Revert "Spanify SRTP key export"
This reverts commit 65ae3245f9380e46b1d755f3f452ba63ab6cdf8d.
Reason for revert: breaks downstream compilation
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: I03ffcda3d6821718f355b243ce78a9c54b4036f3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/365062
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Jeremy Leconte <jleconte@webrtc.org>
Owners-Override: Jeremy Leconte <jleconte@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43202}
diff --git a/p2p/base/dtls_transport.cc b/p2p/base/dtls_transport.cc
index 60d41a8..5ed2a11 100644
--- a/p2p/base/dtls_transport.cc
+++ b/p2p/base/dtls_transport.cc
@@ -354,26 +354,16 @@
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) {
- RTC_DCHECK(!context);
- RTC_DCHECK_EQ(context_len, 0u);
- RTC_DCHECK_EQ(use_context, 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;
+ return (dtls_.get())
+ ? dtls_->ExportKeyingMaterial(label, context, context_len,
+ use_context, result, result_len)
+ : false;
}
bool DtlsTransport::SetupDtls() {
diff --git a/p2p/base/dtls_transport.h b/p2p/base/dtls_transport.h
index a317275..0a988df 100644
--- a/p2p/base/dtls_transport.h
+++ b/p2p/base/dtls_transport.h
@@ -179,16 +179,13 @@
// 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.
- 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,
- uint8_t* result,
- size_t result_len) override;
+ // See the SSLStreamAdapter documentation for info on the specific parameters.
+ bool ExportKeyingMaterial(absl::string_view label,
+ const uint8_t* context,
+ size_t context_len,
+ bool use_context,
+ uint8_t* result,
+ size_t result_len) override;
IceTransportInternal* ice_transport() override;
diff --git a/p2p/base/dtls_transport_internal.h b/p2p/base/dtls_transport_internal.h
index 69881dd..0550145 100644
--- a/p2p/base/dtls_transport_internal.h
+++ b/p2p/base/dtls_transport_internal.h
@@ -89,16 +89,12 @@
virtual std::unique_ptr<rtc::SSLCertChain> GetRemoteSSLCertChain() const = 0;
// Allows key material to be extracted for external encryption.
- 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,
- uint8_t* result,
- size_t result_len) = 0;
+ 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) = 0;
// Set DTLS remote fingerprint. Must be after local identity set.
ABSL_DEPRECATED("Use SetRemoteParameters instead.")
diff --git a/p2p/base/dtls_transport_unittest.cc b/p2p/base/dtls_transport_unittest.cc
index c579421..767fc4d 100644
--- a/p2p/base/dtls_transport_unittest.cc
+++ b/p2p/base/dtls_transport_unittest.cc
@@ -458,33 +458,6 @@
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<
diff --git a/p2p/base/fake_dtls_transport.h b/p2p/base/fake_dtls_transport.h
index f885165..042d93b 100644
--- a/p2p/base/fake_dtls_transport.h
+++ b/p2p/base/fake_dtls_transport.h
@@ -11,7 +11,6 @@
#ifndef P2P_BASE_FAKE_DTLS_TRANSPORT_H_
#define P2P_BASE_FAKE_DTLS_TRANSPORT_H_
-#include <cstring>
#include <memory>
#include <string>
#include <utility>
@@ -228,13 +227,6 @@
}
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,
diff --git a/pc/dtls_srtp_transport.cc b/pc/dtls_srtp_transport.cc
index 7d1cb99..96a7a09 100644
--- a/pc/dtls_srtp_transport.cc
+++ b/pc/dtls_srtp_transport.cc
@@ -20,6 +20,11 @@
#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,
@@ -225,8 +230,10 @@
rtc::ZeroOnFreeBuffer<unsigned char> dtls_buffer(key_len * 2 + salt_len * 2);
// RFC 5705 exporter using the RFC 5764 parameters
- if (!dtls_transport->ExportSrtpKeyingMaterial(dtls_buffer)) {
- RTC_LOG(LS_ERROR) << "DTLS-SRTP key export failed";
+ if (!dtls_transport->ExportKeyingMaterial(kDtlsSrtpExporterLabel, NULL, 0,
+ false, &dtls_buffer[0],
+ dtls_buffer.size())) {
+ RTC_LOG(LS_WARNING) << "DTLS-SRTP key export failed";
RTC_DCHECK_NOTREACHED(); // This should never happen
return false;
}
diff --git a/rtc_base/openssl_stream_adapter.cc b/rtc_base/openssl_stream_adapter.cc
index c60f4a7..bbad2a3 100644
--- a/rtc_base/openssl_stream_adapter.cc
+++ b/rtc_base/openssl_stream_adapter.cc
@@ -63,11 +63,6 @@
#error "webrtc requires at least OpenSSL version 1.1.0, to support DTLS-SRTP"
#endif
-namespace {
-// Value specified in RFC 5764.
-static const char kDtlsSrtpExporterLabel[] = "EXTRACTOR-dtls_srtp";
-} // namespace
-
namespace rtc {
namespace {
using ::webrtc::SafeTask;
@@ -382,19 +377,16 @@
return true;
}
-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,
- sizeof(kDtlsSrtpExporterLabel), nullptr, 0,
- false) != 1) {
+// Key Extractor interface
+bool OpenSSLStreamAdapter::ExportKeyingMaterial(absl::string_view label,
+ const uint8_t* context,
+ size_t context_len,
+ bool use_context,
+ uint8_t* result,
+ size_t result_len) {
+ if (SSL_export_keying_material(ssl_, result, result_len, label.data(),
+ label.length(), context, context_len,
+ use_context) != 1) {
return false;
}
return true;
diff --git a/rtc_base/openssl_stream_adapter.h b/rtc_base/openssl_stream_adapter.h
index 25ca678..a776c1a 100644
--- a/rtc_base/openssl_stream_adapter.h
+++ b/rtc_base/openssl_stream_adapter.h
@@ -109,8 +109,12 @@
const override;
bool GetSslVersionBytes(int* version) const override;
// Key Extractor interface
- bool ExportSrtpKeyingMaterial(
- rtc::ZeroOnFreeBuffer<unsigned char>& keying_material) override;
+ bool ExportKeyingMaterial(absl::string_view label,
+ const uint8_t* context,
+ size_t context_len,
+ bool use_context,
+ uint8_t* result,
+ size_t result_len) override;
uint16_t GetPeerSignatureAlgorithm() const override;
diff --git a/rtc_base/ssl_stream_adapter.cc b/rtc_base/ssl_stream_adapter.cc
index ce26e4f..75e3dda 100644
--- a/rtc_base/ssl_stream_adapter.cc
+++ b/rtc_base/ssl_stream_adapter.cc
@@ -87,6 +87,15 @@
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();
}
diff --git a/rtc_base/ssl_stream_adapter.h b/rtc_base/ssl_stream_adapter.h
index 5019b37..dcaaf3a 100644
--- a/rtc_base/ssl_stream_adapter.h
+++ b/rtc_base/ssl_stream_adapter.h
@@ -198,8 +198,23 @@
virtual bool GetSslVersionBytes(int* version) const = 0;
// Key Exporter interface from RFC 5705
- virtual bool ExportSrtpKeyingMaterial(
- rtc::ZeroOnFreeBuffer<unsigned char>& keying_material) = 0;
+ // Arguments are:
+ // label -- the exporter label.
+ // part of the RFC defining each exporter
+ // usage (IN)
+ // 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);
// Returns the signature algorithm or 0 if not applicable.
virtual uint16_t GetPeerSignatureAlgorithm() const = 0;
diff --git a/rtc_base/ssl_stream_adapter_unittest.cc b/rtc_base/ssl_stream_adapter_unittest.cc
index 02883df..cf42824 100644
--- a/rtc_base/ssl_stream_adapter_unittest.cc
+++ b/rtc_base/ssl_stream_adapter_unittest.cc
@@ -56,6 +56,10 @@
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`
@@ -814,6 +818,21 @@
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;
@@ -1379,23 +1398,22 @@
// Test an exporter
TEST_F(SSLStreamAdapterTestDTLS, TestDTLSExporter) {
- const std::vector<int> crypto_suites = {rtc::kSrtpAes128CmSha1_80};
- SetDtlsSrtpCryptoSuites(crypto_suites, true);
- SetDtlsSrtpCryptoSuites(crypto_suites, false);
-
TestHandshake();
- int selected_crypto_suite;
- 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));
+ unsigned char client_out[EVP_MAX_MD_SIZE];
+ unsigned char server_out[EVP_MAX_MD_SIZE];
- EXPECT_TRUE(client_ssl_->ExportSrtpKeyingMaterial(client_out));
- EXPECT_TRUE(server_ssl_->ExportSrtpKeyingMaterial(server_out));
- EXPECT_EQ(client_out, server_out);
+ bool result;
+ result = ExportKeyingMaterial(kExporterLabel, kExporterContext,
+ kExporterContextLen, true, true, client_out,
+ sizeof(client_out));
+ ASSERT_TRUE(result);
+
+ result = ExportKeyingMaterial(kExporterLabel, kExporterContext,
+ kExporterContextLen, true, false, server_out,
+ sizeof(server_out));
+ ASSERT_TRUE(result);
+
+ ASSERT_TRUE(!memcmp(client_out, server_out, sizeof(client_out)));
}
// Test not yet valid certificates are not rejected.
