diff --git a/p2p/base/dtls_transport.cc b/p2p/base/dtls_transport.cc index 60d41a8441..5ed2a11438 100644 --- a/p2p/base/dtls_transport.cc +++ b/p2p/base/dtls_transport.cc @@ -354,26 +354,16 @@ std::unique_ptr DtlsTransport::GetRemoteSSLCertChain() return dtls_->GetPeerSSLCertChain(); } -bool DtlsTransport::ExportSrtpKeyingMaterial( - rtc::ZeroOnFreeBuffer& 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 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 a317275c9d..0a988dfe38 100644 --- a/p2p/base/dtls_transport.h +++ b/p2p/base/dtls_transport.h @@ -179,16 +179,13 @@ 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. - bool ExportSrtpKeyingMaterial( - rtc::ZeroOnFreeBuffer& 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 69881dd9c6..05501458f8 100644 --- a/p2p/base/dtls_transport_internal.h +++ b/p2p/base/dtls_transport_internal.h @@ -89,16 +89,12 @@ class DtlsTransportInternal : public rtc::PacketTransportInternal { virtual std::unique_ptr GetRemoteSSLCertChain() const = 0; // Allows key material to be extracted for external encryption. - virtual bool ExportSrtpKeyingMaterial( - rtc::ZeroOnFreeBuffer& 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 c57942113a..767fc4d0c6 100644 --- a/p2p/base/dtls_transport_unittest.cc +++ b/p2p/base/dtls_transport_unittest.cc @@ -458,33 +458,6 @@ 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 client1_out(2 * (key_len + salt_len)); - rtc::ZeroOnFreeBuffer 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 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 f885165d9d..042d93b171 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 #include #include #include @@ -228,13 +227,6 @@ class FakeDtlsTransport : public DtlsTransportInternal { } return std::make_unique(remote_cert_->Clone()); } - bool ExportSrtpKeyingMaterial( - rtc::ZeroOnFreeBuffer& 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 7d1cb99061..96a7a09785 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 @@ bool DtlsSrtpTransport::ExtractParams( rtc::ZeroOnFreeBuffer 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 c60f4a7aa2..bbad2a31ac 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 @@ bool OpenSSLStreamAdapter::GetSslVersionBytes(int* version) const { return true; } -bool OpenSSLStreamAdapter::ExportSrtpKeyingMaterial( - rtc::ZeroOnFreeBuffer& 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 25ca678021..a776c1ae5d 100644 --- a/rtc_base/openssl_stream_adapter.h +++ b/rtc_base/openssl_stream_adapter.h @@ -109,8 +109,12 @@ class OpenSSLStreamAdapter final : public SSLStreamAdapter { const override; bool GetSslVersionBytes(int* version) const override; // Key Extractor interface - bool ExportSrtpKeyingMaterial( - rtc::ZeroOnFreeBuffer& 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 ce26e4f201..75e3dda66c 100644 --- a/rtc_base/ssl_stream_adapter.cc +++ b/rtc_base/ssl_stream_adapter.cc @@ -87,6 +87,15 @@ std::unique_ptr 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(); } diff --git a/rtc_base/ssl_stream_adapter.h b/rtc_base/ssl_stream_adapter.h index 5019b37eb8..dcaaf3a0d1 100644 --- a/rtc_base/ssl_stream_adapter.h +++ b/rtc_base/ssl_stream_adapter.h @@ -198,8 +198,23 @@ class SSLStreamAdapter : public StreamInterface { virtual bool GetSslVersionBytes(int* version) const = 0; // Key Exporter interface from RFC 5705 - virtual bool ExportSrtpKeyingMaterial( - rtc::ZeroOnFreeBuffer& 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 02883df97a..cf428245f3 100644 --- a/rtc_base/ssl_stream_adapter_unittest.cc +++ b/rtc_base/ssl_stream_adapter_unittest.cc @@ -56,6 +56,10 @@ 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` @@ -814,6 +818,21 @@ 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; @@ -1379,23 +1398,22 @@ TEST_F(SSLStreamAdapterTestDTLS, TestDTLSSrtpKeyAndSaltLengths) { // Test an exporter TEST_F(SSLStreamAdapterTestDTLS, TestDTLSExporter) { - const std::vector 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 client_out(2 * (key_len + salt_len)); - rtc::ZeroOnFreeBuffer 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.