Allow remote fingerprint update during a call

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 181 matching lines @@
 
   return dtls_->GetSslCipherSuite(cipher);
 }
 
 bool DtlsTransportChannelWrapper::SetRemoteFingerprint(
     const std::string& digest_alg,
     const uint8_t* digest,
     size_t digest_len) {
   rtc::Buffer remote_fingerprint_value(digest, digest_len);
 
+  // When we have local certificate, the same remote fingerprint could be set
+  // multiple times.

[pthatcher1, 2015/11/30 20:23:11]

When we have local => Once we have the local 
could be => can be

[guoweis_webrtc, 2015/12/01 22:05:13]

Done.

   if (dtls_active_ && remote_fingerprint_value_ == remote_fingerprint_value &&
       !digest_alg.empty()) {
     // This may happen during renegotiation.
     LOG_J(LS_INFO, this) << "Ignoring identical remote DTLS fingerprint";
     return true;
   }
 
-  // Allow SetRemoteFingerprint with a NULL digest even if SetLocalCertificate
-  // hasn't been called.
-  if (dtls_ || (!dtls_active_ && !digest_alg.empty())) {
-    LOG_J(LS_ERROR, this) << "Can't set DTLS remote settings in this state.";
-    return false;
-  }
-
+  // If the other side doesn't support DTLS, turn off |dtls_active_|.
   if (digest_alg.empty()) {
+    RTC_DCHECK(!digest_len);
     LOG_J(LS_INFO, this) << "Other side didn't support DTLS.";
     dtls_active_ = false;
     return true;
   }
 
+  // Otherwise, we must have a local certificate before setting remote
+  // 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_ = remote_fingerprint_value.Pass();
   remote_fingerprint_algorithm_ = digest_alg;
 
+  bool reconnect = dtls_;
+
   if (!SetupDtls()) {
     set_dtls_state(DTLS_TRANSPORT_FAILED);
     return false;
   }
 
+  if (reconnect) {
+    set_dtls_state(DTLS_TRANSPORT_NEW);

[pthatcher1, 2015/11/30 20:23:11]

Why do we set the state to new and then assert the state is new in Reconnect()? 
Why not just set the state to new inside of Reconnect()?

And why not set_writable(false) inside of Reconnect as well?

[guoweis_webrtc, 2015/12/01 22:05:13]

Done.

+    set_writable(false);
+    Reconnect();
+  }
+
   return true;
 }
 
 bool DtlsTransportChannelWrapper::GetRemoteSSLCertificate(
     rtc::SSLCertificate** cert) const {
   if (!dtls_) {
     return false;
   }
 
   return dtls_->GetPeerCertificate(cert);
@@ skipping 282 matching lines @@
     if (dtls_->GetState() == rtc::SS_OPEN) {
       // The check for OPEN shouldn't be necessary but let's make
       // sure we don't accidentally frob the state if it's closed.
       set_dtls_state(DTLS_TRANSPORT_CONNECTED);
       set_writable(true);
     }
   }
   if (sig & rtc::SE_READ) {
     char buf[kMaxDtlsPacketLen];
     size_t read;
-    if (dtls_->Read(buf, sizeof(buf), &read, NULL) == rtc::SR_SUCCESS) {
+    rtc::StreamResult result = dtls_->Read(buf, sizeof(buf), &read, NULL);
+    if (result == rtc::SR_SUCCESS) {
       SignalReadPacket(this, buf, read, rtc::CreatePacketTime(0), 0);
+    } else if (result == rtc::SR_EOS) {
+      // If the SSL stream has closed remotely, reset the |sig| to be SE_CLOSE
+      // so it could be handled below.
+      sig = rtc::SE_CLOSE;

[pthatcher1, 2015/11/30 20:23:11]

Shouldn't we have a unit test for dtlstransportchannel that tests low-level
things like this? 

[guoweis_webrtc, 2015/12/01 22:05:13]

Yes, but I'd like to add it in next CL.

     }
   }
   if (sig & rtc::SE_CLOSE) {
     ASSERT(sig == rtc::SE_CLOSE);  // SE_CLOSE should be by itself.
     set_writable(false);
     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;
@@ skipping 64 matching lines @@
   ASSERT(channel == channel_);
   SignalRouteChange(this, candidate);
 }
 
 void DtlsTransportChannelWrapper::OnConnectionRemoved(
     TransportChannelImpl* channel) {
   ASSERT(channel == channel_);
   SignalConnectionRemoved(this);
 }
 
+void DtlsTransportChannelWrapper::Reconnect() {
+  ASSERT(dtls_state() == DTLS_TRANSPORT_NEW);
+  if (channel_->writable()) {
+    OnWritableState(channel_);
+  } else {
+    channel_->Connect();
+  }
+}
+
 }  // namespace cricket