Only set certificate on DTLS transport if fingerprint is found in SDP.
This is used for fallback from DTLS to SDES encryption, which we probably still
want to support. Setting a certificate puts the DTLS transport in a "DTLS
enabled" mode, so it should be delayed until SDP with "a=fingerprint" is set.
BUG=webrtc:6972
Review-Url: https://codereview.webrtc.org/2641633002
Cr-Commit-Position: refs/heads/master@{#16199}
diff --git a/webrtc/api/peerconnectioninterface_unittest.cc b/webrtc/api/peerconnectioninterface_unittest.cc
index 2da3755..be3825c 100644
--- a/webrtc/api/peerconnectioninterface_unittest.cc
+++ b/webrtc/api/peerconnectioninterface_unittest.cc
@@ -293,6 +293,31 @@
"a=ssrc:4 cname:stream1\r\n"
"a=ssrc:4 msid:stream1 videotrack1\r\n";
+static const char kDtlsSdesFallbackSdp[] =
+ "v=0\r\n"
+ "o=xxxxxx 7 2 IN IP4 0.0.0.0\r\n"
+ "s=-\r\n"
+ "c=IN IP4 0.0.0.0\r\n"
+ "t=0 0\r\n"
+ "a=group:BUNDLE audio\r\n"
+ "a=msid-semantic: WMS\r\n"
+ "m=audio 1 RTP/SAVPF 0\r\n"
+ "a=sendrecv\r\n"
+ "a=rtcp-mux\r\n"
+ "a=mid:audio\r\n"
+ "a=ssrc:1 cname:stream1\r\n"
+ "a=ssrc:1 mslabel:stream1\r\n"
+ "a=ssrc:1 label:audiotrack0\r\n"
+ "a=ice-ufrag:e5785931\r\n"
+ "a=ice-pwd:36fb7878390db89481c1d46daa4278d8\r\n"
+ "a=rtpmap:0 pcmu/8000\r\n"
+ "a=fingerprint:sha-1 "
+ "4A:AD:B9:B1:3F:82:18:3B:54:02:12:DF:3E:5D:49:6B:19:E5:7C:AB\r\n"
+ "a=setup:actpass\r\n"
+ "a=crypto:1 AES_CM_128_HMAC_SHA1_32 "
+ "inline:NzB4d1BINUAvLEw6UzF3WSJ+PSdFcGdUJShpX1Zj|2^20|1:32 "
+ "dummy_session_params\r\n";
+
#define MAYBE_SKIP_TEST(feature) \
if (!(feature())) { \
LOG(LS_INFO) << "Feature disabled... skipping"; \
@@ -741,7 +766,8 @@
webrtc::MediaConstraintsInterface::kEnableDtlsSrtp,
&dtls,
nullptr) && dtls) {
- cert_generator.reset(new FakeRTCCertificateGenerator());
+ fake_certificate_generator_ = new FakeRTCCertificateGenerator();
+ cert_generator.reset(fake_certificate_generator_);
}
pc_ = pc_factory_->CreatePeerConnection(
config, constraints, std::move(port_allocator),
@@ -1135,6 +1161,7 @@
}
cricket::FakePortAllocator* port_allocator_ = nullptr;
+ FakeRTCCertificateGenerator* fake_certificate_generator_ = nullptr;
rtc::scoped_refptr<webrtc::PeerConnectionFactoryInterface> pc_factory_;
rtc::scoped_refptr<PeerConnectionFactoryForTest> pc_factory_for_test_;
rtc::scoped_refptr<PeerConnectionInterface> pc_;
@@ -2073,6 +2100,26 @@
#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) {
+ FakeConstraints constraints;
+ constraints.AddMandatory(webrtc::MediaConstraintsInterface::kEnableDtlsSrtp,
+ true);
+ CreatePeerConnection(&constraints);
+ // Wait for fake certificate to be generated. Previously, this is what caused
+ // the "a=crypto" lines to be rejected.
+ AddAudioVideoStream(kStreamLabel1, "audio_label", "video_label");
+ ASSERT_NE(nullptr, fake_certificate_generator_);
+ EXPECT_EQ_WAIT(1, fake_certificate_generator_->generated_certificates(),
+ kTimeout);
+ SessionDescriptionInterface* desc = webrtc::CreateSessionDescription(
+ SessionDescriptionInterface::kOffer, kDtlsSdesFallbackSdp, nullptr);
+ EXPECT_TRUE(DoSetSessionDescription(desc, false));
+ CreateAnswerAsLocalDescription();
+}
+
// Test that we can create an audio only offer and receive an answer with a
// limited set of audio codecs and receive an updated offer with more audio
// codecs, where the added codecs are not supported.
diff --git a/webrtc/api/test/fakertccertificategenerator.h b/webrtc/api/test/fakertccertificategenerator.h
index 34fc1c5..cd1e4bf 100644
--- a/webrtc/api/test/fakertccertificategenerator.h
+++ b/webrtc/api/test/fakertccertificategenerator.h
@@ -134,6 +134,8 @@
void use_original_key() { key_index_ = 0; }
void use_alternate_key() { key_index_ = 1; }
+ int generated_certificates() { return generated_certificates_; }
+
void GenerateCertificateAsync(
const rtc::KeyParams& key_params,
const rtc::Optional<uint64_t>& expires_ms,
@@ -210,6 +212,7 @@
msg->message_id == MSG_SUCCESS_RSA ? rtc::KT_RSA : rtc::KT_ECDSA;
certificate = rtc::RTCCertificate::FromPEM(get_pem(key_type));
RTC_DCHECK(certificate);
+ ++generated_certificates_;
callback->OnSuccess(certificate);
break;
}
@@ -222,6 +225,7 @@
bool should_fail_;
int key_index_ = 0;
+ int generated_certificates_ = 0;
};
#endif // WEBRTC_API_TEST_FAKERTCCERTIFICATEGENERATOR_H_
diff --git a/webrtc/base/sslfingerprint.cc b/webrtc/base/sslfingerprint.cc
index 2c3e1e9..e172a2c 100644
--- a/webrtc/base/sslfingerprint.cc
+++ b/webrtc/base/sslfingerprint.cc
@@ -14,6 +14,7 @@
#include <string>
#include "webrtc/base/helpers.h"
+#include "webrtc/base/logging.h"
#include "webrtc/base/messagedigest.h"
#include "webrtc/base/stringencode.h"
@@ -62,6 +63,22 @@
value_len);
}
+SSLFingerprint* SSLFingerprint::CreateFromCertificate(
+ const RTCCertificate* cert) {
+ std::string digest_alg;
+ if (!cert->ssl_certificate().GetSignatureDigestAlgorithm(&digest_alg)) {
+ LOG(LS_ERROR) << "Failed to retrieve the certificate's digest algorithm";
+ return nullptr;
+ }
+
+ SSLFingerprint* fingerprint = Create(digest_alg, cert->identity());
+ if (!fingerprint) {
+ LOG(LS_ERROR) << "Failed to create identity fingerprint, alg="
+ << digest_alg;
+ }
+ return fingerprint;
+}
+
SSLFingerprint::SSLFingerprint(const std::string& algorithm,
const uint8_t* digest_in,
size_t digest_len)
diff --git a/webrtc/base/sslfingerprint.h b/webrtc/base/sslfingerprint.h
index 4ffb2b0..62b4bc8 100644
--- a/webrtc/base/sslfingerprint.h
+++ b/webrtc/base/sslfingerprint.h
@@ -15,6 +15,7 @@
#include "webrtc/base/basictypes.h"
#include "webrtc/base/copyonwritebuffer.h"
+#include "webrtc/base/rtccertificate.h"
#include "webrtc/base/sslidentity.h"
namespace rtc {
@@ -31,6 +32,10 @@
static SSLFingerprint* CreateFromRfc4572(const std::string& algorithm,
const std::string& fingerprint);
+ // Creates a fingerprint from a certificate, using the same digest algorithm
+ // as the certificate's signature.
+ static SSLFingerprint* CreateFromCertificate(const RTCCertificate* cert);
+
SSLFingerprint(const std::string& algorithm,
const uint8_t* digest_in,
size_t digest_len);
diff --git a/webrtc/p2p/base/faketransportcontroller.h b/webrtc/p2p/base/faketransportcontroller.h
index fd47682..d97b4b6 100644
--- a/webrtc/p2p/base/faketransportcontroller.h
+++ b/webrtc/p2p/base/faketransportcontroller.h
@@ -617,12 +617,20 @@
std::vector<std::string>(),
rtc::CreateRandomString(cricket::ICE_UFRAG_LENGTH),
rtc::CreateRandomString(cricket::ICE_PWD_LENGTH),
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_NONE, nullptr);
+ cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_NONE,
+ certificate_for_testing()
+ ? rtc::SSLFingerprint::CreateFromCertificate(
+ certificate_for_testing())
+ : nullptr);
TransportDescription remote_desc(
std::vector<std::string>(),
rtc::CreateRandomString(cricket::ICE_UFRAG_LENGTH),
rtc::CreateRandomString(cricket::ICE_PWD_LENGTH),
- cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_NONE, nullptr);
+ cricket::ICEMODE_FULL, cricket::CONNECTIONROLE_NONE,
+ dest->certificate_for_testing()
+ ? rtc::SSLFingerprint::CreateFromCertificate(
+ dest->certificate_for_testing())
+ : nullptr);
std::string err;
SetLocalTransportDescription(transport_name, local_desc,
cricket::CA_OFFER, &err);
diff --git a/webrtc/p2p/base/jseptransport.cc b/webrtc/p2p/base/jseptransport.cc
index 159c238..9018a64 100644
--- a/webrtc/p2p/base/jseptransport.cc
+++ b/webrtc/p2p/base/jseptransport.cc
@@ -329,7 +329,12 @@
std::string* error_desc) {
dtls_transport->ice_transport()->SetIceParameters(
local_description_->GetIceParameters());
- return true;
+ bool ret = true;
+ if (certificate_) {
+ ret = dtls_transport->SetLocalCertificate(certificate_);
+ RTC_DCHECK(ret);
+ }
+ return ret;
}
bool JsepTransport::ApplyRemoteTransportDescription(
diff --git a/webrtc/p2p/base/transportcontroller.cc b/webrtc/p2p/base/transportcontroller.cc
index 820645b..1dc2118 100644
--- a/webrtc/p2p/base/transportcontroller.cc
+++ b/webrtc/p2p/base/transportcontroller.cc
@@ -262,10 +262,6 @@
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
@@ -539,16 +535,13 @@
}
certificate_ = certificate;
- // Set certificate both for Transport, which verifies it matches the
- // fingerprint in SDP...
+ // 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.
for (auto& kv : transports_) {
kv.second->SetLocalCertificate(certificate_);
}
- // ... and for the DTLS channel, which needs it for the DTLS handshake.
- for (auto& channel : channels_) {
- bool set_cert_success = channel->dtls()->SetLocalCertificate(certificate);
- RTC_DCHECK(set_cert_success);
- }
return true;
}
diff --git a/webrtc/p2p/base/transportdescriptionfactory.cc b/webrtc/p2p/base/transportdescriptionfactory.cc
index 61e2a9f..238e1b6 100644
--- a/webrtc/p2p/base/transportdescriptionfactory.cc
+++ b/webrtc/p2p/base/transportdescriptionfactory.cc
@@ -110,7 +110,12 @@
// This digest algorithm is used to produce the a=fingerprint lines in SDP.
// RFC 4572 Section 5 requires that those lines use the same hash function as
- // the certificate's signature.
+ // the certificate's signature, which is what CreateFromCertificate does.
+ desc->identity_fingerprint.reset(
+ rtc::SSLFingerprint::CreateFromCertificate(certificate_));
+ if (!desc->identity_fingerprint) {
+ return false;
+ }
std::string digest_alg;
if (!certificate_->ssl_certificate().GetSignatureDigestAlgorithm(
&digest_alg)) {