[Nearby] Revoke only the necessary certificates when contacts change

If contacts are removed from the allowlist, only the selected-contacts
visibility private certificates need to be recreated. If the contact
list changes, only the all-contacts visibility private certificates need
to be recreated. (Ideally, we would only recreate all-contacts
visibility private certificates if contacts are removed from the contact
list, but we do not have this information.)

These revocations need to occur so that unauthorized contacts do not
have access to valid certificates. Previously, all private certificates
were being recreated in both scenarios, which is safe but overkill.

Fixed: b/168022980, 1123134
Change-Id: I9263e02f2cca72c5b350368d42596c5f2d5655e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2412890
Commit-Queue: Josh Nohle <nohle@chromium.org>
Reviewed-by: James Vecore <vecore@google.com>
Cr-Commit-Position: refs/heads/master@{#807478}

chrome/browser/nearby_sharing/certificates/nearby_share_certificate_manager_impl.cc

@@ -336,10 +336,18 @@ void NearbyShareCertificateManagerImpl::OnStop() {
 void NearbyShareCertificateManagerImpl::OnAllowlistChanged(
     bool were_contacts_added_to_allowlist,
     bool were_contacts_removed_from_allowlist) {
+  // Contacts that are newly added to the allowlist can use the existing set of
+  // selected-contacts visibility certificates. The existing certificates will
+  // be distributed to the newly allowed contacts as soon as they check in with
+  // the Nearby Share server.
   if (!were_contacts_removed_from_allowlist)
     return;
 
-  certificate_storage_->ClearPrivateCertificates();
+  // We do not want to continue using the current selected-contacts visibility
+  // certificates because they might have been shared with contacts no longer on
+  // the allowlist.
+  certificate_storage_->ClearPrivateCertificatesOfVisibility(
+      nearby_share::mojom::Visibility::kSelectedContacts);
   private_certificate_expiration_scheduler_->MakeImmediateRequest();
 }
 
@@ -352,7 +360,13 @@ void NearbyShareCertificateManagerImpl::OnContactsUploaded(
   if (!did_contacts_change_since_last_upload)
     return;
 
-  certificate_storage_->ClearPrivateCertificates();
+  // If the contact list changed, all-contacts visibility certificates need to
+  // be recreated because the current certificates might have been shared with
+  // people no longer on the user's contact list. (Ideally, we would only
+  // recreate all-contacts visibility certificates when contacts are *removed*
+  // from the contact list, but we do not have this information.)
+  certificate_storage_->ClearPrivateCertificatesOfVisibility(
+      nearby_share::mojom::Visibility::kAllContacts);
   private_certificate_expiration_scheduler_->MakeImmediateRequest();
 }
 
@@ -363,6 +377,7 @@ void NearbyShareCertificateManagerImpl::OnLocalDeviceDataChanged(
   if (!did_device_name_change && !did_full_name_change && !did_icon_url_change)
     return;
 
+  // Recreate all private certificates to ensure up-to-date metadata.
   certificate_storage_->ClearPrivateCertificates();
   private_certificate_expiration_scheduler_->MakeImmediateRequest();
 }

chrome/browser/nearby_sharing/certificates/nearby_share_certificate_manager_impl_unittest.cc

@@ -540,17 +540,28 @@ TEST_F(NearbyShareCertificateManagerImplTest,
        RevokePrivateCertificates_OnAllowlistChanged) {
   cert_manager_->Start();
 
-  // Destroy and recreate private certificates if contacts were removed from the
-  // user's list of selected contacts.
+  // Destroy and recreate seleted-contacts visibility private certificates if
+  // contacts were removed from the user's list of selected contacts.
   size_t num_expected_calls = 0;
   for (bool were_contacts_added_to_allowlist : {true, false}) {
     for (bool were_contacts_removed_from_allowlist : {true, false}) {
+      cert_store_->ReplacePrivateCertificates(private_certificates_);
       contact_manager_->NotifyAllowlistChanged(
           were_contacts_added_to_allowlist,
           were_contacts_removed_from_allowlist);
+
+      std::vector<NearbySharePrivateCertificate> certs =
+          *cert_store_->GetPrivateCertificates();
+
       if (were_contacts_removed_from_allowlist) {
         ++num_expected_calls;
-        EXPECT_TRUE(cert_store_->GetPrivateCertificates()->empty());
+        EXPECT_EQ(3u, certs.size());
+        for (const NearbySharePrivateCertificate& cert : certs) {
+          EXPECT_NE(nearby_share::mojom::Visibility::kSelectedContacts,
+                    cert.visibility());
+        }
+      } else {
+        EXPECT_EQ(6u, certs.size());
       }
 
       EXPECT_EQ(num_expected_calls,
@@ -563,15 +574,26 @@ TEST_F(NearbyShareCertificateManagerImplTest,
        RevokePrivateCertificates_OnContactsUploaded) {
   cert_manager_->Start();
 
-  // Destroy and recreate private certificates if the user's contact list has
-  // changed since the last upload.
+  // Destroy and recreate all-contacts visibility private certificates if the
+  // user's contact list has changed since the last upload.
   size_t num_expected_calls = 0;
   for (bool did_contacts_change_since_last_upload : {true, false}) {
+    cert_store_->ReplacePrivateCertificates(private_certificates_);
     contact_manager_->NotifyContactsUploaded(
         did_contacts_change_since_last_upload);
+
+    std::vector<NearbySharePrivateCertificate> certs =
+        *cert_store_->GetPrivateCertificates();
+
     if (did_contacts_change_since_last_upload) {
       ++num_expected_calls;
-      EXPECT_TRUE(cert_store_->GetPrivateCertificates()->empty());
+      EXPECT_EQ(3u, certs.size());
+      for (const NearbySharePrivateCertificate& cert : certs) {
+        EXPECT_NE(nearby_share::mojom::Visibility::kAllContacts,
+                  cert.visibility());
+      }
+    } else {
+      EXPECT_EQ(6u, certs.size());
     }
 
     EXPECT_EQ(num_expected_calls,