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}
This commit is contained in:
deadbeef 2017-01-20 21:20:51 -08:00 committed by Commit bot
parent 2197e91860
commit 8662f94023
8 changed files with 100 additions and 16 deletions

View File

@ -293,6 +293,31 @@ static const char kSdpStringMs1Video1[] =
"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 @@ class PeerConnectionInterfaceTest : public testing::Test {
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 @@ class PeerConnectionInterfaceTest : public testing::Test {
}
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 @@ 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) {
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.

View File

@ -134,6 +134,8 @@ class FakeRTCCertificateGenerator
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 @@ class FakeRTCCertificateGenerator
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 @@ class FakeRTCCertificateGenerator
bool should_fail_;
int key_index_ = 0;
int generated_certificates_ = 0;
};
#endif // WEBRTC_API_TEST_FAKERTCCERTIFICATEGENERATOR_H_

View File

@ -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 @@ SSLFingerprint* SSLFingerprint::CreateFromRfc4572(
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)

View File

@ -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 @@ struct SSLFingerprint {
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);

View File

@ -617,12 +617,20 @@ class FakeTransportController : public TransportController {
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);

View File

@ -329,7 +329,12 @@ bool JsepTransport::ApplyLocalTransportDescription(
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(

View File

@ -262,10 +262,6 @@ 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
@ -539,16 +535,13 @@ bool TransportController::SetLocalCertificate_n(
}
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;
}

View File

@ -110,7 +110,12 @@ bool TransportDescriptionFactory::SetSecurityInfo(
// 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)) {