Cache a ClientHello received before the DTLS handshake has started.

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 451 matching lines @@
 
   if (!dtls_active_) {
     // Not doing DTLS.
     SignalReadPacket(this, data, size, packet_time, 0);
     return;
   }
 
   switch (dtls_state()) {
     case DTLS_TRANSPORT_NEW:
       if (dtls_) {
-        // Drop packets received before DTLS has actually started.
-        LOG_J(LS_INFO, this) << "Dropping packet received before DTLS started.";
+        LOG_J(LS_INFO, this) << "Packet received before DTLS started.";
       } else {
-        // Currently drop the packet, but we might in future
-        // decide to take this as evidence that the other
-        // side is ready to do DTLS and start the handshake
-        // on our end.
-        LOG_J(LS_WARNING, this) << "Received packet before we know if we are "
-                                << "doing DTLS or not; dropping.";
+        LOG_J(LS_WARNING, this) << "Packet received before we know if we are "
+                                << "doing DTLS or not.";
+      }
+      // Cache the last DTLS packet (should be a client hello) received before
+      // DTLS has actually started.
+      if (IsDtlsPacket(data, size)) {

[juberti2, 2016/04/25 20:40:18]

We should probably also verify this is a CLIENT HELLO, at least for logging. 

[Taylor Brandstetter, 2016/04/26 22:55:58]

Done.

+        LOG_J(LS_INFO, this) << "Caching DTLS packet until DTLS is started.";
+        cached_dtls_packet_.SetData(data, size);
+      } else {
+        LOG_J(LS_INFO, this) << "Not a DTLS packet; dropping.";
       }
       break;
 
     case DTLS_TRANSPORT_CONNECTING:
     case DTLS_TRANSPORT_CONNECTED:
       // We should only get DTLS or SRTP packets; STUN's already been demuxed.
       // Is this potentially a DTLS packet?
       if (IsDtlsPacket(data, size)) {
         if (!HandleDtlsPacket(data, size)) {
           LOG_J(LS_ERROR, this) << "Failed to handle DTLS packet.";
@@ skipping 78 matching lines @@
 bool DtlsTransportChannelWrapper::MaybeStartDtls() {
   if (dtls_ && channel_->writable()) {
     if (dtls_->StartSSLWithPeer()) {
       LOG_J(LS_ERROR, this) << "Couldn't start DTLS handshake";
       set_dtls_state(DTLS_TRANSPORT_FAILED);
       return false;
     }
     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 packet
+    // (if one exists).
+    if (cached_dtls_packet_.size()) {
+      if (ssl_role_ == rtc::SSL_SERVER) {
+        LOG_J(LS_INFO, this) << "Handling cached DTLS packet.";
+        if (!HandleDtlsPacket(cached_dtls_packet_.data<char>(),
+                              cached_dtls_packet_.size())) {
+          LOG_J(LS_ERROR, this) << "Failed to handle DTLS packet.";
+        }
+      } else {
+        LOG_J(LS_WARNING, this) << "Discarding cached DTLS packet because "

[juberti2, 2016/04/25 20:40:18]

Log text could be clearer to indicate this is a real malfunction by the remote
side as opposed to just a race condition or some such.

[Taylor Brandstetter, 2016/04/26 22:55:58]

Now that the message is changed to "discarding cached ClientHello because we're
not the server" is it more clear?

+                                << "we don't have the server role.";
+      }
+      cached_dtls_packet_.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);
@@ skipping 64 matching lines @@
 
 void DtlsTransportChannelWrapper::Reconnect() {
   set_dtls_state(DTLS_TRANSPORT_NEW);
   set_writable(false);
   if (channel_->writable()) {
     OnWritableState(channel_);
   }
 }
 
 }  // namespace cricket