Add support for GCM cipher suites from RFC 7714.

webrtc/pc/mediasession.cc

 /*
  *  Copyright 2004 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 18 matching lines @@
 
 #ifdef HAVE_SCTP
 #include "webrtc/media/sctp/sctpdataengine.h"
 #else
 static const uint32_t kMaxSctpSid = 1023;
 #endif
 
 namespace {
 const char kInline[] = "inline:";
 
-void GetSupportedCryptoSuiteNames(void (*func)(std::vector<int>*),
+void GetSupportedCryptoSuiteNames(void (*func)(const rtc::CryptoOptions&,
+                                      std::vector<int>*),
+                                  const rtc::CryptoOptions& crypto_options,
                                   std::vector<std::string>* names) {
 #ifdef HAVE_SRTP
   std::vector<int> crypto_suites;
-  func(&crypto_suites);
+  func(crypto_options, &crypto_suites);
   for (const auto crypto : crypto_suites) {
     names->push_back(rtc::SrtpCryptoSuiteToName(crypto));
   }
 #endif
 }
 }  // namespace
 
 namespace cricket {
 
 
@@ skipping 21 matching lines @@
     return false;
   }
 
   const MediaContentDescription* mdesc =
       static_cast<const MediaContentDescription*>(content->description);
   return mdesc && mdesc->type() == media_type;
 }
 
 static bool CreateCryptoParams(int tag, const std::string& cipher,
                                CryptoParams *out) {
+  int key_len;
+  int salt_len;
+  if (!rtc::GetSrtpKeyAndSaltLengths(
+      rtc::SrtpCryptoSuiteFromName(cipher), &key_len, &salt_len)) {
+    return false;
+  }
+
+  int master_key_base64_len = (key_len + salt_len) * 4 / 3;

[mattdr, 2016/05/06 22:34:14]

This is bad.

E.g.
SRTP_AEAD_AES_256_GCM has key_len = 32 bytes and salt_len = 12 bytes. Total is
44 bytes, which requires at least 59 characters to encode in base64 (not
including padding), but we round down to 58.

https://tools.ietf.org/html/rfc4568#section-6.1

   If the length (after being decoded
   from base64) does not match that specified for the crypto-suite, the
   crypto attribute in question MUST be considered invalid.  Each master
   key and salt MUST be a cryptographically random number and MUST be
   unique to the entire SDP message.  When base64 decoding the key and
   salt, padding characters (i.e., one or two "=" at the end of the
   base64-encoded data) are discarded (see [RFC3548] for details).
   Base64 encoding assumes that the base64 encoding input is an integral
   number of octets.  If a given crypto-suite requires the use of a
   concatenated key and salt with a length that is not an integral
   number of octets, said crypto-suite MUST define a padding scheme that
   results in the base64 input being an integral number of octets.  For
   example, if the length defined were 250 bits, then 6 padding bits
   would be needed, which could be defined to be the last 6 bits in a
   256 bit input.

Much more troubling -- the unit tests *cover* AES-256-GCM but *pass*. Something
is wrong with the parsing code (SrtpFilter::ParseKeyParams) or its callers if
this didn't blow up.

[joachim, 2016/05/09 23:21:40]

Right, and sorry for missing this :-( I now changed it to generate binary random
data with the required length and then encode to base64.
The reason why this was not caught by my added tests is that for DTLS-SRTP the
key is extracted from the underlying channel (see "BaseChannel::SetupDtlsSrtp")
which directly returns the binary key.

Also none of my added tests enabled Gcm for cases where keys are read from a SDP
(this codepath is only executed for such keys).

I added "WebRtcSessionTest.VerifyCryptoParamsInSDPGcm" to check the length of
encoded keys in the SDP but I'm not sure if this is the best place to verify
this or if another test should be added somewhere else.

Other tests were added to srtpfilter_unittest.cc together with a fix in
"SrtpFilter::ApplyParams".

+
   std::string key;
-  key.reserve(SRTP_MASTER_KEY_BASE64_LEN);
+  key.reserve(master_key_base64_len);
 
-  if (!rtc::CreateRandomString(SRTP_MASTER_KEY_BASE64_LEN, &key)) {
+  if (!rtc::CreateRandomString(master_key_base64_len, &key)) {
     return false;
   }
   out->tag = tag;
   out->cipher_suite = cipher;
   out->key_params = kInline;
   out->key_params += key;
   return true;
 }
 
 #ifdef HAVE_SRTP
@@ skipping 40 matching lines @@
   for (CryptoParamsVec::const_iterator it = cryptos.begin();
        it != cryptos.end(); ++it) {
     if (crypto.Matches(*it)) {
       *out = *it;
       return true;
     }
   }
   return false;
 }
 
-// For audio, HMAC 32 is prefered because of the low overhead.
-void GetSupportedAudioCryptoSuites(std::vector<int>* crypto_suites) {
+// For audio, HMAC 32 is prefered over HMAC 80 because of the low overhead.
+void GetSupportedAudioCryptoSuites(const rtc::CryptoOptions& crypto_options,
+    std::vector<int>* crypto_suites) {
 #ifdef HAVE_SRTP
+  if (crypto_options.enable_gcm_crypto_suites) {
+    crypto_suites->push_back(rtc::SRTP_AEAD_AES_256_GCM);

[mattdr, 2016/05/06 22:47:52]

Judging by the comments above, I'm inferring these ciphers are listed in
decreasing order of priority, i.e. we'll prefer AES-256-GCM to AES-128-GCM.

Is there an RFC or other explicit recommendation on which we should prefer?

[joachim, 2016/05/09 23:21:41]

No particular reason from my side. I didn't find anything special recommendation
regarding SRTP and GCM, but Mozilla prioritizes AES-256 over -128 for the
webserver setup
(https://wiki.mozilla.org/Security/Server_Side_TLS#Modern_compatibility), so I
decided to use the same ordering here.

+    crypto_suites->push_back(rtc::SRTP_AEAD_AES_128_GCM);
+  }
   crypto_suites->push_back(rtc::SRTP_AES128_CM_SHA1_32);
   crypto_suites->push_back(rtc::SRTP_AES128_CM_SHA1_80);
 #endif
 }
 
-void GetSupportedAudioCryptoSuiteNames(
+void GetSupportedAudioCryptoSuiteNames(const rtc::CryptoOptions& crypto_options,
     std::vector<std::string>* crypto_suite_names) {
   GetSupportedCryptoSuiteNames(GetSupportedAudioCryptoSuites,
-                               crypto_suite_names);
+                               crypto_options, crypto_suite_names);
 }
 
-void GetSupportedVideoCryptoSuites(std::vector<int>* crypto_suites) {
-  GetDefaultSrtpCryptoSuites(crypto_suites);
+void GetSupportedVideoCryptoSuites(const rtc::CryptoOptions& crypto_options,
+    std::vector<int>* crypto_suites) {
+  GetDefaultSrtpCryptoSuites(crypto_options, crypto_suites);
 }
 
-void GetSupportedVideoCryptoSuiteNames(
+void GetSupportedVideoCryptoSuiteNames(const rtc::CryptoOptions& crypto_options,
     std::vector<std::string>* crypto_suite_names) {
   GetSupportedCryptoSuiteNames(GetSupportedVideoCryptoSuites,
-                               crypto_suite_names);
+                               crypto_options, crypto_suite_names);
 }
 
-void GetSupportedDataCryptoSuites(std::vector<int>* crypto_suites) {
-  GetDefaultSrtpCryptoSuites(crypto_suites);
+void GetSupportedDataCryptoSuites(const rtc::CryptoOptions& crypto_options,
+    std::vector<int>* crypto_suites) {
+  GetDefaultSrtpCryptoSuites(crypto_options, crypto_suites);
 }
 
-void GetSupportedDataCryptoSuiteNames(
+void GetSupportedDataCryptoSuiteNames(const rtc::CryptoOptions& crypto_options,
     std::vector<std::string>* crypto_suite_names) {
   GetSupportedCryptoSuiteNames(GetSupportedDataCryptoSuites,
-                               crypto_suite_names);
+                               crypto_options, crypto_suite_names);
 }
 
-void GetDefaultSrtpCryptoSuites(std::vector<int>* crypto_suites) {
+void GetDefaultSrtpCryptoSuites(const rtc::CryptoOptions& crypto_options,
+    std::vector<int>* crypto_suites) {
 #ifdef HAVE_SRTP
+  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_80);
 #endif
 }
 
-void GetDefaultSrtpCryptoSuiteNames(
+void GetDefaultSrtpCryptoSuiteNames(const rtc::CryptoOptions& crypto_options,
     std::vector<std::string>* crypto_suite_names) {
-  GetSupportedCryptoSuiteNames(GetDefaultSrtpCryptoSuites, crypto_suite_names);
+  GetSupportedCryptoSuiteNames(GetDefaultSrtpCryptoSuites,
+                               crypto_options, crypto_suite_names);
 }
 
-// For video support only 80-bit SHA1 HMAC. For audio 32-bit HMAC is
-// tolerated unless bundle is enabled because it is low overhead. Pick the
-// crypto in the list that is supported.
+// 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.
+// Pick the crypto in the list that is supported.
 static bool SelectCrypto(const MediaContentDescription* offer,
                          bool bundle,
+                         const rtc::CryptoOptions& crypto_options,
                          CryptoParams *crypto) {
   bool audio = offer->type() == MEDIA_TYPE_AUDIO;
   const CryptoParamsVec& cryptos = offer->cryptos();
 
   for (CryptoParamsVec::const_iterator i = cryptos.begin();
        i != cryptos.end(); ++i) {
-    if (rtc::CS_AES_CM_128_HMAC_SHA1_80 == i->cipher_suite ||
+    if ((crypto_options.enable_gcm_crypto_suites &&
+        rtc::IsGcmCryptoSuiteName(i->cipher_suite)) ||

[mattdr, 2016/05/06 22:34:14]

one more space on this line to align with the begin of the paren expression
above

[joachim, 2016/05/09 23:21:40]

Done.

+        rtc::CS_AES_CM_128_HMAC_SHA1_80 == i->cipher_suite ||
         (rtc::CS_AES_CM_128_HMAC_SHA1_32 == i->cipher_suite && audio &&
          !bundle)) {
       return CreateCryptoParams(i->tag, i->cipher_suite, crypto);
     }
   }
   return false;
 }
 
 static const StreamParams* FindFirstStreamParamsByCname(
     const StreamParamsVec& params_vec,
@@ skipping 844 matching lines @@
                                &negotiated_rtp_extensions);
   answer->set_rtp_header_extensions(negotiated_rtp_extensions);
 
   answer->set_rtcp_mux(options.rtcp_mux_enabled && offer->rtcp_mux());
   if (answer->type() == cricket::MEDIA_TYPE_VIDEO) {
     answer->set_rtcp_reduced_size(offer->rtcp_reduced_size());
   }
 
   if (sdes_policy != SEC_DISABLED) {
     CryptoParams crypto;
-    if (SelectCrypto(offer, bundle_enabled, &crypto)) {
+    if (SelectCrypto(offer, bundle_enabled, options.crypto_options, &crypto)) {
       if (current_cryptos) {
         FindMatchingCrypto(*current_cryptos, crypto, &crypto);
       }
       answer->AddCrypto(crypto);
     }
   }
 
   if (answer->cryptos().empty() &&
       (offer->crypto_required() == CT_SDES || sdes_policy == SEC_REQUIRED)) {
     return false;
@@ skipping 495 matching lines @@
       GetFirstAudioContent(current_description);
   std::string content_name =
       current_audio_content ? current_audio_content->name : CN_AUDIO;
 
   cricket::SecurePolicy sdes_policy =
       IsDtlsActive(content_name, current_description) ? cricket::SEC_DISABLED
                                                       : secure();
 
   std::unique_ptr<AudioContentDescription> audio(new AudioContentDescription());
   std::vector<std::string> crypto_suites;
-  GetSupportedAudioCryptoSuiteNames(&crypto_suites);
+  GetSupportedAudioCryptoSuiteNames(options.crypto_options, &crypto_suites);
   if (!CreateMediaContentOffer(
           options,
           audio_codecs,
           sdes_policy,
           GetCryptos(GetFirstAudioContentDescription(current_description)),
           crypto_suites,
           audio_rtp_extensions,
           add_legacy_,
           current_streams,
           audio.get())) {
@@ skipping 39 matching lines @@
       GetFirstVideoContent(current_description);
   std::string content_name =
       current_video_content ? current_video_content->name : CN_VIDEO;
 
   cricket::SecurePolicy sdes_policy =
       IsDtlsActive(content_name, current_description) ? cricket::SEC_DISABLED
                                                       : secure();
 
   std::unique_ptr<VideoContentDescription> video(new VideoContentDescription());
   std::vector<std::string> crypto_suites;
-  GetSupportedVideoCryptoSuiteNames(&crypto_suites);
+  GetSupportedVideoCryptoSuiteNames(options.crypto_options, &crypto_suites);
   if (!CreateMediaContentOffer(
           options,
           video_codecs,
           sdes_policy,
           GetCryptos(GetFirstVideoContentDescription(current_description)),
           crypto_suites,
           video_rtp_extensions,
           add_legacy_,
           current_streams,
           video.get())) {
@@ skipping 55 matching lines @@
     // SDES doesn't make sense for SCTP, so we disable it, and we only
     // get SDES crypto suites for RTP-based data channels.
     sdes_policy = cricket::SEC_DISABLED;
     // Unlike SetMediaProtocol below, we need to set the protocol
     // before we call CreateMediaContentOffer.  Otherwise,
     // CreateMediaContentOffer won't know this is SCTP and will
     // generate SSRCs rather than SIDs.
     data->set_protocol(
         secure_transport ? kMediaProtocolDtlsSctp : kMediaProtocolSctp);
   } else {
-    GetSupportedDataCryptoSuiteNames(&crypto_suites);
+    GetSupportedDataCryptoSuiteNames(options.crypto_options, &crypto_suites);
   }
 
   if (!CreateMediaContentOffer(
           options,
           *data_codecs,
           sdes_policy,
           GetCryptos(GetFirstDataContentDescription(current_description)),
           crypto_suites,
           RtpHeaderExtensions(),
           add_legacy_,
@@ skipping 271 matching lines @@
       GetFirstMediaContentDescription(sdesc, MEDIA_TYPE_VIDEO));
 }
 
 const DataContentDescription* GetFirstDataContentDescription(
     const SessionDescription* sdesc) {
   return static_cast<const DataContentDescription*>(
       GetFirstMediaContentDescription(sdesc, MEDIA_TYPE_DATA));
 }
 
 }  // namespace cricket