Add support to GCMKeyStore for multiple keys per app_id

components/gcm_driver/crypto/gcm_encryption_provider_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 "components/gcm_driver/crypto/gcm_encryption_provider.h"
 
 #include <stddef.h>
 
 #include <sstream>
 #include <string>
 
 #include "base/base64url.h"
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/string_util.h"
 #include "base/test/histogram_tester.h"
 #include "components/gcm_driver/common/gcm_messages.h"
 #include "components/gcm_driver/crypto/gcm_key_store.h"
 #include "components/gcm_driver/crypto/gcm_message_cryptographer.h"
 #include "components/gcm_driver/crypto/p256_key_util.h"
 #include "crypto/random.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace gcm {
 namespace {
 
 const char kExampleAppId[] = "my-app-id";
+const char kExampleAuthorizedEntity[] = "my-sender-id";
 const char kExampleMessage[] = "Hello, world, this is the GCM Driver!";
 
 const char kValidEncryptionHeader[] =
     "keyid=foo;salt=MTIzNDU2Nzg5MDEyMzQ1Ng;rs=1024";
 const char kInvalidEncryptionHeader[] = "keyid";
 
 const char kValidCryptoKeyHeader[] =
     "keyid=foo;dh=BL_UGhfudEkXMUd4U4-D4nP5KHxKjQHsW6j88ybbehXM7fqi1OMFefDUEi0eJ"
     "vsKfyVBWYkQjH-lSPJKxjAyslg";
 const char kInvalidCryptoKeyHeader[] = "keyid";
@@ skipping 16 matching lines @@
     // |encryption_provider_| owns a ProtoDatabaseImpl whose destructor deletes
     // the underlying LevelDB database on the task runner.
     base::RunLoop().RunUntilIdle();
   }
 
   // To be used as a callback for GCMEncryptionProvider::GetEncryptionInfo().
   void DidGetEncryptionInfo(std::string* p256dh_out,
                             std::string* auth_secret_out,
                             const std::string& p256dh,
                             const std::string& auth_secret) {
-    *p256dh_out = p256dh;
-    *auth_secret_out = auth_secret;
+    if (p256dh_out)
+      *p256dh_out = p256dh;
+    if (auth_secret_out)
+      *auth_secret_out = auth_secret;
   }
 
