Accept remote offers with current DTLS role, rather than "actpass".

webrtc/p2p/base/jseptransport.cc

 /*
  *  Copyright 2004 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 260 matching lines @@
   if (!needs_ice_restart_) {
     needs_ice_restart_ = true;
     LOG(LS_VERBOSE) << "needs-ice-restart flag set for transport " << mid();
   }
 }
 
 bool JsepTransport::NeedsIceRestart() const {
   return needs_ice_restart_;
 }
 
-void JsepTransport::GetSslRole(rtc::SSLRole* ssl_role) const {
-  RTC_DCHECK(ssl_role);
-  *ssl_role = secure_role_;
+rtc::Optional<rtc::SSLRole> JsepTransport::GetSslRole() const {
+  return ssl_role_;
 }
 
 bool JsepTransport::GetStats(TransportStats* stats) {
   stats->transport_name = mid();
   stats->channel_stats.clear();
   for (auto& kv : channels_) {
     DtlsTransportInternal* dtls_transport = kv.second;
     TransportChannelStats substats;
     substats.component = kv.first;
     dtls_transport->GetSrtpCryptoSuite(&substats.srtp_crypto_suite);
@@ skipping 41 matching lines @@
   if (certificate_) {
     ret = dtls_transport->SetLocalCertificate(certificate_);
     RTC_DCHECK(ret);
   }
   return ret;
 }
 
 bool JsepTransport::ApplyRemoteTransportDescription(
     DtlsTransportInternal* dtls_transport,
     std::string* error_desc) {
-  // Currently, all ICE-related calls still go through this DTLS channel. But
-  // that will change once we get rid of TransportChannelImpl, and the DTLS
-  // channel interface no longer includes ICE-specific methods. Then this class
-  // will need to call dtls->ice()->SetIceRole(), for example, assuming the Dtls
-  // interface will expose its inner ICE channel.
   dtls_transport->ice_transport()->SetRemoteIceParameters(
       remote_description_->GetIceParameters());
   dtls_transport->ice_transport()->SetRemoteIceMode(
       remote_description_->ice_mode);
   return true;
 }
 
 bool JsepTransport::ApplyNegotiatedTransportDescription(
     DtlsTransportInternal* dtls_transport,
     std::string* error_desc) {
   // Set SSL role. Role must be set before fingerprint is applied, which
   // initiates DTLS setup.
-  if (!dtls_transport->SetSslRole(secure_role_)) {
+  if (ssl_role_ && !dtls_transport->SetSslRole(*ssl_role_)) {
     return BadTransportDescription("Failed to set SSL role for the channel.",
                                    error_desc);
   }
   // Apply remote fingerprint.
   if (!dtls_transport->SetRemoteFingerprint(
           remote_fingerprint_->algorithm,
           reinterpret_cast<const uint8_t*>(remote_fingerprint_->digest.data()),
           remote_fingerprint_->digest.size())) {
     return BadTransportDescription("Failed to apply remote fingerprint.",
                                    error_desc);
   }
   return true;
 }
 
-bool JsepTransport::NegotiateTransportDescription(ContentAction local_role,
-                                                  std::string* error_desc) {
+bool JsepTransport::NegotiateTransportDescription(
+    ContentAction local_description_type,
+    std::string* error_desc) {
   if (!local_description_ || !remote_description_) {
     const std::string msg =
         "Applying an answer transport description "
         "without applying any offer.";
     return BadTransportDescription(msg, error_desc);
   }
   rtc::SSLFingerprint* local_fp =
       local_description_->identity_fingerprint.get();
   rtc::SSLFingerprint* remote_fp =
       remote_description_->identity_fingerprint.get();
   if (remote_fp && local_fp) {
     remote_fingerprint_.reset(new rtc::SSLFingerprint(*remote_fp));
-    if (!NegotiateRole(local_role, &secure_role_, error_desc)) {
+    if (!NegotiateRole(local_description_type, error_desc)) {
       return false;
     }
-  } else if (local_fp && (local_role == CA_ANSWER)) {
+  } else if (local_fp && (local_description_type == CA_ANSWER)) {
     return BadTransportDescription(
         "Local fingerprint supplied when caller didn't offer DTLS.",
         error_desc);
   } else {
     // We are not doing DTLS
     remote_fingerprint_.reset(new rtc::SSLFingerprint("", nullptr, 0));
   }
   // Now that we have negotiated everything, push it downward.
   // Note that we cache the result so that if we have race conditions
   // between future SetRemote/SetLocal invocations and new channel
   // creation, we have the negotiation state saved until a new
   // negotiation happens.
   for (const auto& kv : channels_) {
     if (!ApplyNegotiatedTransportDescription(kv.second, error_desc)) {
       return false;
     }
   }
   return true;
 }
 
-bool JsepTransport::NegotiateRole(ContentAction local_role,
-                                  rtc::SSLRole* ssl_role,
-                                  std::string* error_desc) const {
-  RTC_DCHECK(ssl_role);
+bool JsepTransport::NegotiateRole(ContentAction local_description_type,
+                                  std::string* error_desc) {
   if (!local_description_ || !remote_description_) {
     const std::string msg =
         "Local and Remote description must be set before "
         "transport descriptions are negotiated";
     return BadTransportDescription(msg, error_desc);
   }
 
   // From RFC 4145, section-4.1, The following are the values that the
   // 'setup' attribute can take in an offer/answer exchange:
   //       Offer      Answer
@@ skipping 14 matching lines @@
   // latency. setup:active allows the answer and the DTLS handshake to
   // occur in parallel.  Thus, setup:active is RECOMMENDED.  Whichever
   // party is active MUST initiate a DTLS handshake by sending a
   // ClientHello over each flow (host/port quartet).
   // IOW - actpass and passive modes should be treated as server and
   // active as client.
   ConnectionRole local_connection_role = local_description_->connection_role;
   ConnectionRole remote_connection_role = remote_description_->connection_role;
 
   bool is_remote_server = false;
-  if (local_role == CA_OFFER) {
+  if (local_description_type == CA_OFFER) {
     if (local_connection_role != CONNECTIONROLE_ACTPASS) {
       return BadTransportDescription(
           "Offerer must use actpass value for setup attribute.", error_desc);
     }
 
     if (remote_connection_role == CONNECTIONROLE_ACTIVE ||
         remote_connection_role == CONNECTIONROLE_PASSIVE ||
         remote_connection_role == CONNECTIONROLE_NONE) {
       is_remote_server = (remote_connection_role == CONNECTIONROLE_PASSIVE);
     } else {
       const std::string msg =
           "Answerer must use either active or passive value "
           "for setup attribute.";
       return BadTransportDescription(msg, error_desc);
     }
     // If remote is NONE or ACTIVE it will act as client.
   } else {
     if (remote_connection_role != CONNECTIONROLE_ACTPASS &&
         remote_connection_role != CONNECTIONROLE_NONE) {
-      return BadTransportDescription(
-          "Offerer must use actpass value for setup attribute.", error_desc);
+      // Accept a remote role attribute that's not "actpass", but matches the
+      // current negotiated role. This is allowed by dtls-sdp, though our

[pthatcher1, 2017/03/25 03:30:01]

What's "dtls-sdp"?  An RFC a draft?  Being more specific would be nice,
especially if in includes a section number.

[Taylor Brandstetter, 2017/03/27 21:39:28]

Added link and section number.

+      // implementation will never generate such an offer as it's not
+      // recommended.
+      if (!ssl_role_ ||
+          (*ssl_role_ == rtc::SSL_CLIENT &&
+           remote_connection_role == CONNECTIONROLE_ACTIVE) ||
+          (*ssl_role_ == rtc::SSL_SERVER &&
+           remote_connection_role == CONNECTIONROLE_PASSIVE)) {
+        return BadTransportDescription(
+            "Offerer must use actpass value or current negotiated role for "
+            "setup attribute.",
+            error_desc);
+      }
     }
 
     if (local_connection_role == CONNECTIONROLE_ACTIVE ||
         local_connection_role == CONNECTIONROLE_PASSIVE) {
       is_remote_server = (local_connection_role == CONNECTIONROLE_ACTIVE);
     } else {
       const std::string msg =
           "Answerer must use either active or passive value "
           "for setup attribute.";
       return BadTransportDescription(msg, error_desc);
     }
 
     // If local is passive, local will act as server.
   }
 
-  *ssl_role = is_remote_server ? rtc::SSL_CLIENT : rtc::SSL_SERVER;
+  ssl_role_.emplace(is_remote_server ? rtc::SSL_CLIENT : rtc::SSL_SERVER);
   return true;
 }
 
 }  // namespace cricket