Blimp: add support for SSL connections.

blimp/client/session/assignment_source.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/client/session/assignment_source.h"
 
 #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"
 #include "net/base/net_errors.h"
 #include "net/base/url_util.h"
 #include "net/http/http_status_code.h"
 #include "net/proxy/proxy_config_service.h"
 #include "net/proxy/proxy_service.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_builder.h"
 #include "net/url_request/url_request_context_getter.h"
 
 namespace blimp {
 namespace client {
 
 namespace {
 
 // Assignment request JSON keys.
 const char kProtocolVersionKey[] = "protocol_version";
 
 // Assignment response JSON keys.
 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'
 // documentation in blimp_client_switches.cc for details.
-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;
-}
+const char kSSLTransport[] = "ssl";
+const char kTCPTransport[] = "tcp";
+const char kQUICTransport[] = "quic";
 
 GURL GetBlimpAssignerURL() {
   // TODO(dtrainor): Add a way to specify another assigner.
   return GURL(kDefaultAssignerURL);
 }
 
 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_)) {}
 
   // net::URLRequestContextGetter implementation.
   net::URLRequestContext* GetURLRequestContext() override {
     if (!url_request_context_) {
       net::URLRequestContextBuilder builder;
       builder.set_proxy_config_service(std::move(proxy_config_service_));
       builder.DisableHttpCache();
@@ skipping 16 matching lines @@
 
   // Temporary storage for the ProxyConfigService, which needs to be created on
   // the main thread but cleared on the IO thread.  This will be built in the
   // constructor and cleared on the IO thread.  Due to the usage of this class
   // this is safe.
   scoped_ptr<net::ProxyConfigService> proxy_config_service_;
 
   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.
+
+Assignment GetCustomAssignment() {
+  std::string ip_str =
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          switches::kEngineIP);
+  std::string port_str =
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          switches::kEnginePort);
+  std::string transport_str =
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          switches::kEngineTransport);
+
+  if (ip_str.empty() && port_str.empty() && transport_str.empty()) {
+    return Assignment();
+  }
+
+  CHECK(ip_str.empty() == port_str.empty() &&

[Bernhard Bauer, 2016/02/26 16:26:30]

You mention in the review that failing with an error message is useful for
developers, but is CHECKing really what you want? For example, a crash due to a
CHECK will produce a crash report that is uploaded to our servers, which does
not seem quite right for something that could legitimately happen due to a user
error. It also means you can't actually test the behavior if invalid switches
are passed in (unless you use death tests, but those are kinda yuck IMO).

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

Good point about the reporting noise.

I'll switch to DCHECKs/DLOGs with graceful fallback behavior for release builds.

+        ip_str.empty() == transport_str.empty())
+      << "--engine-ip, --engine-port, and --engine-transport must be set "
+         "together.";
+
+  unsigned port_parsed;
+  uint16_t port;
+  CHECK(base::StringToUint(port_str, &port_parsed) && port_parsed > 0 &&
+        port_parsed <= 65535)
+      << "--engine-port must be a value between 0 and 65535.";
+  port = base::checked_cast<uint16_t>(port_parsed);
+
+  net::IPAddress ip_address;
+  CHECK(ip_address.AssignFromIPLiteral(ip_str)) << "Invalid engine IP "
+                                                << ip_str;
+
+  Assignment::TransportProtocol transport =
+      Assignment::TransportProtocol::UNKNOWN;
+  if (transport_str == kSSLTransport) {
+    transport = Assignment::TransportProtocol::SSL;
+  } else if (transport_str == kTCPTransport) {
+    transport = Assignment::TransportProtocol::TCP;
+  } else if (transport_str == kQUICTransport) {
+    transport = Assignment::TransportProtocol::QUIC;
+  } else {
+    LOG(FATAL) << "Invalid engine transport " << transport;
+  }
+
+  scoped_refptr<net::X509Certificate> cert;
+  if (transport == Assignment::TransportProtocol::SSL ||
+      transport == 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 = transport;
+  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) {}
 
 Assignment::~Assignment() {}
 
-bool Assignment::is_null() const {
-  return ip_endpoint.address().empty() || ip_endpoint.port() == 0 ||
-         transport_protocol == TransportProtocol::UNKNOWN;
+bool Assignment::IsValid() const {
+  return !ip_endpoint.address().empty() && ip_endpoint.port() != 0 &&
+         transport_protocol != TransportProtocol::UNKNOWN;
 }
 
 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_)),

[Bernhard Bauer, 2016/02/26 16:26:30]

Actually, I'm still confused. You use this thread for the network request, but
also to synchronously read files? Wouldn't that block any network communication?
Or is just that you don't care because those things happen sequentially?

(TBH, I would still use two different threads here and set
ThreadRestrictions::SetIOAllowed() on the network thread, just to make it clear
in the future which thread should be used for what, but it's up to you, I guess.
Blimp is crazy, I tell ya! 😉)

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

I added a separate task runner for file access. The operations are executed
sequentially, and the file accesses happen once on application start, so it's OK
that they point to the same task runner refptr for the time being, but this
allows for the caller to run the operations on separate threads if needed.

+      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_)
         .Run(AssignmentSource::Result::RESULT_SERVER_INTERRUPTED, Assignment());
   }
   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(
+      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.IsValid()) {
+    base::ResetAndReturn(&callback_)
+        .Run(AssignmentSource::RESULT_OK, custom_assignment);
     return;
   }
 
   // Call out to the network for a real assignment.  Build the network request
   // to hit the assigner.
   url_fetcher_ = net::URLFetcher::Create(GetBlimpAssignerURL(),
                                          net::URLFetcher::POST, this);
   url_fetcher_->SetRequestContext(url_request_context_.get());
   url_fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SAVE_COOKIES |
                              net::LOAD_DO_NOT_SEND_COOKIES);
   url_fetcher_->AddExtraRequestHeader("Authorization: Bearer " +
                                       client_auth_token);
 
   // Write the JSON for the request data.
   base::DictionaryValue dictionary;
   dictionary.SetString(kProtocolVersionKey, blimp::kEngineVersion);
   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);
 
   if (!source->GetStatus().is_success()) {
     DVLOG(1) << "Assignment request failed due to network error: "
              << net::ErrorToString(source->GetStatus().error());
     base::ResetAndReturn(&callback_)
         .Run(AssignmentSource::Result::RESULT_NETWORK_FAILURE, Assignment());
     return;
   }
@@ skipping 36 matching lines @@
   DCHECK_EQ(net::HTTP_OK, url_fetcher_->GetResponseCode());
 
   // Grab the response from the assigner request.
   std::string response;
   if (!url_fetcher_->GetResponseAsString(&response)) {
     base::ResetAndReturn(&callback_)
         .Run(AssignmentSource::Result::RESULT_BAD_RESPONSE, Assignment());
     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()),
+      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_)
         .Run(AssignmentSource::Result::RESULT_BAD_RESPONSE, Assignment());
     return;
   }
 
   // Validate that all the expected fields are present.
   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;
   }
 
   net::IPAddress ip_address;
   if (!ip_address.AssignFromIPLiteral(host)) {
     base::ResetAndReturn(&callback_)
         .Run(AssignmentSource::Result::RESULT_BAD_RESPONSE, Assignment());
     return;
   }
 
   if (!base::IsValueInRangeForNumericType<uint16_t>(port)) {
     base::ResetAndReturn(&callback_)
         .Run(AssignmentSource::Result::RESULT_BAD_RESPONSE, Assignment());
     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