admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "Enable SRTP GCM ciphers by default"

Browse Source 
This reverts commit d8633868b34dc1d841f0a9fd1afe2bc22aa8bde6.

Reason for revert: Breaks downstream project.

Original change's description:
> Enable SRTP GCM ciphers by default
>
> Bug: webrtc:15178
> Change-Id: I0216ce8f194fffc820723d82b9c04a76573c2f4f
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/305381
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Commit-Queue: Philipp Hancke <phancke@microsoft.com>
> Reviewed-by: Victor Boivie <boivie@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#40828}

Bug: webrtc:15178
Change-Id: I88433e899cb4b705eafa3fceff3edc520629f603
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/321863
Owners-Override: Artem Titov <titovartem@webrtc.org>
Auto-Submit: Manashi Sarkar <manashi@google.com>
Commit-Queue: Artem Titov <titovartem@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Artem Titov <titovartem@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40832}
This commit is contained in:
Manashi Sarkar 2023-09-28 14:37:22 +00:00 committed by WebRTC LUCI CQ
parent ebe207f71c
commit c2bbe4b952
4 changed files with 27 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
api/crypto/crypto_options.cc
View File

@ -23,6 +23,13 @@ CryptoOptions::CryptoOptions(const CryptoOptions& other) {
CryptoOptions::~CryptoOptions() {} CryptoOptions::~CryptoOptions() {}
// static
CryptoOptions CryptoOptions::NoGcm() {
CryptoOptions options;
options.srtp.enable_gcm_crypto_suites = false;
return options;
}
std::vector<int> CryptoOptions::GetSupportedDtlsSrtpCryptoSuites() const { std::vector<int> CryptoOptions::GetSupportedDtlsSrtpCryptoSuites() const {
std::vector<int> crypto_suites; std::vector<int> crypto_suites;
// Note: kSrtpAes128CmSha1_80 is what is required to be supported (by // Note: kSrtpAes128CmSha1_80 is what is required to be supported (by

7
api/crypto/crypto_options.h
View File

@ -25,6 +25,11 @@ struct RTC_EXPORT CryptoOptions {
CryptoOptions(const CryptoOptions& other); CryptoOptions(const CryptoOptions& other);
~CryptoOptions(); ~CryptoOptions();
// Helper method to return an instance of the CryptoOptions with GCM crypto
// suites disabled. This method should be used instead of depending on current
// default values set by the constructor.
static CryptoOptions NoGcm();
// Returns a list of the supported DTLS-SRTP Crypto suites based on this set // Returns a list of the supported DTLS-SRTP Crypto suites based on this set
// of crypto options. // of crypto options.
std::vector<int> GetSupportedDtlsSrtpCryptoSuites() const; std::vector<int> GetSupportedDtlsSrtpCryptoSuites() const;
@ -36,7 +41,7 @@ struct RTC_EXPORT CryptoOptions {
struct Srtp { struct Srtp {
// Enable GCM crypto suites from RFC 7714 for SRTP. GCM will only be used // Enable GCM crypto suites from RFC 7714 for SRTP. GCM will only be used
// if both sides enable it. // if both sides enable it.
bool enable_gcm_crypto_suites = true; bool enable_gcm_crypto_suites = false;
// If set to true, the (potentially insecure) crypto cipher // If set to true, the (potentially insecure) crypto cipher
// kSrtpAes128CmSha1_32 will be included in the list of supported ciphers // kSrtpAes128CmSha1_32 will be included in the list of supported ciphers

2
api/peer_connection_interface.h
View File

@ -1493,7 +1493,7 @@ class RTC_EXPORT PeerConnectionFactoryInterface
rtc::SSLProtocolVersion ssl_max_version = rtc::SSL_PROTOCOL_DTLS_12; rtc::SSLProtocolVersion ssl_max_version = rtc::SSL_PROTOCOL_DTLS_12;
// Sets crypto related options, e.g. enabled cipher suites. // Sets crypto related options, e.g. enabled cipher suites.
CryptoOptions crypto_options = {}; CryptoOptions crypto_options = CryptoOptions::NoGcm();
}; };
// Set the options to be used for subsequently created PeerConnections. // Set the options to be used for subsequently created PeerConnections.

25
pc/media_session_unittest.cc
View File

@ -284,7 +284,6 @@ static const char* kMediaProtocolsDtls[] = {
// default changes. // default changes.
static const char* kDefaultSrtpCryptoSuite = kCsAesCm128HmacSha1_80; static const char* kDefaultSrtpCryptoSuite = kCsAesCm128HmacSha1_80;
static const char* kDefaultSrtpCryptoSuiteGcm = kCsAeadAes256Gcm; static const char* kDefaultSrtpCryptoSuiteGcm = kCsAeadAes256Gcm;
static const uint8_t kDefaultCryptoSuiteSize = 3U;
// These constants are used to make the code using "AddMediaDescriptionOptions" // These constants are used to make the code using "AddMediaDescriptionOptions"
// more readable. // more readable.
@ -623,8 +622,9 @@ class MediaSessionDescriptionFactoryTest : public ::testing::Test {
ASSERT_TRUE(video_media_desc); ASSERT_TRUE(video_media_desc);
EXPECT_TRUE(CompareCryptoParams(audio_media_desc->cryptos(), EXPECT_TRUE(CompareCryptoParams(audio_media_desc->cryptos(),
video_media_desc->cryptos())); video_media_desc->cryptos()));
ASSERT_CRYPTO(audio_media_desc, offer ? kDefaultCryptoSuiteSize : 1U, EXPECT_EQ(1u, audio_media_desc->cryptos().size());
kDefaultSrtpCryptoSuite); EXPECT_EQ(kDefaultSrtpCryptoSuite,
audio_media_desc->cryptos()[0].crypto_suite);
// Verify the selected crypto is one from the reference audio // Verify the selected crypto is one from the reference audio
// media content. // media content.
@ -819,7 +819,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateAudioOffer) {
EXPECT_EQ(0U, acd->first_ssrc()); // no sender is attached. EXPECT_EQ(0U, acd->first_ssrc()); // no sender is attached.
EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // default bandwidth (auto) EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // default bandwidth (auto)
EXPECT_TRUE(acd->rtcp_mux()); // rtcp-mux defaults on EXPECT_TRUE(acd->rtcp_mux()); // rtcp-mux defaults on
ASSERT_CRYPTO(acd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite); ASSERT_CRYPTO(acd, 1U, kDefaultSrtpCryptoSuite);
EXPECT_EQ(cricket::kMediaProtocolSavpf, acd->protocol()); EXPECT_EQ(cricket::kMediaProtocolSavpf, acd->protocol());
} }
@ -844,14 +844,14 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateVideoOffer) {
EXPECT_EQ(0U, acd->first_ssrc()); // no sender is attached EXPECT_EQ(0U, acd->first_ssrc()); // no sender is attached
EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // default bandwidth (auto) EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // default bandwidth (auto)
EXPECT_TRUE(acd->rtcp_mux()); // rtcp-mux defaults on EXPECT_TRUE(acd->rtcp_mux()); // rtcp-mux defaults on
ASSERT_CRYPTO(acd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite); ASSERT_CRYPTO(acd, 1U, kDefaultSrtpCryptoSuite);
EXPECT_EQ(cricket::kMediaProtocolSavpf, acd->protocol()); EXPECT_EQ(cricket::kMediaProtocolSavpf, acd->protocol());
EXPECT_EQ(MEDIA_TYPE_VIDEO, vcd->type()); EXPECT_EQ(MEDIA_TYPE_VIDEO, vcd->type());
EXPECT_EQ(f1_.video_sendrecv_codecs(), vcd->codecs()); EXPECT_EQ(f1_.video_sendrecv_codecs(), vcd->codecs());
EXPECT_EQ(0U, vcd->first_ssrc()); // no sender is attached EXPECT_EQ(0U, vcd->first_ssrc()); // no sender is attached
EXPECT_EQ(kAutoBandwidth, vcd->bandwidth()); // default bandwidth (auto) EXPECT_EQ(kAutoBandwidth, vcd->bandwidth()); // default bandwidth (auto)
EXPECT_TRUE(vcd->rtcp_mux()); // rtcp-mux defaults on EXPECT_TRUE(vcd->rtcp_mux()); // rtcp-mux defaults on
ASSERT_CRYPTO(vcd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite); ASSERT_CRYPTO(vcd, 1U, kDefaultSrtpCryptoSuite);
EXPECT_EQ(cricket::kMediaProtocolSavpf, vcd->protocol()); EXPECT_EQ(cricket::kMediaProtocolSavpf, vcd->protocol());
} }
@ -1298,6 +1298,7 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateAudioAnswerGcm) {
f1_.set_secure(SEC_ENABLED); f1_.set_secure(SEC_ENABLED);
f2_.set_secure(SEC_ENABLED); f2_.set_secure(SEC_ENABLED);
MediaSessionOptions opts = CreatePlanBMediaSessionOptions(); MediaSessionOptions opts = CreatePlanBMediaSessionOptions();
opts.crypto_options.srtp.enable_gcm_crypto_suites = true;
std::unique_ptr<SessionDescription> offer = std::unique_ptr<SessionDescription> offer =
f1_.CreateOfferOrError(opts, nullptr).MoveValue(); f1_.CreateOfferOrError(opts, nullptr).MoveValue();
ASSERT_TRUE(offer.get()); ASSERT_TRUE(offer.get());
@ -2474,11 +2475,11 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateMultiStreamVideoOffer) {
EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // default bandwidth (auto) EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // default bandwidth (auto)
EXPECT_TRUE(acd->rtcp_mux()); // rtcp-mux defaults on EXPECT_TRUE(acd->rtcp_mux()); // rtcp-mux defaults on
ASSERT_CRYPTO(acd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite); ASSERT_CRYPTO(acd, 1U, kDefaultSrtpCryptoSuite);
EXPECT_EQ(MEDIA_TYPE_VIDEO, vcd->type()); EXPECT_EQ(MEDIA_TYPE_VIDEO, vcd->type());
EXPECT_EQ(f1_.video_sendrecv_codecs(), vcd->codecs()); EXPECT_EQ(f1_.video_sendrecv_codecs(), vcd->codecs());
ASSERT_CRYPTO(vcd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite); ASSERT_CRYPTO(vcd, 1U, kDefaultSrtpCryptoSuite);
const StreamParamsVec& video_streams = vcd->streams(); const StreamParamsVec& video_streams = vcd->streams();
ASSERT_EQ(1U, video_streams.size()); ASSERT_EQ(1U, video_streams.size());
@ -2511,9 +2512,9 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateMultiStreamVideoOffer) {
EXPECT_EQ(acd->codecs(), updated_acd->codecs()); EXPECT_EQ(acd->codecs(), updated_acd->codecs());
EXPECT_EQ(vcd->type(), updated_vcd->type()); EXPECT_EQ(vcd->type(), updated_vcd->type());
EXPECT_EQ(vcd->codecs(), updated_vcd->codecs()); EXPECT_EQ(vcd->codecs(), updated_vcd->codecs());
ASSERT_CRYPTO(updated_acd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite); ASSERT_CRYPTO(updated_acd, 1U, kDefaultSrtpCryptoSuite);
EXPECT_TRUE(CompareCryptoParams(acd->cryptos(), updated_acd->cryptos())); EXPECT_TRUE(CompareCryptoParams(acd->cryptos(), updated_acd->cryptos()));
ASSERT_CRYPTO(updated_vcd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite); ASSERT_CRYPTO(updated_vcd, 1U, kDefaultSrtpCryptoSuite);
EXPECT_TRUE(CompareCryptoParams(vcd->cryptos(), updated_vcd->cryptos())); EXPECT_TRUE(CompareCryptoParams(vcd->cryptos(), updated_vcd->cryptos()));
const StreamParamsVec& updated_audio_streams = updated_acd->streams(); const StreamParamsVec& updated_audio_streams = updated_acd->streams();
@ -3880,8 +3881,8 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCryptoDtls) {
ASSERT_TRUE(audio_media_desc); ASSERT_TRUE(audio_media_desc);
video_media_desc = offer->GetContentDescriptionByName("video"); video_media_desc = offer->GetContentDescriptionByName("video");
ASSERT_TRUE(video_media_desc); ASSERT_TRUE(video_media_desc);
EXPECT_EQ(kDefaultCryptoSuiteSize, audio_media_desc->cryptos().size()); EXPECT_EQ(1u, audio_media_desc->cryptos().size());
EXPECT_EQ(kDefaultCryptoSuiteSize, video_media_desc->cryptos().size()); EXPECT_EQ(1u, video_media_desc->cryptos().size());
audio_trans_desc = offer->GetTransportDescriptionByName("audio"); audio_trans_desc = offer->GetTransportDescriptionByName("audio");
ASSERT_TRUE(audio_trans_desc); ASSERT_TRUE(audio_trans_desc);