[Cronet-iOS] Public-Key-Pinning Tests

components/cronet/ios/test/cronet_pkp_test.mm

+// Copyright 2017 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.
+
+#import <Cronet/Cronet.h>
+
+#include "components/cronet/ios/test/start_cronet.h"
+#include "components/grpc_support/test/quic_test_server.h"
+#include "cronet_test_base.h"
+#include "net/base/mac/url_conversions.h"
+#include "net/cert/mock_cert_verifier.h"
+#include "net/test/cert_test_util.h"
+#include "net/test/test_data_directory.h"
+
+#include "testing/gtest_mac.h"
+
+namespace {
+const bool kINCLUDE_SUBDOMAINS = true;

[mef, 2017/06/12 22:25:23]

I think constant names are kCamelCase.
Also, if those are ObjC, then I think they should be BOOL and YES?

[kapishnikov, 2017/06/16 20:11:05]

Done.

+const bool kEXCLUDE_SUBDOMAINS = false;
+const bool kSUCCESS = true;
+const bool kERROR = false;
+const std::string kSERVER_CERT = "quic_test.example.com.crt";
+NSDate* distant_future = [NSDate distantFuture];
+}  // namespace
+
+namespace cronet {
+// Tests public-key-pinning functionality.
+class PkpTest : public CronetTestBase {
+ protected:
+  void SetUp() override {
+    CronetTestBase::SetUp();
+
+    NSString* full_url_str = [NSString
+        stringWithFormat:@"%@:%i",
+                         [NSString
+                             stringWithCString:grpc_support::kTestServerUrl
+                                      encoding:NSUTF8StringEncoding],
+                         grpc_support::GetQuicTestServerPort()];
+    request_url_ = [NSURL URLWithString:full_url_str];
+    server_host_ = request_url_.host;
+    NSArray<NSString*>* components = [server_host_
+        componentsSeparatedByCharactersInSet:
+            [NSCharacterSet characterSetWithCharactersInString:@"."]];
+    domain_ =

[mef, 2017/06/12 22:25:23]

Maybe add a comment why we need to extract 'example.com' out of
'test.example.com'? OR just hardcode the domain name next to kServerCert?

[kapishnikov, 2017/06/16 20:11:05]

Added new grpc_support::kTestServerDomain constant.

+        [NSString stringWithFormat:@"%@.%@", components[components.count - 2],
+                                   components[components.count - 1]];
+
+    // Create a Cronet enabled NSURLSession

[mef, 2017/06/12 22:25:23]

nit: Comments should be full sentences ending with period. Here and below.

[kapishnikov, 2017/06/16 20:11:04]

Done.

+    NSURLSessionConfiguration* sessionConfig =
+        [NSURLSessionConfiguration defaultSessionConfiguration];
+    [Cronet installIntoSessionConfiguration:sessionConfig];
+    url_session_ = [NSURLSession sessionWithConfiguration:sessionConfig
+                                                 delegate:delegate_
+                                            delegateQueue:nil];
+
+    // Set mock cert verifier
+    [Cronet setMockCertVerifier:CreateMockCertVerifier({kSERVER_CERT}, YES)];
+  }
+
+  void TearDown() override {
+    [Cronet shutdownForTesting];

[mef, 2017/06/12 22:25:23]

It is strange that we have an explicit shutdown here but not start up in SetUp.
I see why, but maybe add a comment?

[kapishnikov, 2017/06/16 20:11:04]

Added comment. It should be safe to call shutdown even if a test didn't call
StartCronet(). Some future tests may not call StartCronet().

+    CronetTestBase::TearDown();
+  }
+
+  // Sends a request to a given URL, waits for the response and asserts that
+  // the response is either successful or containing an error depending on
+  // the value of the passed |expected_success| parameter.
+  void sendRequestAndAssertResult(NSURL* url, bool expected_success) {
+    NSURLSessionDataTask* dataTask =
+        [url_session_ dataTaskWithURL:request_url_];
+    StartDataTaskAndWaitForCompletion(dataTask);
+    if (expected_success) {
+      ASSERT_TRUE(IsResponseSuccessful());
+    } else {
+      ASSERT_FALSE(IsResponseSuccessful());
+    }
+  }
+
+  // Adds a give public-key-pin and starts a Cronet engine for testing.

[mef, 2017/06/12 22:25:23]

nit: give -> given

[kapishnikov, 2017/06/16 20:11:04]

Done.

+  static void AddPkpAndStartCronet(NSString* host,
+                                   NSData* hash,
+                                   BOOL include_subdomains,
+                                   const NSDate* expiration_date) {
+    NSSet* hashes = [NSSet setWithObject:hash];
+    [Cronet addPublicKeyPinsForHost:host
+                          pinHashes:hashes
+                  includeSubdomains:include_subdomains
+                     expirationDate:(NSDate*)expiration_date];
+    StartCronet(grpc_support::GetQuicTestServerPort());
+  }
+
+  // Returns an arbitrary public key hash that doesn't match with any test
+  // certificate.
+  static NSData* NonMatchingHash() {
+    const int length = 32;
+    u_char hash[length];

[mef, 2017/06/12 22:25:23]

I don't think u_char is normally used in Chromium.
Can this be std::string(length, '\077').c_str() or somesuch?

[kapishnikov, 2017/06/16 20:11:05]

Done.

+    memset(hash, 77, length);
+    return [NSData dataWithBytes:hash length:length];
+  }
+
+  NSURLSession* url_session_;
+  NSURL* request_url_;     // "https://test.example.com/hello.txt:62922"
+  NSString* server_host_;  // test.example.com
+  NSString* domain_;       // example.com
+
+};  // class PkpTest
+
+// Tests the case when a mismatching pin is set for some host that is
+// different from the one the client wants to access. In that case the other
+// host pinning policy should not be applied and the client is expected to
+// receive the successful response with the response code 200.
+TEST_F(PkpTest, TestSuccessIfPinSetForDifferentHost) {
+  AddPkpAndStartCronet(@"some-other-host.com", NonMatchingHash(),
+                       kEXCLUDE_SUBDOMAINS, distant_future);
+  ASSERT_NO_FATAL_FAILURE(sendRequestAndAssertResult(request_url_, kSUCCESS));
+}
+
+// Tests the case when the pin hash does not match. The client is expected to
+// receive the error response.
+TEST_F(PkpTest, TestErrorIfPinDoesNotMatch) {
+  AddPkpAndStartCronet(server_host_, NonMatchingHash(), kEXCLUDE_SUBDOMAINS,
+                       distant_future);
+  ASSERT_NO_FATAL_FAILURE(sendRequestAndAssertResult(request_url_, kERROR));
+}
+
+// Tests the case when the pin hash matches. The client is expected to
+// receive the successful response with the response code 200.
+TEST_F(PkpTest, TestSuccessIfPinMatches) {
+  // Calculate hash of the server certificate
+  scoped_refptr<net::X509Certificate> cert =
+      net::ImportCertFromFile(net::GetTestCertsDirectory(), kSERVER_CERT);
+  net::HashValue hash_value;
+  CalculatePublicKeySha256(*cert, &hash_value);
+  ASSERT_EQ(32ul, hash_value.size());
+  NSData* matching_hash =
+      [NSData dataWithBytes:hash_value.data() length:hash_value.size()];
+
+  AddPkpAndStartCronet(server_host_, matching_hash, kEXCLUDE_SUBDOMAINS,
+                       distant_future);
+  ASSERT_NO_FATAL_FAILURE(sendRequestAndAssertResult(request_url_, kSUCCESS));
+}
+
+// Tests the case when the pin hash does not match and the client accesses the
+// subdomain of the configured PKP host with includeSubdomains flag set to true.
+// The client is expected to receive the error response.
+TEST_F(PkpTest, TestIncludeSubdomainsFlagEqualTrue) {
+  AddPkpAndStartCronet(domain_, NonMatchingHash(), kINCLUDE_SUBDOMAINS,
+                       distant_future);
+  ASSERT_NO_FATAL_FAILURE(sendRequestAndAssertResult(request_url_, kERROR));
+}
+
+// Tests the case when the pin hash does not match and the client accesses the
+// subdomain of the configured PKP host with includeSubdomains flag set to
+// false. The client is expected to receive the successful response with the
+// response code 200.
+TEST_F(PkpTest, TestIncludeSubdomainsFlagEqualFalse) {
+  AddPkpAndStartCronet(domain_, NonMatchingHash(), kEXCLUDE_SUBDOMAINS,
+                       distant_future);
+  ASSERT_NO_FATAL_FAILURE(sendRequestAndAssertResult(request_url_, kSUCCESS));
+}
+
+// Tests a mismatching pin that will expire in 10 seconds. The pins should be
+// still valid and enforced during the request; thus returning the pin match
+// error.
+TEST_F(PkpTest, TestSoonExpiringPin) {
+  AddPkpAndStartCronet(server_host_, NonMatchingHash(), kEXCLUDE_SUBDOMAINS,
+                       [NSDate dateWithTimeIntervalSinceNow:10]);
+  ASSERT_NO_FATAL_FAILURE(sendRequestAndAssertResult(request_url_, kERROR));
+}
+
+// Tests mismatching pin that expired 1 second ago. Since the pin has
+// expired, it should not be enforced during the request; thus a successful
+// response is expected.
+TEST_F(PkpTest, TestRecentlyExpiredPin) {
+  AddPkpAndStartCronet(server_host_, NonMatchingHash(), kEXCLUDE_SUBDOMAINS,
+                       [NSDate dateWithTimeIntervalSinceNow:-1]);
+  ASSERT_NO_FATAL_FAILURE(sendRequestAndAssertResult(request_url_, kSUCCESS));
+}
+
+// Tests that host pinning is not persisted between multiple CronetEngine
+// instances.
+TEST_F(PkpTest, TestPinsAreNotPersisted) {
+  //

[mef, 2017/06/12 22:25:23]

nit: spurious //

[kapishnikov, 2017/06/16 20:11:05]

Done.

+  AddPkpAndStartCronet(server_host_, NonMatchingHash(), kEXCLUDE_SUBDOMAINS,
+                       distant_future);
+  ASSERT_NO_FATAL_FAILURE(sendRequestAndAssertResult(request_url_, kERROR));
+  [Cronet shutdownForTesting];
+
+  // Restart Cronet engine and try the same request again. Since the pins are
+  // not persisted, a successful response is expected.
+  StartCronet(grpc_support::GetQuicTestServerPort());
+  ASSERT_NO_FATAL_FAILURE(sendRequestAndAssertResult(request_url_, kSUCCESS));
+}
+
+}  // namespace cronet