[DeviceSync v2] Add Instance ID field to RemoteDevice

The Instance ID is the primary identifier for devices using v2
DeviceSync. This ID is not present in v1 DeviceSync, which used the
device public key as the primary ID. The device public key is encrypted
in v2 DeviceSync and might not be available during multi-device setup.

Bug: 951969
Change-Id: I09c1ebd6016d651deabb7eca6356581c3c69c0fc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1882300
Commit-Queue: Josh Nohle <nohle@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710966}

chrome/browser/chromeos/login/easy_unlock/easy_unlock_service_signin_chromeos.cc

@@ -523,10 +523,14 @@ void EasyUnlockServiceSignin::OnUserDataLoaded(
       PA_LOG(WARNING) << "No BeaconSeeds were loaded.";
     }
 
+    // Values such as the |instance_id| and |name| of the device are not
+    // provided in the device dictionary that is persisted to the TPM during the
+    // user session. However, in this particular scenario, we do not need these
+    // values to safely construct and use the RemoteDevice objects.
     multidevice::RemoteDevice remote_device(
-        account_id.GetUserEmail(), std::string() /* name */,
-        std::string() /* pii_free_name */, decoded_public_key,
-        decoded_psk /* persistent_symmetric_key */,
+        account_id.GetUserEmail(), std::string() /* instance_id */,
+        std::string() /* name */, std::string() /* pii_free_name */,
+        decoded_public_key, decoded_psk /* persistent_symmetric_key */,
         0L /* last_update_time_millis */, software_features, beacon_seeds);
 
     remote_devices.push_back(remote_device);

chromeos/components/multidevice/mojom/multidevice_mojom_traits.cc

@@ -61,6 +61,13 @@ StructTraits<chromeos::multidevice::mojom::RemoteDeviceDataView,
   return remote_device.user_id;
 }
 
+const std::string&
+StructTraits<chromeos::multidevice::mojom::RemoteDeviceDataView,
+             chromeos::multidevice::RemoteDevice>::
+    instance_id(const chromeos::multidevice::RemoteDevice& remote_device) {
+  return remote_device.instance_id;
+}
+
 const std::string&
 StructTraits<chromeos::multidevice::mojom::RemoteDeviceDataView,
              chromeos::multidevice::RemoteDevice>::
@@ -113,7 +120,8 @@ bool StructTraits<chromeos::multidevice::mojom::RemoteDeviceDataView,
   std::string device_id;
   base::Time last_update_time;
 
