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

webrtc/p2p/base/dtlstransportchannel.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 220 matching lines @@
   // fingerprint.
   if (!dtls_active_) {
     LOG_J(LS_ERROR, this) << "Can't set DTLS remote settings in this state.";
     return false;
   }
 
   // At this point we know we are doing DTLS
   remote_fingerprint_value_ = std::move(remote_fingerprint_value);
   remote_fingerprint_algorithm_ = digest_alg;
 
-  bool reconnect = (dtls_ != nullptr);
+  if (dtls_) {
+    // If the fingerprint is changing, we'll tear down the DTLS association and
+    // create a new one, resetting our state.
+    dtls_.reset(nullptr);
+    set_dtls_state(DTLS_TRANSPORT_NEW);
+    set_writable(false);
+  }
 
   if (!SetupDtls()) {
     set_dtls_state(DTLS_TRANSPORT_FAILED);
     return false;
   }
 
-  if (reconnect) {
-    Reconnect();
-  }
-
   return true;
 }
 
 std::unique_ptr<rtc::SSLCertificate>
 DtlsTransportChannelWrapper::GetRemoteSSLCertificate() const {
   if (!dtls_) {
     return nullptr;
   }
 
   return dtls_->GetPeerCertificate();
@@ skipping 28 matching lines @@
   if (!srtp_ciphers_.empty()) {
     if (!dtls_->SetDtlsSrtpCryptoSuites(srtp_ciphers_)) {
       LOG_J(LS_ERROR, this) << "Couldn't set DTLS-SRTP ciphers.";
       return false;
     }
   } else {
     LOG_J(LS_INFO, this) << "Not using DTLS-SRTP.";
   }
 
   LOG_J(LS_INFO, this) << "DTLS setup complete.";
+
+  // If the underlying channel is already writable at this point, we may be
+  // able to start DTLS right away.
+  MaybeStartDtls();
   return true;
 }
 
 bool DtlsTransportChannelWrapper::SetSrtpCryptoSuites(
     const std::vector<int>& ciphers) {
   if (srtp_ciphers_ == ciphers)
     return true;
 
   if (dtls_state() == DTLS_TRANSPORT_CONNECTING) {
     LOG(LS_WARNING) << "Ignoring new SRTP ciphers while DTLS is negotiating";
@@ skipping 102 matching lines @@
 
   if (!dtls_active_) {
     // Not doing DTLS.
     // Note: SignalWritableState fired by set_writable.
     set_writable(channel_->writable());
     return;
   }
 
   switch (dtls_state()) {
     case DTLS_TRANSPORT_NEW:
-      // This should never fail:
-      // Because we are operating in a nonblocking mode and all
-      // incoming packets come in via OnReadPacket(), which rejects
-      // packets in this state, the incoming queue must be empty. We
-      // ignore write errors, thus any errors must be because of
-      // configuration and therefore are our fault.
-      // Note that in non-debug configurations, failure in
-      // MaybeStartDtls() changes the state to DTLS_TRANSPORT_FAILED.
-      VERIFY(MaybeStartDtls());
+      MaybeStartDtls();
       break;
     case DTLS_TRANSPORT_CONNECTED:
       // Note: SignalWritableState fired by set_writable.
       set_writable(channel_->writable());
       break;
     case DTLS_TRANSPORT_CONNECTING:
       // Do nothing.
       break;
     case DTLS_TRANSPORT_FAILED:
     case DTLS_TRANSPORT_CLOSED:
@@ skipping 123 matching lines @@
     if (!err) {
       LOG_J(LS_INFO, this) << "DTLS channel closed";
       set_dtls_state(DTLS_TRANSPORT_CLOSED);
     } else {
       LOG_J(LS_INFO, this) << "DTLS channel error, code=" << err;
       set_dtls_state(DTLS_TRANSPORT_FAILED);
     }
   }
 }
 
-bool DtlsTransportChannelWrapper::MaybeStartDtls() {
+void DtlsTransportChannelWrapper::MaybeStartDtls() {
   if (dtls_ && channel_->writable()) {
     if (dtls_->StartSSLWithPeer()) {
+      // This should never fail:
+      // Because we are operating in a nonblocking mode and all
+      // incoming packets come in via OnReadPacket(), which rejects
+      // packets in this state, the incoming queue must be empty. We
+      // ignore write errors, thus any errors must be because of
+      // configuration and therefore are our fault.
+      RTC_DCHECK(false) << "StartSSLWithPeer failed.";
       LOG_J(LS_ERROR, this) << "Couldn't start DTLS handshake";
       set_dtls_state(DTLS_TRANSPORT_FAILED);
-      return false;
+      return;
     }
     LOG_J(LS_INFO, this)
       << "DtlsTransportChannelWrapper: Started DTLS handshake";
     set_dtls_state(DTLS_TRANSPORT_CONNECTING);
     // Now that the handshake has started, we can process a cached ClientHello
     // (if one exists).
     if (cached_client_hello_.size()) {
       if (ssl_role_ == rtc::SSL_SERVER) {
         LOG_J(LS_INFO, this) << "Handling cached DTLS ClientHello packet.";
         if (!HandleDtlsPacket(cached_client_hello_.data<char>(),
                               cached_client_hello_.size())) {
           LOG_J(LS_ERROR, this) << "Failed to handle DTLS packet.";
         }
       } else {
         LOG_J(LS_WARNING, this) << "Discarding cached DTLS ClientHello packet "
                                 << "because we don't have the server role.";
       }
       cached_client_hello_.Clear();
     }
   }
-  return true;
 }
 
 // Called from OnReadPacket when a DTLS packet is received.
 bool DtlsTransportChannelWrapper::HandleDtlsPacket(const char* data,
                                                    size_t size) {
   // Sanity check we're not passing junk that
   // just looks like DTLS.
   const uint8_t* tmp_data = reinterpret_cast<const uint8_t*>(data);
   size_t tmp_size = size;
   while (tmp_size > 0) {
@@ skipping 54 matching lines @@
   SignalSelectedCandidatePairChanged(this, selected_candidate_pair,
                                      last_sent_packet_id, ready_to_send);
 }
 
 void DtlsTransportChannelWrapper::OnChannelStateChanged(
     TransportChannelImpl* channel) {
   ASSERT(channel == channel_);
   SignalStateChanged(this);
 }
 
-void DtlsTransportChannelWrapper::Reconnect() {
-  set_dtls_state(DTLS_TRANSPORT_NEW);
-  set_writable(false);
-  if (channel_->writable()) {
-    OnWritableState(channel_);
-  }
-}
-
 }  // namespace cricket