diff --git a/internal/proto/credential.proto b/internal/proto/credential.proto index 1a70c7ff..07129490 100644 --- a/internal/proto/credential.proto +++ b/internal/proto/credential.proto @@ -52,8 +52,8 @@ message SharedCredential { optional IdentityType identity_type = 1; // The unique id of (and hashed based on) a pair of secret - // key (PrivateCredential.verification_key) and X509Certificate's public - // key (PublicCredential.verification_key). + // key (LocalCredential.verification_key) and X509Certificate's public + // key (SharedCredential.verification_key). optional bytes secret_id = 2; // Bytes representation of a Secret Key owned by contact, to decrypt the diff --git a/presence/implementation/BUILD b/presence/implementation/BUILD index 6dd4a82b..bea094ed 100644 --- a/presence/implementation/BUILD +++ b/presence/implementation/BUILD @@ -256,6 +256,7 @@ cc_test( "//net/proto2/contrib/parse_proto:testing", "@com_github_protobuf_matchers//protobuf-matchers", "@com_google_absl//absl/status", + "@com_google_absl//absl/time", "@com_google_googletest//:gtest_main", ], ) diff --git a/presence/implementation/credential_manager_impl.cc b/presence/implementation/credential_manager_impl.cc index 7966bb22..c09bc468 100644 --- a/presence/implementation/credential_manager_impl.cc +++ b/presence/implementation/credential_manager_impl.cc @@ -15,6 +15,8 @@ #include "presence/implementation/credential_manager_impl.h" #include +#include +#include #include #include #include @@ -33,7 +35,6 @@ #include "internal/platform/implementation/crypto.h" #include "internal/platform/logging.h" #include "internal/proto/credential.pb.h" -#include "internal/proto/credential.proto.h" #include "presence/implementation/encryption.h" #include "presence/implementation/ldt.h" @@ -53,7 +54,11 @@ using ::nearby::internal::SharedCredential; // Key to retrieve local device's Private/Public Key Credentials from key store. constexpr char kPairedKeyAliasPrefix[] = "nearby_presence_paired_key_alias_"; -constexpr absl::Duration kTimeout = absl::Seconds(3); +// Returns a random duration in [0, max_duration] range. +absl::Duration RandomDuration(absl::Duration max_duration) { + uint32_t random = ::crypto::RandData(); + return max_duration * random / std::numeric_limits::max(); +} } // namespace @@ -66,21 +71,18 @@ void CredentialManagerImpl::GenerateCredentials( std::vector private_credentials; for (auto identity_type : identity_types) { - // TODO(b/241587906): Get linux time from the platform (like Android) - uint64_t start_time_millis = 0; - const uint64_t gap_millis = credential_life_cycle_days * 24 * 3600 * 1000; - uint64_t end_time_millis = start_time_millis + gap_millis; + absl::Time start_time = SystemClock::ElapsedRealtime(); + absl::Duration gap = credential_life_cycle_days * absl::Hours(24); for (int index = 0; index < contiguous_copy_of_credentials; index++) { auto public_private_credentials = CreatePrivateCredential( - device_metadata, identity_type, start_time_millis, end_time_millis); + device_metadata, identity_type, start_time, start_time + gap); if (public_private_credentials.second.identity_type() != IdentityType::IDENTITY_TYPE_UNSPECIFIED) { private_credentials.push_back(public_private_credentials.first); public_credentials.push_back(public_private_credentials.second); } - start_time_millis += gap_millis; - end_time_millis += gap_millis; + start_time += gap; } } @@ -148,10 +150,10 @@ void CredentialManagerImpl::UpdateRemotePublicCredentials( std::pair CredentialManagerImpl::CreatePrivateCredential( const DeviceMetadata& device_metadata, IdentityType identity_type, - uint64_t start_time_ms, uint64_t end_time_ms) { + absl::Time start_time, absl::Time end_time) { LocalCredential private_credential; - private_credential.set_start_time_millis(start_time_ms); - private_credential.set_end_time_millis(end_time_ms); + private_credential.set_start_time_millis(absl::ToUnixMillis(start_time)); + private_credential.set_end_time_millis(absl::ToUnixMillis(end_time)); private_credential.set_identity_type(identity_type); // Creates an AES key to encrypt the whole broadcast. @@ -198,13 +200,22 @@ CredentialManagerImpl::CreatePrivateCredential( SharedCredential CredentialManagerImpl::CreatePublicCredential( const LocalCredential& private_credential, const std::vector& public_key) { + // The start time in the public credential should be decreased by a random + // value in 0 - 3 hours range. + // The end time should be increased by a random value in 0 - 3 hours range. + // This improves privacy by making it harder to correlate certificates. + absl::Time start_time = + absl::FromUnixMillis(private_credential.start_time_millis()) - + RandomDuration(absl::Hours(3)); + absl::Time end_time = + absl::FromUnixMillis(private_credential.end_time_millis()) + + RandomDuration(absl::Hours(3)); SharedCredential public_credential; public_credential.set_identity_type(private_credential.identity_type()); public_credential.set_secret_id(private_credential.secret_id()); public_credential.set_authenticity_key(private_credential.authenticity_key()); - public_credential.set_start_time_millis( - private_credential.start_time_millis()); - public_credential.set_end_time_millis(private_credential.end_time_millis()); + public_credential.set_start_time_millis(absl::ToUnixMillis(start_time)); + public_credential.set_end_time_millis(absl::ToUnixMillis(end_time)); // set up the public key public_credential.set_verification_key( std::string(public_key.begin(), public_key.end())); diff --git a/presence/implementation/credential_manager_impl.h b/presence/implementation/credential_manager_impl.h index 9388df4e..af207af2 100644 --- a/presence/implementation/credential_manager_impl.h +++ b/presence/implementation/credential_manager_impl.h @@ -24,9 +24,8 @@ #include "absl/base/thread_annotations.h" #include "absl/container/flat_hash_map.h" #include "absl/log/die_if_null.h" -#include "absl/status/status.h" -#include "absl/status/statusor.h" #include "absl/strings/string_view.h" +#include "absl/time/time.h" #include "internal/platform/credential_storage_impl.h" #include "internal/platform/implementation/credential_callbacks.h" #include "internal/platform/runnable.h" @@ -112,7 +111,7 @@ class CredentialManagerImpl : public CredentialManager { nearby::internal::SharedCredential> CreatePrivateCredential( const nearby::internal::DeviceMetadata& device_metadata, - IdentityType identity_type, uint64_t start_time_ms, uint64_t end_time_ms); + IdentityType identity_type, absl::Time start_time, absl::Time end_time); nearby::internal::SharedCredential CreatePublicCredential( const nearby::internal::LocalCredential& private_credential, diff --git a/presence/implementation/credential_manager_impl_test.cc b/presence/implementation/credential_manager_impl_test.cc index e9b136e5..57d035cc 100644 --- a/presence/implementation/credential_manager_impl_test.cc +++ b/presence/implementation/credential_manager_impl_test.cc @@ -14,6 +14,7 @@ #include "presence/implementation/credential_manager_impl.h" +#include #include #include #include @@ -24,13 +25,13 @@ #include "protobuf-matchers/protocol-buffer-matchers.h" #include "gtest/gtest.h" #include "absl/status/status.h" +#include "absl/time/time.h" #include "internal/platform/count_down_latch.h" #include "internal/platform/credential_storage_impl.h" #include "internal/platform/implementation/crypto.h" #include "internal/platform/logging.h" #include "internal/platform/medium_environment.h" #include "internal/proto/credential.pb.h" -#include "internal/proto/credential.proto.h" namespace nearby { namespace presence { @@ -77,10 +78,8 @@ class CredentialManagerImplTest : public ::testing::Test { MOCK_METHOD(void, SaveCredentials, (absl::string_view manager_app_id, absl::string_view account_name, - const std::vector& - private_credentials, - const std::vector& - public_credentials, + const std::vector& private_credentials, + const std::vector& public_credentials, PublicCredentialType public_credential_type, SaveCredentialsResultCallback callback), (override)); @@ -140,20 +139,21 @@ class CredentialManagerImplTest : public ::testing::Test { TEST_F(CredentialManagerImplTest, CreateOneCredentialSuccessfully) { DeviceMetadata device_metadata = CreateTestDeviceMetadata(); + constexpr absl::Time kStartTime = absl::FromUnixSeconds(100000); + constexpr absl::Time kEndTime = absl::FromUnixSeconds(200000); auto credentials = credential_manager_.CreatePrivateCredential( - device_metadata, IDENTITY_TYPE_PRIVATE, /* start_time_ms= */ 0, - /* end_time_ms= */ 1000); + device_metadata, IDENTITY_TYPE_PRIVATE, kStartTime, kEndTime); LocalCredential private_credential = credentials.first; - // Verify the private credential. EXPECT_THAT(private_credential.device_metadata(), EqualsProto(device_metadata)); EXPECT_EQ(private_credential.identity_type(), IDENTITY_TYPE_PRIVATE); EXPECT_FALSE(private_credential.secret_id().empty()); - EXPECT_EQ(private_credential.start_time_millis(), 0); - EXPECT_EQ(private_credential.end_time_millis(), 1000); + EXPECT_EQ(private_credential.start_time_millis(), + absl::ToUnixMillis(kStartTime)); + EXPECT_EQ(private_credential.end_time_millis(), absl::ToUnixMillis(kEndTime)); EXPECT_EQ(private_credential.authenticity_key().size(), CredentialManagerImpl::kAuthenticityKeyByteSize); EXPECT_FALSE(private_credential.verification_key().empty()); @@ -166,8 +166,13 @@ TEST_F(CredentialManagerImplTest, CreateOneCredentialSuccessfully) { EXPECT_FALSE(public_credential.secret_id().empty()); EXPECT_EQ(private_credential.authenticity_key(), public_credential.authenticity_key()); - EXPECT_EQ(public_credential.start_time_millis(), 0); - EXPECT_EQ(public_credential.end_time_millis(), 1000); + EXPECT_LE(public_credential.start_time_millis(), + absl::ToUnixMillis(kStartTime)); + EXPECT_GE(public_credential.start_time_millis(), + absl::ToUnixMillis(kStartTime - absl::Hours(3))); + EXPECT_GE(public_credential.end_time_millis(), absl::ToUnixMillis(kEndTime)); + EXPECT_LE(public_credential.end_time_millis(), + absl::ToUnixMillis(kEndTime + absl::Hours(3))); EXPECT_EQ(Crypto::Sha256(private_credential.metadata_encryption_key()) .AsStringView(), public_credential.metadata_encryption_key_tag()); @@ -186,28 +191,41 @@ TEST_F(CredentialManagerImplTest, CreateOneCredentialSuccessfully) { } TEST_F(CredentialManagerImplTest, GenerateCredentialsSuccessfully) { + constexpr int kLifeCycleDays = 1; + constexpr int kNumCredentials = 5; DeviceMetadata device_metadata = CreateTestDeviceMetadata(); - absl::StatusOr> - public_credentials; + absl::StatusOr> public_credentials; std::vector identityTypes{IDENTITY_TYPE_PRIVATE}; + absl::Time previous_start_time; + absl::Time previous_end_time; credential_manager_.GenerateCredentials( - device_metadata, kManagerAppId, identityTypes, 1, 2, + device_metadata, kManagerAppId, identityTypes, kLifeCycleDays, + kNumCredentials, {.credentials_generated_cb = - [&](absl::StatusOr> - credentials) { + [&](absl::StatusOr> credentials) { public_credentials = std::move(credentials); }}); EXPECT_OK(public_credentials); - EXPECT_EQ(public_credentials->size(), 2); - for (auto& public_credential : *public_credentials) { + EXPECT_EQ(public_credentials->size(), kNumCredentials); + for (int i = 0; i < kNumCredentials; i++) { + SharedCredential& public_credential = public_credentials->at(i); EXPECT_EQ(public_credential.identity_type(), IDENTITY_TYPE_PRIVATE); EXPECT_FALSE(public_credential.secret_id().empty()); - EXPECT_EQ(public_credential.end_time_millis() - - public_credential.start_time_millis(), - 1 * 24 * 3600 * 1000); + absl::Time start_time = + absl::FromUnixMillis(public_credential.start_time_millis()); + absl::Time end_time = + absl::FromUnixMillis(public_credential.end_time_millis()); + if (i > 0) { + EXPECT_GT(start_time, previous_start_time); + EXPECT_GE(previous_end_time, start_time); + EXPECT_GT(end_time, previous_end_time); + } + EXPECT_LT(start_time + absl::Hours(24) * kLifeCycleDays, end_time); EXPECT_FALSE(public_credential.encrypted_metadata_bytes().empty()); + previous_start_time = start_time; + previous_end_time = end_time; } } @@ -309,15 +327,13 @@ TEST_F(CredentialManagerImplTest, })); credential_manager_ = CredentialManagerImpl(&executor_, std::move(credential_storage_ptr)); - absl::StatusOr> - public_credentials; + absl::StatusOr> public_credentials; std::vector identityTypes{IDENTITY_TYPE_PRIVATE}; credential_manager_.GenerateCredentials( device_metadata, kManagerAppId, identityTypes, 1, 2, {.credentials_generated_cb = - [&](absl::StatusOr> - credentials) { + [&](absl::StatusOr> credentials) { public_credentials = std::move(credentials); }}); EXPECT_THAT(public_credentials, @@ -428,8 +444,7 @@ TEST_F(CredentialManagerImplTest, GetPublicCredentialsFailed) { TEST_F(CredentialManagerImplTest, GetCredentialsSuccessfully) { DeviceMetadata device_metadata = CreateTestDeviceMetadata(); - absl::StatusOr> - public_credentials; + absl::StatusOr> public_credentials; std::vector identity_types{IDENTITY_TYPE_PRIVATE}; absl::StatusOr> private_credentials; CredentialSelector credential_selector = BuildDefaultCredentialSelector(); @@ -437,8 +452,7 @@ TEST_F(CredentialManagerImplTest, GetCredentialsSuccessfully) { credential_manager_.GenerateCredentials( device_metadata, kManagerAppId, identity_types, 1, 1, {.credentials_generated_cb = - [&](absl::StatusOr> - credentials) { + [&](absl::StatusOr> credentials) { public_credentials = std::move(credentials); }}); credential_manager_.GetPrivateCredentials( @@ -456,8 +470,7 @@ TEST_F(CredentialManagerImplTest, GetCredentialsSuccessfully) { TEST_F(CredentialManagerImplTest, PublicCredentialsFailEncryption) { DeviceMetadata device_metadata = CreateTestDeviceMetadata(); - absl::StatusOr> - public_credentials; + absl::StatusOr> public_credentials; auto credential_manager_ptr = std::make_unique( &executor_); @@ -471,8 +484,7 @@ TEST_F(CredentialManagerImplTest, PublicCredentialsFailEncryption) { credential_manager_ptr->GenerateCredentials( device_metadata, kManagerAppId, identity_types, 1, 1, {.credentials_generated_cb = - [&](absl::StatusOr> - credentials) { + [&](absl::StatusOr> credentials) { public_credentials = std::move(credentials); }}); diff --git a/presence/scan_request.h b/presence/scan_request.h index ff05e53b..bab03c8a 100644 --- a/presence/scan_request.h +++ b/presence/scan_request.h @@ -114,8 +114,8 @@ struct ScanRequest { // Specifies which manager app to use to get credendentials for scan. std::string manager_app_id; - // Used to specify which types of remote PublicCredential to use during the - // scan. If empty, use all available types of remote PublicCredential. + // Used to specify which types of remote SharedCredential to use during the + // scan. If empty, use all available types of remote SharedCredential. std::vector identity_types; // For new Nearby SDK client (like chromeOs and Android U), use