Allow TransportController to create a QuicTransportChannel

webrtc/p2p/quic/quictransport.cc

+/*
+ *  Copyright 2016 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.
+ */
+
+#include "webrtc/p2p/quic/quictransport.h"
+
+namespace cricket {
+
+QuicTransport::QuicTransport(
+    const std::string& name,
+    PortAllocator* allocator,
+    const rtc::scoped_refptr<rtc::RTCCertificate>& certificate)
+    : Transport(name, allocator),
+      certificate_(certificate),
+      secure_role_(rtc::SSL_CLIENT) {}
+
+QuicTransport::~QuicTransport() {
+  DestroyAllChannels();  // Must be called to avoid assertion error.
+}
+
+void QuicTransport::SetLocalCertificate(
+    const rtc::scoped_refptr<rtc::RTCCertificate>& certificate) {
+  certificate_ = certificate;
+}
+bool QuicTransport::GetLocalCertificate(
+    rtc::scoped_refptr<rtc::RTCCertificate>* certificate) {
+  if (!certificate_)
+    return false;
+
+  *certificate = certificate_;
+  return true;
+}
+
+bool QuicTransport::ApplyLocalTransportDescription(
+    TransportChannelImpl* channel,
+    std::string* error_desc) {
+  rtc::SSLFingerprint* local_fp =
+      local_description()->identity_fingerprint.get();
+
+  if (local_fp) {
+    // Sanity check local fingerprint.
+    if (certificate_) {
+      rtc::scoped_ptr<rtc::SSLFingerprint> local_fp_tmp(
+          rtc::SSLFingerprint::Create(local_fp->algorithm,
+                                      certificate_->identity()));
+      ASSERT(local_fp_tmp.get() != NULL);
+      if (!(*local_fp_tmp == *local_fp)) {
+        std::ostringstream desc;
+        desc << "Local fingerprint does not match identity. Expected: ";
+        desc << local_fp_tmp->ToString();
+        desc << " Got: " << local_fp->ToString();
+        return BadTransportDescription(desc.str(), error_desc);
+      }
+    } else {
+      return BadTransportDescription(
+          "Local fingerprint provided but no identity available.", error_desc);
+    }
+  } else {
+    certificate_ = nullptr;
+  }
+
+  if (!channel->SetLocalCertificate(certificate_)) {
+    return BadTransportDescription("Failed to set local identity.", error_desc);
+  }
+
+  return Transport::ApplyLocalTransportDescription(channel, error_desc);
+}
+
+bool QuicTransport::NegotiateTransportDescription(ContentAction local_role,
+                                                  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);
+  }
+
+  rtc::SSLFingerprint* remote_fp =
+      remote_description()->identity_fingerprint.get();
+
+  if (!remote_fp) {
+    return BadTransportDescription("Remote fingerprint not supplied.",
+                                   error_desc);
+  }
+
+  remote_fingerprint_.reset(new rtc::SSLFingerprint(*remote_fp));
+
+  // TODO(pthatcher): Move all the negotiation up into PeerConnection and
+  // out of the Transport code. Just have a SetRole or similar method here.
+  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_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);
+    }
+
+    if (local_connection_role != CONNECTIONROLE_ACTIVE &&
+        local_connection_role != CONNECTIONROLE_PASSIVE) {
+      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.
+    is_remote_server = (local_connection_role == CONNECTIONROLE_ACTIVE);
+  }
+
+  secure_role_ = is_remote_server ? rtc::SSL_CLIENT : rtc::SSL_SERVER;
+
+  // Now run the negotiation for the Transport class.
+  return Transport::NegotiateTransportDescription(local_role, error_desc);

[Taylor Brandstetter, 2016/04/05 22:13:49]

There's a lot of duplicated code here. Would it maybe make sense to have
DtlsTransport and QuicTransport inherit from a common base class, like
"SecureTransport"? Or alternatively, define an SSL negotiation helper class
that's used by both classes (in order to use composition instead of
inheritance).

[mikescarlett, 2016/04/06 18:43:44]

To avoid code duplication, another solution is that lines 77-135 and the
equivalent code from DtlsTransport could be replaced with a utility function
like:

bool NegotiateSSLRole(ContentAction local_role, const TransportDescription&
local_description, const TransportDescription& remote_description, SSLRole*
ssl_role, std::string* error_desc);

I think moving this code out of the Transport class is a good solution - better
than inheritance if our goal is to eliminate dependence on it. A helper class or
interface makes sense if SSL negotiation depends on a state external to the
negotiated session description, but as far as I can tell a utility function
would be effective.

[Taylor Brandstetter, 2016/04/06 19:27:41]

That sounds like a good idea. Though the advantage of using inheritance or a
helper class is that several more of these methods wouldn't need to be
duplicated (ApplyLocalTD, ApplyNegotiatedTD, etc.). However, if you think
QuicTransport's implementation of these methods may eventually diverge from 
DtlsTransport's, it makes sense to duplicate them for now; it's up to you

[mikescarlett, 2016/04/06 21:18:14]

Good point. My main concern with inheritance is that the class hierarchy would
become:

Transport -> P2PTransport -> SecureTransport -> DtlsTransport
                                             -> QuicTransport

instead of:

Transport -> P2PTransport -> DtlsTransport
          -> QuicTransport

even though the main purpose of subclassing Transport appears to be
creating/destroying transport channels and performing negotiation logic not done
by the Transport base class.

Ideally DtlsTransport would have a TransportChannelFactory as an input
parameter, which creates the ICE and crypto channels, then DtlsTransport would
be renamed to SecureTransport to make the hierarchy:

Transport -> SecureTransport

[mikescarlett, 2016/04/12 00:06:38]

Peter and I discussed this offline. He says that duplicating the code here is
okay as a temporary measure, since the negotiation logic should eventually be
moved to PeerConnection out of Transport.

Since moving the negotiation logic right now would make landing this CL complex,
my plan is to keep QuicTransport as a direct subclass of Transport with the
duplicated code.

+}
+
+QuicTransportChannel* QuicTransport::CreateTransportChannel(int component) {
+  P2PTransportChannel* ice_channel =
+      new P2PTransportChannel(name(), component, port_allocator());
+  QuicTransportChannel* quic_channel = new QuicTransportChannel(ice_channel);
+  ice_channel_by_quic_channel_[quic_channel] = ice_channel;
+  return quic_channel;
+}
+
+void QuicTransport::DestroyTransportChannel(TransportChannelImpl* channel) {
+  const auto& kv = ice_channel_by_quic_channel_.find(channel);
+  RTC_DCHECK(kv != ice_channel_by_quic_channel_.end());
+  delete kv->first;
+  delete kv->second;
+}
+
+bool QuicTransport::GetSslRole(rtc::SSLRole* ssl_role) const {
+  ASSERT(ssl_role != NULL);
+  *ssl_role = secure_role_;
+  return true;
+}
+
+bool QuicTransport::ApplyNegotiatedTransportDescription(
+    TransportChannelImpl* channel,
+    std::string* error_desc) {
+  // Set ssl role and remote fingerprint. These are required for QUIC setup.
+  if (!channel->SetSslRole(secure_role_)) {
+    return BadTransportDescription("Failed to set ssl role for the channel.",
+                                   error_desc);
+  }
+  // Apply remote fingerprint.
+  if (!channel->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 Transport::ApplyNegotiatedTransportDescription(channel, error_desc);
+}
+
+}  // namespace cricket