-  // To be used as a callback for GCMKeyStore::CreateKeys().
-  void DidCreateKeys(KeyPair* pair_out, std::string* auth_secret_out,
-                     const KeyPair& pair, const std::string& auth_secret) {
-    *pair_out = pair;
-    *auth_secret_out = auth_secret;
+  // To be used as a callback for GCMKeyStore::{GetKeys,CreateKeys}.
+  void HandleKeysCallback(KeyPair* pair_out,
+                          std::string* auth_secret_out,
+                          const KeyPair& pair,
+                          const std::string& auth_secret) {
+    if (pair_out)
+      *pair_out = pair;
+    if (auth_secret_out)
+      *auth_secret_out = auth_secret;
   }
 
  protected:
   // Decrypts the |message| and then synchronously waits until either the
   // success or failure callbacks has been invoked.
   void Decrypt(const IncomingMessage& message) {
     encryption_provider_->DecryptMessage(
         kExampleAppId, message,
         base::Bind(&GCMEncryptionProviderTest::DidDecryptMessage,
                    base::Unretained(this)));
 
     // The encryption keys will be read asynchronously.
     base::RunLoop().RunUntilIdle();
   }
 
+  // Checks that the underlying key store has a key for the |kExampleAppId| +
+  // authorized entity pair if and only if |should_have_key| is true. Must wrap
+  // with ASSERT/EXPECT_NO_FATAL_FAILURE.
+  void CheckHasKey(const std::string& instance_id_authorized_entity,
+                   bool should_have_key) {
+    KeyPair pair;
+    std::string auth_secret;
+    encryption_provider()->key_store_->GetKeys(
+        kExampleAppId, instance_id_authorized_entity,
+        false /* fallback_to_empty_authorized_entity */,
+        base::Bind(&GCMEncryptionProviderTest::HandleKeysCallback,
+                   base::Unretained(this), &pair, &auth_secret));
+
+    base::RunLoop().RunUntilIdle();
+
+    if (should_have_key) {
+      ASSERT_GT(pair.public_key().size(), 0u);
+      ASSERT_GT(pair.private_key().size(), 0u);
+      ASSERT_GT(auth_secret.size(), 0u);
+    } else {
+      ASSERT_EQ(0u, pair.public_key().size());
+      ASSERT_EQ(0u, pair.private_key().size());
+      ASSERT_EQ(0u, auth_secret.size());
+    }
+  }
+
   // Returns the result of the previous decryption operation.
   GCMEncryptionProvider::DecryptionResult decryption_result() {
     return decryption_result_;
   }
 
   // Returns the message resulting from the previous decryption operation.
   const IncomingMessage& decrypted_message() { return decrypted_message_; }
 
   GCMEncryptionProvider* encryption_provider() {
     return encryption_provider_.get();
   }
 
+  // Performs a full round-trip test of the encryption feature. Must wrap this
+  // in ASSERT_NO_FATAL_FAILURE.
+  void TestEncryptionRoundTrip(
+      const std::string& app_id,
+      const std::string& instance_id_authorized_entity);
+
  private:
   void DidDecryptMessage(GCMEncryptionProvider::DecryptionResult result,
                          const IncomingMessage& message) {
     decryption_result_ = result;
     decrypted_message_ = message;
   }
 
   base::MessageLoop message_loop_;
   base::ScopedTempDir scoped_temp_dir_;
   base::HistogramTester histogram_tester_;
@@ skipping 86 matching lines @@
   message.data["encryption"] = kValidEncryptionHeader;
   message.data["crypto-key"] = kValidCryptoKeyHeader;
   message.raw_data = "foo";
 
   ASSERT_NO_FATAL_FAILURE(Decrypt(message));
   EXPECT_EQ(GCMEncryptionProvider::DECRYPTION_RESULT_NO_KEYS,
             decryption_result());
 
   std::string public_key, auth_secret;
   encryption_provider()->GetEncryptionInfo(
-      kExampleAppId,
+      kExampleAppId, "" /* empty authorized entity for GCM registration */,
       base::Bind(&GCMEncryptionProviderTest::DidGetEncryptionInfo,
                  base::Unretained(this), &public_key, &auth_secret));
 
   // Getting (or creating) the public key will be done asynchronously.
   base::RunLoop().RunUntilIdle();
 
   ASSERT_GT(public_key.size(), 0u);
   ASSERT_GT(auth_secret.size(), 0u);
 
   ASSERT_NO_FATAL_FAILURE(Decrypt(message));
   EXPECT_NE(GCMEncryptionProvider::DECRYPTION_RESULT_NO_KEYS,
             decryption_result());
 }
 
-TEST_F(GCMEncryptionProviderTest, EncryptionRoundTrip) {
+TEST_F(GCMEncryptionProviderTest, VerifiesKeyRemovalGCMRegistration) {

[Peter Beverloo, 2016/05/09 14:10:10]

While I'm all for tests, this doesn't actually test the logic in the
GCMEncryptionProvider and has a significant amount of overlap with
GCMKeyStoreTest.RemoveKeysWildcardAuthorizedEntity. Could we merge?

[johnme, 2016/05/09 18:15:54]

This (and its twin, VerifiesKeyRemovalInstanceIDToken) is the only test coverage
of GCMEncryptionProvider::RemoveEncryptionInfo, which is the public API to the
crypto component. It's also the only test coverage of calling GetEncryptionInfo
for a key that already exists (as opposed to calling it in order to create a new
key). So it seems worth keeping, though I agree that the duplication is
unfortunate.

+  // Removing encryption info for an InstanceID token shouldn't affect a legacy
+  // GCM registration.
+
+  // Legacy GCM callers pass an empty string for instance_id_authorized_entity.
+  std::string instance_id_authorized_entity_gcm = "";
+  std::string instance_id_authorized_entity_1 =
+      kExampleAuthorizedEntity + std::string("1");
+  std::string instance_id_authorized_entity_2 =
+      kExampleAuthorizedEntity + std::string("2");
+
+  // Should create encryption info.
+  std::string public_key, auth_secret;
+  encryption_provider()->GetEncryptionInfo(
+      kExampleAppId, instance_id_authorized_entity_gcm,
+      base::Bind(&GCMEncryptionProviderTest::DidGetEncryptionInfo,
+                 base::Unretained(this), &public_key, &auth_secret));
+
+  // Should get encryption info created above (although these calls are async

[Peter Beverloo, 2016/05/09 14:10:09]

Instead of the comment in parenthesis, can we just place a RunUntilIdle() in
between for clarity?

[johnme, 2016/05/09 18:15:54]

Sure; instead I expanded GCMKeyStoreTest.CreateGetAndRemoveKeysSynchronously to
cover synchronous gets.

+  // with no RunUntilIdle between, they should get handled in order of posting).
+  std::string read_public_key, read_auth_secret;
+  encryption_provider()->GetEncryptionInfo(
+      kExampleAppId, instance_id_authorized_entity_gcm,
+      base::Bind(&GCMEncryptionProviderTest::DidGetEncryptionInfo,
+                 base::Unretained(this), &read_public_key, &read_auth_secret));
+
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_GT(public_key.size(), 0u);
+  EXPECT_GT(auth_secret.size(), 0u);
+  EXPECT_EQ(public_key, read_public_key);
+  EXPECT_EQ(auth_secret, read_auth_secret);
+
+  EXPECT_NO_FATAL_FAILURE(CheckHasKey(instance_id_authorized_entity_gcm, true));

[Peter Beverloo, 2016/05/09 14:10:10]

This should be an ASSERT_NO_FATAL_FAILURE. There is little point in continuing
if any of the asserts in the CheckHasKey() methods failed.

[johnme, 2016/05/09 18:15:54]

Done (ditto below, since it's always the first failure that matters here).

+
+  encryption_provider()->RemoveEncryptionInfo(kExampleAppId,
+                                              instance_id_authorized_entity_1,
+                                              base::Bind(&base::DoNothing));
+
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_NO_FATAL_FAILURE(CheckHasKey(instance_id_authorized_entity_gcm, true));
+
+  encryption_provider()->RemoveEncryptionInfo(kExampleAppId, "*",
+                                              base::Bind(&base::DoNothing));
+
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_NO_FATAL_FAILURE(CheckHasKey(instance_id_authorized_entity_gcm, true));
+
+  encryption_provider()->RemoveEncryptionInfo(kExampleAppId,
+                                              instance_id_authorized_entity_gcm,
+                                              base::Bind(&base::DoNothing));
+
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_NO_FATAL_FAILURE(
+      CheckHasKey(instance_id_authorized_entity_gcm, false));
+}
+
+TEST_F(GCMEncryptionProviderTest, VerifiesKeyRemovalInstanceIDToken) {
+  // Removing encryption info for a legacy GCM registration shouldn't affect an
+  // InstanceID token.
+
+  // Legacy GCM callers pass an empty string for instance_id_authorized_entity.
+  std::string instance_id_authorized_entity_gcm = "";
+  std::string instance_id_authorized_entity_1 =
+      kExampleAuthorizedEntity + std::string("1");
+  std::string instance_id_authorized_entity_2 =
+      kExampleAuthorizedEntity + std::string("2");
+
+  std::string public_key_1, auth_secret_1;
+  encryption_provider()->GetEncryptionInfo(
+      kExampleAppId, instance_id_authorized_entity_1,
+      base::Bind(&GCMEncryptionProviderTest::DidGetEncryptionInfo,
+                 base::Unretained(this), &public_key_1, &auth_secret_1));
+
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_GT(public_key_1.size(), 0u);
+  EXPECT_GT(auth_secret_1.size(), 0u);
+
+  EXPECT_NO_FATAL_FAILURE(CheckHasKey(instance_id_authorized_entity_1, true));
+  EXPECT_NO_FATAL_FAILURE(CheckHasKey(instance_id_authorized_entity_2, false));
+
+  std::string public_key_2, auth_secret_2;
+  encryption_provider()->GetEncryptionInfo(
+      kExampleAppId, instance_id_authorized_entity_2,
+      base::Bind(&GCMEncryptionProviderTest::DidGetEncryptionInfo,
+                 base::Unretained(this), &public_key_2, &auth_secret_2));
+
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_GT(public_key_2.size(), 0u);
+  EXPECT_GT(auth_secret_2.size(), 0u);
+  EXPECT_NE(public_key_1, public_key_2);
+  EXPECT_NE(auth_secret_1, auth_secret_2);
+
+  EXPECT_NO_FATAL_FAILURE(CheckHasKey(instance_id_authorized_entity_1, true));
+  EXPECT_NO_FATAL_FAILURE(CheckHasKey(instance_id_authorized_entity_2, true));
+
+  std::string read_public_key_1, read_auth_secret_1;
+  encryption_provider()->GetEncryptionInfo(
+      kExampleAppId, instance_id_authorized_entity_1,
+      base::Bind(&GCMEncryptionProviderTest::DidGetEncryptionInfo,
+                 base::Unretained(this), &read_public_key_1,
+                 &read_auth_secret_1));
+
+  base::RunLoop().RunUntilIdle();
+
+  // Should have returned existing info for instance_id_authorized_entity_1.
+  EXPECT_EQ(public_key_1, read_public_key_1);
+  EXPECT_EQ(auth_secret_1, read_auth_secret_1);
+
+  encryption_provider()->RemoveEncryptionInfo(kExampleAppId,
+                                              instance_id_authorized_entity_gcm,
+                                              base::Bind(&base::DoNothing));
+
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_NO_FATAL_FAILURE(CheckHasKey(instance_id_authorized_entity_1, true));
+  EXPECT_NO_FATAL_FAILURE(CheckHasKey(instance_id_authorized_entity_2, true));
+
+  encryption_provider()->RemoveEncryptionInfo(kExampleAppId,
+                                              instance_id_authorized_entity_1,
+                                              base::Bind(&base::DoNothing));
+
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_NO_FATAL_FAILURE(CheckHasKey(instance_id_authorized_entity_1, false));
+  EXPECT_NO_FATAL_FAILURE(CheckHasKey(instance_id_authorized_entity_2, true));
+
+  std::string public_key_1_refreshed, auth_secret_1_refreshed;
+  encryption_provider()->GetEncryptionInfo(
+      kExampleAppId, instance_id_authorized_entity_1,
+      base::Bind(&GCMEncryptionProviderTest::DidGetEncryptionInfo,
+                 base::Unretained(this), &public_key_1_refreshed,
+                 &auth_secret_1_refreshed));
+
+  base::RunLoop().RunUntilIdle();
+
+  // Since the info was removed, GetEncryptionInfo should have created new info.
+  EXPECT_GT(public_key_1_refreshed.size(), 0u);
+  EXPECT_GT(auth_secret_1_refreshed.size(), 0u);
+  EXPECT_NE(public_key_1, public_key_1_refreshed);
+  EXPECT_NE(auth_secret_1, auth_secret_1_refreshed);
+  EXPECT_NE(public_key_2, public_key_1_refreshed);
+  EXPECT_NE(auth_secret_2, auth_secret_1_refreshed);
+
+  EXPECT_NO_FATAL_FAILURE(CheckHasKey(instance_id_authorized_entity_1, true));
+  EXPECT_NO_FATAL_FAILURE(CheckHasKey(instance_id_authorized_entity_2, true));
+
+  encryption_provider()->RemoveEncryptionInfo(kExampleAppId, "*",
+                                              base::Bind(&base::DoNothing));
+
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_NO_FATAL_FAILURE(CheckHasKey(instance_id_authorized_entity_1, false));
+  EXPECT_NO_FATAL_FAILURE(CheckHasKey(instance_id_authorized_entity_2, false));
+}
+
+void GCMEncryptionProviderTest::TestEncryptionRoundTrip(
+    const std::string& app_id,
+    const std::string& instance_id_authorized_entity) {
   // Performs a full round-trip of the encryption feature, including getting a
   // public/private key-pair and performing the cryptographic operations. This
   // is more of an integration test than a unit test.
 
   KeyPair pair, server_pair;
   std::string auth_secret, server_authentication;
 
   // Retrieve the public/private key-pair immediately from the key store, given
   // that the GCMEncryptionProvider will only share the public key with users.
   // Also create a second pair, which will act as the server's keys.
   encryption_provider()->key_store_->CreateKeys(
-      kExampleAppId,
-      base::Bind(&GCMEncryptionProviderTest::DidCreateKeys,
+      app_id, instance_id_authorized_entity,
+      base::Bind(&GCMEncryptionProviderTest::HandleKeysCallback,
                  base::Unretained(this), &pair, &auth_secret));
 
   encryption_provider()->key_store_->CreateKeys(
-      std::string(kExampleAppId) + "-server",
-      base::Bind(&GCMEncryptionProviderTest::DidCreateKeys,
+      "server-" + app_id, instance_id_authorized_entity,
+      base::Bind(&GCMEncryptionProviderTest::HandleKeysCallback,
                  base::Unretained(this), &server_pair, &server_authentication));
 
   // Creating the public keys will be done asynchronously.
   base::RunLoop().RunUntilIdle();
 
   ASSERT_GT(pair.public_key().size(), 0u);
   ASSERT_GT(server_pair.public_key().size(), 0u);
 
   ASSERT_GT(pair.private_key().size(), 0u);
   ASSERT_GT(server_pair.private_key().size(), 0u);
 
   std::string salt;
 
   // Creates a cryptographically secure salt of |salt_size| octets in size, and
   // calculate the shared secret for the message.
   crypto::RandBytes(base::WriteInto(&salt, 16 + 1), 16);
 
   std::string shared_secret;
   ASSERT_TRUE(ComputeSharedP256Secret(
       pair.private_key(), pair.public_key_x509(), server_pair.public_key(),
       &shared_secret));
 
   IncomingMessage message;
   size_t record_size;
 
+  message.sender_id = kExampleAuthorizedEntity;
+
   // Encrypts the |kExampleMessage| using the generated shared key and the
   // random |salt|, storing the result in |record_size| and the message.
   GCMMessageCryptographer cryptographer(
       GCMMessageCryptographer::Label::P256, pair.public_key(),
       server_pair.public_key(), auth_secret);
 
   ASSERT_TRUE(cryptographer.Encrypt(kExampleMessage, shared_secret, salt,
                                     &record_size, &message.raw_data));
 
   std::string encoded_salt, encoded_key;
@@ skipping 16 matching lines @@
 
   // Decrypt the message, and expect everything to go wonderfully well.
   ASSERT_NO_FATAL_FAILURE(Decrypt(message));
   ASSERT_EQ(GCMEncryptionProvider::DECRYPTION_RESULT_DECRYPTED,
             decryption_result());
 
   EXPECT_TRUE(decrypted_message().decrypted);
   EXPECT_EQ(kExampleMessage, decrypted_message().raw_data);
 }
 
+TEST_F(GCMEncryptionProviderTest, EncryptionRoundTripGCMRegistration) {
+  // GCMEncryptionProvider::DecryptMessage should succeed when the message was
+  // sent to a legacy GCM registration (empty instance_id_authorized_entity).
+  ASSERT_NO_FATAL_FAILURE(TestEncryptionRoundTrip(
+      kExampleAppId, "" /* empty authorized entity for GCM registration */));
+}
+
+TEST_F(GCMEncryptionProviderTest, EncryptionRoundTripInstanceIDToken) {
+  // GCMEncryptionProvider::DecryptMessage should succeed when the message was
+  // sent to an InstanceID token (non-empty instance_id_authorized_entity).
+  ASSERT_NO_FATAL_FAILURE(
+      TestEncryptionRoundTrip(kExampleAppId, kExampleAuthorizedEntity));
+}
+
 }  // namespace gcm