Blimp: add support for SSL connections.

blimp/client/session/assignment_source.cc

Index: blimp/client/session/assignment_source.cc
diff --git a/blimp/client/session/assignment_source.cc b/blimp/client/session/assignment_source.cc
index 242d7839122af23edbbeef36cf87feeedd5821d2..d53714360af8b97b61a7f0f11bb77c15db8a014d 100644
--- a/blimp/client/session/assignment_source.cc
+++ b/blimp/client/session/assignment_source.cc
@@ -7,14 +7,18 @@
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
+#include "base/files/file_util.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/location.h"
+#include "base/memory/ref_counted.h"
 #include "base/numerics/safe_conversions.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/task_runner_util.h"
 #include "base/values.h"
 #include "blimp/client/app/blimp_client_switches.h"
 #include "blimp/common/protocol_version.h"
+#include "components/safe_json/safe_json_parser.h"
 #include "net/base/ip_address.h"
 #include "net/base/ip_endpoint.h"
 #include "net/base/load_flags.h"
@@ -40,7 +44,6 @@ const char kProtocolVersionKey[] = "protocol_version";
 const char kClientTokenKey[] = "clientToken";
 const char kHostKey[] = "host";
 const char kPortKey[] = "port";
-const char kCertificateFingerprintKey[] = "certificateFingerprint";
 const char kCertificateKey[] = "certificate";
 
 // URL scheme constants for custom assignments.  See the '--blimplet-endpoint'
@@ -49,47 +52,6 @@ const char kCustomSSLScheme[] = "ssl";
 const char kCustomTCPScheme[] = "tcp";
 const char kCustomQUICScheme[] = "quic";
 