-  if (!in.ReadUserId(&out->user_id) || !in.ReadDeviceName(&out->name) ||
+  if (!in.ReadUserId(&out->user_id) || !in.ReadInstanceId(&out->instance_id) ||
+      !in.ReadDeviceName(&out->name) ||
       !in.ReadPiiFreeDeviceName(&out->pii_free_name) ||
       !in.ReadDeviceId(&device_id) ||
       !in.ReadPersistentSymmetricKey(&out->persistent_symmetric_key) ||

chromeos/components/multidevice/mojom/multidevice_mojom_traits.h

@@ -42,6 +42,8 @@ class StructTraits<chromeos::multidevice::mojom::RemoteDeviceDataView,
       const chromeos::multidevice::RemoteDevice& remote_device);
   static const std::string& user_id(
       const chromeos::multidevice::RemoteDevice& remote_device);
+  static const std::string& instance_id(
+      const chromeos::multidevice::RemoteDevice& remote_device);
   static const std::string& device_name(
       const chromeos::multidevice::RemoteDevice& remote_device);
   static const std::string& pii_free_device_name(

chromeos/components/multidevice/mojom/multidevice_mojom_traits_unittest.cc

@@ -54,6 +54,7 @@ TEST(MultiDeviceMojomStructTraitsTest, RemoteDevice) {
 
   chromeos::multidevice::RemoteDevice input;
   input.user_id = "userId";
+  input.instance_id = "instanceId";
   input.name = "name";
   input.pii_free_name = "piiFreeName";
   input.public_key = "publicKey";
@@ -67,6 +68,7 @@ TEST(MultiDeviceMojomStructTraitsTest, RemoteDevice) {
               chromeos::multidevice::mojom::RemoteDevice>(&input, &output));
 
   EXPECT_EQ("userId", output.user_id);
+  EXPECT_EQ("instanceId", output.instance_id);
   EXPECT_EQ("name", output.name);
   EXPECT_EQ("piiFreeName", output.pii_free_name);
   EXPECT_EQ("publicKey", output.public_key);

chromeos/components/multidevice/mojom/multidevice_types.mojom

@@ -61,6 +61,10 @@ struct RemoteDevice {
   // Identifier for the user to whom this device is registered.
   string user_id;
 
+  // The immutable FCM Instance ID assigned to the device. Used as the primary
+  // identifier for devices using CryptAuth v2 Enrollment and DeviceSync.
+  string instance_id;
+
   // Human-readable device name; by default, this is the name of the device
   // model, but this value is editable.
   string device_name;

chromeos/components/multidevice/remote_device.cc

@@ -30,6 +30,7 @@ RemoteDevice::RemoteDevice() : last_update_time_millis(0L) {}
 
 RemoteDevice::RemoteDevice(
     const std::string& user_id,
+    const std::string& instance_id,
     const std::string& name,
     const std::string& pii_free_name,
     const std::string& public_key,
@@ -38,6 +39,7 @@ RemoteDevice::RemoteDevice(
     const std::map<SoftwareFeature, SoftwareFeatureState>& software_features,
     const std::vector<BeaconSeed>& beacon_seeds)
     : user_id(user_id),
+      instance_id(instance_id),
       name(name),
       pii_free_name(pii_free_name),
       public_key(public_key),
@@ -55,8 +57,8 @@ std::string RemoteDevice::GetDeviceId() const {
 }
 
 bool RemoteDevice::operator==(const RemoteDevice& other) const {
-  return user_id == other.user_id && name == other.name &&
-         pii_free_name == other.pii_free_name &&
+  return user_id == other.user_id && instance_id == other.instance_id &&
+         name == other.name && pii_free_name == other.pii_free_name &&
          public_key == other.public_key &&
          persistent_symmetric_key == other.persistent_symmetric_key &&
          last_update_time_millis == other.last_update_time_millis &&
@@ -68,6 +70,8 @@ bool RemoteDevice::operator<(const RemoteDevice& other) const {
   // |public_key| is the only field guaranteed to be set and is also unique to
   // each RemoteDevice. However, since it can contain null bytes, use
   // GetDeviceId(), which cannot contain null bytes, to compare devices.
+  // TODO(https://crbug.com/1019206): Compare by Instance ID when v1 DeviceSync
+  // is deprecated.
   return GetDeviceId().compare(other.GetDeviceId()) < 0;
 }
 

chromeos/components/multidevice/remote_device.h

@@ -26,6 +26,14 @@ struct RemoteDevice {
   static std::string DerivePublicKey(const std::string& device_id);
 
   std::string user_id;
+
+  // The Instance ID is the primary identifier for devices using CryptAuth v2,
+  // but the Instance ID is not present in CryptAuth v1. This string is empty
+  // for devices not using CryptAuth v2 Enrollment and v2 DeviceSync.
+  // TODO(https://crbug.com/1019206): Remove comments when v1 DeviceSync is
+  // deprecated.
+  std::string instance_id;
+
   std::string name;
   std::string pii_free_name;
   std::string public_key;
@@ -37,6 +45,7 @@ struct RemoteDevice {
   RemoteDevice();
   RemoteDevice(
       const std::string& user_id,
+      const std::string& instance_id,
       const std::string& name,
       const std::string& pii_free_name,
       const std::string& public_key,

chromeos/components/multidevice/remote_device_ref.h

@@ -50,6 +50,7 @@ class RemoteDeviceRef {
   ~RemoteDeviceRef();
 
   const std::string& user_id() const { return remote_device_->user_id; }
+  const std::string& instance_id() const { return remote_device_->instance_id; }
   const std::string& name() const { return remote_device_->name; }
   const std::string& pii_free_name() const {
     return remote_device_->pii_free_name;

chromeos/components/multidevice/remote_device_ref_unittest.cc

@@ -30,7 +30,7 @@ class RemoteDeviceRefTest : public testing::Test {
     std::vector<BeaconSeed> beacon_seeds({BeaconSeed(), BeaconSeed()});
 
     remote_device_ = std::make_shared<RemoteDevice>(
-        "user_id", "name", "pii_free_name", "public_key",
+        "user_id", "instance_id", "name", "pii_free_name", "public_key",
         "persistent_symmetric_key", 42000 /* last_update_time_millis */,
         software_feature_to_state_map /* software_features */,
         beacon_seeds /* beacon_seeds */);
@@ -45,6 +45,7 @@ TEST_F(RemoteDeviceRefTest, TestFields) {
   RemoteDeviceRef remote_device_ref(remote_device_);
 
   EXPECT_EQ(remote_device_->user_id, remote_device_ref.user_id());
+  EXPECT_EQ(remote_device_->instance_id, remote_device_ref.instance_id());
   EXPECT_EQ(remote_device_->name, remote_device_ref.name());
   EXPECT_EQ(remote_device_->pii_free_name, remote_device_ref.pii_free_name());
   EXPECT_EQ(remote_device_->public_key, remote_device_ref.public_key());

chromeos/components/multidevice/remote_device_test_util.cc

@@ -8,6 +8,7 @@
 #include "chromeos/components/multidevice/remote_device_test_util.h"
 
 #include "base/base64.h"
+#include "base/strings/string_number_conversions.h"
 
 namespace chromeos {
 
@@ -15,6 +16,7 @@ namespace multidevice {
 
 // Attributes of the default test remote device.
 const char kTestRemoteDeviceUserId[] = "example@gmail.com";
+const char kTestRemoteDeviceInstanceId[] = "instanceId";
 const char kTestRemoteDeviceName[] = "remote device";
 const char kTestRemoteDevicePiiFreeName[] = "no-pii device";
 const char kTestRemoteDevicePublicKey[] = "public key";
@@ -33,6 +35,12 @@ RemoteDeviceRefBuilder& RemoteDeviceRefBuilder::SetUserId(
   return *this;
 }
 
+RemoteDeviceRefBuilder& RemoteDeviceRefBuilder::SetInstanceId(
+    const std::string& instance_id) {
+  remote_device_->instance_id = instance_id;
+  return *this;
+}
+
 RemoteDeviceRefBuilder& RemoteDeviceRefBuilder::SetName(
     const std::string& name) {
   remote_device_->name = name;
@@ -89,9 +97,9 @@ RemoteDevice CreateRemoteDeviceForTest() {
   software_features[SoftwareFeature::kInstantTetheringHost] =
       SoftwareFeatureState::kSupported;
 
-  return RemoteDevice(kTestRemoteDeviceUserId, kTestRemoteDeviceName,
-                      kTestRemoteDevicePiiFreeName, kTestRemoteDevicePublicKey,
-                      kTestRemoteDevicePSK,
+  return RemoteDevice(kTestRemoteDeviceUserId, kTestRemoteDeviceInstanceId,
+                      kTestRemoteDeviceName, kTestRemoteDevicePiiFreeName,
+                      kTestRemoteDevicePublicKey, kTestRemoteDevicePSK,
                       kTestRemoteDeviceLastUpdateTimeMillis, software_features,
                       {} /* beacon_seeds */);
 }
@@ -106,7 +114,9 @@ RemoteDeviceRefList CreateRemoteDeviceRefListForTest(size_t num_to_create) {
   for (size_t i = 0; i < num_to_create; i++) {
     RemoteDeviceRef remote_device =
         RemoteDeviceRefBuilder()
-            .SetPublicKey("publicKey" + std::to_string(i))
+            .SetInstanceId(kTestRemoteDeviceInstanceId +
+                           base::NumberToString(i))
+            .SetPublicKey("publicKey" + base::NumberToString(i))
             .Build();
     generated_devices.push_back(remote_device);
   }
@@ -119,7 +129,9 @@ RemoteDeviceList CreateRemoteDeviceListForTest(size_t num_to_create) {
 
   for (size_t i = 0; i < num_to_create; i++) {
     RemoteDevice remote_device = CreateRemoteDeviceForTest();
-    remote_device.public_key = "publicKey" + std::to_string(i);
+    remote_device.instance_id =
+        kTestRemoteDeviceInstanceId + base::NumberToString(i);
+    remote_device.public_key = "publicKey" + base::NumberToString(i);
     generated_devices.push_back(remote_device);
   }
 

chromeos/components/multidevice/remote_device_test_util.h

@@ -24,6 +24,7 @@ class RemoteDeviceRefBuilder {
   RemoteDeviceRefBuilder();
   ~RemoteDeviceRefBuilder();
   RemoteDeviceRefBuilder& SetUserId(const std::string& user_id);
+  RemoteDeviceRefBuilder& SetInstanceId(const std::string& instance_id);
   RemoteDeviceRefBuilder& SetName(const std::string& name);
   RemoteDeviceRefBuilder& SetPiiFreeName(const std::string& pii_free_name);
   RemoteDeviceRefBuilder& SetPublicKey(const std::string& public_key);

chromeos/services/device_sync/remote_device_loader.cc

@@ -140,10 +140,13 @@ void RemoteDeviceLoader::OnPSKDerived(
         multidevice::FromCryptAuthSeed(cryptauth_beacon_seed));
   }
 
+  // Because RemoteDeviceLoader does not handle devices using v2 DeviceSync, no
+  // Instance ID is present.
   multidevice::RemoteDevice remote_device(
-      user_id_, device.friendly_device_name(), device.no_pii_device_name(),
-      device.public_key(), psk, device.last_update_time_millis(),
-      GetSoftwareFeatureToStateMap(device), multidevice_beacon_seeds);
+      user_id_, std::string() /* instance_id */, device.friendly_device_name(),
+      device.no_pii_device_name(), device.public_key(), psk,
+      device.last_update_time_millis(), GetSoftwareFeatureToStateMap(device),
+      multidevice_beacon_seeds);
 
   remote_devices_.push_back(remote_device);