[Nearby] Change GetDecryptedPublicCertificate() function signature

Reorder the |salt| and |encrypted_metadata_key| parameters of
NearbyShareEncryptedMetadataKey to align with existing data structures
like Advertisement. Then, use NearbyShareEncryptedMetadataKey in
GetDecryptedPublicCertificate() instead of accepting the individual
|salt| and |encrypted_metadata_key| parameters. Because
NearbyShareEncryptedMetadataKey verifies the expected size of the salt
and encrypted key, this function signature is less error prone.

Fixed: 1115123
Change-Id: I1b22b6e7755b9947355d7ea21cfa661f78cdcf81
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2354555
Commit-Queue: Josh Nohle <nohle@chromium.org>
Reviewed-by: Alex Chau <alexchau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798151}

chrome/browser/nearby_sharing/certificates/fake_nearby_share_certificate_manager.cc

@@ -21,12 +21,9 @@ FakeNearbyShareCertificateManager::Factory::CreateInstance() {
 
 FakeNearbyShareCertificateManager::GetDecryptedPublicCertificateCall::
     GetDecryptedPublicCertificateCall(
-        base::span<const uint8_t> encrypted_metadata_key,
-        base::span<const uint8_t> salt,
+        NearbyShareEncryptedMetadataKey encrypted_metadata_key,
         CertDecryptedCallback callback)
-    : encrypted_metadata_key(encrypted_metadata_key.begin(),
-                             encrypted_metadata_key.end()),
-      salt(salt.begin(), salt.end()),
+    : encrypted_metadata_key(std::move(encrypted_metadata_key)),
       callback(std::move(callback)) {}
 
 FakeNearbyShareCertificateManager::GetDecryptedPublicCertificateCall::
@@ -54,11 +51,10 @@ FakeNearbyShareCertificateManager::GetValidPrivateCertificate(
 }
 
 void FakeNearbyShareCertificateManager::GetDecryptedPublicCertificate(
-    base::span<const uint8_t> encrypted_metadata_key,
-    base::span<const uint8_t> salt,
+    NearbyShareEncryptedMetadataKey encrypted_metadata_key,
     CertDecryptedCallback callback) {
-  get_decrypted_public_certificate_calls_.emplace_back(
-      encrypted_metadata_key, salt, std::move(callback));
+  get_decrypted_public_certificate_calls_.emplace_back(encrypted_metadata_key,
+                                                       std::move(callback));
 }
 
 void FakeNearbyShareCertificateManager::DownloadPublicCertificates() {

chrome/browser/nearby_sharing/certificates/fake_nearby_share_certificate_manager.h

@@ -8,10 +8,10 @@
 #include <memory>
 #include <vector>
 
-#include "base/containers/span.h"
 #include "chrome/browser/nearby_sharing/certificates/nearby_share_certificate_manager.h"
 #include "chrome/browser/nearby_sharing/certificates/nearby_share_certificate_manager_impl.h"
 #include "chrome/browser/nearby_sharing/certificates/nearby_share_decrypted_public_certificate.h"
+#include "chrome/browser/nearby_sharing/certificates/nearby_share_encrypted_metadata_key.h"
 #include "chrome/browser/nearby_sharing/certificates/nearby_share_private_certificate.h"
 
 // A fake implementation of NearbyShareCertificateManager, along with a fake
@@ -42,8 +42,7 @@ class FakeNearbyShareCertificateManager : public NearbyShareCertificateManager {
   class GetDecryptedPublicCertificateCall {
    public:
     GetDecryptedPublicCertificateCall(
-        base::span<const uint8_t> encrypted_metadata_key,
-        base::span<const uint8_t> salt,
+        NearbyShareEncryptedMetadataKey encrypted_metadata_key,
         CertDecryptedCallback callback);
     GetDecryptedPublicCertificateCall(
         GetDecryptedPublicCertificateCall&& other);
@@ -55,8 +54,7 @@ class FakeNearbyShareCertificateManager : public NearbyShareCertificateManager {
         const GetDecryptedPublicCertificateCall&) = delete;
     ~GetDecryptedPublicCertificateCall();
 
-    std::vector<uint8_t> encrypted_metadata_key;
-    std::vector<uint8_t> salt;
+    NearbyShareEncryptedMetadataKey encrypted_metadata_key;
     CertDecryptedCallback callback;
   };
 
@@ -67,8 +65,7 @@ class FakeNearbyShareCertificateManager : public NearbyShareCertificateManager {
   NearbySharePrivateCertificate GetValidPrivateCertificate(
       NearbyShareVisibility visibility) override;
   void GetDecryptedPublicCertificate(
-      base::span<const uint8_t> encrypted_metadata_key,
-      base::span<const uint8_t> salt,
+      NearbyShareEncryptedMetadataKey encrypted_metadata_key,
       CertDecryptedCallback callback) override;
   void DownloadPublicCertificates() override;
 

chrome/browser/nearby_sharing/certificates/nearby_share_certificate_manager.h

@@ -6,11 +6,11 @@
 #define CHROME_BROWSER_NEARBY_SHARING_CERTIFICATES_NEARBY_SHARE_CERTIFICATE_MANAGER_H_
 
 #include "base/callback.h"
-#include "base/containers/span.h"
 #include "base/observer_list.h"
 #include "base/observer_list_types.h"
 #include "base/optional.h"
 #include "chrome/browser/nearby_sharing/certificates/nearby_share_decrypted_public_certificate.h"
+#include "chrome/browser/nearby_sharing/certificates/nearby_share_encrypted_metadata_key.h"
 #include "chrome/browser/nearby_sharing/certificates/nearby_share_private_certificate.h"
 #include "chrome/browser/nearby_sharing/certificates/nearby_share_visibility.h"
 
@@ -51,11 +51,10 @@ class NearbyShareCertificateManager {
       NearbyShareVisibility visibility) = 0;
 
   // Returns in |callback| the public certificate that is able to be decrypted
-  // using |encrypted_metadata_key| and |salt|, and returns base::nullopt if no
-  // such public certificate exists.
+  // using |encrypted_metadata_key|, and returns base::nullopt if no such public
+  // certificate exists.
   virtual void GetDecryptedPublicCertificate(
-      base::span<const uint8_t> encrypted_metadata_key,
-      base::span<const uint8_t> salt,
+      NearbyShareEncryptedMetadataKey encrypted_metadata_key,
       CertDecryptedCallback callback) = 0;
 
   // Makes an RPC call to the Nearby server to retrieve all public certificates

chrome/browser/nearby_sharing/certificates/nearby_share_certificate_manager_impl.cc

@@ -47,8 +47,7 @@ NearbyShareCertificateManagerImpl::GetValidPrivateCertificate(
 }
 
 void NearbyShareCertificateManagerImpl::GetDecryptedPublicCertificate(
-    base::span<const uint8_t> encrypted_metadata_key,
-    base::span<const uint8_t> salt,
+    NearbyShareEncryptedMetadataKey encrypted_metadata_key,
     CertDecryptedCallback callback) {
   NOTIMPLEMENTED();
 }

chrome/browser/nearby_sharing/certificates/nearby_share_certificate_manager_impl.h

@@ -7,8 +7,8 @@
 
 #include <memory>
 
-#include "base/containers/span.h"
 #include "chrome/browser/nearby_sharing/certificates/nearby_share_certificate_manager.h"
+#include "chrome/browser/nearby_sharing/certificates/nearby_share_encrypted_metadata_key.h"
 #include "chrome/browser/nearby_sharing/certificates/nearby_share_private_certificate.h"
 #include "chrome/browser/nearby_sharing/certificates/nearby_share_visibility.h"
 
@@ -37,8 +37,7 @@ class NearbyShareCertificateManagerImpl : public NearbyShareCertificateManager {
   NearbySharePrivateCertificate GetValidPrivateCertificate(
       NearbyShareVisibility visibility) override;
   void GetDecryptedPublicCertificate(
-      base::span<const uint8_t> encrypted_metadata_key,
-      base::span<const uint8_t> salt,
+      NearbyShareEncryptedMetadataKey encrypted_metadata_key,
       CertDecryptedCallback callback) override;
   void DownloadPublicCertificates() override;
   void OnStart() override;

chrome/browser/nearby_sharing/certificates/nearby_share_decrypted_public_certificate_unittest.cc

@@ -37,8 +37,9 @@ TEST(NearbyShareDecryptedPublicCertificateTest, Decrypt_IncorrectKeyFailure) {
   EXPECT_FALSE(NearbyShareDecryptedPublicCertificate::DecryptPublicCertificate(
       GetNearbyShareTestPublicCertificate(),
       NearbyShareEncryptedMetadataKey(
-          std::vector<uint8_t>(kNearbyShareNumBytesMetadataEncryptionKey, 0x00),
           std::vector<uint8_t>(kNearbyShareNumBytesMetadataEncryptionKeySalt,
+                               0x00),
+          std::vector<uint8_t>(kNearbyShareNumBytesMetadataEncryptionKey,
                                0x00))));
 }
 

chrome/browser/nearby_sharing/certificates/nearby_share_encrypted_metadata_key.cc

@@ -6,18 +6,27 @@
 
 #include <utility>
 
-#include "base/strings/string_number_conversions.h"
+#include "base/check.h"
 #include "chrome/browser/nearby_sharing/certificates/constants.h"
 
 NearbyShareEncryptedMetadataKey::NearbyShareEncryptedMetadataKey(
-    std::vector<uint8_t> encrypted_key,
-    std::vector<uint8_t> salt)
-    : encrypted_key_(std::move(encrypted_key)), salt_(std::move(salt)) {
-  DCHECK_EQ(kNearbyShareNumBytesMetadataEncryptionKey, encrypted_key_.size());
+    std::vector<uint8_t> salt,
+    std::vector<uint8_t> encrypted_key)
+    : salt_(std::move(salt)), encrypted_key_(std::move(encrypted_key)) {
   DCHECK_EQ(kNearbyShareNumBytesMetadataEncryptionKeySalt, salt_.size());
+  DCHECK_EQ(kNearbyShareNumBytesMetadataEncryptionKey, encrypted_key_.size());
 }
 
 NearbyShareEncryptedMetadataKey::NearbyShareEncryptedMetadataKey(
+    const NearbyShareEncryptedMetadataKey&) = default;
+
+NearbyShareEncryptedMetadataKey& NearbyShareEncryptedMetadataKey::operator=(
+    const NearbyShareEncryptedMetadataKey&) = default;
+
+NearbyShareEncryptedMetadataKey::NearbyShareEncryptedMetadataKey(
+    NearbyShareEncryptedMetadataKey&&) = default;
+
+NearbyShareEncryptedMetadataKey& NearbyShareEncryptedMetadataKey::operator=(
     NearbyShareEncryptedMetadataKey&&) = default;
 
 NearbyShareEncryptedMetadataKey::~NearbyShareEncryptedMetadataKey() = default;

chrome/browser/nearby_sharing/certificates/nearby_share_encrypted_metadata_key.h

@@ -12,17 +12,21 @@
 // metatdata--as well as the salt used to encrypt the key.
 struct NearbyShareEncryptedMetadataKey {
  public:
-  NearbyShareEncryptedMetadataKey(std::vector<uint8_t> encrypted_key,
-                                  std::vector<uint8_t> salt);
+  NearbyShareEncryptedMetadataKey(std::vector<uint8_t> salt,
+                                  std::vector<uint8_t> encrypted_key);
+  NearbyShareEncryptedMetadataKey(const NearbyShareEncryptedMetadataKey&);
+  NearbyShareEncryptedMetadataKey& operator=(
+      const NearbyShareEncryptedMetadataKey&);
   NearbyShareEncryptedMetadataKey(NearbyShareEncryptedMetadataKey&&);
+  NearbyShareEncryptedMetadataKey& operator=(NearbyShareEncryptedMetadataKey&&);
   ~NearbyShareEncryptedMetadataKey();
 
-  const std::vector<uint8_t>& encrypted_key() const { return encrypted_key_; }
   const std::vector<uint8_t>& salt() const { return salt_; }
+  const std::vector<uint8_t>& encrypted_key() const { return encrypted_key_; }
 
  private:
-  std::vector<uint8_t> encrypted_key_;
   std::vector<uint8_t> salt_;
+  std::vector<uint8_t> encrypted_key_;
 };
 
 #endif  // CHROME_BROWSER_NEARBY_SHARING_CERTIFICATES_NEARBY_SHARE_ENCRYPTED_METADATA_KEY_H_

chrome/browser/nearby_sharing/certificates/nearby_share_private_certificate.cc

@@ -223,7 +223,7 @@ NearbySharePrivateCertificate::EncryptMetadataKey() {
     return base::nullopt;
   }
 
-  return NearbyShareEncryptedMetadataKey(encrypted_metadata_key, *salt);
+  return NearbyShareEncryptedMetadataKey(*salt, encrypted_metadata_key);
 }
 
 base::Optional<std::vector<uint8_t>> NearbySharePrivateCertificate::Sign(

chrome/browser/nearby_sharing/certificates/nearby_share_private_certificate_unittest.cc

@@ -63,10 +63,10 @@ TEST(NearbySharePrivateCertificateTest, EncryptMetadataKey) {
   base::Optional<NearbyShareEncryptedMetadataKey> encrypted_metadata_key =
       private_certificate.EncryptMetadataKey();
   ASSERT_TRUE(encrypted_metadata_key);
-  EXPECT_EQ(kNearbyShareNumBytesMetadataEncryptionKey,
-            encrypted_metadata_key->encrypted_key().size());
   EXPECT_EQ(kNearbyShareNumBytesMetadataEncryptionKeySalt,
             encrypted_metadata_key->salt().size());
+  EXPECT_EQ(kNearbyShareNumBytesMetadataEncryptionKey,
+            encrypted_metadata_key->encrypted_key().size());
 }
 
 TEST(NearbySharePrivateCertificateTest, EncryptMetadataKey_FixedData) {

chrome/browser/nearby_sharing/certificates/test_util.cc

@@ -172,9 +172,9 @@ const NearbyShareEncryptedMetadataKey&
 GetNearbyShareTestEncryptedMetadataKey() {
   static const base::NoDestructor<NearbyShareEncryptedMetadataKey>
       encrypted_metadata_key(
+          GetNearbyShareTestSalt(),
           std::vector<uint8_t>(std::begin(kTestEncryptedMetadataKey),
-                               std::end(kTestEncryptedMetadataKey)),
-          GetNearbyShareTestSalt());
+                               std::end(kTestEncryptedMetadataKey)));
   return *encrypted_metadata_key;
 }
 

chrome/browser/nearby_sharing/nearby_sharing_service_impl.cc

@@ -12,6 +12,7 @@
 #include "base/task_runner_util.h"
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "chrome/browser/nearby_sharing/certificates/nearby_share_certificate_manager_impl.h"
+#include "chrome/browser/nearby_sharing/certificates/nearby_share_encrypted_metadata_key.h"
 #include "chrome/browser/nearby_sharing/client/nearby_share_client_impl.h"
 #include "chrome/browser/nearby_sharing/common/nearby_share_prefs.h"
 #include "chrome/browser/nearby_sharing/contacts/nearby_share_contact_manager_impl.h"
@@ -564,14 +565,12 @@ void NearbySharingServiceImpl::OnOutgoingAdvertisementDecoded(
     return;
   }
 
-  // Once get the advertisement, the first thing to do is decrypt the
+  // Once we get the advertisement, the first thing to do is decrypt the
   // certificate.
-  std::vector<uint8_t> encrypted_metadata_key =
-      advertisement->encrypted_metadata_key;
-  std::vector<uint8_t> salt = advertisement->salt;
-
+  NearbyShareEncryptedMetadataKey encrypted_metadata_key(
+      advertisement->salt, advertisement->encrypted_metadata_key);
   GetCertificateManager()->GetDecryptedPublicCertificate(
-      std::move(encrypted_metadata_key), std::move(salt),
+      std::move(encrypted_metadata_key),
       base::BindOnce(&NearbySharingServiceImpl::OnOutgoingDecryptedCertificate,
                      weak_ptr_factory_.GetWeakPtr(), endpoint_id,
                      std::move(advertisement)));
@@ -1334,12 +1333,10 @@ void NearbySharingServiceImpl::OnIncomingAdvertisementDecoded(
     return;
   }
 
-  std::vector<uint8_t> encrypted_metadata_key =
-      advertisement->encrypted_metadata_key;
-  std::vector<uint8_t> salt = advertisement->salt;
-
+  NearbyShareEncryptedMetadataKey encrypted_metadata_key(
+      advertisement->salt, advertisement->encrypted_metadata_key);
   GetCertificateManager()->GetDecryptedPublicCertificate(
-      std::move(encrypted_metadata_key), std::move(salt),
+      std::move(encrypted_metadata_key),
       base::BindOnce(&NearbySharingServiceImpl::OnIncomingDecryptedCertificate,
                      weak_ptr_factory_.GetWeakPtr(), endpoint_id, connection,
                      std::move(advertisement)));

chrome/browser/nearby_sharing/nearby_sharing_service_impl_unittest.cc

@@ -312,10 +312,10 @@ class NearbySharingServiceImplTest : public testing::Test {
 
     ASSERT_FALSE(calls.empty());
     EXPECT_EQ(expected_num_calls, calls.size());
-    EXPECT_EQ(GetNearbyShareTestEncryptedMetadataKey().encrypted_key(),
-              calls.back().encrypted_metadata_key);
     EXPECT_EQ(GetNearbyShareTestEncryptedMetadataKey().salt(),
-              calls.back().salt);
+              calls.back().encrypted_metadata_key.salt());
+    EXPECT_EQ(GetNearbyShareTestEncryptedMetadataKey().encrypted_key(),
+              calls.back().encrypted_metadata_key.encrypted_key());
 
     if (success) {
       std::move(calls.back().callback)