Production implementation of encryption/decryption.

Fake-* is removed, as well as virtual interface: we are using openssl and AEAD directly.
Also, decryption is moved out from Chrome, since for now we only use it in test.
Once we add it to the server, it will be placed elsewhere and probably wrapped in Java class.

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

chrome/browser/BUILD.gn

@@ -1167,8 +1167,6 @@ static_library("browser") {
     "policy/homepage_location_policy_handler.h",
     "policy/javascript_policy_handler.cc",
     "policy/javascript_policy_handler.h",
-    "policy/messaging_layer/encryption/decryption.cc",
-    "policy/messaging_layer/encryption/decryption.h",
     "policy/messaging_layer/encryption/encryption.cc",
     "policy/messaging_layer/encryption/encryption.h",
     "policy/messaging_layer/encryption/encryption_module.cc",

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

@@ -2,46 +2,148 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <openssl/curve25519.h>
 #include <string>
 
 #include "base/hash/hash.h"
+#include "base/memory/ptr_util.h"
 #include "base/strings/strcat.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_piece.h"
 #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/util/status.h"
+#include "chrome/browser/policy/messaging_layer/util/statusor.h"
+#include "crypto/aead.h"
 
 namespace reporting {
 
-DecryptorBase::Handle::Handle(scoped_refptr<DecryptorBase> decryptor)
-    : decryptor_(decryptor) {}
+Decryptor::Handle::Handle(base::StringPiece symmetric_key,
+                          scoped_refptr<Decryptor> decryptor)
+    : symmetric_key_(symmetric_key), decryptor_(decryptor) {}
 
-DecryptorBase::Handle::~Handle() = default;
+Decryptor::Handle::~Handle() = default;
 
-DecryptorBase::DecryptorBase()
+void Decryptor::Handle::AddToRecord(base::StringPiece data,
+                                    base::OnceCallback<void(Status)> cb) {
+  // Add piece of data to the record.
+  record_.append(data.data(), data.size());
+  std::move(cb).Run(Status::StatusOK());
+}
+
+void Decryptor::Handle::CloseRecord(
+    base::OnceCallback<void(StatusOr<base::StringPiece>)> cb) {
+  // Make sure the record self-destructs when returning from this method.
+  const auto self_destruct = base::WrapUnique(this);
+
+  // Decrypt the data with symmetric key using AEAD interface.
+  crypto::Aead aead(crypto::Aead::CHACHA20_POLY1305);
+
+  // Validate and use the symmetric key.
+  if (symmetric_key_.size() != aead.KeyLength()) {
+    std::move(cb).Run(
+        Status(error::INTERNAL,
+               base::StrCat({"Symmetric key size mismatch, expected=",
+                             base::NumberToString(aead.KeyLength()), " actual=",
+                             base::NumberToString(symmetric_key_.size())})));
+    return;
+  }
+  aead.Init(&symmetric_key_);
+
+  // Set nonce to 0s, since a symmetric key is only used once.
+  // Note: if we ever start reusing the same symmetric key, we will need
+  // to generate new nonce for every record and transfer it to the peer.
+  std::string nonce(aead.NonceLength(), 0);
+
+  // Decrypt collected record.
+  std::string decrypted;
+  if (!aead.Open(record_, nonce, std::string(), &decrypted)) {
+    std::move(cb).Run(Status(error::INTERNAL, "Failed to decrypt"));
+    return;
+  }
+  record_.clear();  // Free unused memory.
+
+  // Return decrypted record.
+  std::move(cb).Run(decrypted);
+}
+
+StatusOr<scoped_refptr<Decryptor>> Decryptor::Create() {
+  return base::WrapRefCounted(new Decryptor());
+}
+
+void Decryptor::OpenRecord(base::StringPiece symmetric_key,
+                           base::OnceCallback<void(StatusOr<Handle*>)> cb) {
+  std::move(cb).Run(new Handle(symmetric_key, this));
+}
+
+StatusOr<std::string> Decryptor::DecryptKey(
+    base::StringPiece private_key,
+    base::StringPiece peer_public_value) {
+  // Verify the keys.
+  if (private_key.size() != X25519_PRIVATE_KEY_LEN) {
+    return Status(
+        error::FAILED_PRECONDITION,
+        base::StrCat({"Private key size mismatch, expected=",
+                      base::NumberToString(X25519_PRIVATE_KEY_LEN),
+                      " actual=", base::NumberToString(private_key.size())}));
+  }
+  if (peer_public_value.size() != X25519_PUBLIC_VALUE_LEN) {
+    return Status(
+        error::FAILED_PRECONDITION,
+        base::StrCat({"Public key size mismatch, expected=",
+                      base::NumberToString(X25519_PUBLIC_VALUE_LEN), " actual=",
+                      base::NumberToString(peer_public_value.size())}));
+  }
+
+  // Compute shared secret which will become a symmetric key.
+  uint8_t out_shared_key[X25519_SHARED_KEY_LEN];
+  if (!X25519(out_shared_key,
+              reinterpret_cast<const uint8_t*>(private_key.data()),
+              reinterpret_cast<const uint8_t*>(peer_public_value.data()))) {
+    return Status(error::DATA_LOSS, "Curve22519 decryption failed");
+  }
+  return std::string(reinterpret_cast<const char*>(out_shared_key),
+                     X25519_SHARED_KEY_LEN);
+}
+
+Decryptor::Decryptor()
     : keys_sequenced_task_runner_(base::ThreadPool::CreateSequencedTaskRunner(
           {base::TaskPriority::BEST_EFFORT, base::MayBlock()})) {
   DETACH_FROM_SEQUENCE(keys_sequence_checker_);
 }
 
-DecryptorBase::~DecryptorBase() = default;
+Decryptor::~Decryptor() = default;
 
-void DecryptorBase::RecordKeyPair(base::StringPiece private_key,
-                                  base::StringPiece public_key,
-                                  base::OnceCallback<void(Status)> cb) {
+void Decryptor::RecordKeyPair(base::StringPiece private_key,
+                              base::StringPiece public_key,
+                              base::OnceCallback<void(Status)> 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(Status)> cb,
-             scoped_refptr<DecryptorBase> decryptor) {
+             scoped_refptr<Decryptor> decryptor) {
             DCHECK_CALLED_ON_VALID_SEQUENCE(decryptor->keys_sequence_checker_);
             Status result;
-            if (!decryptor->keys_
-                     .emplace(base::PersistentHash(public_key.data(),
-                                                   public_key.size()),
-                              key_info)
-                     .second) {
+            if (key_info.private_key.size() != X25519_PRIVATE_KEY_LEN) {
+              result = Status(
+                  error::FAILED_PRECONDITION,
+                  base::StrCat(
+                      {"Private key size mismatch, expected=",
+                       base::NumberToString(X25519_PRIVATE_KEY_LEN), " actual=",
+                       base::NumberToString(key_info.private_key.size())}));
+            } else if (public_key.size() != X25519_PUBLIC_VALUE_LEN) {
+              result = Status(
+                  error::FAILED_PRECONDITION,
+                  base::StrCat(
+                      {"Public key size mismatch, expected=",
+                       base::NumberToString(X25519_PUBLIC_VALUE_LEN),
+                       " actual=", base::NumberToString(public_key.size())}));
+            } else if (!decryptor->keys_
+                            .emplace(base::PersistentHash(public_key), key_info)
+                            .second) {
               result = Status(error::ALREADY_EXISTS,
                               base::StrCat({"Public key='", public_key,
                                             "' already recorded"}));
@@ -59,7 +161,7 @@ void DecryptorBase::RecordKeyPair(base::StringPiece private_key,
           std::move(cb), base::WrapRefCounted(this)));
 }
 
-void DecryptorBase::RetrieveMatchingPrivateKey(
+void Decryptor::RetrieveMatchingPrivateKey(
     uint32_t public_key_id,
     base::OnceCallback<void(StatusOr<std::string>)> cb) {
   // Schedule key retrieval on the sequenced task runner.
@@ -68,9 +170,13 @@ void DecryptorBase::RetrieveMatchingPrivateKey(
       base::BindOnce(
           [](uint32_t public_key_id,
              base::OnceCallback<void(StatusOr<std::string>)> cb,
-             scoped_refptr<DecryptorBase> decryptor) {
+             scoped_refptr<Decryptor> decryptor) {
             DCHECK_CALLED_ON_VALID_SEQUENCE(decryptor->keys_sequence_checker_);
             auto key_info_it = decryptor->keys_.find(public_key_id);
+            if (key_info_it != decryptor->keys_.end()) {
+              DCHECK_EQ(key_info_it->second.private_key.size(),
+                        static_cast<size_t>(X25519_PRIVATE_KEY_LEN));
+            }
             // Schedule response on a generic thread pool.
             base::ThreadPool::PostTask(
                 FROM_HERE,

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

@@ -11,6 +11,7 @@
 #include "base/containers/flat_map.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_refptr.h"
+#include "base/optional.h"
 #include "base/strings/string_piece.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_task_runner_handle.h"
@@ -19,14 +20,16 @@
 
 namespace reporting {
 
-// Interface to the encryption.
+// Full implementation of Decryptor, intended for use in tests and potentially
+// in reporting server (wrapped in a Java class).
+//
+// Curve25519 decryption of the symmetric key with asymmetric private key.
+// ChaCha20_Poly1305 decryption and verification of a record in place with
+// symmetric key.
+//
 // Instantiated by an implementation-specific factory:
-//   StatusOr<scoped_refptr<DecryptorBase>> Create(
-//       implementation-specific parameters);
-// The implementation class should never be used directly by the server code.
-// Note: Production implementation should be written or enclosed in Java code
-// for the server to use.
-class DecryptorBase : public base::RefCountedThreadSafe<DecryptorBase> {
+//   StatusOr<scoped_refptr<Decryptor>> Create();
+class Decryptor : public base::RefCountedThreadSafe<Decryptor> {
  public:
   // Decryption record handle, which is created by |OpenRecord| and can accept
   // pieces of data to be decrypted as one record by calling |AddToRecord|
@@ -34,43 +37,47 @@ class DecryptorBase : public base::RefCountedThreadSafe<DecryptorBase> {
   // is called.
   class Handle {
    public:
-    // Adds piece of data to the record.
-    virtual void AddToRecord(base::StringPiece data,
-                             base::OnceCallback<void(Status)> cb) = 0;
+    Handle(base::StringPiece symmetric_key, scoped_refptr<Decryptor> decryptor);
+    Handle(const Handle& other) = delete;
+    Handle& operator=(const Handle& other) = delete;
+    ~Handle();
+
+    // Adds piece of encrypted data to the record.
+    void AddToRecord(base::StringPiece data,
+                     base::OnceCallback<void(Status)> cb);
 
     // Closes and attempts to decrypt the record. Hands over the decrypted data
     // to be processed by the server (or Status if unsuccessful). Accesses key
     // store to attempt all private keys that are considered to be valid,
     // starting with the one that matches the hash. Self-destructs after the
     // callback.
-    virtual void CloseRecord(
-        base::OnceCallback<void(StatusOr<base::StringPiece>)> cb) = 0;
+    void CloseRecord(base::OnceCallback<void(StatusOr<base::StringPiece>)> cb);
 
-   protected:
-    explicit Handle(scoped_refptr<DecryptorBase> decryptor);
-
-    // Destructor is non-public, because the object can only self-destruct by
-    // |CloseRecord|.
-    virtual ~Handle();
+   private:
+    // Symmetric key.
+    const std::string symmetric_key_;
 
-    DecryptorBase* decryptor() const { return decryptor_.get(); }
+    // Accumulated data to decrypt.
+    std::string record_;
 
-   private:
-    scoped_refptr<DecryptorBase> decryptor_;
+    scoped_refptr<Decryptor> decryptor_;
   };
 
-  // Factory method creates new record to collect data and decrypt them with the
-  // given encrypted key. Hands the handle raw pointer over to the callback, or
-  // error status (e.g., “decryption is not enabled yet”)
-  virtual void OpenRecord(base::StringPiece encrypted_key,
-                          base::OnceCallback<void(StatusOr<Handle*>)> cb) = 0;
+  // Factory method to instantiate the Decryptor.
+  static StatusOr<scoped_refptr<Decryptor>> Create();
+
+  // Factory method creates a new record to collect data and decrypt them with
+  // the given encrypted key. Hands the handle raw pointer over to the callback,
+  // or error status.
+  void OpenRecord(base::StringPiece encrypted_key,
+                  base::OnceCallback<void(StatusOr<Handle*>)> cb);
 
-  // Decrypts symmetric key with asymmetric private key and returns unencrypted
-  // key or error status (e.g., “decryption is not enabled yet”)
-  virtual StatusOr<std::string> DecryptKey(base::StringPiece public_key,
-                                           base::StringPiece encrypted_key) = 0;
+  // Recreates symmetric key with local private key and peer public value and
+  // returns it or error status.
+  StatusOr<std::string> DecryptKey(base::StringPiece public_key,
+                                   base::StringPiece peer_public_value);
 
-  // Records a key pair (store only private key).
+  // Records a key pair (stores only private key).
   // Executes on a sequenced thread, returns with callback.
   void RecordKeyPair(base::StringPiece private_key,
                      base::StringPiece public_key,
@@ -82,23 +89,23 @@ class DecryptorBase : public base::RefCountedThreadSafe<DecryptorBase> {
       uint32_t public_key_id,
       base::OnceCallback<void(StatusOr<std::string>)> cb);
 
- protected:
-  DecryptorBase();
-  virtual ~DecryptorBase();
-
  private:
-  friend base::RefCountedThreadSafe<DecryptorBase>;
+  friend base::RefCountedThreadSafe<Decryptor>;
+  Decryptor();
+  ~Decryptor();
 
-  // Map of hash(public_key)->{public key, private key, time stamp}
+  // Map of hash(public_key)->{private key, time stamp}
   // Private key is located by the hash of a public key, sent together with the
   // encrypted record. Keys older than pre-defined threshold are discarded.
+  // Time stamp allows to drop outdated keys (not implemented yet).
   struct KeyInfo {
     std::string private_key;
     base::Time time_stamp;
   };
   base::flat_map<uint32_t, KeyInfo> keys_;
 
-  // Sequential task runner for all keys_ activities: recording, lookup, purge.
+  // Sequential task runner for all keys_ activities:
+  // recording, lookup, purge.
   scoped_refptr<base::SequencedTaskRunner> keys_sequenced_task_runner_;
 
   SEQUENCE_CHECKER(keys_sequence_checker_);

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

@@ -2,31 +2,132 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "chrome/browser/policy/messaging_layer/encryption/encryption.h"
+
+#include <openssl/curve25519.h>
 #include <string>
 #include <utility>
 
+#include "base/bind.h"
+#include "base/callback.h"
 #include "base/hash/hash.h"
+#include "base/memory/ptr_util.h"
+#include "base/strings/strcat.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/task/post_task.h"
 #include "base/task_runner.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"
 
 namespace reporting {
 
-EncryptorBase::Handle::Handle(scoped_refptr<EncryptorBase> encryptor)
+Encryptor::Handle::Handle(scoped_refptr<Encryptor> encryptor)
     : encryptor_(encryptor) {}
 
-EncryptorBase::Handle::~Handle() = default;
+Encryptor::Handle::~Handle() = default;
+
+void Encryptor::Handle::AddToRecord(base::StringPiece data,
+                                    base::OnceCallback<void(Status)> cb) {
+  // Append new data to the record.
+  record_.append(data.data(), data.size());
+  std::move(cb).Run(Status::StatusOK());
+}
+
+void Encryptor::Handle::CloseRecord(
+    base::OnceCallback<void(StatusOr<EncryptedRecord>)> cb) {
+  // Retrieves asymmetric public key to use.
+  encryptor_->RetrieveAsymmetricKey(base::BindOnce(
+      &Handle::ProduceEncryptedRecord, base::Unretained(this), std::move(cb)));
+}
+
+void Encryptor::Handle::ProduceEncryptedRecord(
+    base::OnceCallback<void(StatusOr<EncryptedRecord>)> cb,
+    StatusOr<std::string> asymmetric_key_result) {
+  // Make sure the record self-destructs when returning from this method.
+  const auto self_destruct = base::WrapUnique(this);
+
+  // Validate keys.
+  if (!asymmetric_key_result.ok()) {
+    std::move(cb).Run(asymmetric_key_result.status());
+    return;
+  }
+  const auto& asymmetric_key = asymmetric_key_result.ValueOrDie();
+  if (asymmetric_key.size() != X25519_PUBLIC_VALUE_LEN) {
+    std::move(cb).Run(Status(
+        error::INTERNAL,
+        base::StrCat({"Asymmetric key size mismatch, expected=",
+                      base::NumberToString(X25519_PUBLIC_VALUE_LEN), " actual=",
+                      base::NumberToString(asymmetric_key.size())})));
+    return;
+  }
+
+  // Generate new pair of private key and public value.
+  uint8_t out_public_value[X25519_PUBLIC_VALUE_LEN];
+  uint8_t out_private_key[X25519_PRIVATE_KEY_LEN];
+  X25519_keypair(out_public_value, out_private_key);
+
+  // Compute shared secret which will become a symmetric key.
+  uint8_t out_shared_key[X25519_SHARED_KEY_LEN];
+  if (!X25519(out_shared_key, out_private_key,
+              reinterpret_cast<const uint8_t*>(asymmetric_key.data()))) {
+    std::move(cb).Run(Status(error::DATA_LOSS, "Curve22519 encryption failed"));
+    return;
+  }
+
+  // COnvert shared secret into a symmetric key for data encryption.
+  std::string symmetric_key(reinterpret_cast<const char*>(out_shared_key),
+                            X25519_SHARED_KEY_LEN);
+
+  // Encrypt the data with symmetric key using AEAD interface.
+  crypto::Aead aead(crypto::Aead::CHACHA20_POLY1305);
+
+  // Validate and use the symmetric key.
+  if (symmetric_key.size() != aead.KeyLength()) {
+    std::move(cb).Run(
+        Status(error::INTERNAL,
+               base::StrCat({"Symmetric key size mismatch, expected=",
+                             base::NumberToString(aead.KeyLength()), " actual=",
+                             base::NumberToString(symmetric_key.size())})));
+    return;
+  }
+  aead.Init(&symmetric_key);
 
-EncryptorBase::EncryptorBase()
+  // Set nonce to 0s, since a symmetric key is only used once.
+  // Note: if we ever start reusing the same symmetric key, we will need
+  // to generate new nonce for every record and transfer it to the peer.
+  std::string nonce(aead.NonceLength(), 0);
+
+  // Prepare encrypted record.
+  EncryptedRecord encrypted_record;
+  encrypted_record.mutable_encryption_info()->set_public_key_id(
+      base::PersistentHash(asymmetric_key));
+  encrypted_record.mutable_encryption_info()->set_encryption_key(
+      reinterpret_cast<const char*>(out_public_value), X25519_PUBLIC_VALUE_LEN);
+
+  // Encrypt the whole record.
+  if (!aead.Seal(record_, nonce, std::string(),
+                 encrypted_record.mutable_encrypted_wrapped_record()) ||
+      encrypted_record.encrypted_wrapped_record().empty()) {
+    std::move(cb).Run(Status(error::INTERNAL, "Failed to encrypt the record"));
+    return;
+  }
+  record_.clear();  // Free unused memory.
+
+  // Return EncryptedRecord.
+  std::move(cb).Run(encrypted_record);
+}
+
+Encryptor::Encryptor()
     : asymmetric_key_sequenced_task_runner_(
           base::ThreadPool::CreateSequencedTaskRunner(
               {base::TaskPriority::BEST_EFFORT, base::MayBlock()})) {
   DETACH_FROM_SEQUENCE(asymmetric_key_sequence_checker_);
 }
 
-EncryptorBase::~EncryptorBase() = default;
+Encryptor::~Encryptor() = default;
 
-void EncryptorBase::UpdateAsymmetricKey(
+void Encryptor::UpdateAsymmetricKey(
     base::StringPiece new_key,
     base::OnceCallback<void(Status)> response_cb) {
   if (new_key.empty()) {
@@ -37,25 +138,29 @@ void EncryptorBase::UpdateAsymmetricKey(
 
   // Schedule key update on the sequenced task runner.
   asymmetric_key_sequenced_task_runner_->PostTask(
-      FROM_HERE, base::BindOnce(
-                     [](base::StringPiece new_key,
-                        scoped_refptr<EncryptorBase> encryptor) {
-                       encryptor->asymmetric_key_ = std::string(new_key);
-                     },
-                     new_key, base::WrapRefCounted(this)));
+      FROM_HERE,
+      base::BindOnce(
+          [](base::StringPiece new_key, scoped_refptr<Encryptor> encryptor) {
+            encryptor->asymmetric_key_ = std::string(new_key);
+          },
+          std::string(new_key), base::WrapRefCounted(this)));
 
   // Response OK not waiting for the update.
   std::move(response_cb).Run(Status::StatusOK());
 }
 
-void EncryptorBase::RetrieveAsymmetricKey(
+void Encryptor::OpenRecord(base::OnceCallback<void(StatusOr<Handle*>)> cb) {
+  std::move(cb).Run(new Handle(this));
+}
+
+void Encryptor::RetrieveAsymmetricKey(
     base::OnceCallback<void(StatusOr<std::string>)> cb) {
   // Schedule key retrueval on the sequenced task runner.
   asymmetric_key_sequenced_task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(
           [](base::OnceCallback<void(StatusOr<std::string>)> cb,
-             scoped_refptr<EncryptorBase> encryptor) {
+             scoped_refptr<Encryptor> encryptor) {
             DCHECK_CALLED_ON_VALID_SEQUENCE(
                 encryptor->asymmetric_key_sequence_checker_);
             StatusOr<std::string> response;
@@ -76,25 +181,8 @@ void EncryptorBase::RetrieveAsymmetricKey(
           std::move(cb), base::WrapRefCounted(this)));
 }
 
-void EncryptorBase::EncryptKey(
-    base::StringPiece symmetric_key,
-    base::OnceCallback<void(StatusOr<std::pair<uint32_t, std::string>>)> cb) {
-  RetrieveAsymmetricKey(base::BindOnce(
-      [](base::StringPiece symmetric_key,
-         scoped_refptr<EncryptorBase> encryptor,
-         base::OnceCallback<void(StatusOr<std::pair<uint32_t, std::string>>)>
-             cb,
-         StatusOr<std::string> asymmetric_key_result) {
-        if (!asymmetric_key_result.ok()) {
-          std::move(cb).Run(asymmetric_key_result.status());
-          return;
-        }
-        const auto& asymmetric_key = asymmetric_key_result.ValueOrDie();
-        std::move(cb).Run(std::make_pair(
-            base::PersistentHash(asymmetric_key),
-            encryptor->EncryptSymmetricKey(symmetric_key, asymmetric_key)));
-      },
-      std::string(symmetric_key), base::WrapRefCounted(this), std::move(cb)));
+StatusOr<scoped_refptr<Encryptor>> Encryptor::Create() {
+  return base::WrapRefCounted(new Encryptor());
 }
 
 }  // namespace reporting

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

@@ -19,12 +19,25 @@
 
 namespace reporting {
 
-// Interface to the encryption.
-// Instantiated by an implementation-specific factory:
-//   StatusOr<scoped_refptr<EncryptorBase>> Create(
-//       implementation-specific parameters);
+// Full implementation of Encryptor, intended for use in reporting client.
+// ChaCha20_Poly1305 AEAD encryption of a record in place with symmetric key.
+// Curve25519 encryption of the symmetric key with asymmetric public key.
+//
+// We generate new Curve25519 public/private keys pair for each record.
+// Then we produce Curve25519 shared secret from our private key and peer's
+// public key, and use it for ChaCha20_Poly1305 AEAD encryption of the record.
+// We send out our public value (calling it encrypted symmetric key) together
+// with encrypted record.
+//
+// Upon receiving the encrypted message the peer will produce the same shared
+// secret by combining their private key and our public key, and use it as
+// a symmetric key for ChaCha20_Poly1305 decryption and validation of the
+// record.
+//
+// Instantiated by a factory:
+//   StatusOr<scoped_refptr<Encryptor>> Create();
 // The implementation class should never be used directly by the client code.
-class EncryptorBase : public base::RefCountedThreadSafe<EncryptorBase> {
+class Encryptor : public base::RefCountedThreadSafe<Encryptor> {
  public:
   // Encryption record handle, which is created by |OpenRecord| and can accept
   // pieces of data to be encrypted as one record by calling |AddToRecord|
@@ -32,30 +45,41 @@ class EncryptorBase : public base::RefCountedThreadSafe<EncryptorBase> {
   // is called.
   class Handle {
    public:
+    explicit Handle(scoped_refptr<Encryptor> encryptor);
+    Handle(const Handle& other) = delete;
+    Handle& operator=(const Handle& other) = delete;
+    ~Handle();
+
     // Adds piece of data to the record.
-    virtual void AddToRecord(base::StringPiece data,
-                             base::OnceCallback<void(Status)> cb) = 0;
+    void AddToRecord(base::StringPiece data,
+                     base::OnceCallback<void(Status)> cb);
 
     // Closes and encrypts the record, hands over the data (encrypted with
     // symmetric key) and the key (encrypted with asymmetric key) to be recorded
     // by the client (or Status if unsuccessful). Self-destructs after the
     // callback.
-    virtual void CloseRecord(
-        base::OnceCallback<void(StatusOr<EncryptedRecord>)> cb) = 0;
-
-   protected:
-    explicit Handle(scoped_refptr<EncryptorBase> encryptor);
+    void CloseRecord(base::OnceCallback<void(StatusOr<EncryptedRecord>)> cb);
 
-    // Destructor is non-public, because the object can only self-destruct by
-    // |CloseRecord|.
-    virtual ~Handle();
+   private:
+    // Helper method to compose EncryptedRecord. Called by |CloseRecord|
+    // as a callback after asynchronous retrieval of the asymmetric key.
+    void ProduceEncryptedRecord(
+        base::OnceCallback<void(StatusOr<EncryptedRecord>)> cb,
+        StatusOr<std::string> asymmetric_key_result);
 
-    EncryptorBase* encryptor() const { return encryptor_.get(); }
+    // Accumulated data to encrypt.
+    std::string record_;
 
-   private:
-    scoped_refptr<EncryptorBase> encryptor_;
+    scoped_refptr<Encryptor> encryptor_;
   };
 
+  // Factory method to instantiate the Encryptor.
+  static StatusOr<scoped_refptr<Encryptor>> Create();
+
+  // Factory method creates new record to collect data and encrypt them.
+  // Hands the Handle raw pointer over to the callback, or error status).
+  void OpenRecord(base::OnceCallback<void(StatusOr<Handle*>)> cb);
+
   // Delivers public asymmetric key to the implementation.
   // To affect specific record, must happen before Handle::CloseRecord
   // (it is OK to do it after OpenRecord and Handle::AddToRecord).
@@ -63,35 +87,16 @@ class EncryptorBase : public base::RefCountedThreadSafe<EncryptorBase> {
   void UpdateAsymmetricKey(base::StringPiece new_key,
                            base::OnceCallback<void(Status)> response_cb);
 
-  // Factory method creates new record to collect data and encrypt them.
-  // Hands the Handle raw pointer over to the callback, or error status
-  // (e.g., “encryption is not enabled yet”).
-  virtual void OpenRecord(base::OnceCallback<void(StatusOr<Handle*>)> cb) = 0;
-
-  // Encrypts symmetric key with asymmetric public key, returns encrypted key
-  // and the hash of the public key used or error status (e.g., “decryption is
-  // not enabled yet”)
-  void EncryptKey(
-      base::StringPiece key,
-      base::OnceCallback<void(StatusOr<std::pair<uint32_t, std::string>>)> cb);
-
- protected:
-  EncryptorBase();
-  virtual ~EncryptorBase();
-
- private:
-  friend class base::RefCountedThreadSafe<EncryptorBase>;
-
-  // Synchronously encrypts symmetric key with asymmetric.
-  // Called by |EncryptKey|.
-  virtual std::string EncryptSymmetricKey(base::StringPiece symmetric_key,
-                                          base::StringPiece asymmetric_key) = 0;
-
   // Retrieves the current public key.
   // Executes on a sequenced thread, returns with callback.
   void RetrieveAsymmetricKey(
       base::OnceCallback<void(StatusOr<std::string>)> cb);
 
+ private:
+  friend class base::RefCountedThreadSafe<Encryptor>;
+  Encryptor();
+  ~Encryptor();
+
   // Public key used for asymmetric encryption of symmetric key.
   base::Optional<std::string> asymmetric_key_;
 

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

-// Copyright 2020 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 "chrome/browser/policy/messaging_layer/encryption/fake_decryption.h"
-
-#include "base/memory/ptr_util.h"
-#include "chrome/browser/policy/messaging_layer/util/status.h"
-#include "chrome/browser/policy/messaging_layer/util/statusor.h"
-
-namespace reporting {
-namespace test {
-
-namespace {
-// Decryption record handle for FakeDecryptor.
-class MockRecordHandle : public DecryptorBase::Handle {
- public:
-  explicit MockRecordHandle(base::StringPiece symmetric_key,
-                            scoped_refptr<DecryptorBase> decryptor)
-      : Handle(decryptor), symmetric_key_(symmetric_key) {}
-
-  MockRecordHandle(const MockRecordHandle& other) = delete;
-  MockRecordHandle& operator=(const MockRecordHandle& other) = delete;
-
-  void AddToRecord(base::StringPiece data,
-                   base::OnceCallback<void(Status)> cb) override {
-    // Add piece of data to the record.
-    record_.append(data.data(), data.size());
-    std::move(cb).Run(Status::StatusOK());
-  }
-
-  void CloseRecord(
-      base::OnceCallback<void(StatusOr<base::StringPiece>)> cb) override {
-    // Decrypt data in place by XORing every byte with the bytes of symmetric
-    // key.
-    size_t key_i = 0;
-    for (auto& record_byte : record_) {
-      record_byte ^= symmetric_key_[key_i++];
-      if (key_i >= symmetric_key_.size()) {
-        key_i = 0;
-      }
-    }
-    std::move(cb).Run(record_);
-    delete this;
-  }
-
- private:
-  // Symmetric key.
-  const std::string symmetric_key_;
-
-  // Accumulated decrypted data.
-  std::string record_;
-};
-
-}  // namespace
-
-StatusOr<scoped_refptr<DecryptorBase>> FakeDecryptor::Create() {
-  return base::WrapRefCounted(new FakeDecryptor());
-}
-
-FakeDecryptor::FakeDecryptor() = default;
-FakeDecryptor::~FakeDecryptor() = default;
-
-void FakeDecryptor::OpenRecord(base::StringPiece encrypted_key,
-                               base::OnceCallback<void(StatusOr<Handle*>)> cb) {
-  std::move(cb).Run(new MockRecordHandle(encrypted_key, this));
-}
-
-StatusOr<std::string> FakeDecryptor::DecryptKey(
-    base::StringPiece private_key,
-    base::StringPiece encrypted_key) {
-  if (private_key.empty()) {
-    return Status{error::FAILED_PRECONDITION, "Private key not provided"};
-  }
-  // Decrypt symmetric key.
-  // Private key is assumed to be a reverse string to the public key.
-  // If symmetric key was encrypted XORing bytes with a public key "012",
-  // decryption will use private key "210" and XOR will be from the last to the
-  // first bytes.
-  std::string unencrypted_key;
-  unencrypted_key.reserve(encrypted_key.size());
-  size_t key_i = 0;
-  for (const auto& key_byte : encrypted_key) {
-    unencrypted_key.push_back(key_byte ^
-                              private_key[private_key.size() - ++key_i]);
-    if (key_i >= private_key.size()) {
-      key_i = 0;
-    }
-  }
-  return unencrypted_key;
-}
-
-}  // namespace test
-}  // namespace reporting

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

-// Copyright 2020 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.
-
-#ifndef CHROME_BROWSER_POLICY_MESSAGING_LAYER_ENCRYPTION_FAKE_DECRYPTION_H_
-#define CHROME_BROWSER_POLICY_MESSAGING_LAYER_ENCRYPTION_FAKE_DECRYPTION_H_
-
-#include <string>
-
-#include "base/callback.h"
-#include "base/memory/scoped_refptr.h"
-#include "base/optional.h"
-#include "base/strings/string_piece.h"
-#include "chrome/browser/policy/messaging_layer/encryption/decryption.h"
-#include "chrome/browser/policy/messaging_layer/util/status.h"
-#include "chrome/browser/policy/messaging_layer/util/statusor.h"
-
-namespace reporting {
-namespace test {
-
-// Fake implementation of DecryptorBase, intended for use in tests of
-// reporting client.
-// Key decryption with asymmetric private key is done by per-byte XOR in reverse
-// order: public and private key are reverse, so if the encryption used XOR with
-// public key "012", decryption will use private key "210" and XOR will be from
-// the last to the first bytes.
-// Record decryption with symmetric key is done by per-byte XOR.
-class FakeDecryptor : public DecryptorBase {
- public:
-  // Factory method
-  static StatusOr<scoped_refptr<DecryptorBase>> Create();
-
-  void OpenRecord(base::StringPiece encrypted_key,
-                  base::OnceCallback<void(StatusOr<Handle*>)> cb) override;
-
-  StatusOr<std::string> DecryptKey(base::StringPiece private_key,
-                                   base::StringPiece encrypted_key) override;
-
- private:
-  FakeDecryptor();
-  ~FakeDecryptor() override;
-};
-
-}  // namespace test
-}  // namespace reporting
-
-#endif  // CHROME_BROWSER_POLICY_MESSAGING_LAYER_ENCRYPTION_FAKE_DECRYPTION_H_

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

-// Copyright 2020 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 "chrome/browser/policy/messaging_layer/encryption/fake_encryption.h"
-
-#include "base/bind.h"
-#include "base/callback.h"
-#include "base/hash/hash.h"
-#include "base/memory/ptr_util.h"
-#include "chrome/browser/policy/messaging_layer/util/status.h"
-#include "chrome/browser/policy/messaging_layer/util/statusor.h"
-#include "crypto/random.h"
-
-namespace reporting {
-namespace test {
-
-namespace {
-// Encryption record handle for FakeEncryptor.
-class MockRecordHandle : public EncryptorBase::Handle {
- public:
-  explicit MockRecordHandle(base::StringPiece symmetric_key,
-                            scoped_refptr<EncryptorBase> encryptor)
-      : Handle(encryptor), symmetric_key_(symmetric_key) {}
-
-  MockRecordHandle(const MockRecordHandle& other) = delete;
-  MockRecordHandle& operator=(const MockRecordHandle& other) = delete;
-
-  void AddToRecord(base::StringPiece data,
-                   base::OnceCallback<void(Status)> cb) override {
-    // Append new data to the record.
-    record_.append(data.data(), data.size());
-    std::move(cb).Run(Status::StatusOK());
-  }
-
-  void CloseRecord(
-      base::OnceCallback<void(StatusOr<EncryptedRecord>)> cb) override {
-    // Encrypt all collected data in place by XORing every byte with the
-    // symmetric key.
-    size_t key_i = 0;
-    for (auto& record_byte : record_) {
-      record_byte ^= symmetric_key_[key_i++];
-      if (key_i >= symmetric_key_.size()) {
-        key_i = 0;
-      }
-    }
-    // Encrypt the symmetric key.
-    encryptor()->EncryptKey(
-        symmetric_key_,
-        base::BindOnce(
-            [](MockRecordHandle* handle,
-               base::OnceCallback<void(StatusOr<EncryptedRecord>)> cb,
-               StatusOr<std::pair<uint32_t, std::string>>
-                   encrypted_key_result) {
-              if (!encrypted_key_result.ok()) {
-                std::move(cb).Run(encrypted_key_result.status());
-              } else {
-                EncryptedRecord encrypted_record;
-                encrypted_record.mutable_encryption_info()->set_public_key_id(
-                    encrypted_key_result.ValueOrDie().first);
-                encrypted_record.mutable_encryption_info()->set_encryption_key(
-                    encrypted_key_result.ValueOrDie().second);
-                encrypted_record.set_encrypted_wrapped_record(handle->record_);
-                std::move(cb).Run(encrypted_record);
-              }
-              delete handle;
-            },
-            base::Unretained(this),  // will self-destruct.
-            std::move(cb)));
-  }
-
- private:
-  // Symmetric key.
-  const std::string symmetric_key_;
-
-  // Accumulated encrypted data.
-  std::string record_;
-};
-
-}  // namespace
-
-StatusOr<scoped_refptr<EncryptorBase>> FakeEncryptor::Create() {
-  return base::WrapRefCounted(new FakeEncryptor());
-}
-
-FakeEncryptor::FakeEncryptor() = default;
-FakeEncryptor::~FakeEncryptor() = default;
-
-void FakeEncryptor::OpenRecord(base::OnceCallback<void(StatusOr<Handle*>)> cb) {
-  // For fake implementation just generate random byte string.
-  constexpr size_t symmetric_key_size = 8;
-  char symmetric_key[8];
-  crypto::RandBytes(symmetric_key, symmetric_key_size);
-  std::move(cb).Run(new MockRecordHandle(
-      std::string(symmetric_key, symmetric_key_size), this));
-}
-
-std::string FakeEncryptor::EncryptSymmetricKey(
-    base::StringPiece symmetric_key,
-    base::StringPiece asymmetric_key) {
-  // Encrypt symmetric key with public asymmetric one: XOR byte by byte.
-  std::string encrypted_key;
-  encrypted_key.reserve(symmetric_key.size());
-  size_t asymmetric_i = 0;
-  for (const auto& key_byte : symmetric_key) {
-    encrypted_key.push_back(key_byte ^ asymmetric_key[asymmetric_i++]);
-    if (asymmetric_i >= asymmetric_key.size()) {
-      asymmetric_i = 0;
-    }
-  }
-  return encrypted_key;
-}
-
-}  // namespace test
-}  // namespace reporting

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

-// Copyright 2020 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.
-
-#ifndef CHROME_BROWSER_POLICY_MESSAGING_LAYER_ENCRYPTION_FAKE_ENCRYPTION_H_
-#define CHROME_BROWSER_POLICY_MESSAGING_LAYER_ENCRYPTION_FAKE_ENCRYPTION_H_
-
-#include <string>
-
-#include "base/callback.h"
-#include "base/hash/hash.h"
-#include "base/memory/ref_counted.h"
-#include "base/strings/string_piece.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"
-
-namespace reporting {
-namespace test {
-
-// Fake implementation of EncryptorBase, intended for use in tests of
-// reporting client.
-// Record encryption with symmetric key is done by per-byte XOR.
-// Key encryption with asymmetric public key is also done by per-byte XOR.
-class FakeEncryptor : public EncryptorBase {
- public:
-  // Factory method
-  static StatusOr<scoped_refptr<EncryptorBase>> Create();
-
-  void OpenRecord(base::OnceCallback<void(StatusOr<Handle*>)> cb) override;
-
- private:
-  FakeEncryptor();
-  ~FakeEncryptor() override;
-
-  std::string EncryptSymmetricKey(base::StringPiece symmetric_key,
-                                  base::StringPiece asymmetric_key) override;
-};
-
-}  // namespace test
-}  // namespace reporting
-
-#endif  // CHROME_BROWSER_POLICY_MESSAGING_LAYER_ENCRYPTION_FAKE_ENCRYPTION_H_

chrome/test/BUILD.gn

@@ -3451,11 +3451,9 @@ test("unit_tests") {
     "../browser/policy/file_selection_dialogs_policy_handler_unittest.cc",
     "../browser/policy/homepage_location_policy_handler_unittest.cc",
     "../browser/policy/javascript_policy_handler_unittest.cc",
-    "../browser/policy/messaging_layer/encryption/fake_decryption.cc",
-    "../browser/policy/messaging_layer/encryption/fake_decryption.h",
-    "../browser/policy/messaging_layer/encryption/fake_encryption.cc",
-    "../browser/policy/messaging_layer/encryption/fake_encryption.h",
-    "../browser/policy/messaging_layer/encryption/fake_encryption_unittest.cc",
+    "../browser/policy/messaging_layer/encryption/decryption.cc",
+    "../browser/policy/messaging_layer/encryption/decryption.h",
+    "../browser/policy/messaging_layer/encryption/encryption_unittest.cc",
     "../browser/policy/messaging_layer/encryption/test_encryption_module.cc",
     "../browser/policy/messaging_layer/encryption/test_encryption_module.h",
     "../browser/policy/messaging_layer/public/report_client_unittest.cc",