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}
diff --git a/api/crypto/crypto_options.cc b/api/crypto/crypto_options.cc
index 22c5dd4..6fe0540 100644
--- a/api/crypto/crypto_options.cc
+++ b/api/crypto/crypto_options.cc
@@ -23,13 +23,6 @@
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> crypto_suites;
// Note: kSrtpAes128CmSha1_80 is what is required to be supported (by
diff --git a/api/crypto/crypto_options.h b/api/crypto/crypto_options.h
index 3179951..b38ea27 100644
--- a/api/crypto/crypto_options.h
+++ b/api/crypto/crypto_options.h
@@ -25,11 +25,6 @@
CryptoOptions(const CryptoOptions& other);
~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
// of crypto options.
std::vector<int> GetSupportedDtlsSrtpCryptoSuites() const;
@@ -41,7 +36,7 @@
struct Srtp {
// Enable GCM crypto suites from RFC 7714 for SRTP. GCM will only be used
// if both sides enable it.
- bool enable_gcm_crypto_suites = false;
+ bool enable_gcm_crypto_suites = true;
// If set to true, the (potentially insecure) crypto cipher
// kSrtpAes128CmSha1_32 will be included in the list of supported ciphers
diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h
index 37dcfbb..e2f284e 100644
--- a/api/peer_connection_interface.h
+++ b/api/peer_connection_interface.h
@@ -1493,7 +1493,7 @@
rtc::SSLProtocolVersion ssl_max_version = rtc::SSL_PROTOCOL_DTLS_12;
// Sets crypto related options, e.g. enabled cipher suites.
- CryptoOptions crypto_options = CryptoOptions::NoGcm();
+ CryptoOptions crypto_options = {};
};
// Set the options to be used for subsequently created PeerConnections.
diff --git a/pc/media_session_unittest.cc b/pc/media_session_unittest.cc
index 753e8cd..aaf2183 100644
--- a/pc/media_session_unittest.cc
+++ b/pc/media_session_unittest.cc
@@ -284,6 +284,7 @@
// default changes.
static const char* kDefaultSrtpCryptoSuite = kCsAesCm128HmacSha1_80;
static const char* kDefaultSrtpCryptoSuiteGcm = kCsAeadAes256Gcm;
+static const uint8_t kDefaultCryptoSuiteSize = 3U;
// These constants are used to make the code using "AddMediaDescriptionOptions"
// more readable.
@@ -622,9 +623,8 @@
ASSERT_TRUE(video_media_desc);
EXPECT_TRUE(CompareCryptoParams(audio_media_desc->cryptos(),
video_media_desc->cryptos()));
- EXPECT_EQ(1u, audio_media_desc->cryptos().size());
- EXPECT_EQ(kDefaultSrtpCryptoSuite,
- audio_media_desc->cryptos()[0].crypto_suite);
+ ASSERT_CRYPTO(audio_media_desc, offer ? kDefaultCryptoSuiteSize : 1U,
+ kDefaultSrtpCryptoSuite);
// Verify the selected crypto is one from the reference audio
// media content.
@@ -819,7 +819,7 @@
EXPECT_EQ(0U, acd->first_ssrc()); // no sender is attached.
EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // default bandwidth (auto)
EXPECT_TRUE(acd->rtcp_mux()); // rtcp-mux defaults on
- ASSERT_CRYPTO(acd, 1U, kDefaultSrtpCryptoSuite);
+ ASSERT_CRYPTO(acd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite);
EXPECT_EQ(cricket::kMediaProtocolSavpf, acd->protocol());
}
@@ -844,14 +844,14 @@
EXPECT_EQ(0U, acd->first_ssrc()); // no sender is attached
EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // default bandwidth (auto)
EXPECT_TRUE(acd->rtcp_mux()); // rtcp-mux defaults on
- ASSERT_CRYPTO(acd, 1U, kDefaultSrtpCryptoSuite);
+ ASSERT_CRYPTO(acd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite);
EXPECT_EQ(cricket::kMediaProtocolSavpf, acd->protocol());
EXPECT_EQ(MEDIA_TYPE_VIDEO, vcd->type());
EXPECT_EQ(f1_.video_sendrecv_codecs(), vcd->codecs());
EXPECT_EQ(0U, vcd->first_ssrc()); // no sender is attached
EXPECT_EQ(kAutoBandwidth, vcd->bandwidth()); // default bandwidth (auto)
EXPECT_TRUE(vcd->rtcp_mux()); // rtcp-mux defaults on
- ASSERT_CRYPTO(vcd, 1U, kDefaultSrtpCryptoSuite);
+ ASSERT_CRYPTO(vcd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite);
EXPECT_EQ(cricket::kMediaProtocolSavpf, vcd->protocol());
}
@@ -1298,7 +1298,6 @@
f1_.set_secure(SEC_ENABLED);
f2_.set_secure(SEC_ENABLED);
MediaSessionOptions opts = CreatePlanBMediaSessionOptions();
- opts.crypto_options.srtp.enable_gcm_crypto_suites = true;
std::unique_ptr<SessionDescription> offer =
f1_.CreateOfferOrError(opts, nullptr).MoveValue();
ASSERT_TRUE(offer.get());
@@ -2475,11 +2474,11 @@
EXPECT_EQ(kAutoBandwidth, acd->bandwidth()); // default bandwidth (auto)
EXPECT_TRUE(acd->rtcp_mux()); // rtcp-mux defaults on
- ASSERT_CRYPTO(acd, 1U, kDefaultSrtpCryptoSuite);
+ ASSERT_CRYPTO(acd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite);
EXPECT_EQ(MEDIA_TYPE_VIDEO, vcd->type());
EXPECT_EQ(f1_.video_sendrecv_codecs(), vcd->codecs());
- ASSERT_CRYPTO(vcd, 1U, kDefaultSrtpCryptoSuite);
+ ASSERT_CRYPTO(vcd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite);
const StreamParamsVec& video_streams = vcd->streams();
ASSERT_EQ(1U, video_streams.size());
@@ -2512,9 +2511,9 @@
EXPECT_EQ(acd->codecs(), updated_acd->codecs());
EXPECT_EQ(vcd->type(), updated_vcd->type());
EXPECT_EQ(vcd->codecs(), updated_vcd->codecs());
- ASSERT_CRYPTO(updated_acd, 1U, kDefaultSrtpCryptoSuite);
+ ASSERT_CRYPTO(updated_acd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite);
EXPECT_TRUE(CompareCryptoParams(acd->cryptos(), updated_acd->cryptos()));
- ASSERT_CRYPTO(updated_vcd, 1U, kDefaultSrtpCryptoSuite);
+ ASSERT_CRYPTO(updated_vcd, kDefaultCryptoSuiteSize, kDefaultSrtpCryptoSuite);
EXPECT_TRUE(CompareCryptoParams(vcd->cryptos(), updated_vcd->cryptos()));
const StreamParamsVec& updated_audio_streams = updated_acd->streams();
@@ -3881,8 +3880,8 @@
ASSERT_TRUE(audio_media_desc);
video_media_desc = offer->GetContentDescriptionByName("video");
ASSERT_TRUE(video_media_desc);
- EXPECT_EQ(1u, audio_media_desc->cryptos().size());
- EXPECT_EQ(1u, video_media_desc->cryptos().size());
+ EXPECT_EQ(kDefaultCryptoSuiteSize, audio_media_desc->cryptos().size());
+ EXPECT_EQ(kDefaultCryptoSuiteSize, video_media_desc->cryptos().size());
audio_trans_desc = offer->GetTransportDescriptionByName("audio");
ASSERT_TRUE(audio_trans_desc);
