Blimp: add support for SSL connections.

blimp/client/session/assignment_source_unittest.cc

Index: blimp/client/session/assignment_source_unittest.cc
diff --git a/blimp/client/session/assignment_source_unittest.cc b/blimp/client/session/assignment_source_unittest.cc
index d893ae3d3a79b4e342b233957b7215cd212e7e16..20993886a25eda18d3bce671741f8f6a30d28edd 100644
--- a/blimp/client/session/assignment_source_unittest.cc
+++ b/blimp/client/session/assignment_source_unittest.cc
@@ -5,6 +5,9 @@
 #include "blimp/client/session/assignment_source.h"
 
 #include "base/command_line.h"
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/files/scoped_temp_dir.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/test/test_simple_task_runner.h"
@@ -17,44 +20,78 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 using testing::_;
+using testing::DoAll;
 using testing::InSequence;
+using testing::NotNull;
+using testing::Return;
+using testing::SetArgPointee;
 
 namespace blimp {
 namespace client {
 namespace {
 
+const char kCertificatePEMEncoded[] =
+    "-----BEGIN CERTIFICATE-----\n"
+    "MIIDNzCCAh+gAwIBAgIJAM6Wyzih3bLNMA0GCSqGSIb3DQEBCwUAMDIxEjAQBgNV\n"
+    "BAMMCWtldmluLmNvbTEPMA0GA1UECgwGR29vZ2xlMQswCQYDVQQGEwJVUzAeFw0x\n"
+    "NjAyMTkyMTU3NDZaFw0xNzAyMTgyMTU3NDZaMDIxEjAQBgNVBAMMCWtldmluLmNv\n"
+    "bTEPMA0GA1UECgwGR29vZ2xlMQswCQYDVQQGEwJVUzCCASIwDQYJKoZIhvcNAQEB\n"
+    "BQADggEPADCCAQoCggEBALTLD9KDNKM5qSLzYct+zndPB8cFCtOIbvW7LFlwLqm7\n"
+    "w2xTOaaWhMPcbWJAo5Z+93q+vwNuNaenuENnKiJ7HFX0nyBUXjOSh8ds5JAA8+MW\n"
+    "qFM5r/+/Ww9mH3/Yhta/bzTIrt35SMclfcytH1YfACDenXNxYPA+GqFfcZnx7eat\n"
+    "Af0EvMi375Xf2qCROghuDfaPeY6FnqVNHhqFCB79f8UQmciO2cJsI3vMSLSAfoXQ\n"
+    "g3Fl4m6Q0PYFKPLECVxh9FLdqlAMBgxJIM2XhLXqR9qbrpCUlq/BVa2kmLcY061E\n"
+    "maTlrjVbtgqm5xJzXyCV1ZNBIdmfBZbO/OOOf4UFDgcCAwEAAaNQME4wHQYDVR0O\n"
+    "BBYEFCvDRIJ15+pept6O/OdVnNChxk8WMB8GA1UdIwQYMBaAFCvDRIJ15+pept6O\n"
+    "/OdVnNChxk8WMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBAI+fgjIl\n"
+    "s0QFFAw+6lupC5BkFwXp5kgnD80mbvq9jeCmd8Rzs46XhI1CsKtODNRpjcqKAakW\n"
+    "PPIMExT1kUpHrbzs4fMbEPTYQryjRk29UNV6gPm264XHJNzOfh4vvV2u8F1Glb5j\n"
+    "Lcipt9+jGFMTcSRZ6fUtAaSl0LxXTCyzqQb86IQgxxWkW61ow99D+I/x1U+gE/w1\n"
+    "8NfudsmClORGinEwy5ZepmwQHDZjpnwauLoJuo3TbTLj6BY4AFPS+VD1ySvec6Lk\n"
+    "OkI7+7+kmbrqn0OJKIUVtw/0t9qMyPH+B87uyvgSd3J/xURnPEnIbpmrqM3bAdP6\n"
+    "j05XNhHVBefITMI=\n"
+    "-----END CERTIFICATE-----";
+
 MATCHER_P(AssignmentEquals, assignment, "") {
   return arg.transport_protocol == assignment.transport_protocol &&
          arg.ip_endpoint == assignment.ip_endpoint &&
          arg.client_token == assignment.client_token &&
-         arg.certificate == assignment.certificate &&
-         arg.certificate_fingerprint == assignment.certificate_fingerprint;
+         (!assignment.cert || arg.cert->Equals(assignment.cert.get()));

[Ryan Sleevi, 2016/02/23 21:01:31]

BUG: Is this right? What if arg->cert != NULL, but assignment.cert == null.
Don't you need an additional check

return ...
  arg.client_token == .. &&
  ((!arg.cert && !assignment.cert) ||
   (arg.cert && assignment.cert && arg.cert->Equals(assignment.cert.get())));

[Kevin M, 2016/02/24 00:31:42]

Done.

 }
 
 net::IPEndPoint BuildIPEndPoint(const std::string& ip, int port) {
   net::IPAddress ip_address;
   EXPECT_TRUE(ip_address.AssignFromIPLiteral(ip));

[Ryan Sleevi, 2016/02/23 21:01:31]

BUG/DESIGN: This does nothing to stop your test early, because it's in a
subroutine. Just wanting to make sure that was expected.

[Kevin M, 2016/02/24 00:31:42]

Helper fn removed.

-
   return net::IPEndPoint(ip_address, port);
 }
 
+scoped_refptr<net::X509Certificate> ParseCert(const std::string& cert_str) {
+  net::CertificateList cert_list =
+      net::X509Certificate::CreateCertificateListFromBytes(
+          kCertificatePEMEncoded, arraysize(kCertificatePEMEncoded),
+          net::X509Certificate::FORMAT_PEM_CERT_SEQUENCE);
+  return cert_list[0];
+}
+
 Assignment BuildValidAssignment() {
   Assignment assignment;
   assignment.transport_protocol = Assignment::TransportProtocol::SSL;
   assignment.ip_endpoint = BuildIPEndPoint("100.150.200.250", 500);

[Ryan Sleevi, 2016/02/23 21:01:31]

TEST DESIGN: You should not use real IPs in tests. Please use one of the
reserved host ranges.

FURTHER TEST DESIGN: Should these be named constants?

[Kevin M, 2016/02/24 00:31:42]

Done, done, also switched to the uint8_t[4] IPEndPoint constructor.

   assignment.client_token = "SecretT0kenz";
-  assignment.certificate_fingerprint = "WhaleWhaleWhale";
-  assignment.certificate = "whaaaaaaaaaaaaale";
+  assignment.cert = ParseCert(kCertificatePEMEncoded);
   return assignment;
 }
 
-std::string BuildResponseFromAssignment(const Assignment& assignment) {
+// Builds simulated JSON response from the Assigner service.
+// |assignment|: The Assignment to convert.
+// |expected_cert_str|: The PEM encoded certificate to include in the response.
+std::string BuildResponseFromAssignment(const Assignment& assignment,
+                                        const std::string& expected_cert_str) {
   base::DictionaryValue dict;
   dict.SetString("clientToken", assignment.client_token);
   dict.SetString("host", assignment.ip_endpoint.address().ToString());
   dict.SetInteger("port", assignment.ip_endpoint.port());
-  dict.SetString("certificateFingerprint", assignment.certificate_fingerprint);
-  dict.SetString("certificate", assignment.certificate);
+  dict.SetString("certificate", expected_cert_str);
 
   std::string json;
   base::JSONWriter::Write(dict, &json);
@@ -66,7 +103,14 @@ class AssignmentSourceTest : public testing::Test {
   AssignmentSourceTest()
       : task_runner_(new base::TestSimpleTaskRunner),
         task_runner_handle_(task_runner_),
-        source_(task_runner_, task_runner_) {}
+        source_(task_runner_) {}
+
+  void SetUp() override {
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+    base::CreateTemporaryFileInDir(temp_dir_.path(), &pem_path_);
+    base::WriteFile(pem_path_, kCertificatePEMEncoded,
+                    arraysize(kCertificatePEMEncoded));

[Ryan Sleevi, 2016/02/23 21:01:31]

DESIGN: This seems wasteful/inefficient for a unit test. Is there a reason you
can't simply check in the file and load during your unit tests?

//net does this extensively with //net/data and //net/base/test_data_directory.h

[Kevin M, 2016/02/24 00:31:42]

Done, created a PEM file for unittest.selfsigned.der.

+  }
 
   // This expects the AssignmentSource::GetAssignment to return a custom
   // endpoint without having to hit the network.  This will typically be used
@@ -90,11 +134,10 @@ class AssignmentSourceTest : public testing::Test {
     source_.GetAssignment(client_auth_token,
                           base::Bind(&AssignmentSourceTest::AssignmentResponse,
                                      base::Unretained(this)));
+    task_runner_->RunUntilIdle();
 
     net::TestURLFetcher* fetcher = factory_.GetFetcherByID(0);
 
-    task_runner_->RunUntilIdle();
-
     EXPECT_NE(nullptr, fetcher);
     EXPECT_EQ(kDefaultAssignerURL, fetcher->GetOriginalURL().spec());
 
@@ -138,6 +181,9 @@ class AssignmentSourceTest : public testing::Test {
 
   net::TestURLFetcherFactory factory_;

[Ryan Sleevi, 2016/02/23 21:01:31]

FWIW my gut is that using EmbeddedTestServer might be easier for this than what
you're doing with TestURLFetcher, but that's more for future
work/experimentation.

[Kevin M, 2016/02/24 00:31:42]

Acknowledged.

 
+  base::ScopedTempDir temp_dir_;
+  base::FilePath pem_path_;
+
   AssignmentSource source_;
 };
 
@@ -146,9 +192,10 @@ TEST_F(AssignmentSourceTest, TestTCPAlternateEndpointSuccess) {
   assignment.transport_protocol = Assignment::TransportProtocol::TCP;
   assignment.ip_endpoint = BuildIPEndPoint("100.150.200.250", 500);
   assignment.client_token = kDummyClientToken;
+  assignment.cert = scoped_refptr<net::X509Certificate>(nullptr);
 
   base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
-      switches::kBlimpletEndpoint, "tcp:100.150.200.250:500");
+      switches::kEngineEndpoint, "tcp:100.150.200.250:500");

[Ryan Sleevi, 2016/02/23 21:01:31]

Same remarks about constants, but also about URL scheme syntax.

[Kevin M, 2016/02/24 00:31:42]

Done.

 
   EXPECT_CALL(*this, AssignmentResponse(AssignmentSource::Result::RESULT_OK,
                                         AssignmentEquals(assignment)))
@@ -162,9 +209,12 @@ TEST_F(AssignmentSourceTest, TestSSLAlternateEndpointSuccess) {
   assignment.transport_protocol = Assignment::TransportProtocol::SSL;
   assignment.ip_endpoint = BuildIPEndPoint("100.150.200.250", 500);
   assignment.client_token = kDummyClientToken;
+  assignment.cert = ParseCert(kCertificatePEMEncoded);
 
   base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
-      switches::kBlimpletEndpoint, "ssl:100.150.200.250:500");
+      switches::kEngineEndpoint, "ssl:100.150.200.250:500");
+  base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+      switches::kEngineCertPath, pem_path_.value());
 
   EXPECT_CALL(*this, AssignmentResponse(AssignmentSource::Result::RESULT_OK,
                                         AssignmentEquals(assignment)))
@@ -178,9 +228,12 @@ TEST_F(AssignmentSourceTest, TestQUICAlternateEndpointSuccess) {
   assignment.transport_protocol = Assignment::TransportProtocol::QUIC;
   assignment.ip_endpoint = BuildIPEndPoint("100.150.200.250", 500);
   assignment.client_token = kDummyClientToken;
+  assignment.cert = ParseCert(kCertificatePEMEncoded);
 
   base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
-      switches::kBlimpletEndpoint, "quic:100.150.200.250:500");
+      switches::kEngineEndpoint, "quic:100.150.200.250:500");
+  base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+      switches::kEngineCertPath, pem_path_.value());
 
   EXPECT_CALL(*this, AssignmentResponse(AssignmentSource::Result::RESULT_OK,
                                         AssignmentEquals(assignment)))
@@ -197,7 +250,8 @@ TEST_F(AssignmentSourceTest, TestSuccess) {
       .Times(1);
 
   GetNetworkAssignmentAndWaitForResponse(
-      net::HTTP_OK, net::Error::OK, BuildResponseFromAssignment(assignment),
+      net::HTTP_OK, net::Error::OK,
+      BuildResponseFromAssignment(assignment, kCertificatePEMEncoded),
       "UserAuthT0kenz", kEngineVersion);
 }
 
@@ -221,7 +275,8 @@ TEST_F(AssignmentSourceTest, TestSecondRequestInterruptsFirst) {
       .RetiresOnSaturation();
 
   GetNetworkAssignmentAndWaitForResponse(
-      net::HTTP_OK, net::Error::OK, BuildResponseFromAssignment(assignment),
+      net::HTTP_OK, net::Error::OK,
+      BuildResponseFromAssignment(assignment, kCertificatePEMEncoded),
       "UserAuthT0kenz", kEngineVersion);
 }
 
@@ -244,7 +299,8 @@ TEST_F(AssignmentSourceTest, TestValidAfterError) {
                                          "", "UserAuthT0kenz", kEngineVersion);
 
   GetNetworkAssignmentAndWaitForResponse(
-      net::HTTP_OK, net::Error::OK, BuildResponseFromAssignment(assignment),
+      net::HTTP_OK, net::Error::OK,
+      BuildResponseFromAssignment(assignment, kCertificatePEMEncoded),
       "UserAuthT0kenz", kEngineVersion);
 }
 
@@ -306,7 +362,8 @@ TEST_F(AssignmentSourceTest, TestInvalidJsonResponse) {
   Assignment assignment = BuildValidAssignment();
 
   // Remove half the response.
-  std::string response = BuildResponseFromAssignment(assignment);
+  std::string response =
+      BuildResponseFromAssignment(assignment, kCertificatePEMEncoded);
   response = response.substr(response.size() / 2);
 
   EXPECT_CALL(*this, AssignmentResponse(
@@ -320,7 +377,6 @@ TEST_F(AssignmentSourceTest, TestMissingResponsePort) {
   base::DictionaryValue dict;
   dict.SetString("clientToken", "SecretT0kenz");
   dict.SetString("host", "happywhales");
-  dict.SetString("certificateFingerprint", "WhaleWhaleWhale");
   dict.SetString("certificate", "whaaaaaaaaaaaaale");
 
   std::string response;
@@ -338,7 +394,6 @@ TEST_F(AssignmentSourceTest, TestInvalidIPAddress) {
   dict.SetString("clientToken", "SecretT0kenz");
   dict.SetString("host", "happywhales");
   dict.SetInteger("port", 500);
-  dict.SetString("certificateFingerprint", "WhaleWhaleWhale");
   dict.SetString("certificate", "whaaaaaaaaaaaaale");
 
   std::string response;
@@ -350,6 +405,37 @@ TEST_F(AssignmentSourceTest, TestInvalidIPAddress) {
                                          "UserAuthT0kenz", kEngineVersion);
 }
 
+TEST_F(AssignmentSourceTest, TestMissingCert) {
+  base::DictionaryValue dict;
+  dict.SetString("clientToken", "SecretT0kenz");
+  dict.SetString("host", "127.0.0.1");
+  dict.SetInteger("port", 500);
+
+  std::string response;
+  base::JSONWriter::Write(dict, &response);
+
+  EXPECT_CALL(*this, AssignmentResponse(
+                         AssignmentSource::Result::RESULT_BAD_RESPONSE, _));
+  GetNetworkAssignmentAndWaitForResponse(net::HTTP_OK, net::Error::OK, response,
+                                         "UserAuthT0kenz", kEngineVersion);
+}
+
+TEST_F(AssignmentSourceTest, TestInvalidCert) {
+  base::DictionaryValue dict;
+  dict.SetString("clientToken", "SecretT0kenz");
+  dict.SetString("host", "127.0.0.1");
+  dict.SetInteger("port", 500);
+  dict.SetString("certificate", "h4x0r c3r7");
+
+  std::string response;
+  base::JSONWriter::Write(dict, &response);
+
+  EXPECT_CALL(*this, AssignmentResponse(
+                         AssignmentSource::Result::RESULT_INVALID_CERT, _));
+  GetNetworkAssignmentAndWaitForResponse(net::HTTP_OK, net::Error::OK, response,
+                                         "UserAuthT0kenz", kEngineVersion);
+}
+
 }  // namespace
 }  // namespace client
 }  // namespace blimp