-Assignment GetCustomBlimpletAssignment() {
-  GURL url(base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-      switches::kBlimpletEndpoint));
-
-  std::string host;
-  int port;
-  if (url.is_empty() || !url.is_valid() || !url.has_scheme() ||
-      !net::ParseHostAndPort(url.path(), &host, &port)) {
-    return Assignment();
-  }
-
-  net::IPAddress ip_address;
-  if (!ip_address.AssignFromIPLiteral(host)) {
-    CHECK(false) << "Invalid BlimpletAssignment host " << host;
-  }
-
-  if (!base::IsValueInRangeForNumericType<uint16_t>(port)) {
-    CHECK(false) << "Invalid BlimpletAssignment port " << port;
-  }
-
-  Assignment::TransportProtocol protocol =
-      Assignment::TransportProtocol::UNKNOWN;
-  if (url.has_scheme()) {
-    if (url.SchemeIs(kCustomSSLScheme)) {
-      protocol = Assignment::TransportProtocol::SSL;
-    } else if (url.SchemeIs(kCustomTCPScheme)) {
-      protocol = Assignment::TransportProtocol::TCP;
-    } else if (url.SchemeIs(kCustomQUICScheme)) {
-      protocol = Assignment::TransportProtocol::QUIC;
-    } else {
-      CHECK(false) << "Invalid BlimpletAssignment scheme " << url.scheme();
-    }
-  }
-
-  Assignment assignment;
-  assignment.transport_protocol = protocol;
-  assignment.ip_endpoint = net::IPEndPoint(ip_address, port);
-  assignment.client_token = kDummyClientToken;
-  return assignment;
-}
-
 GURL GetBlimpAssignerURL() {
   // TODO(dtrainor): Add a way to specify another assigner.
   return GURL(kDefaultAssignerURL);
@@ -98,8 +60,8 @@ GURL GetBlimpAssignerURL() {
 class SimpleURLRequestContextGetter : public net::URLRequestContextGetter {
  public:
   SimpleURLRequestContextGetter(
-      const scoped_refptr<base::SingleThreadTaskRunner>& io_loop_task_runner)
-      : io_loop_task_runner_(io_loop_task_runner),
+      scoped_refptr<base::SingleThreadTaskRunner> io_loop_task_runner)
+      : io_loop_task_runner_(std::move(io_loop_task_runner)),
         proxy_config_service_(net::ProxyService::CreateSystemProxyConfigService(
             io_loop_task_runner_,
             io_loop_task_runner_)) {}
@@ -136,6 +98,74 @@ class SimpleURLRequestContextGetter : public net::URLRequestContextGetter {
   DISALLOW_COPY_AND_ASSIGN(SimpleURLRequestContextGetter);
 };
 
+// Populates an Assignment using command-line parameters, if provided.
+// Returns a null Assignment if no parameters were set.
+// Must be called on the IO thread.

[Wez, 2016/02/26 18:32:09]

nit: s/the/an

[Kevin M, 2016/02/26 19:57:22]

Done.

+

[Wez, 2016/02/26 18:32:09]

nit: Remove this blank line

[Kevin M, 2016/02/26 19:57:23]

Done.

+Assignment GetCustomAssignment() {

[Wez, 2016/02/26 18:32:09]

So should this be called GetAssignmentFromCommandLine()?

[Kevin M, 2016/02/26 19:57:23]

Done.

+  GURL url(base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+      switches::kEngineEndpoint));
+
+  // Our use of nonstandard URL schemes means the URL parser will fall back to
+  // very conservative parsing logic. It lumps everything after the scheme into
+  // the GURL "path" fragment. We need to trim leading slashes (if present) and
+  // parse the host/port pair from the path ourselves.
+  std::string path = url.path();

[Ryan Sleevi, 2016/02/25 22:16:25]

Ideally you would check .is_empty() / is_valid() / has_scheme() before this

Having just reviewed the Headless code, I suspect that in retrospect, trying to
fit this into a GURL (or GURL-shaped string) may not be ideal; it seems like
expressing this as three explicit flags avoids any ambiguity (protocol, host,
port) may be clearer, at least for your use case.

FWIW, there is url::AddStandardScheme() to register your schemes, but that has
global side-effects

[Kevin M, 2016/02/26 00:30:04]

Done.

+  if (path.substr(0, 2) == "//") {
+    path = path.erase(0, 2);
+  }
+  std::string host;
+  int port;
+  if (url.is_empty() || !url.is_valid() || !url.has_scheme() ||
+      !net::ParseHostAndPort(path, &host, &port)) {
+    return Assignment();
+  }
+
+  net::IPAddress ip_address;
+  CHECK(ip_address.AssignFromIPLiteral(host)) << "Invalid Assignment host "
+                                              << host;
+
+  Assignment::TransportProtocol protocol =
+      Assignment::TransportProtocol::UNKNOWN;
+  if (url.SchemeIs(kCustomSSLScheme)) {
+    protocol = Assignment::TransportProtocol::SSL;
+  } else if (url.SchemeIs(kCustomTCPScheme)) {
+    protocol = Assignment::TransportProtocol::TCP;
+  } else if (url.SchemeIs(kCustomQUICScheme)) {
+    protocol = Assignment::TransportProtocol::QUIC;
+  } else {
+    CHECK(false) << "Invalid engine protocol scheme " << url.scheme();
+  }
+
+  scoped_refptr<net::X509Certificate> cert;
+  if (protocol == Assignment::TransportProtocol::SSL ||
+      protocol == Assignment::TransportProtocol::QUIC) {
+    base::FilePath cert_path =
+        base::CommandLine::ForCurrentProcess()->GetSwitchValuePath(
+            switches::kEngineCertPath);
+    CHECK(!cert_path.empty()) << "Missing required parameter --"
+                              << switches::kEngineCertPath << ".";
+    std::string cert_str;
+    CHECK(base::ReadFileToString(cert_path, &cert_str))
+        << "Couldn't read from file: " << cert_path.LossyDisplayName();
+    net::CertificateList cert_list =
+        net::X509Certificate::CreateCertificateListFromBytes(
+            cert_str.data(), cert_str.size(),
+            net::X509Certificate::FORMAT_PEM_CERT_SEQUENCE);
+    CHECK_EQ(1u, cert_list.size())
+        << "Only one cert is allowed in PEM cert list.";
+    cert = std::move(cert_list[0]);
+  }
+
+  Assignment assignment;
+  assignment.transport_protocol = protocol;
+  assignment.ip_endpoint =
+      net::IPEndPoint(ip_address, base::checked_cast<uint16_t>(port));
+  assignment.client_token = kDummyClientToken;
+  assignment.cert = std::move(cert);
+  return assignment;
+}
+
 }  // namespace
 
 Assignment::Assignment() : transport_protocol(TransportProtocol::UNKNOWN) {}
