diff --git a/pc/mediasession.cc b/pc/mediasession.cc index 5bea2a3e98..8a9c1f802b 100644 --- a/pc/mediasession.cc +++ b/pc/mediasession.cc @@ -194,14 +194,17 @@ bool FindMatchingCrypto(const CryptoParamsVec& cryptos, return false; } -// For audio, HMAC 32 is prefered over HMAC 80 because of the low overhead. +// For audio, HMAC 32 (if enabled) is prefered over HMAC 80 because of the +// low overhead. void GetSupportedAudioSdesCryptoSuites(const rtc::CryptoOptions& crypto_options, std::vector* crypto_suites) { if (crypto_options.enable_gcm_crypto_suites) { crypto_suites->push_back(rtc::SRTP_AEAD_AES_256_GCM); crypto_suites->push_back(rtc::SRTP_AEAD_AES_128_GCM); } - crypto_suites->push_back(rtc::SRTP_AES128_CM_SHA1_32); + if (crypto_options.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); } @@ -245,8 +248,8 @@ void GetSupportedDataSdesCryptoSuiteNames( } // Support any GCM cipher (if enabled through options). For video support only -// 80-bit SHA1 HMAC. For audio 32-bit HMAC is tolerated unless bundle is enabled -// because it is low overhead. +// 80-bit SHA1 HMAC. For audio 32-bit HMAC is tolerated (if enabled) unless +// bundle is enabled because it is low overhead. // Pick the crypto in the list that is supported. static bool SelectCrypto(const MediaContentDescription* offer, bool bundle, @@ -261,7 +264,7 @@ static bool SelectCrypto(const MediaContentDescription* offer, rtc::IsGcmCryptoSuiteName(i->cipher_suite)) || rtc::CS_AES_CM_128_HMAC_SHA1_80 == i->cipher_suite || (rtc::CS_AES_CM_128_HMAC_SHA1_32 == i->cipher_suite && audio && - !bundle)) { + !bundle && crypto_options.enable_aes128_sha1_32_crypto_cipher)) { return CreateCryptoParams(i->tag, i->cipher_suite, crypto); } } diff --git a/pc/peerconnection_integrationtest.cc b/pc/peerconnection_integrationtest.cc index ec53298d22..33bf267e18 100644 --- a/pc/peerconnection_integrationtest.cc +++ b/pc/peerconnection_integrationtest.cc @@ -1365,13 +1365,10 @@ class PeerConnectionIntegrationBaseTest : public testing::Test { return expectations_correct; } - void TestGcmNegotiationUsesCipherSuite(bool local_gcm_enabled, - bool remote_gcm_enabled, - int expected_cipher_suite) { - PeerConnectionFactory::Options caller_options; - caller_options.crypto_options.enable_gcm_crypto_suites = local_gcm_enabled; - PeerConnectionFactory::Options callee_options; - callee_options.crypto_options.enable_gcm_crypto_suites = remote_gcm_enabled; + void TestNegotiatedCipherSuite( + const PeerConnectionFactory::Options& caller_options, + const PeerConnectionFactory::Options& callee_options, + int expected_cipher_suite) { ASSERT_TRUE(CreatePeerConnectionWrappersWithOptions(caller_options, callee_options)); rtc::scoped_refptr caller_observer = @@ -1390,6 +1387,17 @@ class PeerConnectionIntegrationBaseTest : public testing::Test { caller()->pc()->RegisterUMAObserver(nullptr); } + void TestGcmNegotiationUsesCipherSuite(bool local_gcm_enabled, + bool remote_gcm_enabled, + int expected_cipher_suite) { + PeerConnectionFactory::Options caller_options; + caller_options.crypto_options.enable_gcm_crypto_suites = local_gcm_enabled; + PeerConnectionFactory::Options callee_options; + callee_options.crypto_options.enable_gcm_crypto_suites = remote_gcm_enabled; + TestNegotiatedCipherSuite(caller_options, callee_options, + expected_cipher_suite); + } + protected: const SdpSemantics sdp_semantics_; @@ -2600,6 +2608,40 @@ TEST_P(PeerConnectionIntegrationTest, CallerDtls10ToCalleeDtls12) { ASSERT_TRUE(ExpectNewFrames(media_expectations)); } +// The three tests below verify that "enable_aes128_sha1_32_crypto_cipher" +// works as expected; the cipher should only be used if enabled by both sides. +TEST_P(PeerConnectionIntegrationTest, + Aes128Sha1_32_CipherNotUsedWhenOnlyCallerSupported) { + PeerConnectionFactory::Options caller_options; + caller_options.crypto_options.enable_aes128_sha1_32_crypto_cipher = true; + PeerConnectionFactory::Options callee_options; + callee_options.crypto_options.enable_aes128_sha1_32_crypto_cipher = false; + int expected_cipher_suite = rtc::SRTP_AES128_CM_SHA1_80; + TestNegotiatedCipherSuite(caller_options, callee_options, + expected_cipher_suite); +} + +TEST_P(PeerConnectionIntegrationTest, + Aes128Sha1_32_CipherNotUsedWhenOnlyCalleeSupported) { + PeerConnectionFactory::Options caller_options; + caller_options.crypto_options.enable_aes128_sha1_32_crypto_cipher = false; + PeerConnectionFactory::Options callee_options; + callee_options.crypto_options.enable_aes128_sha1_32_crypto_cipher = true; + int expected_cipher_suite = rtc::SRTP_AES128_CM_SHA1_80; + TestNegotiatedCipherSuite(caller_options, callee_options, + expected_cipher_suite); +} + +TEST_P(PeerConnectionIntegrationTest, Aes128Sha1_32_CipherUsedWhenSupported) { + PeerConnectionFactory::Options caller_options; + caller_options.crypto_options.enable_aes128_sha1_32_crypto_cipher = true; + PeerConnectionFactory::Options callee_options; + callee_options.crypto_options.enable_aes128_sha1_32_crypto_cipher = true; + int expected_cipher_suite = rtc::SRTP_AES128_CM_SHA1_32; + TestNegotiatedCipherSuite(caller_options, callee_options, + expected_cipher_suite); +} + // Test that a non-GCM cipher is used if both sides only support non-GCM. TEST_P(PeerConnectionIntegrationTest, NonGcmCipherUsedWhenGcmNotSupported) { bool local_gcm_enabled = false; diff --git a/rtc_base/sslstreamadapter.cc b/rtc_base/sslstreamadapter.cc index b09c1440a4..d52dc45629 100644 --- a/rtc_base/sslstreamadapter.cc +++ b/rtc_base/sslstreamadapter.cc @@ -105,7 +105,11 @@ std::vector GetSupportedDtlsSrtpCryptoSuites( // 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. - crypto_suites.push_back(rtc::SRTP_AES128_CM_SHA1_32); + // As the cipher suite is potentially insecure, it will only be used if + // enabled by both peers. + if (crypto_options.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); return crypto_suites; } diff --git a/rtc_base/sslstreamadapter.h b/rtc_base/sslstreamadapter.h index c04fb346c5..11a67e473f 100644 --- a/rtc_base/sslstreamadapter.h +++ b/rtc_base/sslstreamadapter.h @@ -80,6 +80,15 @@ struct CryptoOptions { // if both sides enable it. bool enable_gcm_crypto_suites = false; + // If set to true, the (potentially insecure) crypto cipher + // SRTP_AES128_CM_SHA1_32 will be included in the list of supported ciphers + // during negotiation. It will only be used if both peers support it and no + // other ciphers get preferred. + // TODO(crbug.com/webrtc/7670): Change default to false after sending PSA and + // giving time for users to set this flag to true explicitly, if they still + // want to use this crypto suite. + bool enable_aes128_sha1_32_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/sdk/android/api/org/webrtc/PeerConnectionFactory.java b/sdk/android/api/org/webrtc/PeerConnectionFactory.java index d2eb73acfc..e50e03c05f 100644 --- a/sdk/android/api/org/webrtc/PeerConnectionFactory.java +++ b/sdk/android/api/org/webrtc/PeerConnectionFactory.java @@ -107,6 +107,7 @@ public class PeerConnectionFactory { public int networkIgnoreMask; public boolean disableEncryption; public boolean disableNetworkMonitor; + public boolean enableAes128Sha1_32CryptoCipher; @CalledByNative("Options") int getNetworkIgnoreMask() { @@ -122,6 +123,11 @@ public class PeerConnectionFactory { boolean getDisableNetworkMonitor() { return disableNetworkMonitor; } + + @CalledByNative("Options") + boolean getEnableAes128Sha1_32CryptoCipher() { + return enableAes128Sha1_32CryptoCipher; + } } public static class Builder { diff --git a/sdk/android/src/jni/pc/peerconnectionfactory.cc b/sdk/android/src/jni/pc/peerconnectionfactory.cc index d10455ef85..62873bdf9a 100644 --- a/sdk/android/src/jni/pc/peerconnectionfactory.cc +++ b/sdk/android/src/jni/pc/peerconnectionfactory.cc @@ -52,6 +52,8 @@ JavaToNativePeerConnectionFactoryOptions(JNIEnv* jni, bool disable_encryption = Java_Options_getDisableEncryption(jni, options); bool disable_network_monitor = Java_Options_getDisableNetworkMonitor(jni, options); + bool enable_aes128_sha1_32_crypto_cipher = + Java_Options_getEnableAes128Sha1_32CryptoCipher(jni, options); PeerConnectionFactoryInterface::Options native_options; @@ -60,6 +62,9 @@ JavaToNativePeerConnectionFactoryOptions(JNIEnv* jni, native_options.network_ignore_mask = network_ignore_mask; native_options.disable_encryption = disable_encryption; native_options.disable_network_monitor = disable_network_monitor; + + native_options.crypto_options.enable_aes128_sha1_32_crypto_cipher = + enable_aes128_sha1_32_crypto_cipher; return native_options; } } // namespace diff --git a/sdk/objc/Framework/Classes/PeerConnection/RTCPeerConnectionFactoryOptions.mm b/sdk/objc/Framework/Classes/PeerConnection/RTCPeerConnectionFactoryOptions.mm index f0cc6a6c81..b5e9d74d61 100644 --- a/sdk/objc/Framework/Classes/PeerConnection/RTCPeerConnectionFactoryOptions.mm +++ b/sdk/objc/Framework/Classes/PeerConnection/RTCPeerConnectionFactoryOptions.mm @@ -34,6 +34,7 @@ void setNetworkBit(webrtc::PeerConnectionFactoryInterface::Options* options, @synthesize ignoreCellularNetworkAdapter = _ignoreCellularNetworkAdapter; @synthesize ignoreWiFiNetworkAdapter = _ignoreWiFiNetworkAdapter; @synthesize ignoreEthernetNetworkAdapter = _ignoreEthernetNetworkAdapter; +@synthesize enableAes128Sha1_32CryptoCipher = _enableAes128Sha1_32CryptoCipher; - (instancetype)init { return [super init]; @@ -50,6 +51,8 @@ void setNetworkBit(webrtc::PeerConnectionFactoryInterface::Options* options, setNetworkBit(&options, rtc::ADAPTER_TYPE_WIFI, self.ignoreWiFiNetworkAdapter); setNetworkBit(&options, rtc::ADAPTER_TYPE_ETHERNET, self.ignoreEthernetNetworkAdapter); + options.crypto_options.enable_aes128_sha1_32_crypto_cipher = self.enableAes128Sha1_32CryptoCipher; + return options; } diff --git a/sdk/objc/Framework/Headers/WebRTC/RTCPeerConnectionFactoryOptions.h b/sdk/objc/Framework/Headers/WebRTC/RTCPeerConnectionFactoryOptions.h index a65abc6c8b..33faa1e6aa 100644 --- a/sdk/objc/Framework/Headers/WebRTC/RTCPeerConnectionFactoryOptions.h +++ b/sdk/objc/Framework/Headers/WebRTC/RTCPeerConnectionFactoryOptions.h @@ -31,6 +31,8 @@ RTC_EXPORT @property(nonatomic, assign) BOOL ignoreEthernetNetworkAdapter; +@property(nonatomic, assign) BOOL enableAes128Sha1_32CryptoCipher; + - (instancetype)init NS_DESIGNATED_INITIALIZER; @end