Commit fbd98f96 authored by Josh Nohle's avatar Josh Nohle Committed by Chromium LUCI CQ

[Nearby] Clarify misleading certificate storage logs

Notably, remove the log, "Overwriting private certificates pref with X
certificates." Some certificate might be overwritten but with no
material changes. For example, consumed salts--used to encrypt the
metadata key during advertising--need to be stored in the private
certificate. That update overwrites the certificate, but the certificate
remains essentially the same.

Change-Id: I99b120689d1371f844dac682c2e1c398d65b4c9b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2595191
Commit-Queue: Josh Nohle <nohle@chromium.org>
Commit-Queue: James Vecore <vecore@google.com>
Auto-Submit: Josh Nohle <nohle@chromium.org>
Reviewed-by: default avatarJames Vecore <vecore@google.com>
Cr-Commit-Position: refs/heads/master@{#837836}
parent 8c4aa932
...@@ -4,8 +4,10 @@ ...@@ -4,8 +4,10 @@
#include <algorithm> #include <algorithm>
#include "base/strings/string_number_conversions.h"
#include "chrome/browser/nearby_sharing/certificates/common.h" #include "chrome/browser/nearby_sharing/certificates/common.h"
#include "chrome/browser/nearby_sharing/certificates/nearby_share_certificate_storage.h" #include "chrome/browser/nearby_sharing/certificates/nearby_share_certificate_storage.h"
#include "chrome/browser/nearby_sharing/logging/logging.h"
base::Optional<base::Time> base::Optional<base::Time>
NearbyShareCertificateStorage::NextPrivateCertificateExpirationTime() { NearbyShareCertificateStorage::NextPrivateCertificateExpirationTime() {
...@@ -25,17 +27,24 @@ void NearbyShareCertificateStorage::UpdatePrivateCertificate( ...@@ -25,17 +27,24 @@ void NearbyShareCertificateStorage::UpdatePrivateCertificate(
const NearbySharePrivateCertificate& private_certificate) { const NearbySharePrivateCertificate& private_certificate) {
base::Optional<std::vector<NearbySharePrivateCertificate>> certs = base::Optional<std::vector<NearbySharePrivateCertificate>> certs =
GetPrivateCertificates(); GetPrivateCertificates();
if (!certs) if (!certs) {
NS_LOG(WARNING) << __func__ << ": No private certificates to update.";
return; return;
}
auto it = std::find_if( auto it = std::find_if(
certs->begin(), certs->end(), certs->begin(), certs->end(),
[&private_certificate](const NearbySharePrivateCertificate& cert) { [&private_certificate](const NearbySharePrivateCertificate& cert) {
return cert.id() == private_certificate.id(); return cert.id() == private_certificate.id();
}); });
if (it == certs->end()) if (it == certs->end()) {
NS_LOG(VERBOSE) << __func__ << ": No private certificate with id="
<< base::HexEncode(private_certificate.id());
return; return;
}
NS_LOG(VERBOSE) << __func__ << ": Updating private certificate id="
<< base::HexEncode(private_certificate.id());
*it = private_certificate; *it = private_certificate;
ReplacePrivateCertificates(*certs); ReplacePrivateCertificates(*certs);
} }
...@@ -56,10 +65,17 @@ void NearbyShareCertificateStorage::RemoveExpiredPrivateCertificates( ...@@ -56,10 +65,17 @@ void NearbyShareCertificateStorage::RemoveExpiredPrivateCertificates(
} }
} }
size_t num_removed = certs->size() - unexpired_certs.size();
if (num_removed == 0)
return;
NS_LOG(VERBOSE) << __func__ << ": Removing " << num_removed
<< " expired private certificates.";
ReplacePrivateCertificates(unexpired_certs); ReplacePrivateCertificates(unexpired_certs);
} }
void NearbyShareCertificateStorage::ClearPrivateCertificates() { void NearbyShareCertificateStorage::ClearPrivateCertificates() {
NS_LOG(VERBOSE) << __func__ << ": Removing all private certificates.";
ReplacePrivateCertificates(std::vector<NearbySharePrivateCertificate>()); ReplacePrivateCertificates(std::vector<NearbySharePrivateCertificate>());
} }
...@@ -80,6 +96,10 @@ void NearbyShareCertificateStorage::ClearPrivateCertificatesOfVisibility( ...@@ -80,6 +96,10 @@ void NearbyShareCertificateStorage::ClearPrivateCertificatesOfVisibility(
} }
} }
if (were_certs_removed) if (were_certs_removed) {
NS_LOG(VERBOSE) << __func__
<< ": Removing all private certificates of visibility "
<< visibility;
ReplacePrivateCertificates(new_certs); ReplacePrivateCertificates(new_certs);
}
} }
...@@ -330,7 +330,7 @@ void NearbyShareCertificateStorageImpl:: ...@@ -330,7 +330,7 @@ void NearbyShareCertificateStorageImpl::
} }
NS_LOG(VERBOSE) << __func__ << ": Inserting " << new_entries->size() NS_LOG(VERBOSE) << __func__ << ": Inserting " << new_entries->size()
<< " new public certificates."; << " public certificates.";
db_->UpdateEntries( db_->UpdateEntries(
std::move(new_entries), std::move(new_entries),
/*keys_to_remove=*/std::make_unique<std::vector<std::string>>(), /*keys_to_remove=*/std::make_unique<std::vector<std::string>>(),
...@@ -462,8 +462,6 @@ void NearbyShareCertificateStorageImpl::ReplacePrivateCertificates( ...@@ -462,8 +462,6 @@ void NearbyShareCertificateStorageImpl::ReplacePrivateCertificates(
for (const NearbySharePrivateCertificate& cert : private_certificates) { for (const NearbySharePrivateCertificate& cert : private_certificates) {
list.Append(cert.ToDictionary()); list.Append(cert.ToDictionary());
} }
NS_LOG(VERBOSE) << __func__ << ": Overwriting private certificates pref with "
<< private_certificates.size() << " certificates.";
pref_service_->Set(prefs::kNearbySharingPrivateCertificateListPrefName, list); pref_service_->Set(prefs::kNearbySharingPrivateCertificateListPrefName, list);
} }
...@@ -531,7 +529,7 @@ void NearbyShareCertificateStorageImpl::AddPublicCertificates( ...@@ -531,7 +529,7 @@ void NearbyShareCertificateStorageImpl::AddPublicCertificates(
NS_LOG(VERBOSE) NS_LOG(VERBOSE)
<< __func__ << __func__
<< ": Calling UpdateEntries on public certificate database with " << ": Calling UpdateEntries on public certificate database with "
<< public_certificates.size() << " new certificates."; << public_certificates.size() << " certificates.";
db_->UpdateEntries( db_->UpdateEntries(
std::move(new_entries), std::make_unique<std::vector<std::string>>(), std::move(new_entries), std::make_unique<std::vector<std::string>>(),
base::BindOnce( base::BindOnce(
......
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