@@ -148,17 +178,15 @@ bool Assignment::is_null() const {
 }
 
 AssignmentSource::AssignmentSource(
-    const scoped_refptr<base::SingleThreadTaskRunner>& main_task_runner,
-    const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner)
-    : main_task_runner_(main_task_runner),
-      url_request_context_(new SimpleURLRequestContextGetter(io_task_runner)) {}
+    scoped_refptr<base::SingleThreadTaskRunner> io_task_runner)
+    : io_task_runner_(std::move(io_task_runner)),
+      url_request_context_(new SimpleURLRequestContextGetter(io_task_runner_)),
+      weak_factory_(this) {}
 
 AssignmentSource::~AssignmentSource() {}
 
 void AssignmentSource::GetAssignment(const std::string& client_auth_token,
                                      const AssignmentCallback& callback) {
-  DCHECK(main_task_runner_->BelongsToCurrentThread());
-
   // Cancel any outstanding callback.
   if (!callback_.is_null()) {
     base::ResetAndReturn(&callback_)
@@ -166,12 +194,21 @@ void AssignmentSource::GetAssignment(const std::string& client_auth_token,
   }
   callback_ = AssignmentCallback(callback);
 
-  Assignment assignment = GetCustomBlimpletAssignment();
-  if (!assignment.is_null()) {
-    // Post the result so that the behavior of this function is consistent.
-    main_task_runner_->PostTask(
-        FROM_HERE, base::Bind(base::ResetAndReturn(&callback_),
-                              AssignmentSource::Result::RESULT_OK, assignment));
+  // Try to get a custom assignment on the IO thread first.
+  PostTaskAndReplyWithResult(

[Wez, 2016/02/26 18:32:09]

OT: I <3 PostTaskAndReplyWithResult - so handy!

+      io_task_runner_.get(), FROM_HERE, base::Bind(&GetCustomAssignment),
+      base::Bind(&AssignmentSource::OnGetCustomAssignmentDone,
+                 weak_factory_.GetWeakPtr(), client_auth_token));
+}
+
+void AssignmentSource::OnGetCustomAssignmentDone(
+    const std::string& client_auth_token,
+    Assignment custom_assignment) {
+  // If GetCustomAssignment succeeded, then return the custom assignment
+  // directly.
+  if (!custom_assignment.is_null()) {
+    base::ResetAndReturn(&callback_)
+        .Run(AssignmentSource::RESULT_OK, custom_assignment);
     return;
   }
 
@@ -191,12 +228,10 @@ void AssignmentSource::GetAssignment(const std::string& client_auth_token,
   std::string json;
   base::JSONWriter::Write(dictionary, &json);
   url_fetcher_->SetUploadData("application/json", json);
-
   url_fetcher_->Start();
 }
 
 void AssignmentSource::OnURLFetchComplete(const net::URLFetcher* source) {
-  DCHECK(main_task_runner_->BelongsToCurrentThread());
   DCHECK(!callback_.is_null());
   DCHECK_EQ(url_fetcher_.get(), source);
 
@@ -253,14 +288,14 @@ void AssignmentSource::ParseAssignerResponse() {
     return;
   }
 
-  // Attempt to interpret the response as JSON and treat it as a dictionary.
-  scoped_ptr<base::Value> json = base::JSONReader::Read(response);
-  if (!json) {
-    base::ResetAndReturn(&callback_)
-        .Run(AssignmentSource::Result::RESULT_BAD_RESPONSE, Assignment());
-    return;
-  }
+  safe_json::SafeJsonParser::Parse(
+      response, base::Bind(&AssignmentSource::AssignerJSONParseOK,
+                           weak_factory_.GetWeakPtr()),

[Wez, 2016/02/26 18:32:09]

nit: This line-wrap looks weird - I'd expect the base::Binds to be aligned - did
you apply git cl format?

[Kevin M, 2016/02/26 19:57:23]

Yes, this is formatted.

+      base::Bind(&AssignmentSource::AssignerJSONParseError,
+                 weak_factory_.GetWeakPtr()));
+}
 
+void AssignmentSource::AssignerJSONParseOK(scoped_ptr<base::Value> json) {
   const base::DictionaryValue* dict;
   if (!json->GetAsDictionary(&dict)) {
     base::ResetAndReturn(&callback_)
@@ -272,12 +307,10 @@ void AssignmentSource::ParseAssignerResponse() {
   std::string client_token;
   std::string host;
   int port;
-  std::string cert_fingerprint;
-  std::string cert;
+  std::string cert_str;
   if (!(dict->GetString(kClientTokenKey, &client_token) &&
         dict->GetString(kHostKey, &host) && dict->GetInteger(kPortKey, &port) &&
-        dict->GetString(kCertificateFingerprintKey, &cert_fingerprint) &&
-        dict->GetString(kCertificateKey, &cert))) {
+        dict->GetString(kCertificateKey, &cert_str))) {
     base::ResetAndReturn(&callback_)
         .Run(AssignmentSource::Result::RESULT_BAD_RESPONSE, Assignment());
     return;
@@ -296,18 +329,33 @@ void AssignmentSource::ParseAssignerResponse() {
     return;
   }
 
-  Assignment assignment;
+  net::CertificateList cert_list =
+      net::X509Certificate::CreateCertificateListFromBytes(
+          cert_str.data(), cert_str.size(),
+          net::X509Certificate::FORMAT_PEM_CERT_SEQUENCE);
+  if (cert_list.size() != 1) {
+    base::ResetAndReturn(&callback_)
+        .Run(AssignmentSource::Result::RESULT_INVALID_CERT, Assignment());
+    return;
+  }
+
   // The assigner assumes SSL-only and all engines it assigns only communicate
   // over SSL.
+  Assignment assignment;
   assignment.transport_protocol = Assignment::TransportProtocol::SSL;
   assignment.ip_endpoint = net::IPEndPoint(ip_address, port);
   assignment.client_token = client_token;
-  assignment.certificate = cert;
-  assignment.certificate_fingerprint = cert_fingerprint;
+  assignment.cert = std::move(cert_list[0]);
 
   base::ResetAndReturn(&callback_)
       .Run(AssignmentSource::Result::RESULT_OK, assignment);
 }
 
+void AssignmentSource::AssignerJSONParseError(const std::string& error) {
+  DLOG(ERROR) << "Error while parsing assigner JSON: " << error;
+  base::ResetAndReturn(&callback_)
+      .Run(AssignmentSource::Result::RESULT_BAD_RESPONSE, Assignment());
+}
+
 }  // namespace client
 }  // namespace blimp