diff --git a/api/crypto/crypto_options.cc b/api/crypto/crypto_options.cc index 7892b2ecee..f47e844596 100644 --- a/api/crypto/crypto_options.cc +++ b/api/crypto/crypto_options.cc @@ -32,10 +32,6 @@ CryptoOptions CryptoOptions::NoGcm() { std::vector CryptoOptions::GetSupportedDtlsSrtpCryptoSuites() const { std::vector crypto_suites; - if (srtp.enable_gcm_crypto_suites) { - crypto_suites.push_back(rtc::SRTP_AEAD_AES_256_GCM); - crypto_suites.push_back(rtc::SRTP_AEAD_AES_128_GCM); - } // Note: SRTP_AES128_CM_SHA1_80 is what is required to be supported (by // draft-ietf-rtcweb-security-arch), but SRTP_AES128_CM_SHA1_32 is allowed as // well, and saves a few bytes per packet if it ends up selected. @@ -44,7 +40,18 @@ std::vector CryptoOptions::GetSupportedDtlsSrtpCryptoSuites() const { if (srtp.enable_aes128_sha1_32_crypto_cipher) { crypto_suites.push_back(rtc::SRTP_AES128_CM_SHA1_32); } - crypto_suites.push_back(rtc::SRTP_AES128_CM_SHA1_80); + if (srtp.enable_aes128_sha1_80_crypto_cipher) { + crypto_suites.push_back(rtc::SRTP_AES128_CM_SHA1_80); + } + + // Note: GCM cipher suites are not the top choice since they increase the + // packet size. In order to negotiate them the other side must not support + // SRTP_AES128_CM_SHA1_80. + if (srtp.enable_gcm_crypto_suites) { + crypto_suites.push_back(rtc::SRTP_AEAD_AES_256_GCM); + crypto_suites.push_back(rtc::SRTP_AEAD_AES_128_GCM); + } + RTC_CHECK(!crypto_suites.empty()); return crypto_suites; } @@ -53,6 +60,7 @@ bool CryptoOptions::operator==(const CryptoOptions& other) const { struct Srtp { bool enable_gcm_crypto_suites; bool enable_aes128_sha1_32_crypto_cipher; + bool enable_aes128_sha1_80_crypto_cipher; bool enable_encrypted_rtp_header_extensions; } srtp; struct SFrame { @@ -66,6 +74,8 @@ bool CryptoOptions::operator==(const CryptoOptions& other) const { return srtp.enable_gcm_crypto_suites == other.srtp.enable_gcm_crypto_suites && srtp.enable_aes128_sha1_32_crypto_cipher == other.srtp.enable_aes128_sha1_32_crypto_cipher && + srtp.enable_aes128_sha1_80_crypto_cipher == + other.srtp.enable_aes128_sha1_80_crypto_cipher && srtp.enable_encrypted_rtp_header_extensions == other.srtp.enable_encrypted_rtp_header_extensions && sframe.require_frame_encryption == diff --git a/api/crypto/crypto_options.h b/api/crypto/crypto_options.h index 91a585a6c6..5f6cea6c82 100644 --- a/api/crypto/crypto_options.h +++ b/api/crypto/crypto_options.h @@ -49,6 +49,10 @@ struct RTC_EXPORT CryptoOptions { // other ciphers get preferred. bool enable_aes128_sha1_32_crypto_cipher = false; + // The most commonly used cipher. Can be disabled, mostly for testing + // purposes. + bool enable_aes128_sha1_80_crypto_cipher = true; + // If set to true, encrypted RTP header extensions as defined in RFC 6904 // will be negotiated. They will only be used if both peers support them. bool enable_encrypted_rtp_header_extensions = false; diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc index b06091b3d9..536ad01ebd 100644 --- a/pc/peer_connection_integrationtest.cc +++ b/pc/peer_connection_integrationtest.cc @@ -1635,13 +1635,18 @@ class PeerConnectionIntegrationBaseTest : public ::testing::Test { void TestGcmNegotiationUsesCipherSuite(bool local_gcm_enabled, bool remote_gcm_enabled, + bool aes_ctr_enabled, int expected_cipher_suite) { PeerConnectionFactory::Options caller_options; caller_options.crypto_options.srtp.enable_gcm_crypto_suites = local_gcm_enabled; + caller_options.crypto_options.srtp.enable_aes128_sha1_80_crypto_cipher = + aes_ctr_enabled; PeerConnectionFactory::Options callee_options; callee_options.crypto_options.srtp.enable_gcm_crypto_suites = remote_gcm_enabled; + callee_options.crypto_options.srtp.enable_aes128_sha1_80_crypto_cipher = + aes_ctr_enabled; TestNegotiatedCipherSuite(caller_options, callee_options, expected_cipher_suite); } @@ -3110,38 +3115,21 @@ TEST_P(PeerConnectionIntegrationTest, Aes128Sha1_32_CipherUsedWhenSupported) { TEST_P(PeerConnectionIntegrationTest, NonGcmCipherUsedWhenGcmNotSupported) { bool local_gcm_enabled = false; bool remote_gcm_enabled = false; + bool aes_ctr_enabled = true; int expected_cipher_suite = kDefaultSrtpCryptoSuite; TestGcmNegotiationUsesCipherSuite(local_gcm_enabled, remote_gcm_enabled, - expected_cipher_suite); + aes_ctr_enabled, expected_cipher_suite); } -// Test that a GCM cipher is used if both ends support it. -TEST_P(PeerConnectionIntegrationTest, GcmCipherUsedWhenGcmSupported) { +// Test that a GCM cipher is used if both ends support it and non-GCM is +// disabled. +TEST_P(PeerConnectionIntegrationTest, GcmCipherUsedWhenOnlyGcmSupported) { bool local_gcm_enabled = true; bool remote_gcm_enabled = true; + bool aes_ctr_enabled = false; int expected_cipher_suite = kDefaultSrtpCryptoSuiteGcm; TestGcmNegotiationUsesCipherSuite(local_gcm_enabled, remote_gcm_enabled, - expected_cipher_suite); -} - -// Test that GCM isn't used if only the offerer supports it. -TEST_P(PeerConnectionIntegrationTest, - NonGcmCipherUsedWhenOnlyCallerSupportsGcm) { - bool local_gcm_enabled = true; - bool remote_gcm_enabled = false; - int expected_cipher_suite = kDefaultSrtpCryptoSuite; - TestGcmNegotiationUsesCipherSuite(local_gcm_enabled, remote_gcm_enabled, - expected_cipher_suite); -} - -// Test that GCM isn't used if only the answerer supports it. -TEST_P(PeerConnectionIntegrationTest, - NonGcmCipherUsedWhenOnlyCalleeSupportsGcm) { - bool local_gcm_enabled = false; - bool remote_gcm_enabled = true; - int expected_cipher_suite = kDefaultSrtpCryptoSuite; - TestGcmNegotiationUsesCipherSuite(local_gcm_enabled, remote_gcm_enabled, - expected_cipher_suite); + aes_ctr_enabled, expected_cipher_suite); } // Verify that media can be transmitted end-to-end when GCM crypto suites are @@ -3151,6 +3139,7 @@ TEST_P(PeerConnectionIntegrationTest, TEST_P(PeerConnectionIntegrationTest, EndToEndCallWithGcmCipher) { PeerConnectionFactory::Options gcm_options; gcm_options.crypto_options.srtp.enable_gcm_crypto_suites = true; + gcm_options.crypto_options.srtp.enable_aes128_sha1_80_crypto_cipher = false; ASSERT_TRUE( CreatePeerConnectionWrappersWithOptions(gcm_options, gcm_options)); ConnectFakeSignaling();