From c942282abc1592997ccb344925a8d76be61ba826 Mon Sep 17 00:00:00 2001 From: Matthew Stevenson Date: Tue, 27 Aug 2019 14:57:02 -0700 Subject: [PATCH] Implemented Yihua's review comments --- .../grpcpp/security/tls_credentials_options.h | 47 ++---- src/cpp/common/tls_credentials_options.cc | 156 ++++++------------ .../common/tls_credentials_options_util.cc | 51 ++---- test/cpp/client/credentials_test.cc | 95 ++++++----- 4 files changed, 135 insertions(+), 214 deletions(-) diff --git a/include/grpcpp/security/tls_credentials_options.h b/include/grpcpp/security/tls_credentials_options.h index 87c97dbd217..f5c0545ea0c 100644 --- a/include/grpcpp/security/tls_credentials_options.h +++ b/include/grpcpp/security/tls_credentials_options.h @@ -59,7 +59,7 @@ class TlsKeyMaterialsConfig { /** TLS credential reload arguments, wraps grpc_tls_credential_reload_arg. **/ class TlsCredentialReloadArg { public: - TlsCredentialReloadArg(grpc_tls_credential_reload_arg arg); + TlsCredentialReloadArg(grpc_tls_credential_reload_arg* arg) : c_arg_(arg) { } ~TlsCredentialReloadArg(); /** Getters for member fields. The callback function is not exposed. @@ -83,12 +83,7 @@ class TlsCredentialReloadArg { void OnCredentialReloadDoneCallback(); private: - grpc_tls_credential_reload_arg c_arg_; - /** These boolean variables record whether the corresponding field of the C - * arg was dynamically allocated. This occurs e.g. if one of the above setter functions was - * used, or if the C arg's callback function does so. **/ - bool key_materials_config_alloc_ = false; - bool error_details_alloc_ = false; + grpc_tls_credential_reload_arg* c_arg_; }; /** TLS credential reloag config, wraps grpc_tls_credential_reload_config. **/ @@ -134,7 +129,7 @@ class TlsCredentialReloadConfig { class TlsServerAuthorizationCheckArg { public: - TlsServerAuthorizationCheckArg(grpc_tls_server_authorization_check_arg arg); + TlsServerAuthorizationCheckArg(grpc_tls_server_authorization_check_arg* arg) : c_arg_(arg) { } ~TlsServerAuthorizationCheckArg(); /** Getters for member fields. They return the corresponding fields of the @@ -159,13 +154,7 @@ class TlsServerAuthorizationCheckArg { void OnServerAuthorizationCheckDoneCallback(); private: - grpc_tls_server_authorization_check_arg c_arg_; - /** These boolean variables record whether the corresponding field of the C - * arg was dynamically allocated. This occurs e.g. if one of the above setter functions was - * used, or if the C arg's callback function does so. **/ - bool target_name_alloc_ = false; - bool peer_cert_alloc_ = false; - bool error_details_alloc_ = false; + grpc_tls_server_authorization_check_arg* c_arg_; }; /** TLS server authorization check config, wraps @@ -213,6 +202,12 @@ class TlsServerAuthorizationCheckConfig { /** TLS credentials options, wrapper for grpc_tls_credentials_options. **/ class TlsCredentialsOptions { public: + TlsCredentialsOptions(grpc_ssl_client_certificate_request_type cert_request_type, + std::shared_ptr key_materials_config, + std::shared_ptr credential_reload_config, + std::shared_ptr server_authorization_check_config); + ~TlsCredentialsOptions(); + /** Getters for member fields. **/ grpc_ssl_client_certificate_request_type cert_request_type() const { return cert_request_type_; @@ -227,26 +222,9 @@ class TlsCredentialsOptions { server_authorization_check_config() const { return server_authorization_check_config_; } - - /** Setters for member fields. **/ - void set_cert_request_type( - const grpc_ssl_client_certificate_request_type type) { - cert_request_type_ = type; - } - void set_key_materials_config(std::shared_ptr config) { - key_materials_config_ = std::move(config); + grpc_tls_credentials_options* c_credentials_options() const { + return c_credentials_options_; } - void set_credential_reload_config( - std::shared_ptr config) { - credential_reload_config_ = std::move(config); - } - void set_server_authorization_check_config( - std::shared_ptr config) { - server_authorization_check_config_ = std::move(config); - } - - /** Creates C struct for TLS credential options. **/ - grpc_tls_credentials_options* c_credentials_options() const; private: grpc_ssl_client_certificate_request_type cert_request_type_; @@ -254,6 +232,7 @@ class TlsCredentialsOptions { std::shared_ptr credential_reload_config_; std::shared_ptr server_authorization_check_config_; + grpc_tls_credentials_options* c_credentials_options_; }; } // namespace experimental diff --git a/src/cpp/common/tls_credentials_options.cc b/src/cpp/common/tls_credentials_options.cc index 5a446cd0354..eb9f8e6fe98 100644 --- a/src/cpp/common/tls_credentials_options.cc +++ b/src/cpp/common/tls_credentials_options.cc @@ -21,21 +21,6 @@ #include "src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h" #include "src/cpp/common/tls_credentials_options_util.h" -namespace { - /** This function returns True and frees old_string if old_string and - * new_string are different, and frees the memory allocated to old_string if it was dynamically allocated. - * Otherwise, it whether or not the old_string was dynamically allocated. **/ - bool cleanup_string(const char* old_string, const char* new_string, bool old_string_alloc) { - if (old_string != new_string) { - if (old_string_alloc) { - gpr_free(const_cast(old_string)); - } - return true; - } - return old_string_alloc; - } -} - namespace grpc_impl { namespace experimental { @@ -48,18 +33,10 @@ void TlsKeyMaterialsConfig::set_key_materials( } /** TLS credential reload arg API implementation **/ -TlsCredentialReloadArg::TlsCredentialReloadArg( - grpc_tls_credential_reload_arg arg) { - c_arg_ = arg; -} - -TlsCredentialReloadArg::~TlsCredentialReloadArg() { - if (key_materials_config_alloc_) { gpr_free(c_arg_.key_materials_config); } - if (error_details_alloc_) { gpr_free(const_cast(c_arg_.error_details)); } -} +TlsCredentialReloadArg::~TlsCredentialReloadArg() { } void* TlsCredentialReloadArg::cb_user_data() const { - return c_arg_.cb_user_data; + return c_arg_->cb_user_data; } /** This function creates a new TlsKeyMaterialsConfig instance whose fields are @@ -67,51 +44,41 @@ void* TlsCredentialReloadArg::cb_user_data() const { * TlsCredentialReloadArg instance. **/ std::shared_ptr TlsCredentialReloadArg::key_materials_config() const { - return ConvertToCppKeyMaterialsConfig(c_arg_.key_materials_config); + return ConvertToCppKeyMaterialsConfig(c_arg_->key_materials_config); } grpc_ssl_certificate_config_reload_status TlsCredentialReloadArg::status() const { - return c_arg_.status; + return c_arg_->status; } grpc::string TlsCredentialReloadArg::error_details() const { - grpc::string cpp_error_details(c_arg_.error_details); + grpc::string cpp_error_details(c_arg_->error_details); return cpp_error_details; } void TlsCredentialReloadArg::set_cb_user_data(void* cb_user_data) { - c_arg_.cb_user_data = cb_user_data; + c_arg_->cb_user_data = cb_user_data; } void TlsCredentialReloadArg::set_key_materials_config( const std::shared_ptr& key_materials_config) { - if (key_materials_config_alloc_) { - gpr_free(c_arg_.key_materials_config); - } - c_arg_.key_materials_config = + c_arg_->key_materials_config = ConvertToCKeyMaterialsConfig(key_materials_config); - key_materials_config_alloc_ = true; } void TlsCredentialReloadArg::set_status( grpc_ssl_certificate_config_reload_status status) { - c_arg_.status = status; + c_arg_->status = status; } void TlsCredentialReloadArg::set_error_details( const grpc::string& error_details) { - if (error_details_alloc_) { - gpr_free(const_cast(c_arg_.error_details)); - } - c_arg_.error_details = gpr_strdup(error_details.c_str()); - error_details_alloc_ = true; + c_arg_->error_details = gpr_strdup(error_details.c_str()); } void TlsCredentialReloadArg::OnCredentialReloadDoneCallback() { - const char* error_details = c_arg_.error_details; - c_arg_.cb(&c_arg_); - error_details_alloc_ = cleanup_string(error_details, c_arg_.error_details, error_details_alloc_); + c_arg_->cb(c_arg_); } /** gRPC TLS credential reload config API implementation **/ @@ -119,11 +86,10 @@ TlsCredentialReloadConfig::TlsCredentialReloadConfig( const void* config_user_data, int (*schedule)(void* config_user_data, TlsCredentialReloadArg* arg), void (*cancel)(void* config_user_data, TlsCredentialReloadArg* arg), - void (*destruct)(void* config_user_data)) { - config_user_data_ = const_cast(config_user_data); - schedule_ = schedule; - cancel_ = cancel; - destruct_ = destruct; + void (*destruct)(void* config_user_data)) : config_user_data_(const_cast(config_user_data)), + schedule_(schedule), + cancel_(cancel), + destruct_(destruct) { c_config_ = grpc_tls_credential_reload_config_create( config_user_data_, &TlsCredentialReloadConfigCSchedule, &TlsCredentialReloadConfigCCancel, destruct_); @@ -135,89 +101,63 @@ TlsCredentialReloadConfig::~TlsCredentialReloadConfig() { } /** gRPC TLS server authorization check arg API implementation **/ -TlsServerAuthorizationCheckArg::TlsServerAuthorizationCheckArg( - grpc_tls_server_authorization_check_arg arg) { - c_arg_ = arg; -} - TlsServerAuthorizationCheckArg::~TlsServerAuthorizationCheckArg() { - if (target_name_alloc_) { gpr_free(const_cast(c_arg_.target_name)); } - if (peer_cert_alloc_) { gpr_free(const_cast(c_arg_.peer_cert)); } - if (error_details_alloc_) { gpr_free(const_cast(c_arg_.error_details)); } } void* TlsServerAuthorizationCheckArg::cb_user_data() const { - return c_arg_.cb_user_data; + return c_arg_->cb_user_data; } -int TlsServerAuthorizationCheckArg::success() const { return c_arg_.success; } +int TlsServerAuthorizationCheckArg::success() const { return c_arg_->success; } grpc::string TlsServerAuthorizationCheckArg::target_name() const { - grpc::string cpp_target_name(c_arg_.target_name); + grpc::string cpp_target_name(c_arg_->target_name); return cpp_target_name; } grpc::string TlsServerAuthorizationCheckArg::peer_cert() const { - grpc::string cpp_peer_cert(c_arg_.peer_cert); + grpc::string cpp_peer_cert(c_arg_->peer_cert); return cpp_peer_cert; } grpc_status_code TlsServerAuthorizationCheckArg::status() const { - return c_arg_.status; + return c_arg_->status; } grpc::string TlsServerAuthorizationCheckArg::error_details() const { - grpc::string cpp_error_details(c_arg_.error_details); + grpc::string cpp_error_details(c_arg_->error_details); return cpp_error_details; } void TlsServerAuthorizationCheckArg::set_cb_user_data(void* cb_user_data) { - c_arg_.cb_user_data = cb_user_data; + c_arg_->cb_user_data = cb_user_data; } void TlsServerAuthorizationCheckArg::set_success(int success) { - c_arg_.success = success; + c_arg_->success = success; } void TlsServerAuthorizationCheckArg::set_target_name( const grpc::string& target_name) { - if (target_name_alloc_) { - gpr_free(const_cast(c_arg_.target_name)); - } - c_arg_.target_name = gpr_strdup(target_name.c_str()); - target_name_alloc_ = true; + c_arg_->target_name = gpr_strdup(target_name.c_str()); } void TlsServerAuthorizationCheckArg::set_peer_cert( const grpc::string& peer_cert) { - if (peer_cert_alloc_) { - gpr_free(const_cast(c_arg_.peer_cert)); - } - c_arg_.peer_cert = gpr_strdup(peer_cert.c_str()); - peer_cert_alloc_ = true; + c_arg_->peer_cert = gpr_strdup(peer_cert.c_str()); } void TlsServerAuthorizationCheckArg::set_status(grpc_status_code status) { - c_arg_.status = status; + c_arg_->status = status; } void TlsServerAuthorizationCheckArg::set_error_details( const grpc::string& error_details) { - if (error_details_alloc_) { - gpr_free(const_cast(c_arg_.error_details)); - } - c_arg_.error_details = gpr_strdup(error_details.c_str()); - error_details_alloc_ = true; + c_arg_->error_details = gpr_strdup(error_details.c_str()); } void TlsServerAuthorizationCheckArg::OnServerAuthorizationCheckDoneCallback() { - const char* target_name = c_arg_.target_name; - const char* peer_cert = c_arg_.peer_cert; - const char* error_details = c_arg_.error_details; - c_arg_.cb(&c_arg_); - target_name_alloc_ = cleanup_string(target_name, c_arg_.target_name, target_name_alloc_); - peer_cert_alloc_ = cleanup_string(peer_cert, c_arg_.peer_cert, peer_cert_alloc_); - error_details_alloc_ = cleanup_string(error_details, c_arg_.error_details, error_details_alloc_); + c_arg_->cb(c_arg_); } /** gRPC TLS server authorization check config API implementation **/ @@ -226,11 +166,10 @@ TlsServerAuthorizationCheckConfig::TlsServerAuthorizationCheckConfig( int (*schedule)(void* config_user_data, TlsServerAuthorizationCheckArg* arg), void (*cancel)(void* config_user_data, TlsServerAuthorizationCheckArg* arg), - void (*destruct)(void* config_user_data)) { - config_user_data_ = const_cast(config_user_data); - schedule_ = schedule; - cancel_ = cancel; - destruct_ = destruct; + void (*destruct)(void* config_user_data)) : config_user_data_(const_cast(config_user_data)), + schedule_(schedule), + cancel_(cancel), + destruct_(destruct) { c_config_ = grpc_tls_server_authorization_check_config_create( config_user_data_, &TlsServerAuthorizationCheckConfigCSchedule, &TlsServerAuthorizationCheckConfigCCancel, destruct_); @@ -242,21 +181,24 @@ TlsServerAuthorizationCheckConfig::~TlsServerAuthorizationCheckConfig() { } /** gRPC TLS credential options API implementation **/ -grpc_tls_credentials_options* TlsCredentialsOptions::c_credentials_options() - const { - grpc_tls_credentials_options* c_options = - grpc_tls_credentials_options_create(); - c_options->set_cert_request_type(cert_request_type_); - c_options->set_key_materials_config( - ::grpc_core::RefCountedPtr( - ConvertToCKeyMaterialsConfig(key_materials_config_))); - c_options->set_credential_reload_config( - ::grpc_core::RefCountedPtr( - credential_reload_config_->c_config())); - c_options->set_server_authorization_check_config( - ::grpc_core::RefCountedPtr( - server_authorization_check_config_->c_config())); - return c_options; +TlsCredentialsOptions::TlsCredentialsOptions(grpc_ssl_client_certificate_request_type cert_request_type, + std::shared_ptr key_materials_config, + std::shared_ptr credential_reload_config, + std::shared_ptr server_authorization_check_config) : + cert_request_type_(cert_request_type), + key_materials_config_(key_materials_config), + credential_reload_config_(credential_reload_config), + server_authorization_check_config_(server_authorization_check_config) { + c_credentials_options_ = grpc_tls_credentials_options_create(); + grpc_tls_credentials_options_set_cert_request_type(c_credentials_options_, cert_request_type_); + grpc_tls_credentials_options_set_key_materials_config(c_credentials_options_, ConvertToCKeyMaterialsConfig(key_materials_config_)); + grpc_tls_credentials_options_set_credential_reload_config(c_credentials_options_, credential_reload_config_->c_config()); + grpc_tls_credentials_options_set_server_authorization_check_config( + c_credentials_options_, server_authorization_check_config_->c_config()); +} + +TlsCredentialsOptions::~TlsCredentialsOptions() { + gpr_free(c_credentials_options_); } } // namespace experimental diff --git a/src/cpp/common/tls_credentials_options_util.cc b/src/cpp/common/tls_credentials_options_util.cc index 14de025c075..bb4538973ec 100644 --- a/src/cpp/common/tls_credentials_options_util.cc +++ b/src/cpp/common/tls_credentials_options_util.cc @@ -22,7 +22,8 @@ namespace grpc_impl { namespace experimental { -/** Creates a new C struct for the key materials. Note that the user must free +/** Converts the Cpp key materials to C key materials; this allocates memory for + * the C key materials. Note that the user must free * the underlying pointer to private key and cert chain duplicates; they are not * freed when the UniquePtr member variables of PemKeyCertPair are unused. * Similarly, the user must free the underlying pointer to c_pem_root_certs. **/ @@ -52,7 +53,8 @@ grpc_tls_key_materials_config* ConvertToCKeyMaterialsConfig( return c_config; } -/** Creates a new TlsKeyMaterialsConfig from a C struct config. **/ +/** Converts the C key materials config to a Cpp key materials config; it + * allocates memory for the Cpp config. **/ std::shared_ptr ConvertToCppKeyMaterialsConfig( const grpc_tls_key_materials_config* config) { std::shared_ptr cpp_config( @@ -63,8 +65,6 @@ std::shared_ptr ConvertToCppKeyMaterialsConfig( for (size_t i = 0; i < pem_key_cert_pair_list.size(); i++) { ::grpc_core::PemKeyCertPair key_cert_pair = pem_key_cert_pair_list[i]; TlsKeyMaterialsConfig::PemKeyCertPair p = { - // gpr_strdup(key_cert_pair.private_key()), - // gpr_strdup(key_cert_pair.cert_chain())}; key_cert_pair.private_key(), key_cert_pair.cert_chain()}; cpp_pem_key_cert_pair_list.push_back(::std::move(p)); } @@ -74,63 +74,42 @@ std::shared_ptr ConvertToCppKeyMaterialsConfig( return cpp_config; } -/** The C schedule and cancel functions for the credential reload config. **/ +/** The C schedule and cancel functions for the credential reload config. + * They populate a C credential reload arg with the result of a C++ credential + * reload schedule/cancel API. **/ int TlsCredentialReloadConfigCSchedule( void* config_user_data, grpc_tls_credential_reload_arg* arg) { TlsCredentialReloadConfig* cpp_config = static_cast(arg->config->context()); - TlsCredentialReloadArg cpp_arg(*arg); - int schedule_output = cpp_config->Schedule(&cpp_arg); - arg->cb_user_data = cpp_arg.cb_user_data(); - arg->key_materials_config = - ConvertToCKeyMaterialsConfig(cpp_arg.key_materials_config()); - arg->status = cpp_arg.status(); - arg->error_details = gpr_strdup(cpp_arg.error_details().c_str()); - return schedule_output; + TlsCredentialReloadArg cpp_arg(arg); + return cpp_config->Schedule(&cpp_arg); } void TlsCredentialReloadConfigCCancel( void* config_user_data, grpc_tls_credential_reload_arg* arg) { TlsCredentialReloadConfig* cpp_config = static_cast(arg->config->context()); - TlsCredentialReloadArg cpp_arg(*arg); + TlsCredentialReloadArg cpp_arg(arg); cpp_config->Cancel(&cpp_arg); - arg->cb_user_data = cpp_arg.cb_user_data(); - arg->key_materials_config = - ConvertToCKeyMaterialsConfig(cpp_arg.key_materials_config()); - arg->status = cpp_arg.status(); - arg->error_details = gpr_strdup(cpp_arg.error_details().c_str()); } /** The C schedule and cancel functions for the server authorization check - * config. **/ + * config. They populate a C server authorization check arg with the result + * of a C++ server authorization check schedule/cancel API. **/ int TlsServerAuthorizationCheckConfigCSchedule( void* config_user_data, grpc_tls_server_authorization_check_arg* arg) { TlsServerAuthorizationCheckConfig* cpp_config = static_cast(arg->config->context()); - TlsServerAuthorizationCheckArg cpp_arg(*arg); - int schedule_output = cpp_config->Schedule(&cpp_arg); - arg->cb_user_data = cpp_arg.cb_user_data(); - arg->success = cpp_arg.success(); - arg->target_name = gpr_strdup(cpp_arg.target_name().c_str()); - arg->peer_cert = gpr_strdup(cpp_arg.peer_cert().c_str()); - arg->status = cpp_arg.status(); - arg->error_details = gpr_strdup(cpp_arg.error_details().c_str()); - return schedule_output; + TlsServerAuthorizationCheckArg cpp_arg(arg); + return cpp_config->Schedule(&cpp_arg); } void TlsServerAuthorizationCheckConfigCCancel( void* config_user_data, grpc_tls_server_authorization_check_arg* arg) { TlsServerAuthorizationCheckConfig* cpp_config = static_cast(arg->config->context()); - TlsServerAuthorizationCheckArg cpp_arg(*arg); + TlsServerAuthorizationCheckArg cpp_arg(arg); cpp_config->Cancel(&cpp_arg); - arg->cb_user_data = cpp_arg.cb_user_data(); - arg->success = cpp_arg.success(); - arg->target_name = gpr_strdup(cpp_arg.target_name().c_str()); - arg->peer_cert = gpr_strdup(cpp_arg.peer_cert().c_str()); - arg->status = cpp_arg.status(); - arg->error_details = gpr_strdup(cpp_arg.error_details().c_str()); } } // namespace experimental diff --git a/test/cpp/client/credentials_test.cc b/test/cpp/client/credentials_test.cc index 6b58e18e8da..efd87941f72 100644 --- a/test/cpp/client/credentials_test.cc +++ b/test/cpp/client/credentials_test.cc @@ -71,8 +71,7 @@ static void tls_credential_reload_cancel(void* config_user_data, arg->set_error_details("cancelled"); } -static void tls_server_authorization_check_callback( - grpc_tls_server_authorization_check_arg* arg) { +static void tls_server_authorization_check_callback(grpc_tls_server_authorization_check_arg* arg) { GPR_ASSERT(arg != nullptr); grpc::string cb_user_data = "cb_user_data"; arg->cb_user_data = static_cast(gpr_strdup(cb_user_data.c_str())); @@ -331,7 +330,7 @@ typedef class ::grpc_impl::experimental::TlsCredentialReloadConfig TEST_F(CredentialsTest, TlsCredentialReloadArgCallback) { grpc_tls_credential_reload_arg c_arg; c_arg.cb = tls_credential_reload_callback; - TlsCredentialReloadArg arg = TlsCredentialReloadArg(c_arg); + TlsCredentialReloadArg arg = TlsCredentialReloadArg(&c_arg); arg.set_status(GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_NEW); arg.OnCredentialReloadDoneCallback(); EXPECT_EQ(arg.status(), GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_UNCHANGED); @@ -341,7 +340,7 @@ TEST_F(CredentialsTest, TlsCredentialReloadConfigSchedule) { TlsCredentialReloadConfig config(nullptr, &tls_credential_reload_sync, nullptr, nullptr); grpc_tls_credential_reload_arg c_arg; - TlsCredentialReloadArg arg(c_arg); + TlsCredentialReloadArg arg(&c_arg); arg.set_cb_user_data(static_cast(nullptr)); std::shared_ptr key_materials_config( new TlsKeyMaterialsConfig()); @@ -354,6 +353,9 @@ TEST_F(CredentialsTest, TlsCredentialReloadConfigSchedule) { arg.set_key_materials_config(key_materials_config); arg.set_status(GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_NEW); arg.set_error_details("error_details"); + grpc_tls_key_materials_config* key_materials_config_before_schedule = c_arg.key_materials_config; + const char* error_details_before_schedule = c_arg.error_details; + int schedule_output = config.Schedule(&arg); EXPECT_EQ(schedule_output, 0); EXPECT_EQ(arg.cb_user_data(), nullptr); @@ -369,6 +371,11 @@ TEST_F(CredentialsTest, TlsCredentialReloadConfigSchedule) { EXPECT_STREQ(pair_list[2].cert_chain.c_str(), "cert_chain3"); EXPECT_EQ(arg.status(), GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_NEW); EXPECT_STREQ(arg.error_details().c_str(), "error_details"); + + // Cleanup. + gpr_free(const_cast(error_details_before_schedule)); + ::grpc_core::Delete(key_materials_config_before_schedule); + ::grpc_core::Delete(c_arg.key_materials_config); } TEST_F(CredentialsTest, TlsCredentialReloadConfigCppToC) { @@ -415,17 +422,15 @@ TEST_F(CredentialsTest, TlsCredentialReloadConfigCppToC) { EXPECT_STREQ(pair_list[1].cert_chain(), "cert_chain3"); EXPECT_EQ(c_arg.status, GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_NEW); EXPECT_STREQ(c_arg.error_details, test_error_details.c_str()); - const char* error_details_after_schedule = c_arg.error_details; + grpc_tls_key_materials_config* key_materials_config_after_schedule = c_arg.key_materials_config; c_config->Cancel(&c_arg); EXPECT_EQ(c_arg.status, GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_FAIL); EXPECT_STREQ(c_arg.error_details, "cancelled"); - // Must free the fields of the C arg afterwards, regardless - // of whether or not they were changed. + // Cleanup. + ::grpc_core::Delete(key_materials_config_after_schedule); gpr_free(const_cast(c_arg.error_details)); - gpr_free(const_cast(error_details_after_schedule)); - gpr_free(c_config); } typedef class ::grpc_impl::experimental::TlsServerAuthorizationCheckArg @@ -436,13 +441,17 @@ typedef class ::grpc_impl::experimental::TlsServerAuthorizationCheckConfig TEST_F(CredentialsTest, TlsServerAuthorizationCheckArgCallback) { grpc_tls_server_authorization_check_arg c_arg; c_arg.cb = tls_server_authorization_check_callback; - TlsServerAuthorizationCheckArg arg(c_arg); + TlsServerAuthorizationCheckArg arg(&c_arg); arg.set_cb_user_data(nullptr); arg.set_success(0); arg.set_target_name("target_name"); arg.set_peer_cert("peer_cert"); arg.set_status(GRPC_STATUS_UNAUTHENTICATED); arg.set_error_details("error_details"); + const char* target_name_before_callback = c_arg.target_name; + const char* peer_cert_before_callback = c_arg.peer_cert; + const char* error_details_before_callback = c_arg.error_details; + arg.OnServerAuthorizationCheckDoneCallback(); EXPECT_STREQ(static_cast(arg.cb_user_data()), "cb_user_data"); gpr_free(arg.cb_user_data()); @@ -451,28 +460,49 @@ TEST_F(CredentialsTest, TlsServerAuthorizationCheckArgCallback) { EXPECT_STREQ(arg.peer_cert().c_str(), "callback_peer_cert"); EXPECT_EQ(arg.status(), GRPC_STATUS_OK); EXPECT_STREQ(arg.error_details().c_str(), "callback_error_details"); + + // Cleanup. + gpr_free(const_cast(target_name_before_callback)); + gpr_free(const_cast(peer_cert_before_callback)); + gpr_free(const_cast(error_details_before_callback)); + gpr_free(const_cast(c_arg.target_name)); + gpr_free(const_cast(c_arg.peer_cert)); + gpr_free(const_cast(c_arg.error_details)); } TEST_F(CredentialsTest, TlsServerAuthorizationCheckConfigSchedule) { TlsServerAuthorizationCheckConfig config = TlsServerAuthorizationCheckConfig( nullptr, &tls_server_authorization_check_sync, nullptr, nullptr); grpc_tls_server_authorization_check_arg c_arg; - TlsServerAuthorizationCheckArg arg(c_arg); + TlsServerAuthorizationCheckArg arg(&c_arg); arg.set_cb_user_data(nullptr); arg.set_success(0); arg.set_target_name("target_name"); arg.set_peer_cert("peer_cert"); arg.set_status(GRPC_STATUS_PERMISSION_DENIED); arg.set_error_details("error_details"); + const char* target_name_before_schedule = c_arg.target_name; + const char* peer_cert_before_schedule = c_arg.peer_cert; + const char* error_details_before_schedule = c_arg.error_details; + int schedule_output = config.Schedule(&arg); EXPECT_EQ(schedule_output, 1); EXPECT_STREQ(static_cast(arg.cb_user_data()), "cb_user_data"); - gpr_free(arg.cb_user_data()); EXPECT_EQ(arg.success(), 1); EXPECT_STREQ(arg.target_name().c_str(), "sync_target_name"); EXPECT_STREQ(arg.peer_cert().c_str(), "sync_peer_cert"); EXPECT_EQ(arg.status(), GRPC_STATUS_OK); EXPECT_STREQ(arg.error_details().c_str(), "sync_error_details"); + + // Cleanup. + gpr_free(arg.cb_user_data()); + gpr_free(const_cast(target_name_before_schedule)); + gpr_free(const_cast(peer_cert_before_schedule)); + gpr_free(const_cast(error_details_before_schedule)); + gpr_free(const_cast(c_arg.target_name)); + gpr_free(const_cast(c_arg.peer_cert)); + gpr_free(const_cast(c_arg.error_details)); + } TEST_F(CredentialsTest, TlsServerAuthorizationCheckConfigCppToC) { @@ -487,13 +517,10 @@ TEST_F(CredentialsTest, TlsServerAuthorizationCheckConfigCppToC) { c_arg.peer_cert = "peer_cert"; c_arg.status = GRPC_STATUS_UNAUTHENTICATED; c_arg.error_details = "error_details"; - - grpc_tls_server_authorization_check_config* c_config = config.c_config(); - c_arg.config = c_config; - int c_schedule_output = c_config->Schedule(&c_arg); + c_arg.config = config.c_config(); + int c_schedule_output = (c_arg.config)->Schedule(&c_arg); EXPECT_EQ(c_schedule_output, 1); EXPECT_STREQ(static_cast(c_arg.cb_user_data), "cb_user_data"); - gpr_free(c_arg.cb_user_data); EXPECT_EQ(c_arg.success, 1); EXPECT_STREQ(c_arg.target_name, "sync_target_name"); EXPECT_STREQ(c_arg.peer_cert, "sync_peer_cert"); @@ -504,14 +531,12 @@ TEST_F(CredentialsTest, TlsServerAuthorizationCheckConfigCppToC) { const char* peer_cert_after_schedule = c_arg.peer_cert; const char* error_details_after_schedule = c_arg.error_details; - c_config->Cancel(&c_arg); + (c_arg.config)->Cancel(&c_arg); EXPECT_EQ(c_arg.status, GRPC_STATUS_PERMISSION_DENIED); EXPECT_STREQ(c_arg.error_details, "cancelled"); - // Must free the fields of the C arg afterwards, regardless - // of whether or not they were changed. - gpr_free(const_cast(c_arg.target_name)); - gpr_free(const_cast(c_arg.peer_cert)); + // Cleanup. + gpr_free(c_arg.cb_user_data); gpr_free(const_cast(c_arg.error_details)); gpr_free(const_cast(target_name_after_schedule)); gpr_free(const_cast(peer_cert_after_schedule)); @@ -522,31 +547,26 @@ typedef class ::grpc_impl::experimental::TlsCredentialsOptions TlsCredentialsOptions; TEST_F(CredentialsTest, TlsCredentialsOptionsCppToC) { - TlsCredentialsOptions options; - options.set_cert_request_type(GRPC_SSL_REQUEST_CLIENT_CERTIFICATE_AND_VERIFY); std::shared_ptr key_materials_config( new TlsKeyMaterialsConfig()); struct TlsKeyMaterialsConfig::PemKeyCertPair pair = {"private_key", "cert_chain"}; std::vector pair_list = {pair}; key_materials_config->set_key_materials("pem_root_certs", pair_list); - options.set_key_materials_config(key_materials_config); - std::shared_ptr credential_reload_config( new TlsCredentialReloadConfig(nullptr, &tls_credential_reload_sync, &tls_credential_reload_cancel, nullptr)); - options.set_credential_reload_config(credential_reload_config); std::shared_ptr server_authorization_check_config(new TlsServerAuthorizationCheckConfig( nullptr, &tls_server_authorization_check_sync, &tls_server_authorization_check_cancel, nullptr)); - options.set_server_authorization_check_config( - server_authorization_check_config); - + TlsCredentialsOptions options = TlsCredentialsOptions(GRPC_SSL_REQUEST_CLIENT_CERTIFICATE_AND_VERIFY, + key_materials_config, + credential_reload_config, + server_authorization_check_config); grpc_tls_credentials_options* c_options = options.c_credentials_options(); - EXPECT_EQ(c_options->cert_request_type(), - GRPC_SSL_REQUEST_CLIENT_CERTIFICATE_AND_VERIFY); + EXPECT_EQ(c_options->cert_request_type(), GRPC_SSL_REQUEST_CLIENT_CERTIFICATE_AND_VERIFY); grpc_tls_key_materials_config* c_key_materials_config = c_options->key_materials_config(); grpc_tls_credential_reload_config* c_credential_reload_config = @@ -554,7 +574,7 @@ TEST_F(CredentialsTest, TlsCredentialsOptionsCppToC) { grpc_tls_credential_reload_arg c_credential_reload_arg; c_credential_reload_arg.config = c_credential_reload_config; c_credential_reload_arg.cb_user_data = nullptr; - c_credential_reload_arg.key_materials_config = c_key_materials_config; + c_credential_reload_arg.key_materials_config = c_options->key_materials_config(); c_credential_reload_arg.status = GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_UNCHANGED; grpc::string test_error_details = "error_details"; c_credential_reload_arg.error_details = test_error_details.c_str(); @@ -598,7 +618,6 @@ TEST_F(CredentialsTest, TlsCredentialsOptionsCppToC) { GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_NEW); EXPECT_STREQ(c_credential_reload_arg.error_details, test_error_details.c_str()); - gpr_free(const_cast(c_credential_reload_arg.error_details)); int c_server_authorization_check_schedule_output = c_server_authorization_check_config->Schedule( @@ -607,7 +626,6 @@ TEST_F(CredentialsTest, TlsCredentialsOptionsCppToC) { EXPECT_STREQ( static_cast(c_server_authorization_check_arg.cb_user_data), "cb_user_data"); - gpr_free(c_server_authorization_check_arg.cb_user_data); EXPECT_EQ(c_server_authorization_check_arg.success, 1); EXPECT_STREQ(c_server_authorization_check_arg.target_name, "sync_target_name"); @@ -615,11 +633,14 @@ TEST_F(CredentialsTest, TlsCredentialsOptionsCppToC) { EXPECT_EQ(c_server_authorization_check_arg.status, GRPC_STATUS_OK); EXPECT_STREQ(c_server_authorization_check_arg.error_details, "sync_error_details"); + + // Cleanup. + ::grpc_core::Delete(c_key_materials_config); + ::grpc_core::Delete(c_credential_reload_arg.key_materials_config); + gpr_free(c_server_authorization_check_arg.cb_user_data); gpr_free(const_cast(c_server_authorization_check_arg.target_name)); gpr_free(const_cast(c_server_authorization_check_arg.peer_cert)); gpr_free(const_cast(c_server_authorization_check_arg.error_details)); - - gpr_free(c_options); } } // namespace testing