From 5cfff04ce9c1c52bf3b0f1acfbff32670c526a65 Mon Sep 17 00:00:00 2001 From: Matthew Stevenson Date: Wed, 22 Jan 2020 13:06:22 -0800 Subject: [PATCH] Collect TLS-specific changes from PR 2-568. --- .../grpcpp/security/tls_credentials_options.h | 53 +++++++--------- src/cpp/common/tls_credentials_options.cc | 62 ++++++++++++++----- test/cpp/client/credentials_test.cc | 47 ++++++++++---- 3 files changed, 105 insertions(+), 57 deletions(-) diff --git a/include/grpcpp/security/tls_credentials_options.h b/include/grpcpp/security/tls_credentials_options.h index b5bb7c78b7f..d83226865d8 100644 --- a/include/grpcpp/security/tls_credentials_options.h +++ b/include/grpcpp/security/tls_credentials_options.h @@ -55,12 +55,13 @@ class TlsKeyMaterialsConfig { } int version() const { return version_; } - /** Setter for key materials that will be called by the user. The setter - * transfers ownership of the arguments to the config. **/ - void set_pem_root_certs(grpc::string pem_root_certs); + /** Setter for key materials that will be called by the user. Ownership of the + * arguments will not be transferred. **/ + void set_pem_root_certs(const grpc::string& pem_root_certs); void add_pem_key_cert_pair(const PemKeyCertPair& pem_key_cert_pair); - void set_key_materials(grpc::string pem_root_certs, - std::vector pem_key_cert_pair_list); + void set_key_materials( + const grpc::string& pem_root_certs, + const std::vector& pem_key_cert_pair_list); void set_version(int version) { version_ = version; }; private: @@ -70,40 +71,36 @@ class TlsKeyMaterialsConfig { }; /** TLS credential reload arguments, wraps grpc_tls_credential_reload_arg. It is - * used for experimental purposes for now and it is subject to change. + * used for experimental purposes for now and it is subject to change. * - * The credential reload arg contains all the info necessary to schedule/cancel - * a credential reload request. The callback function must be called after - * finishing the schedule operation. See the description of the - * grpc_tls_credential_reload_arg struct in grpc_security.h for more details. + * The credential reload arg contains all the info necessary to schedule/cancel + * a credential reload request. The callback function must be called after + * finishing the schedule operation. See the description of the + * grpc_tls_credential_reload_arg struct in grpc_security.h for more details. * **/ class TlsCredentialReloadArg { public: /** TlsCredentialReloadArg does not take ownership of the C arg that is passed - * to the constructor. One must remember to free any memory allocated to the C - * arg after using the setter functions below. **/ + * to the constructor. One must remember to free any memory allocated to the + * C arg after using the setter functions below. **/ TlsCredentialReloadArg(grpc_tls_credential_reload_arg* arg); ~TlsCredentialReloadArg(); - /** Getters for member fields. The callback function is not exposed. - * They return the corresponding fields of the underlying C arg. In the case - * of the key materials config, it creates a new instance of the C++ key - * materials config from the underlying C grpc_tls_key_materials_config. **/ + /** Getters for member fields. **/ void* cb_user_data() const; bool is_pem_key_cert_pair_list_empty() const; grpc_ssl_certificate_config_reload_status status() const; grpc::string error_details() const; - /** Setters for member fields. They modify the fields of the underlying C arg. - * The setters for the key_materials_config and the error_details allocate - * memory when modifying c_arg_, so one must remember to free c_arg_'s - * original key_materials_config or error_details after using the appropriate - * setter function. - * **/ + /** Setters for member fields. Ownership of the arguments will not be + * transferred. **/ void set_cb_user_data(void* cb_user_data); void set_pem_root_certs(const grpc::string& pem_root_certs); void add_pem_key_cert_pair( - TlsKeyMaterialsConfig::PemKeyCertPair pem_key_cert_pair); + const TlsKeyMaterialsConfig::PemKeyCertPair& pem_key_cert_pair); + void set_key_materials(const grpc::string& pem_root_certs, + std::vector + pem_key_cert_pair_list); void set_key_materials_config( const std::shared_ptr& key_materials_config); void set_status(grpc_ssl_certificate_config_reload_status status); @@ -187,8 +184,7 @@ class TlsServerAuthorizationCheckArg { TlsServerAuthorizationCheckArg(grpc_tls_server_authorization_check_arg* arg); ~TlsServerAuthorizationCheckArg(); - /** Getters for member fields. They return the corresponding fields of the - * underlying C arg.**/ + /** Getters for member fields. **/ void* cb_user_data() const; int success() const; grpc::string target_name() const; @@ -197,12 +193,7 @@ class TlsServerAuthorizationCheckArg { grpc_status_code status() const; grpc::string error_details() const; - /** Setters for member fields. They modify the fields of the underlying C arg. - * The setters for target_name, peer_cert, and error_details allocate memory - * when modifying c_arg_, so one must remember to free c_arg_'s original - * target_name, peer_cert, or error_details after using the appropriate setter - * function. - * **/ + /** Setters for member fields. **/ void set_cb_user_data(void* cb_user_data); void set_success(int success); void set_target_name(const grpc::string& target_name); diff --git a/src/cpp/common/tls_credentials_options.cc b/src/cpp/common/tls_credentials_options.cc index d5692c6effd..1c65ea66d89 100644 --- a/src/cpp/common/tls_credentials_options.cc +++ b/src/cpp/common/tls_credentials_options.cc @@ -16,19 +16,18 @@ * */ +#include #include #include "src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h" - -#include - #include "src/cpp/common/tls_credentials_options_util.h" namespace grpc_impl { namespace experimental { /** TLS key materials config API implementation **/ -void TlsKeyMaterialsConfig::set_pem_root_certs(grpc::string pem_root_certs) { - pem_root_certs_ = std::move(pem_root_certs); +void TlsKeyMaterialsConfig::set_pem_root_certs( + const grpc::string& pem_root_certs) { + pem_root_certs_ = pem_root_certs; } void TlsKeyMaterialsConfig::add_pem_key_cert_pair( @@ -37,10 +36,10 @@ void TlsKeyMaterialsConfig::add_pem_key_cert_pair( } void TlsKeyMaterialsConfig::set_key_materials( - grpc::string pem_root_certs, - std::vector pem_key_cert_pair_list) { - pem_key_cert_pair_list_ = std::move(pem_key_cert_pair_list); - pem_root_certs_ = std::move(pem_root_certs); + const grpc::string& pem_root_certs, + const std::vector& pem_key_cert_pair_list) { + pem_key_cert_pair_list_ = pem_key_cert_pair_list; + pem_root_certs_ = pem_root_certs; } /** TLS credential reload arg API implementation **/ @@ -59,7 +58,6 @@ TlsCredentialReloadArg::~TlsCredentialReloadArg() {} void* TlsCredentialReloadArg::cb_user_data() const { return c_arg_->cb_user_data; } - bool TlsCredentialReloadArg::is_pem_key_cert_pair_list_empty() const { return c_arg_->key_materials_config->pem_key_cert_pair_list().empty(); } @@ -85,17 +83,46 @@ void TlsCredentialReloadArg::set_pem_root_certs( c_arg_->key_materials_config->set_pem_root_certs(std::move(c_pem_root_certs)); } -void TlsCredentialReloadArg::add_pem_key_cert_pair( - TlsKeyMaterialsConfig::PemKeyCertPair pem_key_cert_pair) { +namespace { + +::grpc_core::PemKeyCertPair ConvertToCorePemKeyCertPair( + const TlsKeyMaterialsConfig::PemKeyCertPair& pem_key_cert_pair) { grpc_ssl_pem_key_cert_pair* ssl_pair = (grpc_ssl_pem_key_cert_pair*)gpr_malloc( sizeof(grpc_ssl_pem_key_cert_pair)); ssl_pair->private_key = gpr_strdup(pem_key_cert_pair.private_key.c_str()); ssl_pair->cert_chain = gpr_strdup(pem_key_cert_pair.cert_chain.c_str()); - ::grpc_core::PemKeyCertPair c_pem_key_cert_pair = - ::grpc_core::PemKeyCertPair(ssl_pair); + return ::grpc_core::PemKeyCertPair(ssl_pair); +} + +} // namespace + +void TlsCredentialReloadArg::add_pem_key_cert_pair( + const TlsKeyMaterialsConfig::PemKeyCertPair& pem_key_cert_pair) { c_arg_->key_materials_config->add_pem_key_cert_pair( - std::move(c_pem_key_cert_pair)); + ConvertToCorePemKeyCertPair(pem_key_cert_pair)); +} + +void TlsCredentialReloadArg::set_key_materials( + const grpc::string& pem_root_certs, + std::vector pem_key_cert_pair_list) { + /** Initialize the |key_materials_config| field of |c_arg_|, if it has not + * already been done. **/ + if (c_arg_->key_materials_config == nullptr) { + c_arg_->key_materials_config = grpc_tls_key_materials_config_create(); + } + /** Convert |pem_key_cert_pair_list| to an inlined vector of ssl pairs. **/ + ::grpc_core::InlinedVector<::grpc_core::PemKeyCertPair, 1> + c_pem_key_cert_pair_list; + for (const auto& key_cert_pair : pem_key_cert_pair_list) { + c_pem_key_cert_pair_list.emplace_back( + ConvertToCorePemKeyCertPair(key_cert_pair)); + } + /** Populate the key materials config field of |c_arg_|. **/ + ::grpc_core::UniquePtr c_pem_root_certs( + gpr_strdup(pem_root_certs.c_str())); + c_arg_->key_materials_config->set_key_materials(std::move(c_pem_root_certs), + c_pem_key_cert_pair_list); } void TlsCredentialReloadArg::set_key_materials_config( @@ -288,6 +315,11 @@ TlsCredentialsOptions::TlsCredentialsOptions( c_credentials_options_, server_verification_option); } +/** Whenever a TlsCredentialsOptions instance is created, the caller takes + * ownership of the c_credentials_options_ pointer (see e.g. the implementation + * of the TlsCredentials API in secure_credentials.cc). For this reason, the + * TlsCredentialsOptions destructor is not responsible for freeing + * c_credentials_options_. **/ TlsCredentialsOptions::~TlsCredentialsOptions() {} } // namespace experimental diff --git a/test/cpp/client/credentials_test.cc b/test/cpp/client/credentials_test.cc index 3cc0ebfb8e9..8d4922e961e 100644 --- a/test/cpp/client/credentials_test.cc +++ b/test/cpp/client/credentials_test.cc @@ -17,6 +17,7 @@ */ #include +#include #include #include @@ -53,10 +54,10 @@ static void tls_credential_reload_callback( class TestTlsCredentialReload : public TlsCredentialReloadInterface { int Schedule(TlsCredentialReloadArg* arg) override { GPR_ASSERT(arg != nullptr); - struct TlsKeyMaterialsConfig::PemKeyCertPair pair3 = {"private_key3", - "cert_chain3"}; + TlsKeyMaterialsConfig::PemKeyCertPair pair = {"private_key3", + "cert_chain3"}; arg->set_pem_root_certs("new_pem_root_certs"); - arg->add_pem_key_cert_pair(pair3); + arg->add_pem_key_cert_pair(pair); arg->set_status(GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_NEW); return 0; } @@ -100,7 +101,6 @@ class TestTlsServerAuthorizationCheck arg->set_error_details("cancelled"); } }; - } // namespace namespace grpc { @@ -293,8 +293,7 @@ TEST_F(CredentialsTest, TlsKeyMaterialsConfigCppToC) { TEST_F(CredentialsTest, TlsKeyMaterialsModifiers) { std::shared_ptr config(new TlsKeyMaterialsConfig()); - struct TlsKeyMaterialsConfig::PemKeyCertPair pair = {"private_key", - "cert_chain"}; + TlsKeyMaterialsConfig::PemKeyCertPair pair = {"private_key", "cert_chain"}; config->add_pem_key_cert_pair(pair); config->set_pem_root_certs("pem_root_certs"); EXPECT_STREQ(config->pem_root_certs().c_str(), "pem_root_certs"); @@ -312,15 +311,28 @@ typedef class ::grpc_impl::experimental::TlsCredentialReloadConfig TEST_F(CredentialsTest, TlsCredentialReloadArgCallback) { grpc_tls_credential_reload_arg* c_arg = new grpc_tls_credential_reload_arg; + c_arg->key_materials_config = grpc_tls_key_materials_config_create(); c_arg->cb = tls_credential_reload_callback; c_arg->context = nullptr; TlsCredentialReloadArg* arg = new TlsCredentialReloadArg(c_arg); + arg->set_pem_root_certs("pem_root_certs"); + TlsKeyMaterialsConfig::PemKeyCertPair pair = {"private_key", "cert_chain"}; + arg->add_pem_key_cert_pair(pair); arg->set_status(GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_NEW); arg->OnCredentialReloadDoneCallback(); EXPECT_EQ(arg->status(), GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_UNCHANGED); + EXPECT_STREQ(c_arg->key_materials_config->pem_root_certs(), "pem_root_certs"); + EXPECT_EQ(c_arg->key_materials_config->pem_key_cert_pair_list().size(), 1); + EXPECT_STREQ( + c_arg->key_materials_config->pem_key_cert_pair_list()[0].private_key(), + "private_key"); + EXPECT_STREQ( + c_arg->key_materials_config->pem_key_cert_pair_list()[0].cert_chain(), + "cert_chain"); // Cleanup. delete arg; + delete c_arg->key_materials_config; delete c_arg; } @@ -332,15 +344,12 @@ TEST_F(CredentialsTest, TlsCredentialReloadConfigSchedule) { grpc_tls_credential_reload_arg* c_arg = new grpc_tls_credential_reload_arg(); c_arg->context = nullptr; TlsCredentialReloadArg* arg = new TlsCredentialReloadArg(c_arg); - std::shared_ptr key_materials_config( - new TlsKeyMaterialsConfig()); struct TlsKeyMaterialsConfig::PemKeyCertPair pair1 = {"private_key1", "cert_chain1"}; struct TlsKeyMaterialsConfig::PemKeyCertPair pair2 = {"private_key2", "cert_chain2"}; std::vector pair_list = {pair1, pair2}; - key_materials_config->set_key_materials("pem_root_certs", pair_list); - arg->set_key_materials_config(key_materials_config); + arg->set_key_materials("pem_root_certs", pair_list); arg->set_status(GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_NEW); arg->set_error_details("error_details"); const char* error_details_before_schedule = c_arg->error_details; @@ -648,7 +657,7 @@ TEST_F(CredentialsTest, TlsCredentialsOptionsCppToC) { delete c_options; } -// This test demonstrates how the SPIFFE credentials will be used. +// This test demonstrates how the TLS credentials will be used. TEST_F(CredentialsTest, LoadTlsChannelCredentials) { std::shared_ptr test_credential_reload( new TestTlsCredentialReload()); @@ -670,6 +679,22 @@ TEST_F(CredentialsTest, LoadTlsChannelCredentials) { GPR_ASSERT(channel_credentials != nullptr); } +// This test demonstrates how the TLS credentials will be used to create +// server credentials. +TEST_F(CredentialsTest, LoadTlsServerCredentials) { + std::shared_ptr test_credential_reload( + new TestTlsCredentialReload()); + std::shared_ptr credential_reload_config( + new TlsCredentialReloadConfig(test_credential_reload)); + + TlsCredentialsOptions options = TlsCredentialsOptions( + GRPC_SSL_REQUEST_CLIENT_CERTIFICATE_AND_VERIFY, + GRPC_TLS_SERVER_VERIFICATION, nullptr, credential_reload_config, nullptr); + std::shared_ptr<::grpc_impl::ServerCredentials> server_credentials = + grpc::experimental::TlsServerCredentials(options); + GPR_ASSERT(server_credentials != nullptr); +} + TEST_F(CredentialsTest, TlsCredentialReloadConfigErrorMessages) { std::shared_ptr config( new TlsCredentialReloadConfig(nullptr));