reorder sdes suites to not prefer gcm
BUG=chromium:713701
Change-Id: I1ef00df7a7b86a83ae97d4c7c5f41d85eb60b391
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/174803
Commit-Queue: Philipp Hancke <philipp.hancke@googlemail.com>
Reviewed-by: Taylor <deadbeef@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31225}
diff --git a/pc/media_session.cc b/pc/media_session.cc
index a9c523d..51885b4 100644
--- a/pc/media_session.cc
+++ b/pc/media_session.cc
@@ -182,14 +182,14 @@
void GetSupportedAudioSdesCryptoSuites(
const webrtc::CryptoOptions& crypto_options,
std::vector<int>* 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 GetSupportedVideoSdesCryptoSuites(
const webrtc::CryptoOptions& crypto_options,
std::vector<int>* 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 GetSupportedDataSdesCryptoSuites(
const webrtc::CryptoOptions& crypto_options,
std::vector<int>* 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 1a4b507..ba4db0a 100644
--- a/pc/media_session_unittest.cc
+++ b/pc/media_session_unittest.cc
@@ -413,6 +413,17 @@
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 @@
std::unique_ptr<SessionDescription> 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<SessionDescription> answer =
f2_.CreateAnswer(offer.get(), answer_opts, NULL);
const ContentInfo* ac = answer->GetContentByName("audio");
@@ -1237,6 +1255,11 @@
opts.crypto_options.srtp.enable_gcm_crypto_suites = true;
std::unique_ptr<SessionDescription> 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<SessionDescription> answer =
f2_.CreateAnswer(offer.get(), opts, NULL);
const ContentInfo* ac = answer->GetContentByName("audio");
@@ -1343,6 +1366,11 @@
f2_.set_secure(SEC_ENABLED);
std::unique_ptr<SessionDescription> 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<SessionDescription> 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 99eb5cd..32e8cbd 100644
--- a/pc/peer_connection_crypto_unittest.cc
+++ b/pc/peer_connection_crypto_unittest.cc
@@ -149,9 +149,12 @@
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 @@
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);
