diff --git a/pc/media_session.cc b/pc/media_session.cc index a9c523d430..51885b4fc4 100644 --- a/pc/media_session.cc +++ b/pc/media_session.cc @@ -182,14 +182,14 @@ bool FindMatchingCrypto(const CryptoParamsVec& cryptos, void GetSupportedAudioSdesCryptoSuites( const webrtc::CryptoOptions& crypto_options, std::vector* crypto_suites) { - if (crypto_options.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); - } if (crypto_options.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 (crypto_options.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); + } } void GetSupportedAudioSdesCryptoSuiteNames( @@ -202,11 +202,11 @@ void GetSupportedAudioSdesCryptoSuiteNames( void GetSupportedVideoSdesCryptoSuites( const webrtc::CryptoOptions& crypto_options, std::vector* crypto_suites) { + crypto_suites->push_back(rtc::SRTP_AES128_CM_SHA1_80); if (crypto_options.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); } - crypto_suites->push_back(rtc::SRTP_AES128_CM_SHA1_80); } void GetSupportedVideoSdesCryptoSuiteNames( @@ -219,11 +219,11 @@ void GetSupportedVideoSdesCryptoSuiteNames( void GetSupportedDataSdesCryptoSuites( const webrtc::CryptoOptions& crypto_options, std::vector* crypto_suites) { + crypto_suites->push_back(rtc::SRTP_AES128_CM_SHA1_80); if (crypto_options.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); } - crypto_suites->push_back(rtc::SRTP_AES128_CM_SHA1_80); } void GetSupportedDataSdesCryptoSuiteNames( diff --git a/pc/media_session_unittest.cc b/pc/media_session_unittest.cc index 1a4b507c2b..ba4db0a674 100644 --- a/pc/media_session_unittest.cc +++ b/pc/media_session_unittest.cc @@ -413,6 +413,17 @@ static MediaSessionOptions CreatePlanBMediaSessionOptions() { return session_options; } +// prefers GCM SDES crypto suites by removing non-GCM defaults. +void PreferGcmCryptoParameters(CryptoParamsVec* cryptos) { + cryptos->erase( + std::remove_if(cryptos->begin(), cryptos->end(), + [](const cricket::CryptoParams& crypto) { + return crypto.cipher_suite != CS_AEAD_AES_256_GCM && + crypto.cipher_suite != CS_AEAD_AES_128_GCM; + }), + cryptos->end()); +} + // TODO(zhihuang): Most of these tests were written while MediaSessionOptions // was designed for Plan B SDP, where only one audio "m=" section and one video // "m=" section could be generated, and ordering couldn't be controlled. Many of @@ -698,6 +709,13 @@ class MediaSessionDescriptionFactoryTest : public ::testing::Test { std::unique_ptr offer = f1_.CreateOffer(offer_opts, NULL); ASSERT_TRUE(offer.get() != NULL); + if (gcm_offer && gcm_answer) { + for (cricket::ContentInfo& content : offer->contents()) { + auto cryptos = content.media_description()->cryptos(); + PreferGcmCryptoParameters(&cryptos); + content.media_description()->set_cryptos(cryptos); + } + } std::unique_ptr answer = f2_.CreateAnswer(offer.get(), answer_opts, NULL); const ContentInfo* ac = answer->GetContentByName("audio"); @@ -1237,6 +1255,11 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateAudioAnswerGcm) { opts.crypto_options.srtp.enable_gcm_crypto_suites = true; std::unique_ptr offer = f1_.CreateOffer(opts, NULL); ASSERT_TRUE(offer.get() != NULL); + for (cricket::ContentInfo& content : offer->contents()) { + auto cryptos = content.media_description()->cryptos(); + PreferGcmCryptoParameters(&cryptos); + content.media_description()->set_cryptos(cryptos); + } std::unique_ptr answer = f2_.CreateAnswer(offer.get(), opts, NULL); const ContentInfo* ac = answer->GetContentByName("audio"); @@ -1343,6 +1366,11 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateDataAnswerGcm) { f2_.set_secure(SEC_ENABLED); std::unique_ptr offer = f1_.CreateOffer(opts, NULL); ASSERT_TRUE(offer.get() != NULL); + for (cricket::ContentInfo& content : offer->contents()) { + auto cryptos = content.media_description()->cryptos(); + PreferGcmCryptoParameters(&cryptos); + content.media_description()->set_cryptos(cryptos); + } std::unique_ptr answer = f2_.CreateAnswer(offer.get(), opts, NULL); const ContentInfo* ac = answer->GetContentByName("audio"); diff --git a/pc/peer_connection_crypto_unittest.cc b/pc/peer_connection_crypto_unittest.cc index 99eb5cd7ac..32e8cbd74c 100644 --- a/pc/peer_connection_crypto_unittest.cc +++ b/pc/peer_connection_crypto_unittest.cc @@ -149,9 +149,12 @@ SdpContentPredicate HaveSdesGcmCryptos(size_t num_crypto_suites) { if (cryptos.size() != num_crypto_suites) { return false; } - const cricket::CryptoParams first_params = cryptos[0]; - return first_params.key_params.size() == 67U && - first_params.cipher_suite == "AEAD_AES_256_GCM"; + for (size_t i = 0; i < cryptos.size(); ++i) { + if (cryptos[i].key_params.size() == 67U && + cryptos[i].cipher_suite == "AEAD_AES_256_GCM") + return true; + } + return false; }; } @@ -333,7 +336,14 @@ TEST_P(PeerConnectionCryptoTest, CorrectCryptoInAnswerWithSdesAndGcm) { auto caller = CreatePeerConnectionWithAudioVideo(config); auto callee = CreatePeerConnectionWithAudioVideo(config); - callee->SetRemoteDescription(caller->CreateOffer()); + auto offer = caller->CreateOffer(); + for (cricket::ContentInfo& content : offer->description()->contents()) { + auto cryptos = content.media_description()->cryptos(); + cryptos.erase(cryptos.begin()); // Assumes that non-GCM is the default. + content.media_description()->set_cryptos(cryptos); + } + + callee->SetRemoteDescription(std::move(offer)); auto answer = callee->CreateAnswer(); ASSERT_TRUE(answer);