Change public key id type to 32 bit integer.

This change is to match Keystore definition.
Also, assigned a symbolic name to it, so further changes (if any)
will only need to touch one line in the code.

Bug: b:170054326
Change-Id: I8e33afb9252fa17acb527453b0ecc3535ee72318
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2533175
Commit-Queue: Leonid Baraz <lbaraz@chromium.org>
Reviewed-by: Zach Trudo <zatrudo@google.com>
Cr-Commit-Position: refs/heads/master@{#826527}

chrome/browser/policy/messaging_layer/encryption/decryption.cc

@@ -14,6 +14,7 @@
 #include "base/task/post_task.h"
 #include "base/task_runner.h"
 #include "chrome/browser/policy/messaging_layer/encryption/decryption.h"
+#include "chrome/browser/policy/messaging_layer/encryption/encryption.h"
 #include "chrome/browser/policy/messaging_layer/util/status.h"
 #include "chrome/browser/policy/messaging_layer/util/statusor.h"
 #include "crypto/aead.h"
@@ -122,18 +123,19 @@ Decryptor::Decryptor()
 
 Decryptor::~Decryptor() = default;
 
-void Decryptor::RecordKeyPair(base::StringPiece private_key,
+void Decryptor::RecordKeyPair(
+    base::StringPiece private_key,
     base::StringPiece public_key,
-                              base::OnceCallback<void(StatusOr<int64_t>)> cb) {
+    base::OnceCallback<void(StatusOr<Encryptor::PublicKeyId>)> cb) {
   // Schedule key recording on the sequenced task runner.
   keys_sequenced_task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(
           [](std::string public_key, KeyInfo key_info,
-             base::OnceCallback<void(StatusOr<int64_t>)> cb,
+             base::OnceCallback<void(StatusOr<Encryptor::PublicKeyId>)> cb,
              scoped_refptr<Decryptor> decryptor) {
             DCHECK_CALLED_ON_VALID_SEQUENCE(decryptor->keys_sequence_checker_);
-            StatusOr<int64_t> result;
+            StatusOr<Encryptor::PublicKeyId> result;
             if (key_info.private_key.size() != X25519_PRIVATE_KEY_LEN) {
               result = Status(
                   error::FAILED_PRECONDITION,
@@ -152,7 +154,7 @@ void Decryptor::RecordKeyPair(base::StringPiece private_key,
               // Assign a random number to be public key id for testing purposes
               // only (in production it will be Java Fingerprint2011 which is
               // 'long').
-              int64_t public_key_id;
+              Encryptor::PublicKeyId public_key_id;
               base::RandBytes(&public_key_id, sizeof(public_key_id));
               if (!decryptor->keys_.emplace(public_key_id, key_info).second) {
                 result = Status(error::ALREADY_EXISTS,
@@ -164,10 +166,12 @@ void Decryptor::RecordKeyPair(base::StringPiece private_key,
             }
             // Schedule response on a generic thread pool.
             base::ThreadPool::PostTask(
-                FROM_HERE,
-                base::BindOnce(
-                    [](base::OnceCallback<void(StatusOr<int64_t>)> cb,
-                       StatusOr<int64_t> result) { std::move(cb).Run(result); },
+                FROM_HERE, base::BindOnce(
+                               [](base::OnceCallback<void(
+                                      StatusOr<Encryptor::PublicKeyId>)> cb,
+                                  StatusOr<Encryptor::PublicKeyId> result) {
+                                 std::move(cb).Run(result);
+                               },
                                std::move(cb), result));
           },
           std::string(public_key),
@@ -177,13 +181,13 @@ void Decryptor::RecordKeyPair(base::StringPiece private_key,
 }
 
 void Decryptor::RetrieveMatchingPrivateKey(
-    int64_t public_key_id,
+    Encryptor::PublicKeyId public_key_id,
     base::OnceCallback<void(StatusOr<std::string>)> cb) {
   // Schedule key retrieval on the sequenced task runner.
   keys_sequenced_task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(
-          [](int64_t public_key_id,
+          [](Encryptor::PublicKeyId public_key_id,
              base::OnceCallback<void(StatusOr<std::string>)> cb,
              scoped_refptr<Decryptor> decryptor) {
             DCHECK_CALLED_ON_VALID_SEQUENCE(decryptor->keys_sequence_checker_);

chrome/browser/policy/messaging_layer/encryption/decryption.h

@@ -15,6 +15,7 @@
 #include "base/strings/string_piece.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "chrome/browser/policy/messaging_layer/encryption/encryption.h"
 #include "chrome/browser/policy/messaging_layer/util/status.h"
 #include "chrome/browser/policy/messaging_layer/util/statusor.h"
 
@@ -79,14 +80,15 @@ class Decryptor : public base::RefCountedThreadSafe<Decryptor> {
 
   // Records a key pair (stores only private key).
   // Executes on a sequenced thread, returns key id or error with callback.
-  void RecordKeyPair(base::StringPiece private_key,
+  void RecordKeyPair(
+      base::StringPiece private_key,
       base::StringPiece public_key,
-                     base::OnceCallback<void(StatusOr<int64_t>)> cb);
+      base::OnceCallback<void(StatusOr<Encryptor::PublicKeyId>)> cb);
 
   // Retrieves private key matching the public key hash.
   // Executes on a sequenced thread, returns with callback.
   void RetrieveMatchingPrivateKey(
-      int64_t public_key_id,
+      Encryptor::PublicKeyId public_key_id,
       base::OnceCallback<void(StatusOr<std::string>)> cb);
 
  private:
@@ -102,7 +104,7 @@ class Decryptor : public base::RefCountedThreadSafe<Decryptor> {
     std::string private_key;
     base::Time time_stamp;
   };
-  base::flat_map<int64_t, KeyInfo> keys_;
+  base::flat_map<Encryptor::PublicKeyId, KeyInfo> keys_;
 
   // Sequential task runner for all keys_ activities:
   // recording, lookup, purge.

chrome/browser/policy/messaging_layer/encryption/encryption.cc

@@ -47,7 +47,7 @@ void Encryptor::Handle::CloseRecord(
 
 void Encryptor::Handle::ProduceEncryptedRecord(
     base::OnceCallback<void(StatusOr<EncryptedRecord>)> cb,
-    StatusOr<std::pair<std::string, int64_t>> asymmetric_key_result) {
+    StatusOr<std::pair<std::string, PublicKeyId>> asymmetric_key_result) {
   // Make sure the record self-destructs when returning from this method.
   const auto self_destruct = base::WrapUnique(this);
 
@@ -133,7 +133,7 @@ Encryptor::~Encryptor() = default;
 
 void Encryptor::UpdateAsymmetricKey(
     base::StringPiece new_public_key,
-    int64_t new_public_key_id,
+    PublicKeyId new_public_key_id,
     base::OnceCallback<void(Status)> response_cb) {
   if (new_public_key.empty()) {
     std::move(response_cb)
@@ -145,7 +145,7 @@ void Encryptor::UpdateAsymmetricKey(
   asymmetric_key_sequenced_task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(
-          [](base::StringPiece new_public_key, int64_t new_public_key_id,
+          [](base::StringPiece new_public_key, PublicKeyId new_public_key_id,
              scoped_refptr<Encryptor> encryptor) {
             encryptor->asymmetric_key_ =
                 std::make_pair(std::string(new_public_key), new_public_key_id);
@@ -162,29 +162,30 @@ void Encryptor::OpenRecord(base::OnceCallback<void(StatusOr<Handle*>)> cb) {
 }
 
 void Encryptor::RetrieveAsymmetricKey(
-    base::OnceCallback<void(StatusOr<std::pair<std::string, int64_t>>)> cb) {
+    base::OnceCallback<void(StatusOr<std::pair<std::string, PublicKeyId>>)>
+        cb) {
   // Schedule key retrieval on the sequenced task runner.
   asymmetric_key_sequenced_task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(
-          [](base::OnceCallback<void(StatusOr<std::pair<std::string, int64_t>>)>
-                 cb,
+          [](base::OnceCallback<void(
+                 StatusOr<std::pair<std::string, PublicKeyId>>)> cb,
              scoped_refptr<Encryptor> encryptor) {
             DCHECK_CALLED_ON_VALID_SEQUENCE(
                 encryptor->asymmetric_key_sequence_checker_);
-            StatusOr<std::pair<std::string, int64_t>> response;
+            StatusOr<std::pair<std::string, PublicKeyId>> response;
             // Schedule response on regular thread pool.
             base::ThreadPool::PostTask(
                 FROM_HERE,
                 base::BindOnce(
                     [](base::OnceCallback<void(
-                           StatusOr<std::pair<std::string, int64_t>>)> cb,
-                       StatusOr<std::pair<std::string, int64_t>> response) {
+                           StatusOr<std::pair<std::string, PublicKeyId>>)> cb,
+                       StatusOr<std::pair<std::string, PublicKeyId>> response) {
                       std::move(cb).Run(response);
                     },
                     std::move(cb),
                     !encryptor->asymmetric_key_.has_value()
-                        ? StatusOr<std::pair<std::string, int64_t>>(Status(
+                        ? StatusOr<std::pair<std::string, PublicKeyId>>(Status(
                               error::NOT_FOUND, "Asymmetric key not set"))
                         : encryptor->asymmetric_key_.value()));
           },

chrome/browser/policy/messaging_layer/encryption/encryption.h

@@ -39,6 +39,9 @@ namespace reporting {
 // The implementation class should never be used directly by the client code.
 class Encryptor : public base::RefCountedThreadSafe<Encryptor> {
  public:
+  // Public key id, as defined by Keystore.
+  using PublicKeyId = int32_t;
+
   // Encryption record handle, which is created by |OpenRecord| and can accept
   // pieces of data to be encrypted as one record by calling |AddToRecord|
   // multiple times. Resulting encrypted record is available once |CloseRecord|
@@ -65,7 +68,7 @@ class Encryptor : public base::RefCountedThreadSafe<Encryptor> {
     // as a callback after asynchronous retrieval of the asymmetric key.
     void ProduceEncryptedRecord(
         base::OnceCallback<void(StatusOr<EncryptedRecord>)> cb,
-        StatusOr<std::pair<std::string, int64_t>> asymmetric_key_result);
+        StatusOr<std::pair<std::string, PublicKeyId>> asymmetric_key_result);
 
     // Accumulated data to encrypt.
     std::string record_;
@@ -85,13 +88,14 @@ class Encryptor : public base::RefCountedThreadSafe<Encryptor> {
   // (it is OK to do it after OpenRecord and Handle::AddToRecord).
   // Executes on a sequenced thread, returns with callback.
   void UpdateAsymmetricKey(base::StringPiece new_public_key,
-                           int64_t new_public_key_id,
+                           PublicKeyId new_public_key_id,
                            base::OnceCallback<void(Status)> response_cb);
 
   // Retrieves the current public key.
   // Executes on a sequenced thread, returns with callback.
   void RetrieveAsymmetricKey(
-      base::OnceCallback<void(StatusOr<std::pair<std::string, int64_t>>)> cb);
+      base::OnceCallback<void(StatusOr<std::pair<std::string, PublicKeyId>>)>
+          cb);
 
  private:
   friend class base::RefCountedThreadSafe<Encryptor>;
@@ -99,7 +103,7 @@ class Encryptor : public base::RefCountedThreadSafe<Encryptor> {
   ~Encryptor();
 
   // Public key used for asymmetric encryption of symmetric key and its id.
-  base::Optional<std::pair<std::string, int64_t>> asymmetric_key_;
+  base::Optional<std::pair<std::string, PublicKeyId>> asymmetric_key_;
 
   // Sequential task runner for all asymmetric_key_ activities: update, read.
   scoped_refptr<base::SequencedTaskRunner>

chrome/browser/policy/messaging_layer/encryption/encryption_module.cc

@@ -88,7 +88,7 @@ void EncryptionModule::EncryptRecord(
 
 void EncryptionModule::UpdateAsymmetricKey(
     base::StringPiece new_public_key,
-    int64_t new_public_key_id,
+    Encryptor::PublicKeyId new_public_key_id,
     base::OnceCallback<void(Status)> response_cb) {
   encryptor_->UpdateAsymmetricKey(new_public_key, new_public_key_id,
                                   std::move(response_cb));

chrome/browser/policy/messaging_layer/encryption/encryption_module.h

@@ -36,7 +36,7 @@ class EncryptionModule : public base::RefCountedThreadSafe<EncryptionModule> {
   // Records current public asymmetric key.
   virtual void UpdateAsymmetricKey(
       base::StringPiece new_public_key,
-      int64_t new_public_key_id,
+      Encryptor::PublicKeyId new_public_key_id,
       base::OnceCallback<void(Status)> response_cb);
 
  protected:

chrome/browser/policy/messaging_layer/encryption/encryption_module_unittest.cc

@@ -14,6 +14,7 @@
 #include "base/test/task_environment.h"
 #include "base/time/time.h"
 #include "chrome/browser/policy/messaging_layer/encryption/decryption.h"
+#include "chrome/browser/policy/messaging_layer/encryption/encryption.h"
 #include "chrome/browser/policy/messaging_layer/util/status.h"
 #include "chrome/browser/policy/messaging_layer/util/status_macros.h"
 #include "chrome/browser/policy/messaging_layer/util/statusor.h"
@@ -114,7 +115,8 @@ class EncryptionModuleTest : public ::testing::Test {
     return decrypted_string;
   }
 
-  StatusOr<std::string> DecryptMatchingSecret(int64_t public_key_id,
+  StatusOr<std::string> DecryptMatchingSecret(
+      Encryptor::PublicKeyId public_key_id,
       base::StringPiece encrypted_key) {
     // Retrieve private key that matches public key hash.
     TestEvent<StatusOr<std::string>> retrieve_private_key;
@@ -133,14 +135,15 @@ class EncryptionModuleTest : public ::testing::Test {
     uint8_t out_private_key[X25519_PRIVATE_KEY_LEN];
     X25519_keypair(out_public_value, out_private_key);
 
-    TestEvent<StatusOr<int64_t>> record_keys;
+    TestEvent<StatusOr<Encryptor::PublicKeyId>> record_keys;
     decryptor_->RecordKeyPair(
         std::string(reinterpret_cast<const char*>(out_private_key),
                     X25519_PRIVATE_KEY_LEN),
         std::string(reinterpret_cast<const char*>(out_public_value),
                     X25519_PUBLIC_VALUE_LEN),
         record_keys.cb());
-    ASSIGN_OR_RETURN(int64_t new_public_key_id, record_keys.result());
+    ASSIGN_OR_RETURN(Encryptor::PublicKeyId new_public_key_id,
+                     record_keys.result());
     TestEvent<Status> set_public_key;
     encryption_module_->UpdateAsymmetricKey(
         std::string(reinterpret_cast<const char*>(out_public_value),
@@ -275,7 +278,7 @@ TEST_F(EncryptionModuleTest, EncryptAndDecryptMultipleParallel) {
     SingleEncryptionContext(
         base::StringPiece test_string,
         base::StringPiece public_key,
-        int64_t public_key_id,
+        Encryptor::PublicKeyId public_key_id,
         scoped_refptr<EncryptionModule> encryption_module,
         base::OnceCallback<void(StatusOr<EncryptedRecord>)> response)
         : test_string_(test_string),
@@ -336,7 +339,7 @@ TEST_F(EncryptionModuleTest, EncryptAndDecryptMultipleParallel) {
    private:
     const std::string test_string_;
     const std::string public_key_;
-    const int64_t public_key_id_;
+    const Encryptor::PublicKeyId public_key_id_;
     const scoped_refptr<EncryptionModule> encryption_module_;
     base::OnceCallback<void(StatusOr<EncryptedRecord>)> response_;
   };
@@ -468,7 +471,7 @@ TEST_F(EncryptionModuleTest, EncryptAndDecryptMultipleParallel) {
   // Public and private key pairs in this test are reversed strings.
   std::vector<std::string> private_key_strings;
   std::vector<std::string> public_value_strings;
-  std::vector<int64_t> public_value_ids;
+  std::vector<Encryptor::PublicKeyId> public_value_ids;
   for (size_t i = 0; i < 3; ++i) {
     // Generate new pair of private key and public value.
     uint8_t out_public_value[X25519_PUBLIC_VALUE_LEN];
@@ -482,7 +485,7 @@ TEST_F(EncryptionModuleTest, EncryptAndDecryptMultipleParallel) {
   }
 
   // Register all key pairs for decryption.
-  std::vector<TestEvent<StatusOr<int64_t>>> record_results(
+  std::vector<TestEvent<StatusOr<Encryptor::PublicKeyId>>> record_results(
       public_value_strings.size());
   for (size_t i = 0; i < public_value_strings.size(); ++i) {
     base::ThreadPool::PostTask(
@@ -490,7 +493,8 @@ TEST_F(EncryptionModuleTest, EncryptAndDecryptMultipleParallel) {
                        [](base::StringPiece private_key_string,
                           base::StringPiece public_key_string,
                           scoped_refptr<Decryptor> decryptor,
-                          base::OnceCallback<void(StatusOr<int64_t>)> done_cb) {
+                          base::OnceCallback<void(
+                              StatusOr<Encryptor::PublicKeyId>)> done_cb) {
                          decryptor->RecordKeyPair(private_key_string,
                                                   public_key_string,
                                                   std::move(done_cb));

chrome/browser/policy/messaging_layer/encryption/encryption_unittest.cc

@@ -134,7 +134,8 @@ class EncryptionTest : public ::testing::Test {
     return decrypted_string;
   }
 
-  StatusOr<std::string> DecryptMatchingSecret(int64_t public_key_id,
+  StatusOr<std::string> DecryptMatchingSecret(
+      Encryptor::PublicKeyId public_key_id,
       base::StringPiece encrypted_key) {
     // Retrieve private key that matches public key hash.
     TestEvent<StatusOr<std::string>> retrieve_private_key;
@@ -153,14 +154,15 @@ class EncryptionTest : public ::testing::Test {
     uint8_t out_private_key[X25519_PRIVATE_KEY_LEN];
     X25519_keypair(out_public_value, out_private_key);
 
-    TestEvent<StatusOr<int64_t>> record_keys;
+    TestEvent<StatusOr<Encryptor::PublicKeyId>> record_keys;
     decryptor_->RecordKeyPair(
         std::string(reinterpret_cast<const char*>(out_private_key),
                     X25519_PRIVATE_KEY_LEN),
         std::string(reinterpret_cast<const char*>(out_public_value),
                     X25519_PUBLIC_VALUE_LEN),
         record_keys.cb());
-    ASSIGN_OR_RETURN(int64_t new_public_key_id, record_keys.result());
+    ASSIGN_OR_RETURN(Encryptor::PublicKeyId new_public_key_id,
+                     record_keys.result());
     TestEvent<Status> set_public_key;
     encryptor_->UpdateAsymmetricKey(
         std::string(reinterpret_cast<const char*>(out_public_value),
@@ -275,7 +277,7 @@ TEST_F(EncryptionTest, EncryptAndDecryptMultipleParallel) {
     SingleEncryptionContext(
         base::StringPiece test_string,
         base::StringPiece public_key,
-        int64_t public_key_id,
+        Encryptor::PublicKeyId public_key_id,
         scoped_refptr<Encryptor> encryptor,
         base::OnceCallback<void(StatusOr<EncryptedRecord>)> response)
         : test_string_(test_string),
@@ -365,7 +367,7 @@ TEST_F(EncryptionTest, EncryptAndDecryptMultipleParallel) {
    private:
     const std::string test_string_;
     const std::string public_key_;
-    const int64_t public_key_id_;
+    const Encryptor::PublicKeyId public_key_id_;
     const scoped_refptr<Encryptor> encryptor_;
     base::OnceCallback<void(StatusOr<EncryptedRecord>)> response_;
   };
@@ -497,7 +499,7 @@ TEST_F(EncryptionTest, EncryptAndDecryptMultipleParallel) {
   // Public and private key pairs in this test are reversed strings.
   std::vector<std::string> private_key_strings;
   std::vector<std::string> public_value_strings;
-  std::vector<int64_t> public_value_ids;
+  std::vector<Encryptor::PublicKeyId> public_value_ids;
   for (size_t i = 0; i < 3; ++i) {
     // Generate new pair of private key and public value.
     uint8_t out_public_value[X25519_PUBLIC_VALUE_LEN];
@@ -511,7 +513,7 @@ TEST_F(EncryptionTest, EncryptAndDecryptMultipleParallel) {
   }
 
   // Register all key pairs for decryption.
-  std::vector<TestEvent<StatusOr<int64_t>>> record_results(
+  std::vector<TestEvent<StatusOr<Encryptor::PublicKeyId>>> record_results(
       public_value_strings.size());
   for (size_t i = 0; i < public_value_strings.size(); ++i) {
     base::ThreadPool::PostTask(
@@ -519,7 +521,8 @@ TEST_F(EncryptionTest, EncryptAndDecryptMultipleParallel) {
                        [](base::StringPiece private_key_string,
                           base::StringPiece public_key_string,
                           scoped_refptr<Decryptor> decryptor,
-                          base::OnceCallback<void(StatusOr<int64_t>)> done_cb) {
+                          base::OnceCallback<void(
+                              StatusOr<Encryptor::PublicKeyId>)> done_cb) {
                          decryptor->RecordKeyPair(private_key_string,
                                                   public_key_string,
                                                   std::move(done_cb));

chrome/browser/policy/messaging_layer/encryption/test_encryption_module.cc

@@ -6,6 +6,7 @@
 
 #include "base/callback.h"
 #include "base/strings/string_piece.h"
+#include "chrome/browser/policy/messaging_layer/encryption/encryption.h"
 #include "chrome/browser/policy/messaging_layer/util/statusor.h"
 #include "components/policy/proto/record.pb.h"
 
@@ -28,7 +29,7 @@ TestEncryptionModuleStrict::TestEncryptionModuleStrict() {
 
 void TestEncryptionModuleStrict::UpdateAsymmetricKey(
     base::StringPiece new_public_key,
-    int64_t new_public_key_id,
+    Encryptor::PublicKeyId new_public_key_id,
     base::OnceCallback<void(Status)> response_cb) {
   std::move(response_cb)
       .Run(Status(error::UNIMPLEMENTED,

chrome/browser/policy/messaging_layer/encryption/test_encryption_module.h

@@ -7,6 +7,7 @@
 
 #include "base/callback.h"
 #include "base/strings/string_piece.h"
+#include "chrome/browser/policy/messaging_layer/encryption/encryption.h"
 #include "chrome/browser/policy/messaging_layer/public/report_queue.h"
 #include "chrome/browser/policy/messaging_layer/util/statusor.h"
 #include "components/policy/proto/record.pb.h"
@@ -29,7 +30,7 @@ class TestEncryptionModuleStrict : public EncryptionModule {
 
   void UpdateAsymmetricKey(
       base::StringPiece new_public_key,
-      int64_t new_public_key_id,
+      Encryptor::PublicKeyId new_public_key_id,
       base::OnceCallback<void(Status)> response_cb) override;
 
  protected: