do not offer gcm as the preferred cipher suite
Move the GCM srtp cipher suites below the default SRTP_AES128_CM_SHA1_80 one. This will not negotiate them by default since they have an impact on packet overhead for audio-only calls. GCM can still be negotiated if the peer offers it as preferred cipher suite or answers with just that cipher suite. BUG=chromium:713701 Change-Id: I79bd4ab827e5c7f55f5550d14db3f4217a7eff86 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/158404 Reviewed-by: Justin Uberti <juberti@google.com> Reviewed-by: Justin Uberti <juberti@webrtc.org> Reviewed-by: Harald Alvestrand <hta@webrtc.org> Commit-Queue: Justin Uberti <juberti@webrtc.org> Cr-Commit-Position: refs/heads/master@{#29672}
This commit is contained in:

committed by
Commit Bot

parent
3ce44a3540
commit
2ebbff83ee
@ -32,10 +32,6 @@ CryptoOptions CryptoOptions::NoGcm() {
|
||||
|
||||
std::vector<int> CryptoOptions::GetSupportedDtlsSrtpCryptoSuites() const {
|
||||
std::vector<int> 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<int> CryptoOptions::GetSupportedDtlsSrtpCryptoSuites() const {
|
||||
if (srtp.enable_aes128_sha1_32_crypto_cipher) {
|
||||
crypto_suites.push_back(rtc::SRTP_AES128_CM_SHA1_32);
|
||||
}
|
||||
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 ==
|
||||
|
@ -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;
|
||||
|
@ -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();
|
||||
|
Reference in New Issue
Block a user