Add public key and signature verification to browser-side API keys

content/common/experiments/api_key_unittest.cc

 // Copyright 2015 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 "content/common/experiments/api_key.h"
 
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/string_util.h"
 #include "base/test/simple_test_clock.h"
 #include "base/time/time.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace content {
 
 namespace {
 
+/*
+This is a sample public key for testing the API. The corresponding private
+key (use this to generate new samples for this test file) is
+
+    0x83, 0x67, 0xf4, 0xcd, 0x2a, 0x1f, 0x0e, 0x04, 0x0d, 0x43,
+    0x13, 0x4c, 0x67, 0xc4, 0xf4, 0x28, 0xc9, 0x90, 0x15, 0x02,
+    0xe2, 0xba, 0xfd, 0xbb, 0xfa, 0xbc, 0x92, 0x76, 0x8a, 0x2c,
+    0x4b, 0xc7, 0x75, 0x10, 0xac, 0xf9, 0x3a, 0x1c, 0xb8, 0xa9,
+    0x28, 0x70, 0xd2, 0x9a, 0xd0, 0x0b, 0x59, 0xe1, 0xac, 0x2b,
+    0xb7, 0xd5, 0xca, 0x1f, 0x64, 0x90, 0x08, 0x8e, 0xa8, 0xe0,
+    0x56, 0x3a, 0x04, 0xd0
+*/
+const uint8_t kTestPublicKey[] = {
+    0x75, 0x10, 0xac, 0xf9, 0x3a, 0x1c, 0xb8, 0xa9, 0x28, 0x70, 0xd2,
+    0x9a, 0xd0, 0x0b, 0x59, 0xe1, 0xac, 0x2b, 0xb7, 0xd5, 0xca, 0x1f,
+    0x64, 0x90, 0x08, 0x8e, 0xa8, 0xe0, 0x56, 0x3a, 0x04, 0xd0,
+};
+const size_t kTestPublicKeyLength = sizeof(kTestPublicKey);

[chasej, 2016/01/11 16:11:25]

This can be removed, once the key length parameter is removed from the
validation methods on APIKey.

[iclelland, 2016/01/11 18:59:40]

Done.

+
+// This is a good key, signed with the above test private key.
 const char* kSampleAPIKey =
-    "Signature|https://valid.example.com|Frobulate|1458766277";
-
-const char* kExpectedAPIKeySignature = "Signature";
+    "UsEO0cNxoUtBnHDJdGPWTlXuLENjXcEIPL7Bs7sbvicPCcvAtyqhQuTJ9h/u1R3VZpWigtI+S"
+    "dUwk7Dyk/qbDw==|https://valid.example.com|Frobulate|1458766277";
+const char* kExpectedAPIKeySignature =
+    "UsEO0cNxoUtBnHDJdGPWTlXuLENjXcEIPL7Bs7sbvicPCcvAtyqhQuTJ9h/u1R3VZpWigtI+S"
+    "dUwk7Dyk/qbDw==";
 const char* kExpectedAPIKeyData =
     "https://valid.example.com|Frobulate|1458766277";
 const char* kExpectedAPIName = "Frobulate";
 const char* kExpectedOrigin = "https://valid.example.com";
 const uint64_t kExpectedExpiry = 1458766277;
 
 // The key should not be valid for this origin, or for this API.
 const char* kInvalidOrigin = "https://invalid.example.com";
 const char* kInsecureOrigin = "http://valid.example.com";
 const char* kInvalidAPIName = "Grokalyze";
 
 // The key should be valid if the current time is kValidTimestamp or earlier.
 double kValidTimestamp = 1458766276.0;
 
 // The key should be invalid if the current time is kInvalidTimestamp or later.
 double kInvalidTimestamp = 1458766278.0;
 
+// Well-formed API key with an invalid signature.
+const char* kInvalidSignatureAPIKey =
+    "CO8hDne98QeFeOJ0DbRZCBN3uE0nyaPgaLlkYhSWnbRoDfEAg+TXELaYfQPfEvKYFauBg/hnx"
+    "mba765hz0mXMc==|https://valid.example.com|Frobulate|1458766277";
+
 // Various ill-formed API keys. These should all fail to parse.
 const char* kInvalidAPIKeys[] = {
     // Invalid - only one part
     "abcde",
     // Not enough parts
     "https://valid.example.com|APIName|1458766277",
     // Delimiter in API Name
     "Signature|https://valid.example.com|API|Name|1458766277",
     // Extra string field
     "Signature|https://valid.example.com|APIName|1458766277|SomethingElse",
@@ skipping 18 matching lines @@
     return api_key->ValidateOrigin(origin);
   }
 
   bool ValidateApiName(ApiKey* api_key, const char* api_name) {
     return api_key->ValidateApiName(api_name);
   }
 
   bool ValidateDate(ApiKey* api_key, const base::Time& now) {
     return api_key->ValidateDate(now);
   }
+
+  bool ValidateSignature(ApiKey* api_key,
+                         const uint8_t* public_key,
+                         size_t public_key_length) {
+    return api_key->ValidateSignature(public_key, public_key_length);
+  }
 };
 
 TEST_F(ApiKeyTest, ParseEmptyString) {
   scoped_ptr<ApiKey> empty_key = ApiKey::Parse("");
   EXPECT_FALSE(empty_key);
 }
 
 TEST_F(ApiKeyTest, ParseInvalidStrings) {
   for (size_t i = 0; i < kNumInvalidAPIKeys; ++i) {
     scoped_ptr<ApiKey> empty_key = ApiKey::Parse(kInvalidAPIKeys[i]);
@@ skipping 32 matching lines @@
   EXPECT_TRUE(key->IsAppropriate(kExpectedOrigin, kExpectedAPIName));
   EXPECT_TRUE(key->IsAppropriate(kExpectedOrigin,
                                  base::ToUpperASCII(kExpectedAPIName)));
   EXPECT_TRUE(key->IsAppropriate(kExpectedOrigin,
                                  base::ToLowerASCII(kExpectedAPIName)));
   EXPECT_FALSE(key->IsAppropriate(kInvalidOrigin, kExpectedAPIName));
   EXPECT_FALSE(key->IsAppropriate(kInsecureOrigin, kExpectedAPIName));
   EXPECT_FALSE(key->IsAppropriate(kExpectedOrigin, kInvalidAPIName));
 }
 
+TEST_F(ApiKeyTest, ValidateValidSignature) {
+  scoped_ptr<ApiKey> key = ApiKey::Parse(kSampleAPIKey);
+  ASSERT_TRUE(key);
+  EXPECT_TRUE(
+      ValidateSignature(key.get(), kTestPublicKey, kTestPublicKeyLength));
+}
+
+TEST_F(ApiKeyTest, ValidateInvalidSignature) {
+  scoped_ptr<ApiKey> key = ApiKey::Parse(kInvalidSignatureAPIKey);
+  ASSERT_TRUE(key);
+  EXPECT_FALSE(
+      ValidateSignature(key.get(), kTestPublicKey, kTestPublicKeyLength));
+}
+
+TEST_F(ApiKeyTest, ValidateSignatureOnWrongKey) {
+  scoped_ptr<ApiKey> key = ApiKey::Parse(kSampleAPIKey);
+  ASSERT_TRUE(key);
+  // Signature will be invalid if tested against the real public key
+  EXPECT_FALSE(key->IsValid(base::Time::FromDoubleT(kInvalidTimestamp)));

[chasej, 2016/01/11 16:11:25]

Should this be passing kValidTimestamp?  Otherwise, this isn't reliably testing
the signature validation.  That is, IsValid() could return false due to the
timestamp, not necessarily because the signature was deemed invalid.

[iclelland, 2016/01/11 18:59:40]

Thanks, that was a typo introduced back in PS#4. Fixed.

+}

[chasej, 2016/01/11 16:11:25]

Do we need more tests for ApiKey.IsValid()?  I know it's a simple method, but
the result is now a combination of conditions. The only test for IsValid() is
the ValidateSignatureOnWrongKey above.  This is an open question, I could be
convinced either way (i.e. more tests could certainly be overkill).

[iclelland, 2016/01/11 18:59:40]

IsValid is deliberately kept very simple, and by design uses the real public
key(s). 

Without shipping a valid API key in tests, which is subject to change all the
time, and will make the tests fragile here, I think the best thing to do is to
provide exhaustive unit tests for the various Validate*() methods that it calls,
and then ensure that IsValid() is obviously correct.

Once I've added proper key management, it should be possible to "preload" the
validator with keys -- they won't be hard-coded like they are now. At that
point, it will be much easier to actually use IsValid() in tests, and I'll be
happy to test all permutations of valid/not-valid cases that we need.

+
+TEST_F(ApiKeyTest, ValidateWhenNotExpired) {
+  scoped_ptr<ApiKey> key = ApiKey::Parse(kSampleAPIKey);
+  ASSERT_TRUE(key);
+}
+
 }  // namespace content