Index: webrtc/p2p/base/dtlstransportchannel_unittest.cc |
diff --git a/webrtc/p2p/base/dtlstransportchannel_unittest.cc b/webrtc/p2p/base/dtlstransportchannel_unittest.cc |
index 705df2d95f0d6e28a37a1e38f63feaf087969b68..1481c7cc1eb73f7acee0b4133bfff31a2e0a5ade 100644 |
--- a/webrtc/p2p/base/dtlstransportchannel_unittest.cc |
+++ b/webrtc/p2p/base/dtlstransportchannel_unittest.cc |
@@ -124,74 +124,110 @@ class DtlsTestClient : public sigslot::has_slots<> { |
local_role, remote_role, flags); |
} |
- // Allow any DTLS configuration to be specified (including invalid ones). |
- void Negotiate(const rtc::scoped_refptr<rtc::RTCCertificate>& local_cert, |
- const rtc::scoped_refptr<rtc::RTCCertificate>& remote_cert, |
- cricket::ContentAction action, |
- ConnectionRole local_role, |
- ConnectionRole remote_role, |
- int flags) { |
- std::unique_ptr<rtc::SSLFingerprint> local_fingerprint; |
- std::unique_ptr<rtc::SSLFingerprint> remote_fingerprint; |
- if (local_cert) { |
- std::string digest_algorithm; |
- ASSERT_TRUE(local_cert->ssl_certificate().GetSignatureDigestAlgorithm( |
- &digest_algorithm)); |
- ASSERT_FALSE(digest_algorithm.empty()); |
- local_fingerprint.reset(rtc::SSLFingerprint::Create( |
- digest_algorithm, local_cert->identity())); |
- ASSERT_TRUE(local_fingerprint.get() != NULL); |
- EXPECT_EQ(rtc::DIGEST_SHA_256, digest_algorithm); |
- } |
- if (remote_cert) { |
+ std::unique_ptr<rtc::SSLFingerprint> MakeFingerprintFromCertificate( |
+ const rtc::scoped_refptr<rtc::RTCCertificate>& cert) { |
+ std::unique_ptr<rtc::SSLFingerprint> fingerprint; |
+ if (cert) { |
pthatcher1
2016/07/13 00:04:19
Maybe have it be early return?
if (!cert) {
ret
Taylor Brandstetter
2016/07/13 16:17:41
Done.
|
std::string digest_algorithm; |
- ASSERT_TRUE(remote_cert->ssl_certificate().GetSignatureDigestAlgorithm( |
+ EXPECT_TRUE(cert->ssl_certificate().GetSignatureDigestAlgorithm( |
&digest_algorithm)); |
- ASSERT_FALSE(digest_algorithm.empty()); |
- remote_fingerprint.reset(rtc::SSLFingerprint::Create( |
- digest_algorithm, remote_cert->identity())); |
- ASSERT_TRUE(remote_fingerprint.get() != NULL); |
+ EXPECT_FALSE(digest_algorithm.empty()); |
+ fingerprint.reset( |
+ rtc::SSLFingerprint::Create(digest_algorithm, cert->identity())); |
+ EXPECT_TRUE(fingerprint.get() != NULL); |
EXPECT_EQ(rtc::DIGEST_SHA_256, digest_algorithm); |
} |
+ return fingerprint; |
+ } |
- if (use_dtls_srtp_ && !(flags & NF_REOFFER)) { |
- // SRTP ciphers will be set only in the beginning. |
- for (std::vector<cricket::DtlsTransportChannelWrapper*>::iterator it = |
- channels_.begin(); it != channels_.end(); ++it) { |
- std::vector<int> ciphers; |
- ciphers.push_back(rtc::SRTP_AES128_CM_SHA1_80); |
- ASSERT_TRUE((*it)->SetSrtpCryptoSuites(ciphers)); |
- } |
- } |
- |
+ void SetLocalTransportDescription( |
+ const rtc::scoped_refptr<rtc::RTCCertificate>& cert, |
+ cricket::ContentAction action, |
+ ConnectionRole role, |
+ int flags) { |
+ auto fingerprint = MakeFingerprintFromCertificate(cert); |
cricket::TransportDescription local_desc( |
std::vector<std::string>(), kIceUfrag1, kIcePwd1, cricket::ICEMODE_FULL, |
- local_role, |
- // If remote if the offerer and has no DTLS support, answer will be |
- // without any fingerprint. |
- (action == cricket::CA_ANSWER && !remote_cert) |
- ? nullptr |
- : local_fingerprint.get()); |
+ role, fingerprint.get()); |
+ |
+ if (action == cricket::CA_OFFER) { |
+ if (use_dtls_srtp_ && !(flags & NF_REOFFER)) { |
+ // SRTP ciphers will be set only in the beginning. |
+ for (std::vector<cricket::DtlsTransportChannelWrapper*>::iterator it = |
+ channels_.begin(); |
+ it != channels_.end(); ++it) { |
pthatcher1
2016/07/13 00:04:19
c++11 style for loop?
Taylor Brandstetter
2016/07/13 16:17:41
Done.
|
+ std::vector<int> ciphers; |
+ ciphers.push_back(rtc::SRTP_AES128_CM_SHA1_80); |
+ EXPECT_TRUE((*it)->SetSrtpCryptoSuites(ciphers)); |
+ } |
+ } |
+ |
+ EXPECT_TRUE(transport_->SetLocalTransportDescription(local_desc, action, |
+ nullptr)); |
+ } else { |
+ // If |expect_success| is false, expect SRTD or SLTD to fail when |
+ // content action is CA_ANSWER. |
+ bool expect_success = (flags & NF_EXPECT_FAILURE) ? false : true; |
+ EXPECT_EQ(expect_success, transport_->SetLocalTransportDescription( |
+ local_desc, action, nullptr)); |
+ } |
+ set_local_cert_ = (cert != nullptr); |
pthatcher1
2016/07/13 00:04:20
Would has_local_cert_ be a better name?
Taylor Brandstetter
2016/07/13 16:17:41
The method is called SetLocalTransportDescription
Taylor Brandstetter
2016/07/13 16:20:16
Oh I misread your comment. I thought you were aski
|
+ } |
+ void SetRemoteTransportDescription( |
+ const rtc::scoped_refptr<rtc::RTCCertificate>& cert, |
+ cricket::ContentAction action, |
+ ConnectionRole role, |
+ int flags) { |
+ auto fingerprint = MakeFingerprintFromCertificate(cert); |
cricket::TransportDescription remote_desc( |
std::vector<std::string>(), kIceUfrag1, kIcePwd1, cricket::ICEMODE_FULL, |
- remote_role, remote_fingerprint.get()); |
+ role, fingerprint.get()); |
- bool expect_success = (flags & NF_EXPECT_FAILURE) ? false : true; |
- // If |expect_success| is false, expect SRTD or SLTD to fail when |
- // content action is CA_ANSWER. |
if (action == cricket::CA_OFFER) { |
- ASSERT_TRUE(transport_->SetLocalTransportDescription( |
- local_desc, cricket::CA_OFFER, NULL)); |
- ASSERT_EQ(expect_success, transport_->SetRemoteTransportDescription( |
- remote_desc, cricket::CA_ANSWER, NULL)); |
+ if (use_dtls_srtp_ && !(flags & NF_REOFFER)) { |
+ // SRTP ciphers will be set only in the beginning. |
+ for (std::vector<cricket::DtlsTransportChannelWrapper*>::iterator it = |
+ channels_.begin(); |
+ it != channels_.end(); ++it) { |
+ std::vector<int> ciphers; |
+ ciphers.push_back(rtc::SRTP_AES128_CM_SHA1_80); |
+ EXPECT_TRUE((*it)->SetSrtpCryptoSuites(ciphers)); |
+ } |
+ } |
pthatcher1
2016/07/13 00:04:19
It seems like a MakeTransportDescription(action, c
Taylor Brandstetter
2016/07/13 16:17:41
Setting the ciphers you mean? I'll make a separate
|
+ |
+ EXPECT_TRUE(transport_->SetRemoteTransportDescription(remote_desc, action, |
+ nullptr)); |
} else { |
- ASSERT_TRUE(transport_->SetRemoteTransportDescription( |
- remote_desc, cricket::CA_OFFER, NULL)); |
- ASSERT_EQ(expect_success, transport_->SetLocalTransportDescription( |
- local_desc, cricket::CA_ANSWER, NULL)); |
+ // If |expect_success| is false, expect SRTD or SLTD to fail when |
+ // content action is CA_ANSWER. |
+ bool expect_success = (flags & NF_EXPECT_FAILURE) ? false : true; |
+ EXPECT_EQ(expect_success, transport_->SetRemoteTransportDescription( |
+ remote_desc, action, nullptr)); |
+ } |
+ set_remote_cert_ = (cert != nullptr); |
+ } |
+ |
+ // Allow any DTLS configuration to be specified (including invalid ones). |
+ void Negotiate(const rtc::scoped_refptr<rtc::RTCCertificate>& local_cert, |
+ const rtc::scoped_refptr<rtc::RTCCertificate>& remote_cert, |
+ cricket::ContentAction action, |
+ ConnectionRole local_role, |
+ ConnectionRole remote_role, |
+ int flags) { |
+ if (action == cricket::CA_OFFER) { |
+ SetLocalTransportDescription(local_cert, cricket::CA_OFFER, local_role, |
+ flags); |
+ SetRemoteTransportDescription(remote_cert, cricket::CA_ANSWER, |
+ remote_role, flags); |
+ } else { |
+ SetRemoteTransportDescription(remote_cert, cricket::CA_OFFER, remote_role, |
+ flags); |
+ // If remote if the offerer and has no DTLS support, answer will be |
+ // without any fingerprint. |
+ SetLocalTransportDescription(remote_cert ? local_cert : nullptr, |
+ cricket::CA_ANSWER, local_role, flags); |
} |
- negotiated_dtls_ = (local_cert && remote_cert); |
} |
bool Connect(DtlsTestClient* peer, bool asymmetric) { |
@@ -227,6 +263,8 @@ class DtlsTestClient : public sigslot::has_slots<> { |
return received_dtls_client_hellos_; |
} |
+ bool negotiated_dtls() const { return set_local_cert_ && set_remote_cert_; } |
+ |
void CheckRole(rtc::SSLRole role) { |
if (role == rtc::SSL_CLIENT) { |
ASSERT_EQ(0, received_dtls_client_hellos_); |
@@ -243,7 +281,7 @@ class DtlsTestClient : public sigslot::has_slots<> { |
int crypto_suite; |
bool rv = (*it)->GetSrtpCryptoSuite(&crypto_suite); |
- if (negotiated_dtls_ && expected_crypto_suite) { |
+ if (negotiated_dtls() && expected_crypto_suite) { |
ASSERT_TRUE(rv); |
ASSERT_EQ(crypto_suite, expected_crypto_suite); |
@@ -259,7 +297,7 @@ class DtlsTestClient : public sigslot::has_slots<> { |
int cipher; |
bool rv = (*it)->GetSslCipherSuite(&cipher); |
- if (negotiated_dtls_) { |
+ if (negotiated_dtls()) { |
ASSERT_TRUE(rv); |
EXPECT_TRUE( |
@@ -388,7 +426,7 @@ class DtlsTestClient : public sigslot::has_slots<> { |
} else if (data[13] == 2) { |
++received_dtls_server_hellos_; |
} |
- } else if (negotiated_dtls_ && !(data[0] >= 20 && data[0] <= 22)) { |
+ } else if (negotiated_dtls() && !(data[0] >= 20 && data[0] <= 22)) { |
ASSERT_TRUE(data[0] == 23 || IsRtpLeadByte(data[0])); |
if (data[0] == 23) { |
ASSERT_TRUE(VerifyEncryptedPacket(data, size)); |
@@ -407,7 +445,8 @@ class DtlsTestClient : public sigslot::has_slots<> { |
std::set<int> received_; |
bool use_dtls_srtp_ = false; |
rtc::SSLProtocolVersion ssl_max_version_ = rtc::SSL_PROTOCOL_DTLS_12; |
- bool negotiated_dtls_ = false; |
+ bool set_local_cert_ = false; |
+ bool set_remote_cert_ = false; |
int received_dtls_client_hellos_ = 0; |
int received_dtls_server_hellos_ = 0; |
rtc::SentPacket sent_packet_; |
@@ -457,10 +496,33 @@ class DtlsTransportChannelTest : public testing::Test { |
use_dtls_srtp_ = true; |
} |
- bool Connect(ConnectionRole client1_role, ConnectionRole client2_role) { |
- Negotiate(client1_role, client2_role); |
+ // Negotiate local/remote fingerprint before or after the underlying |
+ // tranpsort is connected? |
+ enum NegotiateOrdering { NEGOTIATE_BEFORE_CONNECT, CONNECT_BEFORE_NEGOTIATE }; |
+ bool Connect(ConnectionRole client1_role, |
+ ConnectionRole client2_role, |
+ NegotiateOrdering ordering = NEGOTIATE_BEFORE_CONNECT) { |
+ bool rv; |
+ if (ordering == NEGOTIATE_BEFORE_CONNECT) { |
+ Negotiate(client1_role, client2_role); |
+ rv = client1_.Connect(&client2_, false); |
+ } else { |
+ client1_.SetupChannels(channel_ct_, cricket::ICEROLE_CONTROLLING); |
+ client2_.SetupChannels(channel_ct_, cricket::ICEROLE_CONTROLLED); |
+ // This is equivalent to an offer being processed on both sides, but an |
+ // answer not yet being received on the initiating side. So the |
+ // connection will be made before negotiation has finished on both sides. |
+ client1_.SetLocalTransportDescription(client1_.certificate(), |
+ cricket::CA_OFFER, client1_role, 0); |
+ client2_.SetRemoteTransportDescription( |
+ client1_.certificate(), cricket::CA_OFFER, client1_role, 0); |
+ client2_.SetLocalTransportDescription( |
+ client2_.certificate(), cricket::CA_ANSWER, client2_role, 0); |
+ rv = client1_.Connect(&client2_, false); |
+ client1_.SetRemoteTransportDescription( |
+ client2_.certificate(), cricket::CA_ANSWER, client2_role, 0); |
+ } |
- bool rv = client1_.Connect(&client2_, false); |
EXPECT_TRUE(rv); |
if (!rv) |
return false; |
@@ -1018,3 +1080,14 @@ TEST_F(DtlsTransportChannelTest, TestRetransmissionSchedule) { |
EXPECT_EQ(++expected_hellos, client1_.received_dtls_client_hellos()); |
} |
} |
+ |
+// Test that a DTLS connection can be made even if the underlying transport |
+// is connected before DTLS fingerprints/roles have been negotiated. |
+TEST_F(DtlsTransportChannelTest, TestConnectBeforeNegotiate) { |
+ MAYBE_SKIP_TEST(HaveDtls); |
+ PrepareDtls(true, true, rtc::KT_DEFAULT); |
+ ASSERT_TRUE(Connect(cricket::CONNECTIONROLE_ACTPASS, |
+ cricket::CONNECTIONROLE_ACTIVE, |
+ CONNECT_BEFORE_NEGOTIATE)); |
+ TestTransfer(0, 1000, 100, false); |
+} |