[Nearby] Refresh advertising data when private certificates change

Currently, if private certificate data changes, the Chromebook might
continue advertising the stale certificate data until periodic
advertisement rotation occurs. A symptom could be other devices failing
to discover the Chromebook for up to 15 minutes after the Chromebook's
initial onboarding, for instance. In this CL, we refresh the advertising
data whenever private certificates change.

Manually verified that a phone can immediately discover a Chromebook
after 1) initial Chromebook Nearby Share onboarding and 2) changing
selected contacts on the Chromebook (which results in private
certificate recreation).

Fixed: 1154064
Change-Id: I4839d7b91df57759382e646fb2db8ee482d53601
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2566749
Auto-Submit: Josh Nohle <nohle@chromium.org>
Commit-Queue: James Vecore <vecore@google.com>
Reviewed-by: James Vecore <vecore@google.com>
Cr-Commit-Position: refs/heads/master@{#832228}

chrome/browser/nearby_sharing/nearby_sharing_service_impl.cc

@@ -272,6 +272,7 @@ NearbySharingServiceImpl::NearbySharingServiceImpl(
 
   nearby_process_observer_.Add(process_manager_);
   power_client_->AddObserver(this);
+  certificate_manager_->AddObserver(this);
 
   settings_.AddSettingsObserver(settings_receiver_.BindNewPipeAndPassRemote());
 
@@ -318,6 +319,7 @@ void NearbySharingServiceImpl::Shutdown() {
     process_manager_->StopProcess(profile_);
 
   power_client_->RemoveObserver(this);
+  certificate_manager_->RemoveObserver(this);
 
   // TODO(crbug/1147652): The call to update the advertising interval is
   // removed to prevent a Bluez crash. We need to either reduce the global
@@ -361,6 +363,7 @@ void NearbySharingServiceImpl::Shutdown() {
   }
 
   process_shutdown_pending_timer_.Stop();
+  rotate_background_advertisement_timer_.Stop();
 
   // |profile_| has now been shut down so we shouldn't use it anymore.
   profile_ = nullptr;
@@ -958,6 +961,22 @@ void NearbySharingServiceImpl::OnAllowedContactsChanged(
   // TODO(vecore): handle visible contacts change
 }
 
+void NearbySharingServiceImpl::OnPublicCertificatesDownloaded() {
+  // TODO(https://crbug.com/1152158): Possibly restart scanning after public
+  // certificates are downloaded.
+}
+
+void NearbySharingServiceImpl::OnPrivateCertificatesChanged() {
+  // If we are currently advertising, restart advertising using the updated
+  // private certificates.
+  if (rotate_background_advertisement_timer_.IsRunning()) {
+    NS_LOG(VERBOSE)
+        << __func__
+        << ": Private certificates changed; rotating background advertisement.";
+    rotate_background_advertisement_timer_.FireNow();
+  }
+}
+
 void NearbySharingServiceImpl::OnEndpointDiscovered(
     const std::string& endpoint_id,
     const std::vector<uint8_t>& endpoint_info) {
@@ -1054,6 +1073,10 @@ NearbySharingServiceImpl::CreateEndpointInfo(
     if (encrypted_metadata_key) {
       salt = encrypted_metadata_key->salt();
       encrypted_key = encrypted_metadata_key->encrypted_key();
+    } else {
+      NS_LOG(WARNING) << __func__
+                      << ": Failed to encrypt private certificate metadata key "
+                      << "for advertisement.";
     }
   }
 
@@ -1607,7 +1630,7 @@ void NearbySharingServiceImpl::InvalidateAdvertisingState() {
 
   nearby_connections_manager_->StartAdvertising(
       *endpoint_info,
-      /* listener= */ this, power_level, data_usage,
+      /*listener=*/this, power_level, data_usage,
       base::BindOnce(&NearbySharingServiceImpl::OnStartAdvertisingResult,
                      weak_ptr_factory_.GetWeakPtr(), device_name.has_value()));
 
@@ -1664,7 +1687,7 @@ void NearbySharingServiceImpl::StartScanning() {
   ClearOutgoingShareTargetInfoMap();
 
   nearby_connections_manager_->StartDiscovery(
-      /* listener= */ this, settings_.GetDataUsage(),
+      /*listener=*/this, settings_.GetDataUsage(),
       base::BindOnce([](NearbyConnectionsManager::ConnectionsStatus status) {
         NS_LOG(VERBOSE) << __func__
                         << ": Scanning start attempted over Nearby Connections "

chrome/browser/nearby_sharing/nearby_sharing_service_impl.h

@@ -57,6 +57,7 @@ class Profile;
 class NearbySharingServiceImpl
     : public NearbySharingService,
       public nearby_share::mojom::NearbyShareSettingsObserver,
+      public NearbyShareCertificateManager::Observer,
       public NearbyProcessManager::Observer,
       public device::BluetoothAdapter::Observer,
       public NearbyConnectionsManager::IncomingConnectionListener,
@@ -135,6 +136,10 @@ class NearbySharingServiceImpl
   void OnAllowedContactsChanged(
       const std::vector<std::string>& allowed_contacts) override;
 
+  // NearbyShareCertificateManager::Observer:
+  void OnPublicCertificatesDownloaded() override;
+  void OnPrivateCertificatesChanged() override;
+
   // NearbyConnectionsManager::DiscoveryListener:
   void OnEndpointDiscovered(const std::string& endpoint_id,
                             const std::vector<uint8_t>& endpoint_info) override;

chrome/browser/nearby_sharing/nearby_sharing_service_impl_unittest.cc

@@ -3569,8 +3569,7 @@ TEST_F(NearbySharingServiceImplTest, ShutdownCallsObservers) {
   service_.reset();
 }
 
-TEST_F(NearbySharingServiceImplTest,
-       PeriodicallyRotateBackgroundAdvertisement) {
+TEST_F(NearbySharingServiceImplTest, RotateBackgroundAdvertisement_Periodic) {
   certificate_manager()->set_next_salt({0x00, 0x01});
   SetVisibility(nearby_share::mojom::Visibility::kAllContacts);
   NiceMock<MockTransferUpdateCallback> callback;
@@ -3588,3 +3587,23 @@ TEST_F(NearbySharingServiceImplTest,
       fake_nearby_connections_manager_->advertising_endpoint_info();
   EXPECT_NE(endpoint_info_initial, endpoint_info_rotated);
 }
+
+TEST_F(NearbySharingServiceImplTest,
+       RotateBackgroundAdvertisement_PrivateCertificatesChange) {
+  certificate_manager()->set_next_salt({0x00, 0x01});
+  SetVisibility(nearby_share::mojom::Visibility::kAllContacts);
+  NiceMock<MockTransferUpdateCallback> callback;
+  NearbySharingService::StatusCodes result = service_->RegisterReceiveSurface(
+      &callback, NearbySharingService::ReceiveSurfaceState::kBackground);
+  EXPECT_EQ(result, NearbySharingService::StatusCodes::kOk);
+  EXPECT_TRUE(fake_nearby_connections_manager_->IsAdvertising());
+  auto endpoint_info_initial =
+      fake_nearby_connections_manager_->advertising_endpoint_info();
+
+  certificate_manager()->set_next_salt({0x00, 0x02});
+  certificate_manager()->NotifyPrivateCertificatesChanged();
+  EXPECT_TRUE(fake_nearby_connections_manager_->IsAdvertising());
+  auto endpoint_info_rotated =
+      fake_nearby_connections_manager_->advertising_endpoint_info();
+  EXPECT_NE(endpoint_info_initial, endpoint_info_rotated);
+}