Blimp: add support for SSL connections.

blimp/net/ssl_client_transport.cc

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "blimp/net/ssl_client_transport.h"
+
+#include "base/callback.h"
+#include "base/callback_helpers.h"
+#include "blimp/net/exact_match_cert_verifier.h"
+#include "blimp/net/stream_socket_connection.h"
+#include "net/base/host_port_pair.h"
+#include "net/cert/x509_certificate.h"
+#include "net/socket/client_socket_factory.h"
+#include "net/socket/client_socket_handle.h"
+#include "net/socket/ssl_client_socket.h"
+#include "net/socket/stream_socket.h"
+#include "net/socket/tcp_client_socket.h"
+#include "net/ssl/ssl_config.h"
+
+namespace blimp {
+
+SSLClientTransport::SSLClientTransport(const net::AddressList& addresses,
+                                       scoped_refptr<net::X509Certificate> cert,
+                                       net::NetLog* net_log)
+    : TCPClientTransport(addresses, net_log), cert_verifier_(cert) {}

[Ryan Sleevi, 2016/02/19 22:56:08]

STYLE: You're passing a scoped_refptr<> as a non-const, so you need to
std::move() this to avoid unnecessary refcount churn.

[Kevin M, 2016/02/22 22:53:32]

Thanks, but I'll just make this const& for consistency.

+
+SSLClientTransport::~SSLClientTransport() {}
+
+const std::string SSLClientTransport::GetName() const {
+  return "SSL";
+}
+
+void SSLClientTransport::OnTCPConnectComplete(int result) {
+  DCHECK_NE(net::ERR_IO_PENDING, result);
+
+  scoped_ptr<net::StreamSocket> tcp_socket = TCPClientTransport::TakeSocket();
+
+  DVLOG(1) << "TCP connection result=" << result;
+  if (result != net::OK) {
+    OnConnectComplete(result);
+    return;
+  }
+
+  // Forward connection details to the SSL layer.
+  net::IPEndPoint connected_endpoint;
+  result = tcp_socket->GetPeerAddress(&connected_endpoint);
+  if (result != net::OK) {
+    OnConnectComplete(result);
+    return;
+  }
+
+  // Construct arguments to use for the SSL socket factory.
+  scoped_ptr<net::ClientSocketHandle> socket_handle(
+      new net::ClientSocketHandle);
+  socket_handle->SetSocket(std::move(tcp_socket));
+
+  net::HostPortPair host_port_pair =
+      net::HostPortPair::FromIPEndPoint(connected_endpoint);

[Ryan Sleevi, 2016/02/19 22:56:08]

DESIGN: In production code, this would be categorically unsafe for real SSL.

Though you're not attempting to properly implement TLS at this time, your API
should explicitly take the desired hostname (or IP address, in string-form), and
pass that through the appropriate layers, as this is necessary for TLS
connections.

I have trouble l-g-t-m'ing this change, knowing that the API encourages unsafe
practices in its present form.

[Kevin M, 2016/02/22 22:53:32]

Done - the original address is used here. I can also check if the connected
socket's peer address matches, too, but I'm not sure if that would be adding any
security.

+
+  net::SSLClientSocketContext create_context;
+  create_context.cert_verifier = &cert_verifier_;
+  create_context.transport_security_state = &transport_security_state_;
+
+  scoped_ptr<net::StreamSocket> ssl_socket(
+      socket_factory()->CreateSSLClientSocket(std::move(socket_handle),
+                                              host_port_pair, net::SSLConfig(),
+                                              create_context));
+
+  if (!ssl_socket) {
+    OnConnectComplete(net::ERR_SSL_PROTOCOL_ERROR);
+    return;
+  }
+
+  result = ssl_socket->Connect(base::Bind(
+      &SSLClientTransport::OnSSLConnectComplete, base::Unretained(this)));
+  SetSocket(std::move(ssl_socket));
+
+  if (result == net::ERR_IO_PENDING) {
+    // SSL connection will complete asynchronously.
+    return;
+  }
+
+  OnSSLConnectComplete(result);
+}
+
+void SSLClientTransport::OnSSLConnectComplete(int result) {
+  DCHECK_NE(net::ERR_IO_PENDING, result);
+  DVLOG(1) << "SSL connection result=" << result;
+
+  OnConnectComplete(result);
+}
+
+}  // namespace blimp