Exposing DTLS transport state from TransportChannel.

This is necessary in order to support the RTCPeerConnectionState enum in
the future, as well as a correct RTCIceConnectionState (which isn't a
combination ICE and DTLS state).

Review URL: https://codereview.webrtc.org/1414363002

Cr-Commit-Position: refs/heads/master@{#10419}

webrtc/p2p/base/dtlstransportchannel.cc

@@ -94,7 +94,6 @@ DtlsTransportChannelWrapper::DtlsTransportChannelWrapper(
       worker_thread_(rtc::Thread::Current()),
       channel_(channel),
       downward_(NULL),
-      dtls_state_(STATE_NONE),
       ssl_role_(rtc::SSL_CLIENT),
       ssl_max_version_(rtc::SSL_PROTOCOL_DTLS_10) {
   channel_->SignalWritableState.connect(this,
@@ -124,15 +123,13 @@ DtlsTransportChannelWrapper::~DtlsTransportChannelWrapper() {
 
 void DtlsTransportChannelWrapper::Connect() {
   // We should only get a single call to Connect.
-  ASSERT(dtls_state_ == STATE_NONE ||
+  ASSERT(dtls_state() == DTLS_TRANSPORT_NEW);
-         dtls_state_ == STATE_OFFERED ||
-         dtls_state_ == STATE_ACCEPTED);
   channel_->Connect();
 }
 
 bool DtlsTransportChannelWrapper::SetLocalCertificate(
     const rtc::scoped_refptr<rtc::RTCCertificate>& certificate) {
-  if (dtls_state_ != STATE_NONE) {
+  if (dtls_active_) {
     if (certificate == local_certificate_) {
       // This may happen during renegotiation.
       LOG_J(LS_INFO, this) << "Ignoring identical DTLS identity";
@@ -145,7 +142,7 @@ bool DtlsTransportChannelWrapper::SetLocalCertificate(
 
   if (certificate) {
     local_certificate_ = certificate;
-    dtls_state_ = STATE_OFFERED;
+    dtls_active_ = true;
   } else {
     LOG_J(LS_INFO, this) << "NULL DTLS identity supplied. Not doing DTLS";
   }
@@ -160,7 +157,7 @@ DtlsTransportChannelWrapper::GetLocalCertificate() const {
 
 bool DtlsTransportChannelWrapper::SetSslMaxProtocolVersion(
     rtc::SSLProtocolVersion version) {
-  if (dtls_state_ != STATE_NONE) {
+  if (dtls_active_) {
     LOG(LS_ERROR) << "Not changing max. protocol version "
                   << "while DTLS is negotiating";
     return false;
@@ -171,7 +168,7 @@ bool DtlsTransportChannelWrapper::SetSslMaxProtocolVersion(
 }
 
 bool DtlsTransportChannelWrapper::SetSslRole(rtc::SSLRole role) {
-  if (dtls_state_ == STATE_OPEN) {
+  if (dtls_state() == DTLS_TRANSPORT_CONNECTED) {
     if (ssl_role_ != role) {
       LOG(LS_ERROR) << "SSL Role can't be reversed after the session is setup.";
       return false;
@@ -189,7 +186,7 @@ bool DtlsTransportChannelWrapper::GetSslRole(rtc::SSLRole* role) const {
 }
 
 bool DtlsTransportChannelWrapper::GetSslCipherSuite(int* cipher) {
-  if (dtls_state_ != STATE_OPEN) {
+  if (dtls_state() != DTLS_TRANSPORT_CONNECTED) {
     return false;
   }
 
@@ -202,8 +199,7 @@ bool DtlsTransportChannelWrapper::SetRemoteFingerprint(
     size_t digest_len) {
   rtc::Buffer remote_fingerprint_value(digest, digest_len);
 
-  if (dtls_state_ != STATE_NONE &&
+  if (dtls_active_ && remote_fingerprint_value_ == remote_fingerprint_value &&
-      remote_fingerprint_value_ == remote_fingerprint_value &&
       !digest_alg.empty()) {
     // This may happen during renegotiation.
     LOG_J(LS_INFO, this) << "Ignoring identical remote DTLS fingerprint";
@@ -212,15 +208,14 @@ bool DtlsTransportChannelWrapper::SetRemoteFingerprint(
 
   // Allow SetRemoteFingerprint with a NULL digest even if SetLocalCertificate
   // hasn't been called.
-  if (dtls_state_ > STATE_OFFERED ||
+  if (dtls_ || (!dtls_active_ && !digest_alg.empty())) {
-      (dtls_state_ == STATE_NONE && !digest_alg.empty())) {
     LOG_J(LS_ERROR, this) << "Can't set DTLS remote settings in this state.";
     return false;
   }
 
   if (digest_alg.empty()) {
     LOG_J(LS_INFO, this) << "Other side didn't support DTLS.";
-    dtls_state_ = STATE_NONE;
+    dtls_active_ = false;
     return true;
   }
 
@@ -229,18 +224,18 @@ bool DtlsTransportChannelWrapper::SetRemoteFingerprint(
   remote_fingerprint_algorithm_ = digest_alg;
 
   if (!SetupDtls()) {
-    dtls_state_ = STATE_CLOSED;
+    set_dtls_state(DTLS_TRANSPORT_FAILED);
     return false;
   }
 
-  dtls_state_ = STATE_ACCEPTED;
   return true;
 }
 
 bool DtlsTransportChannelWrapper::GetRemoteSSLCertificate(
     rtc::SSLCertificate** cert) const {
-  if (!dtls_)
+  if (!dtls_) {
     return false;
+  }
 
   return dtls_->GetPeerCertificate(cert);
 }
@@ -277,7 +272,7 @@ bool DtlsTransportChannelWrapper::SetupDtls() {
       return false;
     }
   } else {
-    LOG_J(LS_INFO, this) << "Not using DTLS.";
+    LOG_J(LS_INFO, this) << "Not using DTLS-SRTP.";
   }
 
   LOG_J(LS_INFO, this) << "DTLS setup complete.";
@@ -286,15 +281,16 @@ bool DtlsTransportChannelWrapper::SetupDtls() {
 
 bool DtlsTransportChannelWrapper::SetSrtpCiphers(
     const std::vector<std::string>& ciphers) {
-  if (srtp_ciphers_ == ciphers)
+  if (srtp_ciphers_ == ciphers) {
     return true;
+  }
 
-  if (dtls_state_ == STATE_STARTED) {
+  if (dtls_state() == DTLS_TRANSPORT_CONNECTING) {
     LOG(LS_WARNING) << "Ignoring new SRTP ciphers while DTLS is negotiating";
     return true;
   }
 
-  if (dtls_state_ == STATE_OPEN) {
+  if (dtls_state() == DTLS_TRANSPORT_CONNECTED) {
     // We don't support DTLS renegotiation currently. If new set of srtp ciphers
     // are different than what's being used currently, we will not use it.
     // So for now, let's be happy (or sad) with a warning message.
@@ -320,10 +316,7 @@ bool DtlsTransportChannelWrapper::SetSrtpCiphers(
     return true;
   }
 
-  if (dtls_state_ != STATE_NONE &&
+  if (!VERIFY(dtls_state() == DTLS_TRANSPORT_NEW)) {
-      dtls_state_ != STATE_OFFERED &&
-      dtls_state_ != STATE_ACCEPTED) {
-    ASSERT(false);
     return false;
   }
 
@@ -332,7 +325,7 @@ bool DtlsTransportChannelWrapper::SetSrtpCiphers(
 }
 
 bool DtlsTransportChannelWrapper::GetSrtpCryptoSuite(std::string* cipher) {
-  if (dtls_state_ != STATE_OPEN) {
+  if (dtls_state() != DTLS_TRANSPORT_CONNECTED) {
     return false;
   }
 
@@ -344,45 +337,40 @@ bool DtlsTransportChannelWrapper::GetSrtpCryptoSuite(std::string* cipher) {
 int DtlsTransportChannelWrapper::SendPacket(
     const char* data, size_t size,
     const rtc::PacketOptions& options, int flags) {
-  int result = -1;
+  if (!dtls_active_) {
+    // Not doing DTLS.
+    return channel_->SendPacket(data, size, options);
+  }
 
-  switch (dtls_state_) {
+  switch (dtls_state()) {
-    case STATE_OFFERED:
+    case DTLS_TRANSPORT_NEW:
-      // We don't know if we are doing DTLS yet, so we can't send a packet.
+      // Can't send data until the connection is active.
-      // TODO(ekr@rtfm.com): assert here?
+      // TODO(ekr@rtfm.com): assert here if dtls_ is NULL?
-      result = -1;
+      return -1;
-      break;
+    case DTLS_TRANSPORT_CONNECTING:
-
+      // Can't send data until the connection is active.
-    case STATE_STARTED:
+      return -1;
-    case STATE_ACCEPTED:
+    case DTLS_TRANSPORT_CONNECTED:
-      // Can't send data until the connection is active
-      result = -1;
-      break;
-
-    case STATE_OPEN:
       if (flags & PF_SRTP_BYPASS) {
         ASSERT(!srtp_ciphers_.empty());
         if (!IsRtpPacket(data, size)) {
-          result = -1;
+          return -1;
-          break;
         }
 
-        result = channel_->SendPacket(data, size, options);
+        return channel_->SendPacket(data, size, options);
       } else {
-        result = (dtls_->WriteAll(data, size, NULL, NULL) ==
+        return (dtls_->WriteAll(data, size, NULL, NULL) == rtc::SR_SUCCESS)
-          rtc::SR_SUCCESS) ? static_cast<int>(size) : -1;
+                   ? static_cast<int>(size)
+                   : -1;
       }
-      break;
+    case DTLS_TRANSPORT_FAILED:
-      // Not doing DTLS.
+    case DTLS_TRANSPORT_CLOSED:
-    case STATE_NONE:
+      // Can't send anything when we're closed.
-      result = channel_->SendPacket(data, size, options);
+      return -1;
-      break;
+    default:
-
+      ASSERT(false);
-    case STATE_CLOSED:  // Can't send anything when we're closed.
       return -1;
   }
-
-  return result;
 }
 
 // The state transition logic here is as follows:
@@ -402,37 +390,35 @@ void DtlsTransportChannelWrapper::OnWritableState(TransportChannel* channel) {
       << "DTLSTransportChannelWrapper: channel writable state changed to "
       << channel_->writable();
 
-  switch (dtls_state_) {
+  if (!dtls_active_) {
-    case STATE_NONE:
+    // Not doing DTLS.
-    case STATE_OPEN:
+    // Note: SignalWritableState fired by set_writable.
-      set_writable(channel_->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());
+      break;
+    case DTLS_TRANSPORT_CONNECTED:
       // Note: SignalWritableState fired by set_writable.
+      set_writable(channel_->writable());
       break;
-
+    case DTLS_TRANSPORT_CONNECTING:
-    case STATE_OFFERED:
+      // Do nothing.
-      // Do nothing
       break;
-
+    case DTLS_TRANSPORT_FAILED:
-    case STATE_ACCEPTED:
+    case DTLS_TRANSPORT_CLOSED:
-      if (!MaybeStartDtls()) {
+      // Should not happen. Do nothing.
-        // This should never happen:
-        // 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 STATE_CLOSED.
-        ASSERT(false);
-      }
-      break;
-
-    case STATE_STARTED:
-      // Do nothing
-      break;
-
-    case STATE_CLOSED:
-      // Should not happen. Do nothing
       break;
   }
 }
@@ -443,7 +429,7 @@ void DtlsTransportChannelWrapper::OnReceivingState(TransportChannel* channel) {
   LOG_J(LS_VERBOSE, this)
       << "DTLSTransportChannelWrapper: channel receiving state changed to "
       << channel_->receiving();
-  if (dtls_state_ == STATE_NONE || dtls_state_ == STATE_OPEN) {
+  if (!dtls_active_ || dtls_state() == DTLS_TRANSPORT_CONNECTED) {
     // Note: SignalReceivingState fired by set_receiving.
     set_receiving(channel_->receiving());
   }
@@ -456,28 +442,29 @@ void DtlsTransportChannelWrapper::OnReadPacket(
   ASSERT(channel == channel_);
   ASSERT(flags == 0);
 
-  switch (dtls_state_) {
+  if (!dtls_active_) {
-    case STATE_NONE:
+    // Not doing DTLS.
-      // We are not doing DTLS
+    SignalReadPacket(this, data, size, packet_time, 0);
-      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.";
+      } 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.";
+      }
       break;
 
-    case STATE_OFFERED:
+    case DTLS_TRANSPORT_CONNECTING:
-      // Currently drop the packet, but we might in future
+    case DTLS_TRANSPORT_CONNECTED:
-      // 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.";
-      break;
-
-    case STATE_ACCEPTED:
-      // Drop packets received before DTLS has actually started
-      LOG_J(LS_INFO, this) << "Dropping packet received before DTLS started.";
-      break;
-
-    case STATE_STARTED:
-    case STATE_OPEN:
       // We should only get DTLS or SRTP packets; STUN's already been demuxed.
       // Is this potentially a DTLS packet?
       if (IsDtlsPacket(data, size)) {
@@ -487,7 +474,7 @@ void DtlsTransportChannelWrapper::OnReadPacket(
         }
       } else {
         // Not a DTLS packet; our handshake should be complete by now.
-        if (dtls_state_ != STATE_OPEN) {
+        if (dtls_state() != DTLS_TRANSPORT_CONNECTED) {
           LOG_J(LS_ERROR, this) << "Received non-DTLS packet before DTLS "
                                 << "complete.";
           return;
@@ -506,8 +493,9 @@ void DtlsTransportChannelWrapper::OnReadPacket(
         SignalReadPacket(this, data, size, packet_time, PF_SRTP_BYPASS);
       }
       break;
-    case STATE_CLOSED:
+    case DTLS_TRANSPORT_FAILED:
-      // This shouldn't be happening. Drop the packet
+    case DTLS_TRANSPORT_CLOSED:
+      // This shouldn't be happening. Drop the packet.
       break;
   }
 }
@@ -536,7 +524,7 @@ void DtlsTransportChannelWrapper::OnDtlsEvent(rtc::StreamInterface* dtls,
     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.
-      dtls_state_ = STATE_OPEN;
+      set_dtls_state(DTLS_TRANSPORT_CONNECTED);
       set_writable(true);
     }
   }
@@ -549,27 +537,27 @@ void DtlsTransportChannelWrapper::OnDtlsEvent(rtc::StreamInterface* dtls,
   }
   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;
+      set_dtls_state(DTLS_TRANSPORT_FAILED);
     }
-    set_writable(false);
-    dtls_state_ = STATE_CLOSED;
   }
 }
 
 bool DtlsTransportChannelWrapper::MaybeStartDtls() {
-  if (channel_->writable()) {
+  if (dtls_ && channel_->writable()) {
     if (dtls_->StartSSLWithPeer()) {
       LOG_J(LS_ERROR, this) << "Couldn't start DTLS handshake";
-      dtls_state_ = STATE_CLOSED;
+      set_dtls_state(DTLS_TRANSPORT_FAILED);
       return false;
     }
     LOG_J(LS_INFO, this)
       << "DtlsTransportChannelWrapper: Started DTLS handshake";
-
+    set_dtls_state(DTLS_TRANSPORT_CONNECTING);
-    dtls_state_ = STATE_STARTED;
   }
   return true;
 }

webrtc/p2p/base/dtlstransportchannel.h

@@ -81,15 +81,6 @@ class StreamInterfaceChannel : public rtc::StreamInterface {
 //     which translates it into packet writes on channel_.
 class DtlsTransportChannelWrapper : public TransportChannelImpl {
  public:
-    enum State {
-      STATE_NONE,      // No state or rejected.
-      STATE_OFFERED,   // Our identity has been set.
-      STATE_ACCEPTED,  // The other side sent a fingerprint.
-      STATE_STARTED,   // We are negotiating.
-      STATE_OPEN,      // Negotiation complete.
-      STATE_CLOSED     // Connection closed.
-    };
-
   // The parameters here are:
   // transport -- the DtlsTransport that created us
   // channel -- the TransportChannel we are wrapping
@@ -106,7 +97,10 @@ class DtlsTransportChannelWrapper : public TransportChannelImpl {
   bool SetRemoteFingerprint(const std::string& digest_alg,
                             const uint8_t* digest,
                             size_t digest_len) override;
-  bool IsDtlsActive() const override { return dtls_state_ != STATE_NONE; }
+
+  // Returns false if no local certificate was set, or if the peer doesn't
+  // support DTLS.
+  bool IsDtlsActive() const override { return dtls_active_; }
 
   // Called to send a packet (via DTLS, if turned on).
   int SendPacket(const char* data,
@@ -230,7 +224,7 @@ class DtlsTransportChannelWrapper : public TransportChannelImpl {
   rtc::scoped_ptr<rtc::SSLStreamAdapter> dtls_;  // The DTLS stream
   StreamInterfaceChannel* downward_;  // Wrapper for channel_, owned by dtls_.
   std::vector<std::string> srtp_ciphers_;  // SRTP ciphers to use with DTLS.
-  State dtls_state_;
+  bool dtls_active_ = false;
   rtc::scoped_refptr<rtc::RTCCertificate> local_certificate_;
   rtc::SSLRole ssl_role_;
   rtc::SSLProtocolVersion ssl_max_version_;

webrtc/p2p/base/transport.h

@@ -56,6 +56,19 @@ enum IceConnectionState {
   kIceConnectionCompleted,
 };
 
+enum DtlsTransportState {
+  // Haven't started negotiating.
+  DTLS_TRANSPORT_NEW = 0,
+  // Have started negotiating.
+  DTLS_TRANSPORT_CONNECTING,
+  // Negotiated, and has a secure connection.
+  DTLS_TRANSPORT_CONNECTED,
+  // Transport is closed.
+  DTLS_TRANSPORT_CLOSED,
+  // Failed due to some error in the handshake process.
+  DTLS_TRANSPORT_FAILED,
+};
+
 // TODO(deadbeef): Unify with PeerConnectionInterface::IceConnectionState
 // once /talk/ and /webrtc/ are combined, and also switch to ENUM_NAME naming
 // style.

webrtc/p2p/base/transportchannel.cc

@@ -32,15 +32,26 @@ void TransportChannel::set_receiving(bool receiving) {
 }
 
 void TransportChannel::set_writable(bool writable) {
-  if (writable_ != writable) {
+  if (writable_ == writable) {
-    LOG_J(LS_VERBOSE, this) << "set_writable from:" << writable_ << " to "
+    return;
-                            << writable;
-    writable_ = writable;
-    if (writable_) {
-      SignalReadyToSend(this);
-    }
-    SignalWritableState(this);
   }
+  LOG_J(LS_VERBOSE, this) << "set_writable from:" << writable_ << " to "
+                          << writable;
+  writable_ = writable;
+  if (writable_) {
+    SignalReadyToSend(this);
+  }
+  SignalWritableState(this);
+}
+
+void TransportChannel::set_dtls_state(DtlsTransportState state) {
+  if (dtls_state_ == state) {
+    return;
+  }
+  LOG_J(LS_VERBOSE, this) << "set_dtls_state from:" << dtls_state_ << " to "
+                          << state;
+  dtls_state_ = state;
+  SignalDtlsState(this);
 }
 
 }  // namespace cricket

webrtc/p2p/base/transportchannel.h

@@ -46,6 +46,8 @@ enum TransportChannelState {
 
 // A TransportChannel represents one logical stream of packets that are sent
 // between the two sides of a session.
+// TODO(deadbeef): This interface currently represents the unity of an ICE
+// transport and a DTLS transport. They need to be separated apart.
 class TransportChannel : public sigslot::has_slots<> {
  public:
   TransportChannel(const std::string& transport_name, int component)
@@ -72,10 +74,13 @@ class TransportChannel : public sigslot::has_slots<> {
   // a signal is raised.  These states are aggregated by the TransportManager.
   bool writable() const { return writable_; }
   bool receiving() const { return receiving_; }
+  DtlsTransportState dtls_state() const { return dtls_state_; }
   sigslot::signal1<TransportChannel*> SignalWritableState;
   // Emitted when the TransportChannel's ability to send has changed.
   sigslot::signal1<TransportChannel*> SignalReadyToSend;
   sigslot::signal1<TransportChannel*> SignalReceivingState;
+  // Emitted when the DtlsTransportState has changed.
+  sigslot::signal1<TransportChannel*> SignalDtlsState;
 
   // Attempts to send the given packet.  The return value is < 0 on failure.
   // TODO: Remove the default argument once channel code is updated.
@@ -158,12 +163,16 @@ class TransportChannel : public sigslot::has_slots<> {
   // Sets the receiving state, signaling if necessary.
   void set_receiving(bool receiving);
 
+  // Sets the DTLS state, signaling if necessary.
+  void set_dtls_state(DtlsTransportState state);
+
  private:
   // Used mostly for debugging.
   std::string transport_name_;
   int component_;
   bool writable_;
   bool receiving_;
+  DtlsTransportState dtls_state_ = DTLS_TRANSPORT_NEW;
 
   RTC_DISALLOW_COPY_AND_ASSIGN(TransportChannel);
 };