From b19012e6cce70380135e20e29a6631b0ab14f48d Mon Sep 17 00:00:00 2001 From: zhihuang Date: Tue, 19 Sep 2017 13:47:59 -0700 Subject: [PATCH] Remove the support of fallback from DTLS to SDES. The support of fallback from DTLS to SDES is removed in this CL. Setting an SDP with both DTLS fingerprint and SDES crypto would fail. BUG=webrtc:8266 Review-Url: https://codereview.webrtc.org/3011133002 Cr-Commit-Position: refs/heads/master@{#19903} --- p2p/base/jseptransport.cc | 7 +------ p2p/base/transportcontroller.cc | 13 ++++++++++--- p2p/base/transportdescriptionfactory.cc | 14 -------------- pc/peerconnectioninterface_unittest.cc | 11 +++++------ 4 files changed, 16 insertions(+), 29 deletions(-) diff --git a/p2p/base/jseptransport.cc b/p2p/base/jseptransport.cc index cd564b20aa..74094fa07f 100644 --- a/p2p/base/jseptransport.cc +++ b/p2p/base/jseptransport.cc @@ -330,12 +330,7 @@ bool JsepTransport::ApplyLocalTransportDescription( std::string* error_desc) { dtls_transport->ice_transport()->SetIceParameters( local_description_->GetIceParameters()); - bool ret = true; - if (certificate_) { - ret = dtls_transport->SetLocalCertificate(certificate_); - RTC_DCHECK(ret); - } - return ret; + return true; } bool JsepTransport::ApplyRemoteTransportDescription( diff --git a/p2p/base/transportcontroller.cc b/p2p/base/transportcontroller.cc index cfe43d23d4..12ea94fcd4 100644 --- a/p2p/base/transportcontroller.cc +++ b/p2p/base/transportcontroller.cc @@ -257,6 +257,10 @@ DtlsTransportInternal* TransportController::CreateDtlsTransport_n( dtls->ice_transport()->SetIceRole(ice_role_); dtls->ice_transport()->SetIceTiebreaker(ice_tiebreaker_); dtls->ice_transport()->SetIceConfig(ice_config_); + if (certificate_) { + bool set_cert_success = dtls->SetLocalCertificate(certificate_); + RTC_DCHECK(set_cert_success); + } // Connect to signals offered by the channels. Currently, the DTLS channel // forwards signals from the ICE channel, so we only need to connect to the @@ -535,12 +539,15 @@ bool TransportController::SetLocalCertificate_n( certificate_ = certificate; // Set certificate for JsepTransport, which verifies it matches the - // fingerprint in SDP, and only applies it to the DTLS transport if a - // fingerprint attribute is present in SDP. This is used for fallback from - // DTLS to SDES. + // fingerprint in SDP, and DTLS transport. + // Fallback from DTLS to SDES is not supported. for (auto& kv : transports_) { kv.second->SetLocalCertificate(certificate_); } + for (auto& channel : channels_) { + bool set_cert_success = channel->dtls()->SetLocalCertificate(certificate_); + RTC_DCHECK(set_cert_success); + } return true; } diff --git a/p2p/base/transportdescriptionfactory.cc b/p2p/base/transportdescriptionfactory.cc index 78fbf541d1..69663c4eb5 100644 --- a/p2p/base/transportdescriptionfactory.cc +++ b/p2p/base/transportdescriptionfactory.cc @@ -119,20 +119,6 @@ bool TransportDescriptionFactory::SetSecurityInfo( if (!desc->identity_fingerprint) { return false; } - std::string digest_alg; - if (!certificate_->ssl_certificate().GetSignatureDigestAlgorithm( - &digest_alg)) { - LOG(LS_ERROR) << "Failed to retrieve the certificate's digest algorithm"; - return false; - } - - desc->identity_fingerprint.reset( - rtc::SSLFingerprint::Create(digest_alg, certificate_->identity())); - if (!desc->identity_fingerprint.get()) { - LOG(LS_ERROR) << "Failed to create identity fingerprint, alg=" - << digest_alg; - return false; - } // Assign security role. desc->connection_role = role; diff --git a/pc/peerconnectioninterface_unittest.cc b/pc/peerconnectioninterface_unittest.cc index a3cf3159a8..8154b2d4d5 100644 --- a/pc/peerconnectioninterface_unittest.cc +++ b/pc/peerconnectioninterface_unittest.cc @@ -2249,10 +2249,10 @@ TEST_F(PeerConnectionInterfaceTest, ReceiveFireFoxOffer) { #endif } -// Test that an offer can be received which offers DTLS with SDES fallback. -// Regression test for issue: -// https://bugs.chromium.org/p/webrtc/issues/detail?id=6972 -TEST_F(PeerConnectionInterfaceTest, ReceiveDtlsSdesFallbackOffer) { +// Test that fallback from DTLS to SDES is not supported. +// The fallback was previously supported but was removed to simplify the code +// and because it's non-standard. +TEST_F(PeerConnectionInterfaceTest, DtlsSdesFallbackNotSupported) { FakeConstraints constraints; constraints.AddMandatory(webrtc::MediaConstraintsInterface::kEnableDtlsSrtp, true); @@ -2266,8 +2266,7 @@ TEST_F(PeerConnectionInterfaceTest, ReceiveDtlsSdesFallbackOffer) { std::unique_ptr desc( webrtc::CreateSessionDescription(SessionDescriptionInterface::kOffer, kDtlsSdesFallbackSdp, nullptr)); - EXPECT_TRUE(DoSetSessionDescription(std::move(desc), false)); - CreateAnswerAsLocalDescription(); + EXPECT_FALSE(DoSetSessionDescription(std::move(desc), false)); } // Test that we can create an audio only offer and receive an answer with a