Commit b6f3f4bc authored by Josh Nohle's avatar Josh Nohle Committed by Commit Bot

[Nearby] Destroy and recreate private certificates if contacts change

The certificate manager observer for events from the contact manager.
Destroy and recreate all private certificates if either
  1) the user's contact list has changed since the last upload to the
     NearbyShare server, or
  2) contacts are removed from the allowlist--relevant to
     selected-contacts visibility mode.

This ensures that sharing can only happen with trusted contacts.

Fixed: b/166113568, 1121320
Change-Id: I92ad994602003bbeea0bdc752b03965c11f1265e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2377895
Commit-Queue: Josh Nohle <nohle@chromium.org>
Reviewed-by: default avatarRyan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805338}
parent ccba8cdb
...@@ -29,6 +29,7 @@ source_set("certificates") { ...@@ -29,6 +29,7 @@ source_set("certificates") {
"//base/util/values:values_util", "//base/util/values:values_util",
"//chrome/browser/nearby_sharing/client", "//chrome/browser/nearby_sharing/client",
"//chrome/browser/nearby_sharing/common", "//chrome/browser/nearby_sharing/common",
"//chrome/browser/nearby_sharing/contacts",
"//chrome/browser/nearby_sharing/local_device_data", "//chrome/browser/nearby_sharing/local_device_data",
"//chrome/browser/nearby_sharing/logging", "//chrome/browser/nearby_sharing/logging",
"//chrome/browser/nearby_sharing/proto", "//chrome/browser/nearby_sharing/proto",
...@@ -83,6 +84,7 @@ source_set("unit_tests") { ...@@ -83,6 +84,7 @@ source_set("unit_tests") {
"//base/util/values:values_util", "//base/util/values:values_util",
"//chrome/browser/nearby_sharing/client:test_support", "//chrome/browser/nearby_sharing/client:test_support",
"//chrome/browser/nearby_sharing/common", "//chrome/browser/nearby_sharing/common",
"//chrome/browser/nearby_sharing/contacts:test_support",
"//chrome/browser/nearby_sharing/local_device_data:test_support", "//chrome/browser/nearby_sharing/local_device_data:test_support",
"//chrome/browser/nearby_sharing/proto", "//chrome/browser/nearby_sharing/proto",
"//chrome/browser/nearby_sharing/scheduling:test_support", "//chrome/browser/nearby_sharing/scheduling:test_support",
......
...@@ -14,6 +14,7 @@ FakeNearbyShareCertificateManager::Factory::~Factory() = default; ...@@ -14,6 +14,7 @@ FakeNearbyShareCertificateManager::Factory::~Factory() = default;
std::unique_ptr<NearbyShareCertificateManager> std::unique_ptr<NearbyShareCertificateManager>
FakeNearbyShareCertificateManager::Factory::CreateInstance( FakeNearbyShareCertificateManager::Factory::CreateInstance(
NearbyShareLocalDeviceDataManager* local_device_data_manager, NearbyShareLocalDeviceDataManager* local_device_data_manager,
NearbyShareContactManager* contact_manager,
PrefService* pref_service, PrefService* pref_service,
leveldb_proto::ProtoDatabaseProvider* proto_database_provider, leveldb_proto::ProtoDatabaseProvider* proto_database_provider,
const base::FilePath& profile_path, const base::FilePath& profile_path,
......
...@@ -39,6 +39,7 @@ class FakeNearbyShareCertificateManager : public NearbyShareCertificateManager { ...@@ -39,6 +39,7 @@ class FakeNearbyShareCertificateManager : public NearbyShareCertificateManager {
// NearbyShareCertificateManagerImpl::Factory: // NearbyShareCertificateManagerImpl::Factory:
std::unique_ptr<NearbyShareCertificateManager> CreateInstance( std::unique_ptr<NearbyShareCertificateManager> CreateInstance(
NearbyShareLocalDeviceDataManager* local_device_data_manager, NearbyShareLocalDeviceDataManager* local_device_data_manager,
NearbyShareContactManager* contact_manager,
PrefService* pref_service, PrefService* pref_service,
leveldb_proto::ProtoDatabaseProvider* proto_database_provider, leveldb_proto::ProtoDatabaseProvider* proto_database_provider,
const base::FilePath& profile_path, const base::FilePath& profile_path,
......
...@@ -134,6 +134,7 @@ NearbyShareCertificateManagerImpl::Factory* ...@@ -134,6 +134,7 @@ NearbyShareCertificateManagerImpl::Factory*
std::unique_ptr<NearbyShareCertificateManager> std::unique_ptr<NearbyShareCertificateManager>
NearbyShareCertificateManagerImpl::Factory::Create( NearbyShareCertificateManagerImpl::Factory::Create(
NearbyShareLocalDeviceDataManager* local_device_data_manager, NearbyShareLocalDeviceDataManager* local_device_data_manager,
NearbyShareContactManager* contact_manager,
PrefService* pref_service, PrefService* pref_service,
leveldb_proto::ProtoDatabaseProvider* proto_database_provider, leveldb_proto::ProtoDatabaseProvider* proto_database_provider,
const base::FilePath& profile_path, const base::FilePath& profile_path,
...@@ -142,14 +143,14 @@ NearbyShareCertificateManagerImpl::Factory::Create( ...@@ -142,14 +143,14 @@ NearbyShareCertificateManagerImpl::Factory::Create(
DCHECK(clock); DCHECK(clock);
if (test_factory_) { if (test_factory_) {
return test_factory_->CreateInstance(local_device_data_manager, return test_factory_->CreateInstance(
pref_service, proto_database_provider, local_device_data_manager, contact_manager, pref_service,
profile_path, client_factory, clock); proto_database_provider, profile_path, client_factory, clock);
} }
return base::WrapUnique(new NearbyShareCertificateManagerImpl( return base::WrapUnique(new NearbyShareCertificateManagerImpl(
local_device_data_manager, pref_service, proto_database_provider, local_device_data_manager, contact_manager, pref_service,
profile_path, client_factory, clock)); proto_database_provider, profile_path, client_factory, clock));
} }
// static // static
...@@ -162,12 +163,14 @@ NearbyShareCertificateManagerImpl::Factory::~Factory() = default; ...@@ -162,12 +163,14 @@ NearbyShareCertificateManagerImpl::Factory::~Factory() = default;
NearbyShareCertificateManagerImpl::NearbyShareCertificateManagerImpl( NearbyShareCertificateManagerImpl::NearbyShareCertificateManagerImpl(
NearbyShareLocalDeviceDataManager* local_device_data_manager, NearbyShareLocalDeviceDataManager* local_device_data_manager,
NearbyShareContactManager* contact_manager,
PrefService* pref_service, PrefService* pref_service,
leveldb_proto::ProtoDatabaseProvider* proto_database_provider, leveldb_proto::ProtoDatabaseProvider* proto_database_provider,
const base::FilePath& profile_path, const base::FilePath& profile_path,
NearbyShareClientFactory* client_factory, NearbyShareClientFactory* client_factory,
base::Clock* clock) base::Clock* clock)
: local_device_data_manager_(local_device_data_manager), : local_device_data_manager_(local_device_data_manager),
contact_manager_(contact_manager),
pref_service_(pref_service), pref_service_(pref_service),
client_factory_(client_factory), client_factory_(client_factory),
clock_(clock), clock_(clock),
...@@ -226,10 +229,13 @@ NearbyShareCertificateManagerImpl::NearbyShareCertificateManagerImpl( ...@@ -226,10 +229,13 @@ NearbyShareCertificateManagerImpl::NearbyShareCertificateManagerImpl(
/*page_token=*/base::nullopt, /*page_token=*/base::nullopt,
/*page_number=*/1, /*page_number=*/1,
/*certificate_count=*/0), /*certificate_count=*/0),
clock_)) {} clock_)) {
contact_manager_->AddObserver(this);
}
NearbyShareCertificateManagerImpl::~NearbyShareCertificateManagerImpl() = NearbyShareCertificateManagerImpl::~NearbyShareCertificateManagerImpl() {
default; contact_manager_->RemoveObserver(this);
}
NearbySharePrivateCertificate NearbySharePrivateCertificate
NearbyShareCertificateManagerImpl::GetValidPrivateCertificate( NearbyShareCertificateManagerImpl::GetValidPrivateCertificate(
...@@ -286,6 +292,29 @@ void NearbyShareCertificateManagerImpl::OnStop() { ...@@ -286,6 +292,29 @@ void NearbyShareCertificateManagerImpl::OnStop() {
download_public_certificates_scheduler_->Stop(); download_public_certificates_scheduler_->Stop();
} }
void NearbyShareCertificateManagerImpl::OnAllowlistChanged(
bool were_contacts_added_to_allowlist,
bool were_contacts_removed_from_allowlist) {
if (!were_contacts_removed_from_allowlist)
return;
certificate_storage_->ClearPrivateCertificates();
private_certificate_expiration_scheduler_->Reschedule();
}
void NearbyShareCertificateManagerImpl::OnContactsDownloaded(
const std::set<std::string>& allowed_contact_ids,
const std::vector<nearbyshare::proto::ContactRecord>& contacts) {}
void NearbyShareCertificateManagerImpl::OnContactsUploaded(
bool did_contacts_change_since_last_upload) {
if (!did_contacts_change_since_last_upload)
return;
certificate_storage_->ClearPrivateCertificates();
private_certificate_expiration_scheduler_->Reschedule();
}
base::Optional<base::Time> base::Optional<base::Time>
NearbyShareCertificateManagerImpl::NextPrivateCertificateExpirationTime() { NearbyShareCertificateManagerImpl::NextPrivateCertificateExpirationTime() {
// We enforce that a fixed number--kNearbyShareNumPrivateCertificates for each // We enforce that a fixed number--kNearbyShareNumPrivateCertificates for each
......
...@@ -21,6 +21,7 @@ ...@@ -21,6 +21,7 @@
#include "chrome/browser/nearby_sharing/certificates/nearby_share_private_certificate.h" #include "chrome/browser/nearby_sharing/certificates/nearby_share_private_certificate.h"
#include "chrome/browser/nearby_sharing/certificates/nearby_share_visibility.h" #include "chrome/browser/nearby_sharing/certificates/nearby_share_visibility.h"
#include "chrome/browser/nearby_sharing/common/nearby_share_http_result.h" #include "chrome/browser/nearby_sharing/common/nearby_share_http_result.h"
#include "chrome/browser/nearby_sharing/contacts/nearby_share_contact_manager.h"
#include "chrome/browser/nearby_sharing/proto/rpc_resources.pb.h" #include "chrome/browser/nearby_sharing/proto/rpc_resources.pb.h"
class NearbyShareClient; class NearbyShareClient;
...@@ -49,16 +50,26 @@ class ListPublicCertificatesResponse; ...@@ -49,16 +50,26 @@ class ListPublicCertificatesResponse;
// 2) downloading, storing, and decrypting public certificates from trusted // 2) downloading, storing, and decrypting public certificates from trusted
// contacts, as well as removing expired public certificates. // contacts, as well as removing expired public certificates.
// //
// This implementation destroys and recreates all private certificates if either
// a) the user's contact list has changed, or
// b) contacts are removed from the allowlist--relevant to selected-contacts
// visibility mode.
// TODO(b/168022980): Only destroy the private certificates of the relevant
// visiblity: all-contacts and selected-contacts visibility, respectively.
//
// TODO(https://crbug.com/1121443): Add the following if we remove // TODO(https://crbug.com/1121443): Add the following if we remove
// GetValidPrivateCertificate() and perform all private certificate crypto // GetValidPrivateCertificate() and perform all private certificate crypto
// operations internally: "This implementation also provides the high-level // operations internally: "This implementation also provides the high-level
// interface for performing cryptographic operations related to certificates." // interface for performing cryptographic operations related to certificates."
class NearbyShareCertificateManagerImpl : public NearbyShareCertificateManager { class NearbyShareCertificateManagerImpl
: public NearbyShareCertificateManager,
public NearbyShareContactManager::Observer {
public: public:
class Factory { class Factory {
public: public:
static std::unique_ptr<NearbyShareCertificateManager> Create( static std::unique_ptr<NearbyShareCertificateManager> Create(
NearbyShareLocalDeviceDataManager* local_device_data_manager, NearbyShareLocalDeviceDataManager* local_device_data_manager,
NearbyShareContactManager* contact_manager,
PrefService* pref_service, PrefService* pref_service,
leveldb_proto::ProtoDatabaseProvider* proto_database_provider, leveldb_proto::ProtoDatabaseProvider* proto_database_provider,
const base::FilePath& profile_path, const base::FilePath& profile_path,
...@@ -70,6 +81,7 @@ class NearbyShareCertificateManagerImpl : public NearbyShareCertificateManager { ...@@ -70,6 +81,7 @@ class NearbyShareCertificateManagerImpl : public NearbyShareCertificateManager {
virtual ~Factory(); virtual ~Factory();
virtual std::unique_ptr<NearbyShareCertificateManager> CreateInstance( virtual std::unique_ptr<NearbyShareCertificateManager> CreateInstance(
NearbyShareLocalDeviceDataManager* local_device_data_manager, NearbyShareLocalDeviceDataManager* local_device_data_manager,
NearbyShareContactManager* contact_manager,
PrefService* pref_service, PrefService* pref_service,
leveldb_proto::ProtoDatabaseProvider* proto_database_provider, leveldb_proto::ProtoDatabaseProvider* proto_database_provider,
const base::FilePath& profile_path, const base::FilePath& profile_path,
...@@ -85,6 +97,7 @@ class NearbyShareCertificateManagerImpl : public NearbyShareCertificateManager { ...@@ -85,6 +97,7 @@ class NearbyShareCertificateManagerImpl : public NearbyShareCertificateManager {
private: private:
NearbyShareCertificateManagerImpl( NearbyShareCertificateManagerImpl(
NearbyShareLocalDeviceDataManager* local_device_data_manager, NearbyShareLocalDeviceDataManager* local_device_data_manager,
NearbyShareContactManager* contact_manager,
PrefService* pref_service, PrefService* pref_service,
leveldb_proto::ProtoDatabaseProvider* proto_database_provider, leveldb_proto::ProtoDatabaseProvider* proto_database_provider,
const base::FilePath& profile_path, const base::FilePath& profile_path,
...@@ -104,6 +117,14 @@ class NearbyShareCertificateManagerImpl : public NearbyShareCertificateManager { ...@@ -104,6 +117,14 @@ class NearbyShareCertificateManagerImpl : public NearbyShareCertificateManager {
void OnStart() override; void OnStart() override;
void OnStop() override; void OnStop() override;
// NearbyShareContactManager::Observer:
void OnAllowlistChanged(bool were_contacts_added_to_allowlist,
bool were_contacts_removed_from_allowlist) override;
void OnContactsDownloaded(
const std::set<std::string>& allowed_contact_ids,
const std::vector<nearbyshare::proto::ContactRecord>& contacts) override;
void OnContactsUploaded(bool did_contacts_change_since_last_upload) override;
// Used by the private certificate expiration scheduler to determine the next // Used by the private certificate expiration scheduler to determine the next
// private certificate expiration time. Returns base::Time::Min() if // private certificate expiration time. Returns base::Time::Min() if
// certificates are missing. This function never returns base::nullopt. // certificates are missing. This function never returns base::nullopt.
...@@ -165,6 +186,7 @@ class NearbyShareCertificateManagerImpl : public NearbyShareCertificateManager { ...@@ -165,6 +186,7 @@ class NearbyShareCertificateManagerImpl : public NearbyShareCertificateManager {
size_t certificate_count); size_t certificate_count);
NearbyShareLocalDeviceDataManager* local_device_data_manager_ = nullptr; NearbyShareLocalDeviceDataManager* local_device_data_manager_ = nullptr;
NearbyShareContactManager* contact_manager_ = nullptr;
PrefService* pref_service_ = nullptr; PrefService* pref_service_ = nullptr;
NearbyShareClientFactory* client_factory_ = nullptr; NearbyShareClientFactory* client_factory_ = nullptr;
base::Clock* clock_ = nullptr; base::Clock* clock_ = nullptr;
......
...@@ -220,6 +220,7 @@ NearbySharingServiceImpl::NearbySharingServiceImpl( ...@@ -220,6 +220,7 @@ NearbySharingServiceImpl::NearbySharingServiceImpl(
local_device_data_manager_.get())), local_device_data_manager_.get())),
certificate_manager_(NearbyShareCertificateManagerImpl::Factory::Create( certificate_manager_(NearbyShareCertificateManagerImpl::Factory::Create(
local_device_data_manager_.get(), local_device_data_manager_.get(),
contact_manager_.get(),
prefs, prefs,
content::BrowserContext::GetDefaultStoragePartition(profile) content::BrowserContext::GetDefaultStoragePartition(profile)
->GetProtoDatabaseProvider(), ->GetProtoDatabaseProvider(),
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment