diff --git a/src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h b/src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h index 78f32ab2479..e27eca82d1b 100644 --- a/src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h +++ b/src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h @@ -69,6 +69,13 @@ struct grpc_tls_credential_reload_config void set_context(void* context) { context_ = context; } int Schedule(grpc_tls_credential_reload_arg* arg) const { + if (schedule_ == nullptr) { + gpr_log(GPR_ERROR, "schedule API is nullptr"); + return 1; + } + if (arg != nullptr && context_ != nullptr) { + arg->config = const_cast(this); + } return schedule_(config_user_data_, arg); } void Cancel(grpc_tls_credential_reload_arg* arg) const { @@ -76,6 +83,9 @@ struct grpc_tls_credential_reload_config gpr_log(GPR_ERROR, "cancel API is nullptr."); return; } + if (arg != nullptr && context_ != nullptr) { + arg->config = const_cast(this); + } cancel_(config_user_data_, arg); } @@ -125,6 +135,14 @@ struct grpc_tls_server_authorization_check_config void set_context(void* context) { context_ = context; } int Schedule(grpc_tls_server_authorization_check_arg* arg) const { + if (schedule_ == nullptr) { + gpr_log(GPR_ERROR, "schedule API is nullptr"); + return 1; + } + if (arg != nullptr && context_ != nullptr) { + arg->config = + const_cast(this); + } return schedule_(config_user_data_, arg); } void Cancel(grpc_tls_server_authorization_check_arg* arg) const { @@ -132,6 +150,10 @@ struct grpc_tls_server_authorization_check_config gpr_log(GPR_ERROR, "cancel API is nullptr."); return; } + if (arg != nullptr && context_ != nullptr) { + arg->config = + const_cast(this); + } cancel_(config_user_data_, arg); } diff --git a/src/cpp/common/tls_credentials_options.cc b/src/cpp/common/tls_credentials_options.cc index 21e3ce3548d..88b78dd1a2d 100644 --- a/src/cpp/common/tls_credentials_options.cc +++ b/src/cpp/common/tls_credentials_options.cc @@ -78,6 +78,10 @@ void TlsCredentialReloadArg::set_error_details( } void TlsCredentialReloadArg::OnCredentialReloadDoneCallback() { + if (c_arg_->cb == nullptr) { + gpr_log(GPR_ERROR, "credential reload arg callback API is nullptr"); + return; + } c_arg_->cb(c_arg_); } @@ -92,13 +96,17 @@ TlsCredentialReloadConfig::TlsCredentialReloadConfig( cancel_(cancel), destruct_(destruct) { c_config_ = grpc_tls_credential_reload_config_create( - config_user_data_, &TlsCredentialReloadConfigCSchedule, - &TlsCredentialReloadConfigCCancel, destruct_); + config_user_data_, + schedule != nullptr ? &TlsCredentialReloadConfigCSchedule : nullptr, + cancel != nullptr ? &TlsCredentialReloadConfigCCancel : nullptr, + destruct_); c_config_->set_context(static_cast(this)); } TlsCredentialReloadConfig::~TlsCredentialReloadConfig() { - ::grpc_core::Delete(c_config_); + if (destruct_ != nullptr) { + destruct_(config_user_data_); + } } /** gRPC TLS server authorization check arg API implementation **/ @@ -157,6 +165,10 @@ void TlsServerAuthorizationCheckArg::set_error_details( } void TlsServerAuthorizationCheckArg::OnServerAuthorizationCheckDoneCallback() { + if (c_arg_->cb == nullptr) { + gpr_log(GPR_ERROR, "server authorizaton check arg callback API is nullptr"); + return; + } c_arg_->cb(c_arg_); } @@ -172,13 +184,18 @@ TlsServerAuthorizationCheckConfig::TlsServerAuthorizationCheckConfig( cancel_(cancel), destruct_(destruct) { c_config_ = grpc_tls_server_authorization_check_config_create( - config_user_data_, &TlsServerAuthorizationCheckConfigCSchedule, - &TlsServerAuthorizationCheckConfigCCancel, destruct_); + config_user_data_, + schedule != nullptr ? &TlsServerAuthorizationCheckConfigCSchedule + : nullptr, + cancel != nullptr ? &TlsServerAuthorizationCheckConfigCCancel : nullptr, + destruct_); c_config_->set_context(static_cast(this)); } TlsServerAuthorizationCheckConfig::~TlsServerAuthorizationCheckConfig() { - ::grpc_core::Delete(c_config_); + if (destruct_ != nullptr) { + destruct_(config_user_data_); + } } /** gRPC TLS credential options API implementation **/ @@ -199,14 +216,20 @@ TlsCredentialsOptions::TlsCredentialsOptions( 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()); + if (credential_reload_config_ != nullptr) { + grpc_tls_credentials_options_set_credential_reload_config( + c_credentials_options_, credential_reload_config_->c_config()); + } + if (server_authorization_check_config_ != nullptr) { + 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_); + if (c_credentials_options_ != nullptr) { + 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 8879b44eaae..680aad162e4 100644 --- a/src/cpp/common/tls_credentials_options_util.cc +++ b/src/cpp/common/tls_credentials_options_util.cc @@ -29,6 +29,9 @@ namespace experimental { * Similarly, the user must free the underlying pointer to c_pem_root_certs. **/ grpc_tls_key_materials_config* ConvertToCKeyMaterialsConfig( const std::shared_ptr& config) { + if (config == nullptr) { + return nullptr; + } grpc_tls_key_materials_config* c_config = grpc_tls_key_materials_config_create(); ::grpc_core::InlinedVector<::grpc_core::PemKeyCertPair, 1> @@ -57,6 +60,9 @@ grpc_tls_key_materials_config* ConvertToCKeyMaterialsConfig( * allocates memory for the Cpp config. **/ std::shared_ptr ConvertToCppKeyMaterialsConfig( const grpc_tls_key_materials_config* config) { + if (config == nullptr) { + return nullptr; + } std::shared_ptr cpp_config( new TlsKeyMaterialsConfig()); std::vector cpp_pem_key_cert_pair_list; @@ -79,6 +85,11 @@ std::shared_ptr ConvertToCppKeyMaterialsConfig( * reload schedule/cancel API. **/ int TlsCredentialReloadConfigCSchedule(void* config_user_data, grpc_tls_credential_reload_arg* arg) { + if (arg == nullptr || arg->config == nullptr || + arg->config->context() == nullptr) { + gpr_log(GPR_ERROR, "credential reload arg was not properly initialized"); + return 1; + } TlsCredentialReloadConfig* cpp_config = static_cast(arg->config->context()); TlsCredentialReloadArg cpp_arg(arg); @@ -87,6 +98,11 @@ int TlsCredentialReloadConfigCSchedule(void* config_user_data, void TlsCredentialReloadConfigCCancel(void* config_user_data, grpc_tls_credential_reload_arg* arg) { + if (arg == nullptr || arg->config == nullptr || + arg->config->context() == nullptr) { + gpr_log(GPR_ERROR, "credential reload arg was not properly initialized"); + return; + } TlsCredentialReloadConfig* cpp_config = static_cast(arg->config->context()); TlsCredentialReloadArg cpp_arg(arg); @@ -98,6 +114,12 @@ void TlsCredentialReloadConfigCCancel(void* config_user_data, * of a C++ server authorization check schedule/cancel API. **/ int TlsServerAuthorizationCheckConfigCSchedule( void* config_user_data, grpc_tls_server_authorization_check_arg* arg) { + if (arg == nullptr || arg->config == nullptr || + arg->config->context() == nullptr) { + gpr_log(GPR_ERROR, + "server authorization check arg was not properly initialized"); + return 1; + } TlsServerAuthorizationCheckConfig* cpp_config = static_cast(arg->config->context()); TlsServerAuthorizationCheckArg cpp_arg(arg); @@ -106,6 +128,12 @@ int TlsServerAuthorizationCheckConfigCSchedule( void TlsServerAuthorizationCheckConfigCCancel( void* config_user_data, grpc_tls_server_authorization_check_arg* arg) { + if (arg == nullptr || arg->config == nullptr || + arg->config->context() == nullptr) { + gpr_log(GPR_ERROR, + "server authorization check arg was not properly initialized"); + return; + } TlsServerAuthorizationCheckConfig* cpp_config = static_cast(arg->config->context()); TlsServerAuthorizationCheckArg cpp_arg(arg); diff --git a/test/cpp/client/credentials_test.cc b/test/cpp/client/credentials_test.cc index adc2865f226..b5b7f563c21 100644 --- a/test/cpp/client/credentials_test.cc +++ b/test/cpp/client/credentials_test.cc @@ -53,6 +53,7 @@ static int tls_credential_reload_sync(void* config_user_data, "cert_chain3"}; std::shared_ptr key_materials_config = arg->key_materials_config(); + GPR_ASSERT(key_materials_config != nullptr); std::vector pair_list = key_materials_config->pem_key_cert_pair_list(); pair_list.push_back(pair3); @@ -378,6 +379,7 @@ TEST_F(CredentialsTest, TlsCredentialReloadConfigSchedule) { gpr_free(const_cast(error_details_before_schedule)); ::grpc_core::Delete(key_materials_config_before_schedule); ::grpc_core::Delete(c_arg.key_materials_config); + ::grpc_core::Delete(config.c_config()); } TEST_F(CredentialsTest, TlsCredentialReloadConfigCppToC) { @@ -434,6 +436,7 @@ TEST_F(CredentialsTest, TlsCredentialReloadConfigCppToC) { // Cleanup. ::grpc_core::Delete(key_materials_config_after_schedule); gpr_free(const_cast(c_arg.error_details)); + ::grpc_core::Delete(config.c_config()); } typedef class ::grpc_impl::experimental::TlsServerAuthorizationCheckArg @@ -505,6 +508,7 @@ TEST_F(CredentialsTest, TlsServerAuthorizationCheckConfigSchedule) { gpr_free(const_cast(c_arg.target_name)); gpr_free(const_cast(c_arg.peer_cert)); gpr_free(const_cast(c_arg.error_details)); + ::grpc_core::Delete(config.c_config()); } TEST_F(CredentialsTest, TlsServerAuthorizationCheckConfigCppToC) { @@ -543,6 +547,7 @@ TEST_F(CredentialsTest, TlsServerAuthorizationCheckConfigCppToC) { gpr_free(const_cast(target_name_after_schedule)); gpr_free(const_cast(peer_cert_after_schedule)); gpr_free(const_cast(error_details_after_schedule)); + ::grpc_core::Delete(config.c_config()); } typedef class ::grpc_impl::experimental::TlsCredentialsOptions @@ -574,7 +579,6 @@ TEST_F(CredentialsTest, TlsCredentialsOptionsCppToC) { grpc_tls_credential_reload_config* c_credential_reload_config = c_options->credential_reload_config(); 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_options->key_materials_config(); @@ -585,7 +589,6 @@ TEST_F(CredentialsTest, TlsCredentialsOptionsCppToC) { c_server_authorization_check_config = c_options->server_authorization_check_config(); grpc_tls_server_authorization_check_arg c_server_authorization_check_arg; - c_server_authorization_check_arg.config = c_server_authorization_check_config; c_server_authorization_check_arg.cb = tls_server_authorization_check_callback; c_server_authorization_check_arg.cb_user_data = nullptr; c_server_authorization_check_arg.success = 0; @@ -593,7 +596,6 @@ TEST_F(CredentialsTest, TlsCredentialsOptionsCppToC) { c_server_authorization_check_arg.peer_cert = "peer_cert"; c_server_authorization_check_arg.status = GRPC_STATUS_UNAUTHENTICATED; c_server_authorization_check_arg.error_details = "error_details"; - EXPECT_STREQ(c_key_materials_config->pem_root_certs(), "pem_root_certs"); EXPECT_EQ( static_cast(c_key_materials_config->pem_key_cert_pair_list().size()), @@ -604,6 +606,7 @@ TEST_F(CredentialsTest, TlsCredentialsOptionsCppToC) { EXPECT_STREQ(c_key_materials_config->pem_key_cert_pair_list()[0].cert_chain(), "cert_chain"); + GPR_ASSERT(c_credential_reload_config != nullptr); int c_credential_reload_schedule_output = c_credential_reload_config->Schedule(&c_credential_reload_arg); EXPECT_EQ(c_credential_reload_schedule_output, 0); @@ -644,6 +647,8 @@ TEST_F(CredentialsTest, TlsCredentialsOptionsCppToC) { 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)); + ::grpc_core::Delete(c_credential_reload_config); + ::grpc_core::Delete(c_server_authorization_check_config); } } // namespace testing