Reland "Spanify SRTP key export"

This is a reland of commit 65ae3245f9380e46b1d755f3f452ba63ab6cdf8d
with more backward compat which also fixes the off-by-one issue which caused wrong SRTP keys to be extracted.

Original change's description:
> Spanify SRTP key export
>
> and simplify the interface used as this is only used for exporting
> SRTP keys and passing arcane OpenSSL arguments around does not make
> much sense.
>
> BUG=webrtc:357776213
>
> Change-Id: I9e5a94fe368b77975e48b6dd5ab6a2d2575d6382
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/364521
> Commit-Queue: Philipp Hancke <phancke@meta.com>
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Reviewed-by: Florent Castelli <orphis@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#43198}

Bug: webrtc:357776213
Change-Id: I5d43dc23f90ef630834fb400751979fcc5e18203
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/365180
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Jeremy Leconte <jleconte@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@meta.com>
Cr-Commit-Position: refs/heads/main@{#43225}

p2p/base/dtls_transport.cc

@@ -354,16 +354,26 @@ std::unique_ptr<rtc::SSLCertChain> DtlsTransport::GetRemoteSSLCertChain()
   return dtls_->GetPeerSSLCertChain();
 }
 
+bool DtlsTransport::ExportSrtpKeyingMaterial(
+    rtc::ZeroOnFreeBuffer<unsigned char>& keying_material) {
+  return dtls_ ? dtls_->ExportSrtpKeyingMaterial(keying_material) : false;
+}
+
 bool DtlsTransport::ExportKeyingMaterial(absl::string_view label,
                                          const uint8_t* context,
                                          size_t context_len,
                                          bool use_context,
                                          uint8_t* result,
                                          size_t result_len) {
-  return (dtls_.get())
+  RTC_DCHECK(!context);
-             ? dtls_->ExportKeyingMaterial(label, context, context_len,
+  RTC_DCHECK_EQ(context_len, 0u);
-                                           use_context, result, result_len)
+  RTC_DCHECK_EQ(use_context, false);
-             : false;
+  rtc::ZeroOnFreeBuffer<unsigned char> temporary_result(result_len);
+  if (ExportSrtpKeyingMaterial(temporary_result)) {
+    std::memcpy(result, temporary_result.data(), result_len);
+    return true;
+  }
+  return false;
 }
 
 bool DtlsTransport::SetupDtls() {

p2p/base/dtls_transport.h

@@ -179,8 +179,11 @@ class DtlsTransport : public DtlsTransportInternal {
   // Once DTLS has established (i.e., this ice_transport is writable), this
   // method extracts the keys negotiated during the DTLS handshake, for use in
   // external encryption. DTLS-SRTP uses this to extract the needed SRTP keys.
-  // See the SSLStreamAdapter documentation for info on the specific parameters.
+  bool ExportSrtpKeyingMaterial(
-  bool ExportKeyingMaterial(absl::string_view label,
+      rtc::ZeroOnFreeBuffer<unsigned char>& keying_material) override;
+
+  [[deprecated("Use ExportSrtpKeyingMaterial instead")]] bool
+  ExportKeyingMaterial(absl::string_view label,
                        const uint8_t* context,
                        size_t context_len,
                        bool use_context,

p2p/base/dtls_transport_internal.h

@@ -89,7 +89,11 @@ class DtlsTransportInternal : public rtc::PacketTransportInternal {
   virtual std::unique_ptr<rtc::SSLCertChain> GetRemoteSSLCertChain() const = 0;
 
   // Allows key material to be extracted for external encryption.
-  virtual bool ExportKeyingMaterial(absl::string_view label,
+  virtual bool ExportSrtpKeyingMaterial(
+      rtc::ZeroOnFreeBuffer<unsigned char>& keying_material) = 0;
+
+  [[deprecated("Use ExportSrtpKeyingMaterial instead")]] virtual bool
+  ExportKeyingMaterial(absl::string_view label,
                        const uint8_t* context,
                        size_t context_len,
                        bool use_context,

p2p/base/dtls_transport_unittest.cc

@@ -458,6 +458,33 @@ TEST_F(DtlsTransportTest, TestTransferDtlsCombineRecords) {
   TestTransfer(500, 100, /*srtp=*/false);
 }
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+TEST_F(DtlsTransportTest, KeyingMaterialExporter) {
+  PrepareDtls(rtc::KT_DEFAULT);
+  ASSERT_TRUE(Connect());
+
+  int crypto_suite;
+  EXPECT_TRUE(client1_.dtls_transport()->GetSrtpCryptoSuite(&crypto_suite));
+  int key_len;
+  int salt_len;
+  EXPECT_TRUE(rtc::GetSrtpKeyAndSaltLengths(crypto_suite, &key_len, &salt_len));
+  rtc::ZeroOnFreeBuffer<unsigned char> client1_out(2 * (key_len + salt_len));
+  rtc::ZeroOnFreeBuffer<unsigned char> client2_out(2 * (key_len + salt_len));
+  EXPECT_TRUE(client1_.dtls_transport()->ExportSrtpKeyingMaterial(client1_out));
+  EXPECT_TRUE(client2_.dtls_transport()->ExportSrtpKeyingMaterial(client2_out));
+  EXPECT_EQ(client1_out, client2_out);
+
+  // Legacy variant using the deprecated API.
+  rtc::ZeroOnFreeBuffer<unsigned char> client1_out_legacy(2 *
+                                                          (key_len + salt_len));
+  EXPECT_TRUE(client1_.dtls_transport()->ExportKeyingMaterial(
+      "EXTRACTOR-dtls_srtp", nullptr, 0, false, client1_out_legacy.data(),
+      client1_out_legacy.size()));
+  EXPECT_EQ(client1_out, client1_out_legacy);
+}
+#pragma clang diagnostic pop
+
 class DtlsTransportVersionTest
     : public DtlsTransportTestBase,
       public ::testing::TestWithParam<

p2p/base/fake_dtls_transport.h

@@ -11,6 +11,7 @@
 #ifndef P2P_BASE_FAKE_DTLS_TRANSPORT_H_
 #define P2P_BASE_FAKE_DTLS_TRANSPORT_H_
 
+#include <cstring>
 #include <memory>
 #include <string>
 #include <utility>
@@ -227,6 +228,13 @@ class FakeDtlsTransport : public DtlsTransportInternal {
     }
     return std::make_unique<rtc::SSLCertChain>(remote_cert_->Clone());
   }
+  bool ExportSrtpKeyingMaterial(
+      rtc::ZeroOnFreeBuffer<unsigned char>& keying_material) override {
+    if (do_dtls_) {
+      std::memset(keying_material.data(), 0xff, keying_material.size());
+    }
+    return do_dtls_;
+  }
   bool ExportKeyingMaterial(absl::string_view label,
                             const uint8_t* context,
                             size_t context_len,

pc/dtls_srtp_transport.cc

@@ -20,11 +20,6 @@
 #include "rtc_base/logging.h"
 #include "rtc_base/ssl_stream_adapter.h"
 
-namespace {
-// Value specified in RFC 5764.
-static const char kDtlsSrtpExporterLabel[] = "EXTRACTOR-dtls_srtp";
-}  // namespace
-
 namespace webrtc {
 
 DtlsSrtpTransport::DtlsSrtpTransport(bool rtcp_mux_enabled,
@@ -230,10 +225,8 @@ bool DtlsSrtpTransport::ExtractParams(
   rtc::ZeroOnFreeBuffer<unsigned char> dtls_buffer(key_len * 2 + salt_len * 2);
 
   // RFC 5705 exporter using the RFC 5764 parameters
-  if (!dtls_transport->ExportKeyingMaterial(kDtlsSrtpExporterLabel, NULL, 0,
+  if (!dtls_transport->ExportSrtpKeyingMaterial(dtls_buffer)) {
-                                            false, &dtls_buffer[0],
+    RTC_LOG(LS_ERROR) << "DTLS-SRTP key export failed";
-                                            dtls_buffer.size())) {
-    RTC_LOG(LS_WARNING) << "DTLS-SRTP key export failed";
     RTC_DCHECK_NOTREACHED();  // This should never happen
     return false;
   }

rtc_base/openssl_stream_adapter.cc

@@ -63,6 +63,12 @@
 #error "webrtc requires at least OpenSSL version 1.1.0, to support DTLS-SRTP"
 #endif
 
+namespace {
+// Value specified in RFC 5764.
+static constexpr absl::string_view kDtlsSrtpExporterLabel =
+    "EXTRACTOR-dtls_srtp";
+}  // namespace
+
 namespace rtc {
 namespace {
 using ::webrtc::SafeTask;
@@ -377,7 +383,24 @@ bool OpenSSLStreamAdapter::GetSslVersionBytes(int* version) const {
   return true;
 }
 
-// Key Extractor interface
+bool OpenSSLStreamAdapter::ExportSrtpKeyingMaterial(
+    rtc::ZeroOnFreeBuffer<unsigned char>& keying_material) {
+  // Arguments are:
+  // keying material/len -- a buffer to hold the keying material.
+  // label               -- the exporter label.
+  //                        part of the RFC defining each exporter
+  //                        usage. We only use RFC 5764 for DTLS-SRTP.
+  // context/context_len -- a context to bind to for this connection;
+  // use_context            optional, can be null, 0 (IN). Not used by WebRTC.
+  if (SSL_export_keying_material(
+          ssl_, keying_material.data(), keying_material.size(),
+          kDtlsSrtpExporterLabel.data(), kDtlsSrtpExporterLabel.size(), nullptr,
+          0, false) != 1) {
+    return false;
+  }
+  return true;
+}
+
 bool OpenSSLStreamAdapter::ExportKeyingMaterial(absl::string_view label,
                                                 const uint8_t* context,
                                                 size_t context_len,

rtc_base/openssl_stream_adapter.h

@@ -109,7 +109,10 @@ class OpenSSLStreamAdapter final : public SSLStreamAdapter {
       const override;
   bool GetSslVersionBytes(int* version) const override;
   // Key Extractor interface
-  bool ExportKeyingMaterial(absl::string_view label,
+  bool ExportSrtpKeyingMaterial(
+      rtc::ZeroOnFreeBuffer<unsigned char>& keying_material) override;
+  [[deprecated("Use ExportSrtpKeyingMaterial instead")]] bool
+  ExportKeyingMaterial(absl::string_view label,
                        const uint8_t* context,
                        size_t context_len,
                        bool use_context,

rtc_base/ssl_stream_adapter.cc

@@ -87,15 +87,6 @@ std::unique_ptr<SSLStreamAdapter> SSLStreamAdapter::Create(
                                                 std::move(handshake_error));
 }
 
-bool SSLStreamAdapter::ExportKeyingMaterial(absl::string_view label,
-                                            const uint8_t* context,
-                                            size_t context_len,
-                                            bool use_context,
-                                            uint8_t* result,
-                                            size_t result_len) {
-  return false;  // Default is unsupported
-}
-
 bool SSLStreamAdapter::IsBoringSsl() {
   return OpenSSLStreamAdapter::IsBoringSsl();
 }

rtc_base/ssl_stream_adapter.h

@@ -198,23 +198,17 @@ class SSLStreamAdapter : public StreamInterface {
   virtual bool GetSslVersionBytes(int* version) const = 0;
 
   // Key Exporter interface from RFC 5705
-  // Arguments are:
+  virtual bool ExportSrtpKeyingMaterial(
-  // label               -- the exporter label.
+      rtc::ZeroOnFreeBuffer<unsigned char>& keying_material) = 0;
-  //                        part of the RFC defining each exporter
+  [[deprecated("Use ExportSrtpKeyingMaterial instead")]] virtual bool
-  //                        usage (IN)
+  ExportKeyingMaterial(absl::string_view label,
-  // context/context_len -- a context to bind to for this connection;
-  //                        optional, can be null, 0 (IN)
-  // use_context         -- whether to use the context value
-  //                        (needed to distinguish no context from
-  //                        zero-length ones).
-  // result              -- where to put the computed value
-  // result_len          -- the length of the computed value
-  virtual bool ExportKeyingMaterial(absl::string_view label,
                        const uint8_t* context,
                        size_t context_len,
                        bool use_context,
                        uint8_t* result,
-                                    size_t result_len);
+                       size_t result_len) {
+    return false;
+  }
 
   // Returns the signature algorithm or 0 if not applicable.
   virtual uint16_t GetPeerSignatureAlgorithm() const = 0;

rtc_base/ssl_stream_adapter_unittest.cc

@@ -56,10 +56,6 @@ using ::testing::Values;
 using ::testing::WithParamInterface;
 using ::webrtc::SafeTask;
 
-static const char kExporterLabel[] = "label";
-static const unsigned char kExporterContext[] = "context";
-static int kExporterContextLen = sizeof(kExporterContext);
-
 // A private key used for testing, broken into pieces in order to avoid
 // issues with Git's checks for private keys in repos.
 // Generated using `openssl genrsa -out key.pem 2048`
@@ -818,21 +814,6 @@ class SSLStreamAdapterTestBase : public ::testing::Test,
       return server_ssl_->GetSslVersionBytes(version);
   }
 
-  bool ExportKeyingMaterial(absl::string_view label,
-                            const unsigned char* context,
-                            size_t context_len,
-                            bool use_context,
-                            bool client,
-                            unsigned char* result,
-                            size_t result_len) {
-    if (client)
-      return client_ssl_->ExportKeyingMaterial(label, context, context_len,
-                                               use_context, result, result_len);
-    else
-      return server_ssl_->ExportKeyingMaterial(label, context, context_len,
-                                               use_context, result, result_len);
-  }
-
   // To be implemented by subclasses.
   virtual void WriteData() = 0;
   virtual void ReadData(rtc::StreamInterface* stream) = 0;
@@ -1397,24 +1378,35 @@ TEST_F(SSLStreamAdapterTestDTLS, TestDTLSSrtpKeyAndSaltLengths) {
 }
 
 // Test an exporter
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
 TEST_F(SSLStreamAdapterTestDTLS, TestDTLSExporter) {
+  const std::vector<int> crypto_suites = {rtc::kSrtpAes128CmSha1_80};
+  SetDtlsSrtpCryptoSuites(crypto_suites, true);
+  SetDtlsSrtpCryptoSuites(crypto_suites, false);
+
   TestHandshake();
-  unsigned char client_out[EVP_MAX_MD_SIZE];
+  int selected_crypto_suite;
-  unsigned char server_out[EVP_MAX_MD_SIZE];
+  EXPECT_TRUE(GetDtlsSrtpCryptoSuite(/*client=*/false, &selected_crypto_suite));
+  int key_len;
+  int salt_len;
+  ASSERT_TRUE(rtc::GetSrtpKeyAndSaltLengths(selected_crypto_suite, &key_len,
+                                            &salt_len));
+  rtc::ZeroOnFreeBuffer<unsigned char> client_out(2 * (key_len + salt_len));
+  rtc::ZeroOnFreeBuffer<unsigned char> server_out(2 * (key_len + salt_len));
 
-  bool result;
+  EXPECT_TRUE(client_ssl_->ExportSrtpKeyingMaterial(client_out));
-  result = ExportKeyingMaterial(kExporterLabel, kExporterContext,
+  EXPECT_TRUE(server_ssl_->ExportSrtpKeyingMaterial(server_out));
-                                kExporterContextLen, true, true, client_out,
+  EXPECT_EQ(client_out, server_out);
-                                sizeof(client_out));
-  ASSERT_TRUE(result);
 
-  result = ExportKeyingMaterial(kExporterLabel, kExporterContext,
+  // Legacy variant.
-                                kExporterContextLen, true, false, server_out,
+  rtc::ZeroOnFreeBuffer<unsigned char> legacy_out(2 * (key_len + salt_len));
-                                sizeof(server_out));
+  EXPECT_TRUE(client_ssl_->ExportKeyingMaterial("EXTRACTOR-dtls_srtp", nullptr,
-  ASSERT_TRUE(result);
+                                                0, false, legacy_out.data(),
-
+                                                legacy_out.size()));
-  ASSERT_TRUE(!memcmp(client_out, server_out, sizeof(client_out)));
+  EXPECT_EQ(client_out, legacy_out);
 }
+#pragma clang diagnostic pop
 
 // Test not yet valid certificates are not rejected.
 TEST_F(SSLStreamAdapterTestDTLS, TestCertNotYetValid) {