From 07119e80de0167b6aba5896a489ee6ef95376240 Mon Sep 17 00:00:00 2001 From: Janusz Sobczak Date: Wed, 10 Aug 2022 11:44:54 -0700 Subject: [PATCH] Merge CertificateManager and CredentialManager Refactoring change. Merge CertificateManager and CredentialManager into a single interface. Hide AdvertisementFactory in implementation/. The client app will not be using that class directly. Clean up build targets. PiperOrigin-RevId: 466748133 --- internal/platform/BUILD | 2 +- presence/BUILD | 54 +------------------ presence/certificate_manager.h | 46 ---------------- presence/implementation/BUILD | 21 +++++++- .../implementation/advertisement_decoder.h | 2 +- .../advertisement_factory.cc | 6 +-- .../advertisement_factory.h | 8 +-- .../advertisement_factory_test.cc | 23 ++++---- presence/implementation/credential_manager.h | 11 ++++ .../implementation/credential_manager_impl.h | 12 +++++ presence/implementation/mediums/BUILD | 5 +- 11 files changed, 65 insertions(+), 125 deletions(-) delete mode 100644 presence/certificate_manager.h rename presence/{ => implementation}/advertisement_factory.cc (95%) rename presence/{ => implementation}/advertisement_factory.h (87%) rename presence/{ => implementation}/advertisement_factory_test.cc (83%) diff --git a/internal/platform/BUILD b/internal/platform/BUILD index dbc06cb8..3de64cb0 100644 --- a/internal/platform/BUILD +++ b/internal/platform/BUILD @@ -176,7 +176,7 @@ cc_library( "//connections/implementation:__subpackages__", "//internal/platform:__pkg__", "//internal/platform/implementation:__subpackages__", - "//presence:__pkg__", + "//presence:__subpackages__", ], deps = [ "//internal/platform/implementation:types", diff --git a/presence/BUILD b/presence/BUILD index c85d62cd..205d8878 100644 --- a/presence/BUILD +++ b/presence/BUILD @@ -74,7 +74,6 @@ cc_library( deps = [ ":credential", ":encryption", - ":types", "//internal/platform:logging", "@com_google_absl//absl/strings", "@com_google_absl//absl/types:variant", @@ -96,35 +95,6 @@ cc_library( ], ) -cc_library( - name = "certificate_manager", - hdrs = ["certificate_manager.h"], - deps = [ - ":credential", - ":types", - "@com_google_absl//absl/status:statusor", - "@com_google_absl//absl/strings", - ], -) - -cc_library( - name = "advertisement_factory", - srcs = ["advertisement_factory.cc"], - hdrs = ["advertisement_factory.h"], - deps = [ - ":broadcast_request", - ":certificate_manager", - ":credential", - ":types", - "//internal/platform:logging", - "//internal/platform:uuid", - "//internal/platform/implementation:comm", - "@com_google_absl//absl/status", - "@com_google_absl//absl/status:statusor", - "@com_google_absl//absl/strings:str_format", - ], -) - cc_library( name = "action_factory", srcs = ["action_factory.cc"], @@ -146,28 +116,6 @@ cc_library( visibility = [ "//third_party/nearby:__subpackages__", ], - deps = [ - "//presence/proto:credential_cc_proto", - "//presence/proto:device_metadata_cc_proto", - ], -) - -cc_test( - name = "advertisement_factory_test", - size = "small", - srcs = ["advertisement_factory_test.cc"], - deps = [ - ":action_factory", - ":advertisement_factory", - ":certificate_manager", - ":credential", - ":types", - "//internal/platform/implementation/g3", # build_cleaner: keep - "@com_github_protobuf_matchers//protobuf-matchers", - "@com_google_absl//absl/status", - "@com_google_absl//absl/strings", - "@com_google_googletest//:gtest_main", - ], ) cc_test( @@ -263,7 +211,7 @@ cc_test( shard_count = 6, deps = [ ":presence", - "//internal/platform/implementation/g3", + "//internal/platform/implementation/g3", # build_cleaner: keep "@com_github_protobuf_matchers//protobuf-matchers", "@com_google_googletest//:gtest_main", ], diff --git a/presence/certificate_manager.h b/presence/certificate_manager.h deleted file mode 100644 index a8bdc28c..00000000 --- a/presence/certificate_manager.h +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright 2022 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// https://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef THIRD_PARTY_NEARBY_PRESENCE_CERTIFICATE_MANAGER_H_ -#define THIRD_PARTY_NEARBY_PRESENCE_CERTIFICATE_MANAGER_H_ -#include - -#include "absl/status/statusor.h" -#include "absl/strings/string_view.h" -#include "presence/presence_identity.h" - -namespace nearby { -namespace presence { - -/** Stores and manages credentials/certificates on local device, and uses the - * certificates for encryption and deceryption. */ -class CertificateManager { - public: - virtual ~CertificateManager() = default; - - /** Returns encrypted metadata key associated with `identity` for Base NP - * advertisement */ - virtual absl::StatusOr GetBaseEncryptedMetadataKey( - const PresenceIdentity& identity) = 0; - /** Encrypts `data_elements` using certificate associated with `identity` and - * `salt` */ - virtual absl::StatusOr EncryptDataElements( - const PresenceIdentity& identity, absl::string_view salt, - absl::string_view data_elements) = 0; -}; - -} // namespace presence -} // namespace nearby - -#endif // THIRD_PARTY_NEARBY_PRESENCE_CERTIFICATE_MANAGER_H_ diff --git a/presence/implementation/BUILD b/presence/implementation/BUILD index f5c64b33..7785e780 100644 --- a/presence/implementation/BUILD +++ b/presence/implementation/BUILD @@ -17,10 +17,12 @@ cc_library( name = "internal", srcs = [ "advertisement_decoder.cc", + "advertisement_factory.cc", "credential_manager_impl.cc", ], hdrs = [ "advertisement_decoder.h", + "advertisement_factory.h", "broadcast_manager.h", "credential_manager.h", "credential_manager_impl.h", @@ -37,10 +39,11 @@ cc_library( "//internal/platform:base", "//internal/platform:comm", "//internal/platform:logging", + "//internal/platform:uuid", "//internal/platform/implementation:comm", "//internal/platform/implementation:types", "//presence:action_factory", - "//presence:advertisement_factory", + "//presence:broadcast_request", "//presence:credential", "//presence:encryption", "//presence:types", @@ -67,6 +70,22 @@ cc_test( ], ) +cc_test( + name = "advertisement_factory_test", + size = "small", + srcs = ["advertisement_factory_test.cc"], + deps = [ + ":internal", + "//internal/platform/implementation/g3", # build_cleaner: keep + "//presence:action_factory", + "//presence:types", + "@com_github_protobuf_matchers//protobuf-matchers", + "@com_google_absl//absl/status", + "@com_google_absl//absl/strings", + "@com_google_googletest//:gtest_main", + ], +) + cc_test( name = "credential_manager_impl_test", size = "small", diff --git a/presence/implementation/advertisement_decoder.h b/presence/implementation/advertisement_decoder.h index d2f8ef0f..28b469a5 100644 --- a/presence/implementation/advertisement_decoder.h +++ b/presence/implementation/advertisement_decoder.h @@ -20,8 +20,8 @@ #include "absl/status/status.h" #include "absl/status/statusor.h" -#include "presence/advertisement_factory.h" #include "presence/data_element.h" +#include "presence/implementation/advertisement_factory.h" #include "presence/implementation/credential_manager.h" #include "presence/presence_identity.h" diff --git a/presence/advertisement_factory.cc b/presence/implementation/advertisement_factory.cc similarity index 95% rename from presence/advertisement_factory.cc rename to presence/implementation/advertisement_factory.cc index ec0ed8bc..514a270c 100644 --- a/presence/advertisement_factory.cc +++ b/presence/implementation/advertisement_factory.cc @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "presence/advertisement_factory.h" +#include "presence/implementation/advertisement_factory.h" #include #include @@ -96,7 +96,7 @@ AdvertisementFactory::CreateBaseNpAdvertisement( } } absl::StatusOr identity = - certificate_manager_.GetBaseEncryptedMetadataKey(presence.identity); + credential_manager_.GetBaseEncryptedMetadataKey(presence.identity); if (!identity.ok()) { return identity.status(); } @@ -122,7 +122,7 @@ AdvertisementFactory::CreateBaseNpAdvertisement( return result; } if (!identity->empty()) { - auto encrypted = certificate_manager_.EncryptDataElements( + auto encrypted = credential_manager_.EncryptDataElements( presence.identity, request.salt, data_elements); if (!encrypted.ok()) { return encrypted.status(); diff --git a/presence/advertisement_factory.h b/presence/implementation/advertisement_factory.h similarity index 87% rename from presence/advertisement_factory.h rename to presence/implementation/advertisement_factory.h index d0db4dfc..3f44d395 100644 --- a/presence/advertisement_factory.h +++ b/presence/implementation/advertisement_factory.h @@ -21,7 +21,7 @@ #include "absl/status/statusor.h" #include "internal/platform/implementation/ble_v2.h" #include "presence/broadcast_request.h" -#include "presence/certificate_manager.h" +#include "presence/implementation/credential_manager.h" #include "presence/presence_identity.h" namespace nearby { @@ -33,8 +33,8 @@ using ::location::nearby::api::ble_v2::BleAdvertisementData; /** Builds BLE advertisements from broadcast requests. */ class AdvertisementFactory { public: - explicit AdvertisementFactory(CertificateManager* certificate_manager) - : certificate_manager_(*certificate_manager) {} + explicit AdvertisementFactory(CredentialManager* credential_manager) + : credential_manager_(*credential_manager) {} /** Returns a BLE advertisement for given `request` */ absl::StatusOr CreateAdvertisement( @@ -44,7 +44,7 @@ class AdvertisementFactory { absl::StatusOr CreateBaseNpAdvertisement( const BroadcastRequest& request) const; - CertificateManager& certificate_manager_; + CredentialManager& credential_manager_; }; } // namespace presence diff --git a/presence/advertisement_factory_test.cc b/presence/implementation/advertisement_factory_test.cc similarity index 83% rename from presence/advertisement_factory_test.cc rename to presence/implementation/advertisement_factory_test.cc index 75534fd7..ac49976b 100644 --- a/presence/advertisement_factory_test.cc +++ b/presence/implementation/advertisement_factory_test.cc @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "presence/advertisement_factory.h" +#include "presence/implementation/advertisement_factory.h" #include #include @@ -23,8 +23,8 @@ #include "absl/status/status.h" #include "absl/strings/escaping.h" #include "presence/action_factory.h" -#include "presence/certificate_manager.h" #include "presence/data_element.h" +#include "presence/implementation/credential_manager_impl.h" namespace nearby { namespace presence { @@ -35,7 +35,7 @@ using ::testing::NiceMock; using ::testing::Return; using ::testing::status::StatusIs; -class MockCertificateManager : public CertificateManager { +class MockCredentialManager : public CredentialManagerImpl { public: MOCK_METHOD(absl::StatusOr, GetBaseEncryptedMetadataKey, (const PresenceIdentity& identity), (override)); @@ -47,7 +47,7 @@ class MockCertificateManager : public CertificateManager { TEST(AdvertisementFactory, CreateAdvertisementFromPrivateIdentity) { std::string salt = "AB"; - NiceMock certificate_manager; + NiceMock credential_manager; PresenceIdentity identity; std::vector data_elements; data_elements.emplace_back(DataElement::kActionFieldType, @@ -58,14 +58,14 @@ TEST(AdvertisementFactory, CreateAdvertisementFromPrivateIdentity) { .SetSalt(salt) .SetTxPower(5) .SetAction(action)); - EXPECT_CALL(certificate_manager, GetBaseEncryptedMetadataKey(identity)) + EXPECT_CALL(credential_manager, GetBaseEncryptedMetadataKey(identity)) .WillOnce(Return(absl::HexStringToBytes("1011121314151617181920212223"))); EXPECT_CALL( - certificate_manager, + credential_manager, EncryptDataElements(identity, salt, absl::HexStringToBytes("1505260080"))) .WillOnce(Return(absl::HexStringToBytes("5051525354"))); - AdvertisementFactory factory(&certificate_manager); + AdvertisementFactory factory(&credential_manager); absl::StatusOr result = factory.CreateAdvertisement(request); @@ -80,9 +80,8 @@ TEST(AdvertisementFactory, CreateAdvertisementFromPrivateIdentity) { } } -TEST(AdvertisementFactory, - CreateAdvertisementFailsWhenCertificateManagerFails) { - NiceMock certificate_manager; +TEST(AdvertisementFactory, CreateAdvertisementFailsWhenCredentialManagerFails) { + NiceMock credential_manager; PresenceIdentity identity; std::vector data_elements; data_elements.emplace_back(DataElement::kActionFieldType, @@ -93,11 +92,11 @@ TEST(AdvertisementFactory, .SetSalt("AB") .SetTxPower(5) .SetAction(action)); - EXPECT_CALL(certificate_manager, GetBaseEncryptedMetadataKey(identity)) + EXPECT_CALL(credential_manager, GetBaseEncryptedMetadataKey(identity)) .WillOnce(Return(absl::UnimplementedError( "GetBaseEncryptedMetadataKey not implemented"))); - AdvertisementFactory factory(&certificate_manager); + AdvertisementFactory factory(&credential_manager); EXPECT_THAT(factory.CreateAdvertisement(request), StatusIs(absl::StatusCode::kUnimplemented)); } diff --git a/presence/implementation/credential_manager.h b/presence/implementation/credential_manager.h index 5775a495..37129466 100644 --- a/presence/implementation/credential_manager.h +++ b/presence/implementation/credential_manager.h @@ -83,6 +83,17 @@ class CredentialManager { virtual absl::StatusOr DecryptDataElements( absl::string_view metadata_key, absl::string_view salt, absl::string_view data_elements) = 0; + + // Returns encrypted metadata key associated with `identity` for Base NP + // advertisement. + virtual absl::StatusOr GetBaseEncryptedMetadataKey( + const PresenceIdentity& identity) = 0; + + // Encrypts `data_elements` using certificate associated with `identity` and + // `salt`. + virtual absl::StatusOr EncryptDataElements( + const PresenceIdentity& identity, absl::string_view salt, + absl::string_view data_elements) = 0; }; } // namespace presence diff --git a/presence/implementation/credential_manager_impl.h b/presence/implementation/credential_manager_impl.h index cc75a4b0..e10e671a 100644 --- a/presence/implementation/credential_manager_impl.h +++ b/presence/implementation/credential_manager_impl.h @@ -69,6 +69,18 @@ class CredentialManagerImpl : public CredentialManager { return absl::UnimplementedError("DecryptDataElements unimplemented"); } + absl::StatusOr GetBaseEncryptedMetadataKey( + const PresenceIdentity& identity) override { + return absl::UnimplementedError( + "GetBaseEncryptedMetadataKey unimplemented"); + } + + absl::StatusOr EncryptDataElements( + const PresenceIdentity& identity, absl::string_view salt, + absl::string_view data_elements) override { + return absl::UnimplementedError("EncryptDataElements unimplemented"); + } + private: FRIEND_TEST(CredentialManagerImpl, CreateOneCredentialSuccessfully); diff --git a/presence/implementation/mediums/BUILD b/presence/implementation/mediums/BUILD index bcc1aae1..3993ad82 100644 --- a/presence/implementation/mediums/BUILD +++ b/presence/implementation/mediums/BUILD @@ -23,8 +23,5 @@ cc_library( visibility = [ "//presence/implementation:__subpackages__", ], - deps = [ - "//internal/platform:base", - "//internal/platform:comm", - ], + deps = ["//internal/platform:comm"], )