admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Reland "dtls: allow dtls role to change during DTLS restart"

Browse Source 
This is a reland of commit 02b5f3c9c12cddf3fc6e9125238b77ddb44f3b53
without making SetRemoteFingerprint private (but adding a deprecation warning)

Original change's description:
> dtls: allow dtls role to change during DTLS restart
>
> which is characterized by a change in remote fingerprint and
> causes a new DTLS handshake. This allows renegotiating the
> client/server role as well.
> Spec guidance is provided by
>   https://www.rfc-editor.org/rfc/rfc5763#section-6.6
>
> BUG=webrtc:5768
>
> Change-Id: I0e8630c0c5907cc92720762a4320ad21a6190d28
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/271680
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Commit-Queue: Philipp Hancke <phancke@microsoft.com>
> Cr-Commit-Position: refs/heads/main@{#37821}

Bug: webrtc:5768
Change-Id: I8dd674db8b683160013e1b4aa7776775d130978f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/272221
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#37838}
This commit is contained in:
Philipp Hancke 2022-08-18 14:48:21 +02:00 committed by WebRTC LUCI CQ
parent bc8a62b244
commit 4a3b5ccfd5
7 changed files with 122 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
p2p/base/dtls_transport.cc
View File

@ -227,6 +227,34 @@ bool DtlsTransport::GetSslCipherSuite(int* cipher) {
return dtls_->GetSslCipherSuite(cipher);
}
webrtc::RTCError DtlsTransport::SetRemoteParameters(
absl::string_view digest_alg,
const uint8_t* digest,
size_t digest_len,
absl::optional<rtc::SSLRole> role) {
rtc::Buffer remote_fingerprint_value(digest, digest_len);
bool is_dtls_restart =
dtls_active_ && remote_fingerprint_value_ != remote_fingerprint_value;
// Set SSL role. Role must be set before fingerprint is applied, which
// initiates DTLS setup.
if (role) {
if (is_dtls_restart) {
dtls_role_ = *role;
} else {
if (!SetDtlsRole(*role)) {
return webrtc::RTCError(webrtc::RTCErrorType::INVALID_PARAMETER,
"Failed to set SSL role for the transport.");
}
}
}
// Apply remote fingerprint.
if (!SetRemoteFingerprint(digest_alg, digest, digest_len)) {
return webrtc::RTCError(webrtc::RTCErrorType::INVALID_PARAMETER,
"Failed to apply remote fingerprint.");
}
return webrtc::RTCError::OK();
}
bool DtlsTransport::SetRemoteFingerprint(absl::string_view digest_alg,
const uint8_t* digest,
size_t digest_len) {

7
p2p/base/dtls_transport.h
View File

@ -140,6 +140,13 @@ class DtlsTransport : public DtlsTransportInternal {
const uint8_t* digest,
size_t digest_len) override;
// SetRemoteParameters must be called after SetLocalCertificate.
webrtc::RTCError SetRemoteParameters(
absl::string_view digest_alg,
const uint8_t* digest,
size_t digest_len,
absl::optional<rtc::SSLRole> role) override;
// Called to send a packet (via DTLS, if turned on).
int SendPacket(const char* data,
size_t size,

8
p2p/base/dtls_transport_internal.h
View File

@ -90,10 +90,18 @@ class DtlsTransportInternal : public rtc::PacketTransportInternal {
size_t result_len) = 0;
// Set DTLS remote fingerprint. Must be after local identity set.
ABSL_DEPRECATED("Use SetRemoteParameters instead.")
virtual bool SetRemoteFingerprint(absl::string_view digest_alg,
const uint8_t* digest,
size_t digest_len) = 0;
// Set DTLS remote fingerprint and role. Must be after local identity set.
virtual webrtc::RTCError SetRemoteParameters(
absl::string_view digest_alg,
const uint8_t* digest,
size_t digest_len,
absl::optional<rtc::SSLRole> role) = 0;
ABSL_DEPRECATED("Set the max version via construction.")
bool SetSslMaxProtocolVersion(rtc::SSLProtocolVersion version) {
return true;

14
p2p/base/dtls_transport_unittest.cc
View File

@ -56,11 +56,15 @@ void SetRemoteFingerprintFromCert(
if (modify_digest) {
++fingerprint->digest.MutableData()[0];
}
// Even if digest is verified to be incorrect, should fail asynchrnously.
EXPECT_TRUE(transport->SetRemoteFingerprint(
fingerprint->algorithm,
reinterpret_cast<const uint8_t*>(fingerprint->digest.data()),
fingerprint->digest.size()));
// Even if digest is verified to be incorrect, should fail asynchronously.
EXPECT_TRUE(
transport
->SetRemoteParameters(
fingerprint->algorithm,
reinterpret_cast<const uint8_t*>(fingerprint->digest.data()),
fingerprint->digest.size(), absl::nullopt)
.ok());
}
class DtlsTestClient : public sigslot::has_slots<> {

12
p2p/base/fake_dtls_transport.h
View File

@ -141,9 +141,19 @@ class FakeDtlsTransport : public DtlsTransportInternal {
const rtc::SSLFingerprint& dtls_fingerprint() const {
return dtls_fingerprint_;
}
webrtc::RTCError SetRemoteParameters(absl::string_view alg,
const uint8_t* digest,
size_t digest_len,
absl::optional<rtc::SSLRole> role) {
if (role) {
SetDtlsRole(*role);
}
SetRemoteFingerprint(alg, digest, digest_len);
return webrtc::RTCError::OK();
}
bool SetRemoteFingerprint(absl::string_view alg,
const uint8_t* digest,
size_t digest_len) override {
size_t digest_len) {
dtls_fingerprint_ =
rtc::SSLFingerprint(alg, rtc::MakeArrayView(digest, digest_len));
return true;

18
pc/jsep_transport.cc
View File

@ -420,21 +420,9 @@ webrtc::RTCError JsepTransport::SetNegotiatedDtlsParameters(
absl::optional<rtc::SSLRole> dtls_role,
rtc::SSLFingerprint* remote_fingerprint) {
RTC_DCHECK(dtls_transport);
// Set SSL role. Role must be set before fingerprint is applied, which
// initiates DTLS setup.
if (dtls_role && !dtls_transport->SetDtlsRole(*dtls_role)) {
return webrtc::RTCError(webrtc::RTCErrorType::INVALID_PARAMETER,
"Failed to set SSL role for the transport.");
}
// Apply remote fingerprint.
if (!remote_fingerprint ||
!dtls_transport->SetRemoteFingerprint(
remote_fingerprint->algorithm, remote_fingerprint->digest.cdata(),
remote_fingerprint->digest.size())) {
return webrtc::RTCError(webrtc::RTCErrorType::INVALID_PARAMETER,
"Failed to apply remote fingerprint.");
}
return webrtc::RTCError::OK();
return dtls_transport->SetRemoteParameters(
remote_fingerprint->algorithm, remote_fingerprint->digest.cdata(),
remote_fingerprint->digest.size(), dtls_role);
}
bool JsepTransport::SetRtcpMux(bool enable,

57
pc/jsep_transport_unittest.cc
View File

@ -881,6 +881,61 @@ TEST_F(JsepTransport2Test, RemoteOfferThatChangesNegotiatedDtlsRole) {
.ok());
}
// Test that a remote offer which changes both fingerprint and role is accepted.
TEST_F(JsepTransport2Test, RemoteOfferThatChangesFingerprintAndDtlsRole) {
rtc::scoped_refptr<rtc::RTCCertificate> certificate =
rtc::RTCCertificate::Create(
rtc::SSLIdentity::Create("testing1", rtc::KT_ECDSA));
rtc::scoped_refptr<rtc::RTCCertificate> certificate2 =
rtc::RTCCertificate::Create(
rtc::SSLIdentity::Create("testing2", rtc::KT_ECDSA));
bool rtcp_mux_enabled = true;
jsep_transport_ = CreateJsepTransport2(rtcp_mux_enabled, SrtpMode::kDtlsSrtp);
jsep_transport_->SetLocalCertificate(certificate);
JsepTransportDescription remote_desc =
MakeJsepTransportDescription(rtcp_mux_enabled, kIceUfrag1, kIcePwd1,
certificate, CONNECTIONROLE_ACTPASS);
JsepTransportDescription remote_desc2 =
MakeJsepTransportDescription(rtcp_mux_enabled, kIceUfrag1, kIcePwd1,
certificate2, CONNECTIONROLE_ACTPASS);
JsepTransportDescription local_desc =
MakeJsepTransportDescription(rtcp_mux_enabled, kIceUfrag2, kIcePwd2,
certificate, CONNECTIONROLE_ACTIVE);
// Normal initial offer/answer with "actpass" in the offer and "active" in
// the answer.
ASSERT_TRUE(
jsep_transport_
->SetRemoteJsepTransportDescription(remote_desc, SdpType::kOffer)
.ok());
ASSERT_TRUE(
jsep_transport_
->SetLocalJsepTransportDescription(local_desc, SdpType::kAnswer)
.ok());
// Sanity check that role was actually negotiated.
absl::optional<rtc::SSLRole> role = jsep_transport_->GetDtlsRole();
ASSERT_TRUE(role);
EXPECT_EQ(rtc::SSL_CLIENT, *role);
// Subsequent exchange with new remote fingerprint and different role.
local_desc.transport_desc.connection_role = CONNECTIONROLE_PASSIVE;
EXPECT_TRUE(
jsep_transport_
->SetRemoteJsepTransportDescription(remote_desc2, SdpType::kOffer)
.ok());
EXPECT_TRUE(
jsep_transport_
->SetLocalJsepTransportDescription(local_desc, SdpType::kAnswer)
.ok());
role = jsep_transport_->GetDtlsRole();
ASSERT_TRUE(role);
EXPECT_EQ(rtc::SSL_SERVER, *role);
}
// Testing that a legacy client that doesn't use the setup attribute will be
// interpreted as having an active role.
TEST_F(JsepTransport2Test, DtlsSetupWithLegacyAsAnswerer) {
@ -912,7 +967,7 @@ TEST_F(JsepTransport2Test, DtlsSetupWithLegacyAsAnswerer) {
absl::optional<rtc::SSLRole> role = jsep_transport_->GetDtlsRole();
ASSERT_TRUE(role);
// Since legacy answer ommitted setup atribute, and we offered actpass, we
// Since legacy answer omitted setup atribute, and we offered actpass, we
// should act as passive (server).
EXPECT_EQ(rtc::SSL_SERVER, *role);
}