diff --git a/p2p/base/dtls_transport.cc b/p2p/base/dtls_transport.cc index 5ed2a11438..60d41a8441 100644 --- a/p2p/base/dtls_transport.cc +++ b/p2p/base/dtls_transport.cc @@ -354,16 +354,26 @@ 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) { - return (dtls_.get()) - ? dtls_->ExportKeyingMaterial(label, context, context_len, - use_context, result, result_len) - : false; + 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; } bool DtlsTransport::SetupDtls() { diff --git a/p2p/base/dtls_transport.h b/p2p/base/dtls_transport.h index 0a988dfe38..a317275c9d 100644 --- a/p2p/base/dtls_transport.h +++ b/p2p/base/dtls_transport.h @@ -179,13 +179,16 @@ 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 ExportKeyingMaterial(absl::string_view label, - const uint8_t* context, - size_t context_len, - bool use_context, - uint8_t* result, - size_t result_len) override; + 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; IceTransportInternal* ice_transport() override; diff --git a/p2p/base/dtls_transport_internal.h b/p2p/base/dtls_transport_internal.h index 05501458f8..69881dd9c6 100644 --- a/p2p/base/dtls_transport_internal.h +++ b/p2p/base/dtls_transport_internal.h @@ -89,12 +89,16 @@ class DtlsTransportInternal : public rtc::PacketTransportInternal { virtual std::unique_ptr GetRemoteSSLCertChain() const = 0; // Allows key material to be extracted for external encryption. - 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 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; // 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 767fc4d0c6..c57942113a 100644 --- a/p2p/base/dtls_transport_unittest.cc +++ b/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 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 042d93b171..f885165d9d 100644 --- a/p2p/base/fake_dtls_transport.h +++ b/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 #include #include #include @@ -227,6 +228,13 @@ 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 96a7a09785..7d1cb99061 100644 --- a/pc/dtls_srtp_transport.cc +++ b/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 dtls_buffer(key_len * 2 + salt_len * 2); // RFC 5705 exporter using the RFC 5764 parameters - if (!dtls_transport->ExportKeyingMaterial(kDtlsSrtpExporterLabel, NULL, 0, - false, &dtls_buffer[0], - dtls_buffer.size())) { - RTC_LOG(LS_WARNING) << "DTLS-SRTP key export failed"; + if (!dtls_transport->ExportSrtpKeyingMaterial(dtls_buffer)) { + RTC_LOG(LS_ERROR) << "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 bbad2a31ac..c60f4a7aa2 100644 --- a/rtc_base/openssl_stream_adapter.cc +++ b/rtc_base/openssl_stream_adapter.cc @@ -63,6 +63,11 @@ #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; @@ -377,16 +382,19 @@ bool OpenSSLStreamAdapter::GetSslVersionBytes(int* version) const { return true; } -// 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) { +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) { return false; } return true; diff --git a/rtc_base/openssl_stream_adapter.h b/rtc_base/openssl_stream_adapter.h index a776c1ae5d..25ca678021 100644 --- a/rtc_base/openssl_stream_adapter.h +++ b/rtc_base/openssl_stream_adapter.h @@ -109,12 +109,8 @@ class OpenSSLStreamAdapter final : public SSLStreamAdapter { const override; bool GetSslVersionBytes(int* version) const override; // Key Extractor interface - 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; + bool ExportSrtpKeyingMaterial( + rtc::ZeroOnFreeBuffer& keying_material) override; uint16_t GetPeerSignatureAlgorithm() const override; diff --git a/rtc_base/ssl_stream_adapter.cc b/rtc_base/ssl_stream_adapter.cc index 75e3dda66c..ce26e4f201 100644 --- a/rtc_base/ssl_stream_adapter.cc +++ b/rtc_base/ssl_stream_adapter.cc @@ -87,15 +87,6 @@ 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 dcaaf3a0d1..5019b37eb8 100644 --- a/rtc_base/ssl_stream_adapter.h +++ b/rtc_base/ssl_stream_adapter.h @@ -198,23 +198,8 @@ class SSLStreamAdapter : public StreamInterface { virtual bool GetSslVersionBytes(int* version) const = 0; // Key Exporter interface from RFC 5705 - // 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); + virtual bool ExportSrtpKeyingMaterial( + rtc::ZeroOnFreeBuffer& keying_material) = 0; // 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 cf428245f3..02883df97a 100644 --- a/rtc_base/ssl_stream_adapter_unittest.cc +++ b/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; @@ -1398,22 +1379,23 @@ 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(); - unsigned char client_out[EVP_MAX_MD_SIZE]; - unsigned char server_out[EVP_MAX_MD_SIZE]; + 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)); - 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))); + EXPECT_TRUE(client_ssl_->ExportSrtpKeyingMaterial(client_out)); + EXPECT_TRUE(server_ssl_->ExportSrtpKeyingMaterial(server_out)); + EXPECT_EQ(client_out, server_out); } // Test not yet valid certificates are not rejected.