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}
diff --git a/p2p/base/dtls_transport.cc b/p2p/base/dtls_transport.cc
index 2ebc983..904a0cb 100644
--- a/p2p/base/dtls_transport.cc
+++ b/p2p/base/dtls_transport.cc
@@ -227,6 +227,34 @@
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) {
diff --git a/p2p/base/dtls_transport.h b/p2p/base/dtls_transport.h
index a78226c..24ac16e 100644
--- a/p2p/base/dtls_transport.h
+++ b/p2p/base/dtls_transport.h
@@ -133,12 +133,12 @@
const rtc::scoped_refptr<rtc::RTCCertificate>& certificate) override;
rtc::scoped_refptr<rtc::RTCCertificate> GetLocalCertificate() const override;
- // SetRemoteFingerprint must be called after SetLocalCertificate, and any
- // other methods like SetDtlsRole. It's what triggers the actual DTLS setup.
- // TODO(deadbeef): Rename to "Start" like in ORTC?
- bool SetRemoteFingerprint(absl::string_view digest_alg,
- 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,
@@ -226,6 +226,13 @@
// Sets the DTLS state, signaling if necessary.
void set_dtls_state(webrtc::DtlsTransportState state);
+ // SetRemoteFingerprint must be called after SetLocalCertificate, and any
+ // other methods like SetDtlsRole. It's what triggers the actual DTLS setup.
+ // TODO(deadbeef): Rename to "Start" like in ORTC?
+ bool SetRemoteFingerprint(absl::string_view digest_alg,
+ const uint8_t* digest,
+ size_t digest_len);
+
webrtc::SequenceChecker thread_checker_;
const int component_;
diff --git a/p2p/base/dtls_transport_internal.h b/p2p/base/dtls_transport_internal.h
index 061d1e9..d975530 100644
--- a/p2p/base/dtls_transport_internal.h
+++ b/p2p/base/dtls_transport_internal.h
@@ -89,10 +89,12 @@
uint8_t* result,
size_t result_len) = 0;
- // Set DTLS remote fingerprint. Must be after local identity set.
- 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) {
diff --git a/p2p/base/dtls_transport_unittest.cc b/p2p/base/dtls_transport_unittest.cc
index cb04bf7..69c36f8 100644
--- a/p2p/base/dtls_transport_unittest.cc
+++ b/p2p/base/dtls_transport_unittest.cc
@@ -56,11 +56,14 @@
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<> {
diff --git a/p2p/base/fake_dtls_transport.h b/p2p/base/fake_dtls_transport.h
index 23d4710..571c0ed 100644
--- a/p2p/base/fake_dtls_transport.h
+++ b/p2p/base/fake_dtls_transport.h
@@ -141,12 +141,15 @@
const rtc::SSLFingerprint& dtls_fingerprint() const {
return dtls_fingerprint_;
}
- bool SetRemoteFingerprint(absl::string_view alg,
- const uint8_t* digest,
- size_t digest_len) override {
- dtls_fingerprint_ =
- rtc::SSLFingerprint(alg, rtc::MakeArrayView(digest, digest_len));
- return true;
+ 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 SetDtlsRole(rtc::SSLRole role) override {
dtls_role_ = std::move(role);
@@ -283,6 +286,14 @@
SignalNetworkRouteChanged(network_route);
}
+ bool SetRemoteFingerprint(absl::string_view alg,
+ const uint8_t* digest,
+ size_t digest_len) {
+ dtls_fingerprint_ =
+ rtc::SSLFingerprint(alg, rtc::MakeArrayView(digest, digest_len));
+ return true;
+ }
+
FakeIceTransport* ice_transport_;
std::unique_ptr<FakeIceTransport> owned_ice_transport_;
std::string transport_name_;
diff --git a/pc/jsep_transport.cc b/pc/jsep_transport.cc
index a2ac757..dad415b 100644
--- a/pc/jsep_transport.cc
+++ b/pc/jsep_transport.cc
@@ -420,21 +420,9 @@
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,
diff --git a/pc/jsep_transport_unittest.cc b/pc/jsep_transport_unittest.cc
index d0cdb75..f057d37 100644
--- a/pc/jsep_transport_unittest.cc
+++ b/pc/jsep_transport_unittest.cc
@@ -881,6 +881,61 @@
.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 @@
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);
}
