Let ZeroOnFreeBuffer do the memcpy for DTLS-SRTP key extraction

and use uint8_t instead of unsigned char. Follow-up from
  https://webrtc-review.googlesource.com/c/src/+/365274

BUG=webrtc:357776213

Change-Id: Ibc97e5cc85316ba69b4133b7f3c42e3afbdd7abd
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/365540
Commit-Queue: Philipp Hancke <phancke@meta.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Jeremy Leconte <jleconte@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43263}
This commit is contained in:
Philipp Hancke 2024-10-14 14:19:03 +02:00 committed by WebRTC LUCI CQ
parent cecee51bc4
commit 03b2c9f6fc
12 changed files with 44 additions and 50 deletions

View File

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

View File

@ -180,7 +180,7 @@ class DtlsTransport : public DtlsTransportInternal {
// method extracts the keys negotiated during the DTLS handshake, for use in // method extracts the keys negotiated during the DTLS handshake, for use in
// external encryption. DTLS-SRTP uses this to extract the needed SRTP keys. // external encryption. DTLS-SRTP uses this to extract the needed SRTP keys.
bool ExportSrtpKeyingMaterial( bool ExportSrtpKeyingMaterial(
rtc::ZeroOnFreeBuffer<unsigned char>& keying_material) override; rtc::ZeroOnFreeBuffer<uint8_t>& keying_material) override;
[[deprecated("Use ExportSrtpKeyingMaterial instead")]] bool [[deprecated("Use ExportSrtpKeyingMaterial instead")]] bool
ExportKeyingMaterial(absl::string_view label, ExportKeyingMaterial(absl::string_view label,

View File

@ -90,7 +90,7 @@ class DtlsTransportInternal : public rtc::PacketTransportInternal {
// Allows key material to be extracted for external encryption. // Allows key material to be extracted for external encryption.
virtual bool ExportSrtpKeyingMaterial( virtual bool ExportSrtpKeyingMaterial(
rtc::ZeroOnFreeBuffer<unsigned char>& keying_material) = 0; rtc::ZeroOnFreeBuffer<uint8_t>& keying_material) = 0;
[[deprecated("Use ExportSrtpKeyingMaterial instead")]] virtual bool [[deprecated("Use ExportSrtpKeyingMaterial instead")]] virtual bool
ExportKeyingMaterial(absl::string_view label, ExportKeyingMaterial(absl::string_view label,

View File

@ -469,15 +469,14 @@ TEST_F(DtlsTransportTest, KeyingMaterialExporter) {
int key_len; int key_len;
int salt_len; int salt_len;
EXPECT_TRUE(rtc::GetSrtpKeyAndSaltLengths(crypto_suite, &key_len, &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<uint8_t> client1_out(2 * (key_len + salt_len));
rtc::ZeroOnFreeBuffer<unsigned char> client2_out(2 * (key_len + salt_len)); rtc::ZeroOnFreeBuffer<uint8_t> client2_out(2 * (key_len + salt_len));
EXPECT_TRUE(client1_.dtls_transport()->ExportSrtpKeyingMaterial(client1_out)); EXPECT_TRUE(client1_.dtls_transport()->ExportSrtpKeyingMaterial(client1_out));
EXPECT_TRUE(client2_.dtls_transport()->ExportSrtpKeyingMaterial(client2_out)); EXPECT_TRUE(client2_.dtls_transport()->ExportSrtpKeyingMaterial(client2_out));
EXPECT_EQ(client1_out, client2_out); EXPECT_EQ(client1_out, client2_out);
// Legacy variant using the deprecated API. // Legacy variant using the deprecated API.
rtc::ZeroOnFreeBuffer<unsigned char> client1_out_legacy(2 * rtc::ZeroOnFreeBuffer<uint8_t> client1_out_legacy(2 * (key_len + salt_len));
(key_len + salt_len));
EXPECT_TRUE(client1_.dtls_transport()->ExportKeyingMaterial( EXPECT_TRUE(client1_.dtls_transport()->ExportKeyingMaterial(
"EXTRACTOR-dtls_srtp", nullptr, 0, false, client1_out_legacy.data(), "EXTRACTOR-dtls_srtp", nullptr, 0, false, client1_out_legacy.data(),
client1_out_legacy.size())); client1_out_legacy.size()));

View File

@ -229,7 +229,7 @@ class FakeDtlsTransport : public DtlsTransportInternal {
return std::make_unique<rtc::SSLCertChain>(remote_cert_->Clone()); return std::make_unique<rtc::SSLCertChain>(remote_cert_->Clone());
} }
bool ExportSrtpKeyingMaterial( bool ExportSrtpKeyingMaterial(
rtc::ZeroOnFreeBuffer<unsigned char>& keying_material) override { rtc::ZeroOnFreeBuffer<uint8_t>& keying_material) override {
if (do_dtls_) { if (do_dtls_) {
std::memset(keying_material.data(), 0xff, keying_material.size()); std::memset(keying_material.data(), 0xff, keying_material.size());
} }

View File

@ -155,8 +155,8 @@ void DtlsSrtpTransport::SetupRtpDtlsSrtp() {
} }
int selected_crypto_suite; int selected_crypto_suite;
rtc::ZeroOnFreeBuffer<unsigned char> send_key; rtc::ZeroOnFreeBuffer<uint8_t> send_key;
rtc::ZeroOnFreeBuffer<unsigned char> recv_key; rtc::ZeroOnFreeBuffer<uint8_t> recv_key;
if (!ExtractParams(rtp_dtls_transport_, &selected_crypto_suite, &send_key, if (!ExtractParams(rtp_dtls_transport_, &selected_crypto_suite, &send_key,
&recv_key) || &recv_key) ||
@ -184,8 +184,8 @@ void DtlsSrtpTransport::SetupRtcpDtlsSrtp() {
} }
int selected_crypto_suite; int selected_crypto_suite;
rtc::ZeroOnFreeBuffer<unsigned char> rtcp_send_key; rtc::ZeroOnFreeBuffer<uint8_t> rtcp_send_key;
rtc::ZeroOnFreeBuffer<unsigned char> rtcp_recv_key; rtc::ZeroOnFreeBuffer<uint8_t> rtcp_recv_key;
if (!ExtractParams(rtcp_dtls_transport_, &selected_crypto_suite, if (!ExtractParams(rtcp_dtls_transport_, &selected_crypto_suite,
&rtcp_send_key, &rtcp_recv_key) || &rtcp_send_key, &rtcp_recv_key) ||
!SetRtcpParams(selected_crypto_suite, rtcp_send_key, send_extension_ids, !SetRtcpParams(selected_crypto_suite, rtcp_send_key, send_extension_ids,
@ -198,8 +198,8 @@ void DtlsSrtpTransport::SetupRtcpDtlsSrtp() {
bool DtlsSrtpTransport::ExtractParams( bool DtlsSrtpTransport::ExtractParams(
cricket::DtlsTransportInternal* dtls_transport, cricket::DtlsTransportInternal* dtls_transport,
int* selected_crypto_suite, int* selected_crypto_suite,
rtc::ZeroOnFreeBuffer<unsigned char>* send_key, rtc::ZeroOnFreeBuffer<uint8_t>* send_key,
rtc::ZeroOnFreeBuffer<unsigned char>* recv_key) { rtc::ZeroOnFreeBuffer<uint8_t>* recv_key) {
if (!dtls_transport || !dtls_transport->IsDtlsActive()) { if (!dtls_transport || !dtls_transport->IsDtlsActive()) {
return false; return false;
} }
@ -222,7 +222,7 @@ bool DtlsSrtpTransport::ExtractParams(
} }
// OK, we're now doing DTLS (RFC 5764) // OK, we're now doing DTLS (RFC 5764)
rtc::ZeroOnFreeBuffer<unsigned char> dtls_buffer(key_len * 2 + salt_len * 2); rtc::ZeroOnFreeBuffer<uint8_t> dtls_buffer(key_len * 2 + salt_len * 2);
// RFC 5705 exporter using the RFC 5764 parameters // RFC 5705 exporter using the RFC 5764 parameters
if (!dtls_transport->ExportSrtpKeyingMaterial(dtls_buffer)) { if (!dtls_transport->ExportSrtpKeyingMaterial(dtls_buffer)) {
@ -232,16 +232,16 @@ bool DtlsSrtpTransport::ExtractParams(
} }
// Sync up the keys with the DTLS-SRTP interface // Sync up the keys with the DTLS-SRTP interface
rtc::ZeroOnFreeBuffer<unsigned char> client_write_key(key_len + salt_len); // https://datatracker.ietf.org/doc/html/rfc5764#section-4.2
rtc::ZeroOnFreeBuffer<unsigned char> server_write_key(key_len + salt_len); // The keying material is in the format:
size_t offset = 0; // client_write_key|server_write_key|client_write_salt|server_write_salt
memcpy(&client_write_key[0], &dtls_buffer[offset], key_len); rtc::ZeroOnFreeBuffer<uint8_t> client_write_key(&dtls_buffer[0], key_len,
offset += key_len; key_len + salt_len);
memcpy(&server_write_key[0], &dtls_buffer[offset], key_len); rtc::ZeroOnFreeBuffer<uint8_t> server_write_key(&dtls_buffer[key_len],
offset += key_len; key_len, key_len + salt_len);
memcpy(&client_write_key[key_len], &dtls_buffer[offset], salt_len); client_write_key.AppendData(&dtls_buffer[key_len + key_len], salt_len);
offset += salt_len; server_write_key.AppendData(&dtls_buffer[key_len + key_len + salt_len],
memcpy(&server_write_key[key_len], &dtls_buffer[offset], salt_len); salt_len);
rtc::SSLRole role; rtc::SSLRole role;
if (!dtls_transport->GetDtlsRole(&role)) { if (!dtls_transport->GetDtlsRole(&role)) {

View File

@ -64,8 +64,8 @@ class DtlsSrtpTransport : public SrtpTransport {
void SetupRtcpDtlsSrtp(); void SetupRtcpDtlsSrtp();
bool ExtractParams(cricket::DtlsTransportInternal* dtls_transport, bool ExtractParams(cricket::DtlsTransportInternal* dtls_transport,
int* selected_crypto_suite, int* selected_crypto_suite,
rtc::ZeroOnFreeBuffer<unsigned char>* send_key, rtc::ZeroOnFreeBuffer<uint8_t>* send_key,
rtc::ZeroOnFreeBuffer<unsigned char>* recv_key); rtc::ZeroOnFreeBuffer<uint8_t>* recv_key);
void SetDtlsTransport(cricket::DtlsTransportInternal* new_dtls_transport, void SetDtlsTransport(cricket::DtlsTransportInternal* new_dtls_transport,
cricket::DtlsTransportInternal** old_dtls_transport); cricket::DtlsTransportInternal** old_dtls_transport);
void SetRtpDtlsTransport(cricket::DtlsTransportInternal* rtp_dtls_transport); void SetRtpDtlsTransport(cricket::DtlsTransportInternal* rtp_dtls_transport);

View File

@ -9,7 +9,6 @@
*/ */
#include <cstdint> #include <cstdint>
#include <memory>
#include "call/rtp_demuxer.h" #include "call/rtp_demuxer.h"
#include "media/base/fake_rtp.h" #include "media/base/fake_rtp.h"
@ -128,24 +127,20 @@ class DtlsSrtpTransportIntegrationTest : public ::testing::Test {
&salt_len)); &salt_len));
// Extract the keys. The order depends on the role! // Extract the keys. The order depends on the role!
rtc::ZeroOnFreeBuffer<unsigned char> dtls_buffer(key_len * 2 + rtc::ZeroOnFreeBuffer<uint8_t> dtls_buffer(key_len * 2 + salt_len * 2);
salt_len * 2);
ASSERT_TRUE(server_dtls_transport_->ExportSrtpKeyingMaterial(dtls_buffer)); ASSERT_TRUE(server_dtls_transport_->ExportSrtpKeyingMaterial(dtls_buffer));
rtc::ZeroOnFreeBuffer<unsigned char> send_key(key_len + salt_len); rtc::ZeroOnFreeBuffer<unsigned char> client_write_key(
rtc::ZeroOnFreeBuffer<unsigned char> recv_key(key_len + salt_len); &dtls_buffer[0], key_len, key_len + salt_len);
size_t offset = 0; rtc::ZeroOnFreeBuffer<unsigned char> server_write_key(
std::memcpy(&recv_key[0], &dtls_buffer[offset], key_len); &dtls_buffer[key_len], key_len, key_len + salt_len);
offset += key_len; client_write_key.AppendData(&dtls_buffer[key_len + key_len], salt_len);
std::memcpy(&send_key[0], &dtls_buffer[offset], key_len); server_write_key.AppendData(&dtls_buffer[key_len + key_len + salt_len],
offset += key_len; salt_len);
std::memcpy(&recv_key[key_len], &dtls_buffer[offset], salt_len);
offset += salt_len;
std::memcpy(&send_key[key_len], &dtls_buffer[offset], salt_len);
EXPECT_TRUE(srtp_transport_.SetRtpParams(selected_crypto_suite, send_key, EXPECT_TRUE(srtp_transport_.SetRtpParams(
{}, selected_crypto_suite, selected_crypto_suite, server_write_key, {}, selected_crypto_suite,
recv_key, {})); client_write_key, {}));
} }
rtc::CopyOnWriteBuffer CreateRtpPacket() { rtc::CopyOnWriteBuffer CreateRtpPacket() {

View File

@ -384,7 +384,7 @@ bool OpenSSLStreamAdapter::GetSslVersionBytes(int* version) const {
} }
bool OpenSSLStreamAdapter::ExportSrtpKeyingMaterial( bool OpenSSLStreamAdapter::ExportSrtpKeyingMaterial(
rtc::ZeroOnFreeBuffer<unsigned char>& keying_material) { rtc::ZeroOnFreeBuffer<uint8_t>& keying_material) {
// Arguments are: // Arguments are:
// keying material/len -- a buffer to hold the keying material. // keying material/len -- a buffer to hold the keying material.
// label -- the exporter label. // label -- the exporter label.

View File

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

View File

@ -199,7 +199,7 @@ class SSLStreamAdapter : public StreamInterface {
// Key Exporter interface from RFC 5705 // Key Exporter interface from RFC 5705
virtual bool ExportSrtpKeyingMaterial( virtual bool ExportSrtpKeyingMaterial(
rtc::ZeroOnFreeBuffer<unsigned char>& keying_material) = 0; rtc::ZeroOnFreeBuffer<uint8_t>& keying_material) = 0;
[[deprecated("Use ExportSrtpKeyingMaterial instead")]] virtual bool [[deprecated("Use ExportSrtpKeyingMaterial instead")]] virtual bool
ExportKeyingMaterial(absl::string_view label, ExportKeyingMaterial(absl::string_view label,
const uint8_t* context, const uint8_t* context,

View File

@ -1392,15 +1392,15 @@ TEST_F(SSLStreamAdapterTestDTLS, TestDTLSExporter) {
int salt_len; int salt_len;
ASSERT_TRUE(rtc::GetSrtpKeyAndSaltLengths(selected_crypto_suite, &key_len, ASSERT_TRUE(rtc::GetSrtpKeyAndSaltLengths(selected_crypto_suite, &key_len,
&salt_len)); &salt_len));
rtc::ZeroOnFreeBuffer<unsigned char> client_out(2 * (key_len + salt_len)); rtc::ZeroOnFreeBuffer<uint8_t> client_out(2 * (key_len + salt_len));
rtc::ZeroOnFreeBuffer<unsigned char> server_out(2 * (key_len + salt_len)); rtc::ZeroOnFreeBuffer<uint8_t> server_out(2 * (key_len + salt_len));
EXPECT_TRUE(client_ssl_->ExportSrtpKeyingMaterial(client_out)); EXPECT_TRUE(client_ssl_->ExportSrtpKeyingMaterial(client_out));
EXPECT_TRUE(server_ssl_->ExportSrtpKeyingMaterial(server_out)); EXPECT_TRUE(server_ssl_->ExportSrtpKeyingMaterial(server_out));
EXPECT_EQ(client_out, server_out); EXPECT_EQ(client_out, server_out);
// Legacy variant. // Legacy variant.
rtc::ZeroOnFreeBuffer<unsigned char> legacy_out(2 * (key_len + salt_len)); rtc::ZeroOnFreeBuffer<uint8_t> legacy_out(2 * (key_len + salt_len));
EXPECT_TRUE(client_ssl_->ExportKeyingMaterial("EXTRACTOR-dtls_srtp", nullptr, EXPECT_TRUE(client_ssl_->ExportKeyingMaterial("EXTRACTOR-dtls_srtp", nullptr,
0, false, legacy_out.data(), 0, false, legacy_out.data(),
legacy_out.size())); legacy_out.size()));