Fixing issue with DTLS channel when using "presume writable" flag.

webrtc/p2p/base/dtlstransportchannel_unittest.cc

 /*
  *  Copyright 2011 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 27 matching lines @@
 static bool IsRtpLeadByte(uint8_t b) {
   return ((b & 0xC0) == 0x80);
 }
 
 cricket::TransportDescription MakeTransportDescription(
     const rtc::scoped_refptr<rtc::RTCCertificate>& cert,
     cricket::ConnectionRole role) {
   std::unique_ptr<rtc::SSLFingerprint> fingerprint;
   if (cert) {
     std::string digest_algorithm;
-    cert->ssl_certificate().GetSignatureDigestAlgorithm(&digest_algorithm);
+    EXPECT_TRUE(
+        cert->ssl_certificate().GetSignatureDigestAlgorithm(&digest_algorithm));
+    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 cricket::TransportDescription(std::vector<std::string>(), kIceUfrag1,
                                        kIcePwd1, cricket::ICEMODE_FULL, role,
                                        fingerprint.get());
 }
 
 using cricket::ConnectionRole;
 
 enum Flags { NF_REOFFER = 0x1, NF_EXPECT_FAILURE = 0x2 };
 
@@ skipping 56 matching lines @@
 
   // Offer DTLS if we have an identity; pass in a remote fingerprint only if
   // both sides support DTLS.
   void Negotiate(DtlsTestClient* peer, cricket::ContentAction action,
                  ConnectionRole local_role, ConnectionRole remote_role,
                  int flags) {
     Negotiate(certificate_, certificate_ ? peer->certificate_ : nullptr, action,
               local_role, remote_role, flags);
   }
 
+  void MaybeSetSrtpCryptoSuites() {
+    if (!use_dtls_srtp_) {
+      return;
+    }
+    std::vector<int> ciphers;
+    ciphers.push_back(rtc::SRTP_AES128_CM_SHA1_80);
+    // SRTP ciphers will be set only in the beginning.
+    for (cricket::DtlsTransportChannelWrapper* channel : channels_) {
+      EXPECT_TRUE(channel->SetSrtpCryptoSuites(ciphers));
+    }
+  }
+
+  void SetLocalTransportDescription(
+      const rtc::scoped_refptr<rtc::RTCCertificate>& cert,
+      cricket::ContentAction action,
+      ConnectionRole role,
+      int flags) {
+    // If |NF_EXPECT_FAILURE| is set, expect SRTD or SLTD to fail when
+    // content action is CA_ANSWER.
+    bool expect_success =
+        !((action == cricket::CA_ANSWER) && (flags & NF_EXPECT_FAILURE));
+    EXPECT_EQ(expect_success,
+              transport_->SetLocalTransportDescription(
+                  MakeTransportDescription(cert, role), action, nullptr));
+    set_local_cert_ = (cert != nullptr);
+  }
+
+  void SetRemoteTransportDescription(
+      const rtc::scoped_refptr<rtc::RTCCertificate>& cert,
+      cricket::ContentAction action,
+      ConnectionRole role,
+      int flags) {
+    // If |NF_EXPECT_FAILURE| is set, expect SRTD or SLTD to fail when
+    // content action is CA_ANSWER.
+    bool expect_success =
+        !((action == cricket::CA_ANSWER) && (flags & NF_EXPECT_FAILURE));
+    EXPECT_EQ(expect_success,
+              transport_->SetRemoteTransportDescription(
+                  MakeTransportDescription(cert, role), 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) {
-    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 (!(flags & NF_REOFFER)) {
+      // SRTP ciphers will be set only in the beginning.
+      MaybeSetSrtpCryptoSuites();
     }
-    if (remote_cert) {
-      std::string digest_algorithm;
-      ASSERT_TRUE(remote_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_EQ(rtc::DIGEST_SHA_256, digest_algorithm);
+    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);
     }
-
-    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));
-      }
-    }
-
-    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());
-
-    cricket::TransportDescription remote_desc(
-        std::vector<std::string>(), kIceUfrag1, kIcePwd1, cricket::ICEMODE_FULL,
-        remote_role, remote_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));
-    } else {
-      ASSERT_TRUE(transport_->SetRemoteTransportDescription(
-          remote_desc, cricket::CA_OFFER, NULL));
-      ASSERT_EQ(expect_success, transport_->SetLocalTransportDescription(
-          local_desc, cricket::CA_ANSWER, NULL));
-    }
-    negotiated_dtls_ = (local_cert && remote_cert);
   }
 
   bool Connect(DtlsTestClient* peer, bool asymmetric) {
     transport_->SetDestination(peer->transport_.get(), asymmetric);
     return true;
   }
 
   bool all_channels_writable() const {
     if (channels_.empty()) {
       return false;
@@ skipping 15 matching lines @@
         return false;
       }
     }
     return true;
   }
 
   int received_dtls_client_hellos() const {
     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_);
       ASSERT_GT(received_dtls_server_hellos_, 0);
     } else {
       ASSERT_GT(received_dtls_client_hellos_, 0);
       ASSERT_EQ(0, received_dtls_server_hellos_);
     }
   }
 
   void CheckSrtp(int expected_crypto_suite) {
     for (std::vector<cricket::DtlsTransportChannelWrapper*>::iterator it =
            channels_.begin(); it != channels_.end(); ++it) {
       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);
       } else {
         ASSERT_FALSE(rv);
       }
     }
   }
 
   void CheckSsl() {
     for (std::vector<cricket::DtlsTransportChannelWrapper*>::iterator it =
            channels_.begin(); it != channels_.end(); ++it) {
       int cipher;
 
       bool rv = (*it)->GetSslCipherSuite(&cipher);
-      if (negotiated_dtls_) {
+      if (negotiated_dtls()) {
         ASSERT_TRUE(rv);
 
         EXPECT_TRUE(
             rtc::SSLStreamAdapter::IsAcceptableCipher(cipher, rtc::KT_DEFAULT));
       } else {
         ASSERT_FALSE(rv);
       }
     }
   }
 
@@ skipping 108 matching lines @@
     ASSERT_EQ(0, flags);
 
     // Look at the handshake packets to see what role we played.
     // Check that non-handshake packets are DTLS data or SRTP bypass.
     if (data[0] == 22 && size > 17) {
       if (data[13] == 1) {
         ++received_dtls_client_hellos_;
       } 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));
       } else if (IsRtpLeadByte(data[0])) {
         ASSERT_TRUE(VerifyPacket(data, size, NULL));
       }
     }
   }
 
  private:
   std::string name_;
   rtc::scoped_refptr<rtc::RTCCertificate> certificate_;
   std::unique_ptr<cricket::FakeTransport> transport_;
   std::vector<cricket::DtlsTransportChannelWrapper*> channels_;
   size_t packet_size_ = 0u;
   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_;
 };
 
 // Note that this test always uses a FakeClock, due to the |fake_clock_| member
 // variable.
 class DtlsTransportChannelTest : public testing::Test {
  public:
   DtlsTransportChannelTest()
@@ skipping 29 matching lines @@
 
     if (c1)
       client1_.SetupSrtp();
     if (c2)
       client2_.SetupSrtp();
 
     if (c1 && c2)
       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);
+      client1_.MaybeSetSrtpCryptoSuites();
+      client2_.MaybeSetSrtpCryptoSuites();
+      // 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;
 
     EXPECT_TRUE_WAIT(
         client1_.all_channels_writable() && client2_.all_channels_writable(),
         kTimeout);
     if (!client1_.all_channels_writable() || !client2_.all_channels_writable())
       return false;
 
@@ skipping 537 matching lines @@
     // millisecond before the expected time and verify that no unexpected
     // retransmissions were sent. Then advance it the final millisecond and
     // verify that the expected retransmission was sent.
     fake_clock_.AdvanceTime(
         rtc::TimeDelta::FromMilliseconds(timeout_schedule_ms[i] - 1));
     EXPECT_EQ(expected_hellos, client1_.received_dtls_client_hellos());
     fake_clock_.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1));
     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);
+}