[DeviceSync v2] Do not share group private key if no devices need it

If no devices request the group private key in the SyncMetadata
response, do not attempt to share the group private key.

Before this CL, an empty map might be sent to
CryptAuthGroupPrivateKeySharer::ShareGroupPrivateKey(), which would
subsequently crash from a DCHECK requiring the map to be non-empty.

Bug: 951969
Change-Id: I0561538ab2bad53b8e065d2e216b92c2e2b25933
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1851325Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Josh Nohle <nohle@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704868}

chromeos/services/device_sync/cryptauth_device_syncer_impl.cc

@@ -304,10 +304,10 @@ void CryptAuthDeviceSyncerImpl::SetGroupKey(const CryptAuthKey& new_group_key) {
     if (*current_group_key == new_group_key)
       return;
 
-    PA_LOG(VERBOSE) << "Deleting old DeviceSync BetterTogether group key: "
+    PA_LOG(VERBOSE) << "Deleting old DeviceSync BetterTogether group key:\n"
                     << "public = "
                     << util::EncodeAsString(current_group_key->public_key())
-                    << ", private = "
+                    << ",\nprivate = "
                     << (current_group_key->private_key().empty()
                             ? "[empty]"
                             : "[not empty]");
@@ -316,10 +316,10 @@ void CryptAuthDeviceSyncerImpl::SetGroupKey(const CryptAuthKey& new_group_key) {
         current_group_key->handle());
   }
 
-  PA_LOG(VERBOSE) << "New DeviceSync BetterTogether group key: "
+  PA_LOG(VERBOSE) << "New DeviceSync BetterTogether group key:\n"
                   << "public = "
                   << util::EncodeAsString(new_group_key.public_key())
-                  << ", private = "
+                  << ",\nprivate = "
                   << (new_group_key.private_key().empty() ? "[empty]"
                                                           : "[not empty]");
 
@@ -584,6 +584,13 @@ void CryptAuthDeviceSyncerImpl::ShareGroupPrivateKey() {
         id_packet_pair.first, id_packet_pair.second.device_public_key());
   }
 
+  // No device needs the group private key.
+  if (id_to_encrypting_key_map.empty()) {
+    OnShareGroupPrivateKeyFinished(
+        CryptAuthDeviceSyncResult::ResultCode::kSuccess);
+    return;
+  }
+
   const CryptAuthKey* group_key = key_registry_->GetActiveKey(
       CryptAuthKeyBundle::Name::kDeviceSyncBetterTogetherGroupKey);
   DCHECK(group_key);

chromeos/services/device_sync/cryptauth_device_syncer_impl_unittest.cc

@@ -499,6 +499,54 @@ TEST_F(DeviceSyncCryptAuthDeviceSyncerImplTest, Success_InitialGroupKeyValid) {
       GetAllTestDevices());
 }
 
+TEST_F(DeviceSyncCryptAuthDeviceSyncerImplTest,
+       Success_InitialGroupKeyValid_NoDevicesNeedGroupPrivateKey) {
+  // Add the correct group key to the registry.
+  AddInitialGroupKeyToRegistry(GetGroupKey());
+
+  CallSync();
+
+  std::string encrypted_group_private_key = MakeFakeEncryptedString(
+      GetGroupKey().private_key(),
+      GetLocalDeviceForTest().device_better_together_public_key);
+
+  // Only return the local device metadata, noting that it does not need the
+  // group private key. So, there is no need to share the group private key.
+  std::vector<cryptauthv2::DeviceMetadataPacket> device_metadata_packets = {
+      ConvertTestDeviceToMetadataPacket(GetLocalDeviceForTest(),
+                                        kGroupPublicKey,
+                                        false /* need_group_private_key */)};
+  VerifyMetadataSyncerInput(&GetGroupKey());
+
+  // The initial group key is valid, so a new group key was not created.
+  FinishMetadataSyncerAttempt(
+      device_metadata_packets, base::nullopt /* new_group_key */,
+      encrypted_group_private_key, cryptauthv2::GetClientDirectiveForTest(),
+      CryptAuthDeviceSyncResult::ResultCode::kSuccess);
+
+  VerifyGroupKeyInRegistry(GetGroupKey());
+
+  base::flat_set<std::string> device_ids = {
+      GetLocalDeviceForTest().instance_id()};
+  VerifyFeatureStatusGetterInput(device_ids);
+  FinishFeatureStatusGetterAttempt(
+      device_ids, CryptAuthDeviceSyncResult::ResultCode::kSuccess);
+
+  // Even though we have the unencrypted group private key in the key registry,
+  // we decrypt the group private key from CryptAuth and check consistency.
+  RunGroupPrivateKeyDecryptor(encrypted_group_private_key, true /* succeed */);
+
+  RunDeviceMetadataDecryptor(device_metadata_packets,
+                             GetGroupKey().private_key(),
+                             {} /* device_ids_to_fail */);
+
+  VerifyDeviceSyncResult(
+      CryptAuthDeviceSyncResult(CryptAuthDeviceSyncResult::ResultCode::kSuccess,
+                                true /* device_registry_changed */,
+                                cryptauthv2::GetClientDirectiveForTest()),
+      {GetLocalDeviceForTest()});
+}
+
 TEST_F(DeviceSyncCryptAuthDeviceSyncerImplTest,
        Success_InitialGroupPublicKeyValid_NeedGroupPrivateKey) {
   // Add the correct group public key to the registry. Note that we still need