Skip to content

Commit

Permalink
Some refactoring of GcpKmsClient and unit test
Browse files Browse the repository at this point in the history 
 * Make GcpKmsClient move only
 * Make the constructor accept key_name and kms_stub
 * Modify GetKeyName to return a failed status in case the key_uri doesn't have the correct prefix. Update DoesSupport and GetAead accordingly
 * Other minor fixes to conform to the C++ style guide

PiperOrigin-RevId: 528567191
Change-Id: Icb2e2149ae86bd2890dc8c348477f54a63f4cd73
  • Loading branch information
@morambro @copybara-github
morambro authored and copybara-github committed May 1, 2023
1 parent 4105d58 commit e0658e4
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 108 deletions.
140 changes: 70 additions & 70 deletions tink/integration/gcpkms/gcp_kms_client.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,8 @@
#include "grpcpp/security/credentials.h"
#include "absl/memory/memory.h"
#include "absl/status/status.h"
#include "absl/strings/ascii.h"
#include "absl/strings/match.h"
#include "absl/strings/str_cat.h"
#include "absl/strings/str_split.h"
#include "absl/strings/string_view.h"
#include "tink/integration/gcpkms/gcp_kms_aead.h"
#include "tink/kms_clients.h"
Expand All @@ -44,128 +43,129 @@ namespace gcpkms {

namespace {

using crypto::tink::Version;
using crypto::tink::util::Status;
using crypto::tink::util::StatusOr;
using google::cloud::kms::v1::KeyManagementService;
using grpc::ChannelArguments;
using grpc::ChannelCredentials;
using ::google::cloud::kms::v1::KeyManagementService;

static constexpr char kKeyUriPrefix[] = "gcp-kms://";
static constexpr char kGcpKmsServer[] = "cloudkms.googleapis.com";
static constexpr char kTinkUserAgentPrefix[] = "Tink/";
static constexpr absl::string_view kKeyUriPrefix = "gcp-kms://";
static constexpr absl::string_view kGcpKmsServer = "cloudkms.googleapis.com";
static constexpr absl::string_view kTinkUserAgentPrefix = "Tink/";

StatusOr<std::string> ReadFile(absl::string_view filename) {
util::StatusOr<std::string> ReadFile(absl::string_view filename) {
std::ifstream input_stream;
input_stream.open(std::string(filename), std::ifstream::in);
if (!input_stream.is_open()) {
return Status(absl::StatusCode::kInvalidArgument,
absl::StrCat("Error reading file ", filename));
return util::Status(absl::StatusCode::kInvalidArgument,
absl::StrCat("Error reading file ", filename));
}
std::stringstream input;
input << input_stream.rdbuf();
input_stream.close();
return input.str();
}

StatusOr<std::shared_ptr<ChannelCredentials>> GetCredentials(
util::StatusOr<std::shared_ptr<grpc::ChannelCredentials>> GetCredentials(
absl::string_view credentials_path) {
if (credentials_path.empty()) {
auto creds = grpc::GoogleDefaultCredentials();
std::shared_ptr<grpc::ChannelCredentials> creds =
grpc::GoogleDefaultCredentials();
if (creds == nullptr) {
return Status(absl::StatusCode::kInternal,
"Could not read default credentials");
return util::Status(absl::StatusCode::kInternal,
"Could not read default credentials");
}
return creds;
}

// Try reading credentials from a file.
auto json_creds_result = ReadFile(credentials_path);
if (!json_creds_result.ok()) return json_creds_result.status();
auto creds =
util::StatusOr<std::string> json_creds_result = ReadFile(credentials_path);
if (!json_creds_result.ok()) {
return json_creds_result.status();
}
std::shared_ptr<grpc::CallCredentials> creds =
grpc::ServiceAccountJWTAccessCredentials(json_creds_result.value());
if (creds != nullptr) {
// Creating "empty" 'channel_creds', to convert 'creds'
// to ChannelCredentials via CompositeChannelCredentials().
auto channel_creds = grpc::SslCredentials(grpc::SslCredentialsOptions());
return grpc::CompositeChannelCredentials(channel_creds, creds);
if (creds == nullptr) {
return util::Status(absl::StatusCode::kInvalidArgument,
absl::StrCat("Could not load credentials from file ",
credentials_path));
}
return Status(
absl::StatusCode::kInvalidArgument,
absl::StrCat("Could not load credentials from file ", credentials_path));
// Creating "empty" 'channel_creds', to convert 'creds' to ChannelCredentials
// via CompositeChannelCredentials().
std::shared_ptr<grpc::ChannelCredentials> channel_creds =
grpc::SslCredentials(grpc::SslCredentialsOptions());
return grpc::CompositeChannelCredentials(channel_creds, creds);
}

// Returns GCP KMS key name contained in 'key_uri'.
// If 'key_uri' does not refer to an GCP key, returns an empty string.
std::string GetKeyName(absl::string_view key_uri) {
if (!absl::StartsWithIgnoreCase(key_uri, kKeyUriPrefix)) return "";
return std::string(key_uri.substr(std::string(kKeyUriPrefix).length()));
// Returns GCP KMS key name contained in `key_uri`. If `key_uri` does not refer
// to a GCP key, returns an error status.
util::StatusOr<std::string> GetKeyName(absl::string_view key_uri) {
if (!absl::StartsWithIgnoreCase(key_uri, kKeyUriPrefix)) {
return util::Status(absl::StatusCode::kInvalidArgument,
absl::StrCat("The key URI ", key_uri,
" does not start with ", kKeyUriPrefix));
}
return std::string(key_uri.substr(kKeyUriPrefix.length()));
}

} // namespace

// static
StatusOr<std::unique_ptr<GcpKmsClient>> GcpKmsClient::New(
util::StatusOr<std::unique_ptr<GcpKmsClient>> GcpKmsClient::New(
absl::string_view key_uri, absl::string_view credentials_path) {
std::unique_ptr<GcpKmsClient> client(new GcpKmsClient());

// If a specific key is given, create a GCP KMSClient.
// Empty key name by default.
std::string key_name = "";
if (!key_uri.empty()) {
client->key_name_ = GetKeyName(key_uri);
if (client->key_name_.empty()) {
return Status(absl::StatusCode::kInvalidArgument,
absl::StrCat("Key '", key_uri, "' not supported"));
util::StatusOr<std::string> key_name_from_uri = GetKeyName(key_uri);
if (!key_name_from_uri.ok()) {
return key_name_from_uri.status();
}
key_name = key_name_from_uri.value();
}

// Read credentials.
auto creds_result = GetCredentials(credentials_path);
util::StatusOr<std::shared_ptr<grpc::ChannelCredentials>> creds_result =
GetCredentials(credentials_path);
if (!creds_result.ok()) {
return creds_result.status();
}

// Create a KMS stub.
ChannelArguments args;
grpc::ChannelArguments args;
args.SetUserAgentPrefix(
absl::StrCat(kTinkUserAgentPrefix, Version::kTinkVersion, " CPP-Python"));
client->kms_stub_ = KeyManagementService::NewStub(
grpc::CreateCustomChannel(kGcpKmsServer, creds_result.value(), args));
return std::move(client);
absl::StrCat(kTinkUserAgentPrefix, Version::kTinkVersion, " CPP"));
std::shared_ptr<KeyManagementService::Stub> kms_stub =
KeyManagementService::NewStub(grpc::CreateCustomChannel(
std::string(kGcpKmsServer), creds_result.value(), args));
return absl::WrapUnique(new GcpKmsClient(key_name, std::move(kms_stub)));
}

bool GcpKmsClient::DoesSupport(absl::string_view key_uri) const {
if (!key_name_.empty()) {
return key_name_ == GetKeyName(key_uri);
util::StatusOr<std::string> key_name = GetKeyName(key_uri);
if (!key_name.ok()) {
return false;
}
return !GetKeyName(key_uri).empty();
return key_name_.empty() ? true : key_name_ == *key_name;
}

StatusOr<std::unique_ptr<Aead>> GcpKmsClient::GetAead(
util::StatusOr<std::unique_ptr<Aead>> GcpKmsClient::GetAead(
absl::string_view key_uri) const {
if (!DoesSupport(key_uri)) {
if (!key_name_.empty()) {
return Status(absl::StatusCode::kInvalidArgument,
absl::StrCat("This client is bound to ", key_name_,
" and cannot use key ", key_uri));
} else {
return Status(absl::StatusCode::kInvalidArgument,
absl::StrCat("This client does not support key ", key_uri));
}
util::StatusOr<std::string> key_name_from_key_uri = GetKeyName(key_uri);
// key_uri is invalid.
if (!key_name_from_key_uri.ok()) {
return key_name_from_key_uri.status();
}
if (!key_name_.empty()) { // This client is bound to a specific key.
return GcpKmsAead::New(key_name_, kms_stub_);
} else { // Create an GCP KMSClient for the given key.
auto key_name = GetKeyName(key_uri);
return GcpKmsAead::New(key_name, kms_stub_);
// key_uri is valid, but if key_name_ is not empty key_name_from_key_uri must
// be equal to key_name_.
if (!key_name_.empty() && key_name_ != *key_name_from_key_uri) {
return util::Status(absl::StatusCode::kInvalidArgument,
absl::StrCat("This client is bound to ", key_name_,
" and cannot use key ", key_uri));
}
return GcpKmsAead::New(*key_name_from_key_uri, kms_stub_);
}

Status GcpKmsClient::RegisterNewClient(absl::string_view key_uri,
absl::string_view credentials_path) {
util::Status GcpKmsClient::RegisterNewClient(
absl::string_view key_uri, absl::string_view credentials_path) {
auto client_result = GcpKmsClient::New(key_uri, credentials_path);
if (!client_result.ok()) {
return client_result.status();
}

return KmsClients::Add(std::move(client_result.value()));
}

Expand Down
45 changes: 27 additions & 18 deletions tink/integration/gcpkms/gcp_kms_client.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include <memory>
#include <string>
#include <utility>

#include "google/cloud/kms/v1/service.grpc.pb.h"
#include "grpcpp/channel.h"
Expand All @@ -33,40 +34,48 @@ namespace tink {
namespace integration {
namespace gcpkms {


// GcpKmsClient is an implementation of KmsClient for
// <a href="https://cloud.google.com/kms/">Google Cloud KMS</a>.
class GcpKmsClient : public crypto::tink::KmsClient {
// GcpKmsClient is an implementation of KmsClient for Google Cloud KMS
// (https://cloud.google.com/kms/).
class GcpKmsClient : public crypto::tink::KmsClient {
public:
// Creates a new GcpKmsClient that is bound to the key specified in 'key_uri',
// and that uses the specifed credentials when communicating with the KMS.
// Move only.
GcpKmsClient(GcpKmsClient&& other) = default;
GcpKmsClient& operator=(GcpKmsClient&& other) = default;
GcpKmsClient(const GcpKmsClient&) = delete;
GcpKmsClient& operator=(const GcpKmsClient&) = delete;

// Creates a new GcpKmsClient that is bound to the key specified in `key_uri`,
// and that uses the specified credentials when communicating with the KMS.
//
// Either of arguments can be empty.
// If 'key_uri' is empty, then the client is not bound to any particular key.
// If 'credential_path' is empty, then default credentials will be used.
static crypto::tink::util::StatusOr<std::unique_ptr<GcpKmsClient>>
New(absl::string_view key_uri, absl::string_view credentials_path);
// Either argument can be empty.
// If `key_uri` is empty, then the client is not bound to any particular key.
// If `credential_path` is empty, then default credentials will be used.
static crypto::tink::util::StatusOr<std::unique_ptr<GcpKmsClient>> New(
absl::string_view key_uri, absl::string_view credentials_path);

// Creates a new client and registers it in KMSClients.
static crypto::tink::util::Status RegisterNewClient(
absl::string_view key_uri, absl::string_view credentials_path);

// Returns true iff this client does support KMS key specified by 'key_uri'.
// Returns true iff this client does support KMS key specified by `key_uri`.
bool DoesSupport(absl::string_view key_uri) const override;

// Returns an Aead-primitive backed by KMS key specified by 'key_uri',
// provided that this KmsClient does support 'key_uri'.
crypto::tink::util::StatusOr<std::unique_ptr<Aead>>
GetAead(absl::string_view key_uri) const override;
// Returns an Aead-primitive backed by KMS key specified by `key_uri`,
// provided that this KmsClient does support `key_uri`.
crypto::tink::util::StatusOr<std::unique_ptr<Aead>> GetAead(
absl::string_view key_uri) const override;

private:
GcpKmsClient() {}
explicit GcpKmsClient(
std::string key_name,
std::shared_ptr<google::cloud::kms::v1::KeyManagementService::Stub>
kms_stub)
: key_name_(key_name), kms_stub_(std::move(kms_stub)) {}

std::string key_name_;
std::shared_ptr<google::cloud::kms::v1::KeyManagementService::Stub> kms_stub_;
};


} // namespace gcpkms
} // namespace integration
} // namespace tink
Expand Down
40 changes: 20 additions & 20 deletions tink/integration/gcpkms/gcp_kms_client_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,14 @@
#include "tink/util/test_matchers.h"
#include "tink/util/test_util.h"

using ::crypto::tink::test::IsOk;
using ::crypto::tink::test::StatusIs;

namespace crypto {
namespace tink {
namespace integration {
namespace gcpkms {
namespace {

using crypto::tink::integration::gcpkms::GcpKmsClient;
using ::crypto::tink::test::IsOk;
using ::crypto::tink::test::StatusIs;

TEST(GcpKmsClientTest, ClientNotBoundToAKey) {
std::string gcp_key1 = "gcp-kms://projects/someProject/.../cryptoKeys/key1";
Expand All @@ -47,12 +45,12 @@ TEST(GcpKmsClientTest, ClientNotBoundToAKey) {
std::string creds_file =
std::string(getenv("TEST_SRCDIR")) + "/tink_cc_gcpkms/testdata/gcp/credential.json";

auto client_result = GcpKmsClient::New("", creds_file);
EXPECT_TRUE(client_result.ok()) << client_result.status();
auto client = std::move(client_result.value());
EXPECT_TRUE(client->DoesSupport(gcp_key1));
EXPECT_TRUE(client->DoesSupport(gcp_key2));
EXPECT_FALSE(client->DoesSupport(non_gcp_key));
util::StatusOr<std::unique_ptr<GcpKmsClient>> client =
GcpKmsClient::New("", creds_file);
ASSERT_THAT(client, IsOk());
EXPECT_TRUE((*client)->DoesSupport(gcp_key1));
EXPECT_TRUE((*client)->DoesSupport(gcp_key2));
EXPECT_FALSE((*client)->DoesSupport(non_gcp_key));
}

TEST(GcpKmsClientTest, ClientBoundToASpecificKey) {
Expand All @@ -62,23 +60,24 @@ TEST(GcpKmsClientTest, ClientBoundToASpecificKey) {
std::string creds_file =
std::string(getenv("TEST_SRCDIR")) + "/tink_cc_gcpkms/testdata/gcp/credential.json";

auto client_result = GcpKmsClient::New(gcp_key1, creds_file);
EXPECT_TRUE(client_result.ok()) << client_result.status();
auto client = std::move(client_result.value());
EXPECT_TRUE(client->DoesSupport(gcp_key1));
EXPECT_FALSE(client->DoesSupport(gcp_key2));
EXPECT_FALSE(client->DoesSupport(non_gcp_key));
util::StatusOr<std::unique_ptr<GcpKmsClient>> client =
GcpKmsClient::New(gcp_key1, creds_file);
ASSERT_THAT(client, IsOk());
EXPECT_TRUE((*client)->DoesSupport(gcp_key1));
EXPECT_FALSE((*client)->DoesSupport(gcp_key2));
EXPECT_FALSE((*client)->DoesSupport(non_gcp_key));
}

TEST(GcpKmsClientTest, ClientCreationAndRegistry) {
std::string gcp_key1 = "gcp-kms://projects/someProject/.../cryptoKeys/key1";
std::string creds_file =
absl::StrCat(getenv("TEST_SRCDIR"), "/tink_cc_gcpkms/testdata/gcp/credential.json");

auto client_result = GcpKmsClient::RegisterNewClient(gcp_key1, creds_file);
EXPECT_THAT(client_result, IsOk());
util::Status client_result =
GcpKmsClient::RegisterNewClient(gcp_key1, creds_file);
ASSERT_THAT(client_result, IsOk());

auto registry_result = KmsClients::Get(gcp_key1);
util::StatusOr<const KmsClient*> registry_result = KmsClients::Get(gcp_key1);
EXPECT_THAT(registry_result, IsOk());
}

Expand All @@ -87,7 +86,8 @@ TEST(GcpKmsClientTest, ClientCreationInvalidRegistry) {
std::string creds_file =
std::string(getenv("TEST_SRCDIR")) + "/tink_cc_gcpkms/testdata/gcp/credential.json";

auto client_result = GcpKmsClient::RegisterNewClient(non_gcp_key, creds_file);
util::Status client_result =
GcpKmsClient::RegisterNewClient(non_gcp_key, creds_file);
EXPECT_THAT(client_result, StatusIs(absl::StatusCode::kInvalidArgument));
}

Expand Down

0 comments on commit e0658e4

Please sign in to comment.