From 00cce90adf8956091a4edcbc2c476baf8e91924b Mon Sep 17 00:00:00 2001 From: Matthew Stevenson Date: Tue, 1 Oct 2019 16:06:01 -0700 Subject: [PATCH] Changes requested by Yihua. --- .../grpcpp/security/tls_credentials_options.h | 24 +++- .../tls/grpc_tls_credentials_options.h | 26 +++- src/cpp/common/tls_credentials_options.cc | 2 + test/cpp/client/credentials_test.cc | 124 +++++++++++++----- 4 files changed, 142 insertions(+), 34 deletions(-) diff --git a/include/grpcpp/security/tls_credentials_options.h b/include/grpcpp/security/tls_credentials_options.h index 8cd17c03da4..76ff9ed2939 100644 --- a/include/grpcpp/security/tls_credentials_options.h +++ b/include/grpcpp/security/tls_credentials_options.h @@ -139,6 +139,11 @@ class TlsCredentialReloadConfig { int Schedule(TlsCredentialReloadArg* arg) const { if (credential_reload_interface_ == nullptr) { gpr_log(GPR_ERROR, "credential reload interface is nullptr"); + if (arg != nullptr) { + arg->set_status(GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_FAIL); + arg->set_error_details( + "the interface of the credential reload config is nullptr"); + } return 1; } return credential_reload_interface_->Schedule(arg); @@ -147,6 +152,11 @@ class TlsCredentialReloadConfig { void Cancel(TlsCredentialReloadArg* arg) const { if (credential_reload_interface_ == nullptr) { gpr_log(GPR_ERROR, "credential reload interface is nullptr"); + if (arg != nullptr) { + arg->set_status(GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_FAIL); + arg->set_error_details( + "the interface of the credential reload config is nullptr"); + } return; } credential_reload_interface_->Cancel(arg); @@ -233,6 +243,12 @@ class TlsServerAuthorizationCheckConfig { int Schedule(TlsServerAuthorizationCheckArg* arg) const { if (server_authorization_check_interface_ == nullptr) { gpr_log(GPR_ERROR, "server authorization check interface is nullptr"); + if (arg != nullptr) { + arg->set_status(GRPC_STATUS_NOT_FOUND); + arg->set_error_details( + "the interface of the server authorization check config is " + "nullptr"); + } return 1; } return server_authorization_check_interface_->Schedule(arg); @@ -241,12 +257,18 @@ class TlsServerAuthorizationCheckConfig { void Cancel(TlsServerAuthorizationCheckArg* arg) const { if (server_authorization_check_interface_ == nullptr) { gpr_log(GPR_ERROR, "server authorization check interface is nullptr"); + if (arg != nullptr) { + arg->set_status(GRPC_STATUS_NOT_FOUND); + arg->set_error_details( + "the interface of the server authorization check config is " + "nullptr"); + } return; } server_authorization_check_interface_->Cancel(arg); } - /** Creates C struct for the server authorization check config. **/ + /** Returns C struct for the server authorization check config. **/ grpc_tls_server_authorization_check_config* c_config() const { return c_config_; } 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 b9ee16c2f1e..a7f99822121 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 @@ -77,9 +77,14 @@ struct grpc_tls_credential_reload_config int Schedule(grpc_tls_credential_reload_arg* arg) const { if (schedule_ == nullptr) { gpr_log(GPR_ERROR, "schedule API is nullptr"); + if (arg != nullptr) { + arg->status = GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_FAIL; + arg->error_details = + gpr_strdup("schedule API in credential reload config is nullptr"); + } return 1; } - if (arg != nullptr && context_ != nullptr) { + if (arg != nullptr) { arg->config = const_cast(this); } return schedule_(config_user_data_, arg); @@ -87,9 +92,14 @@ struct grpc_tls_credential_reload_config void Cancel(grpc_tls_credential_reload_arg* arg) const { if (cancel_ == nullptr) { gpr_log(GPR_ERROR, "cancel API is nullptr."); + if (arg != nullptr) { + arg->status = GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_FAIL; + arg->error_details = + gpr_strdup("cancel API in credential reload config is nullptr"); + } return; } - if (arg != nullptr && context_ != nullptr) { + if (arg != nullptr) { arg->config = const_cast(this); } cancel_(config_user_data_, arg); @@ -143,6 +153,11 @@ struct grpc_tls_server_authorization_check_config int Schedule(grpc_tls_server_authorization_check_arg* arg) const { if (schedule_ == nullptr) { gpr_log(GPR_ERROR, "schedule API is nullptr"); + if (arg != nullptr) { + arg->status = GRPC_STATUS_NOT_FOUND; + arg->error_details = gpr_strdup( + "schedule API in server authorization check config is nullptr"); + } return 1; } if (arg != nullptr && context_ != nullptr) { @@ -154,9 +169,14 @@ struct grpc_tls_server_authorization_check_config void Cancel(grpc_tls_server_authorization_check_arg* arg) const { if (cancel_ == nullptr) { gpr_log(GPR_ERROR, "cancel API is nullptr."); + if (arg != nullptr) { + arg->status = GRPC_STATUS_NOT_FOUND; + arg->error_details = gpr_strdup( + "schedule API in server authorization check config is nullptr"); + } return; } - if (arg != nullptr && context_ != nullptr) { + if (arg != nullptr) { arg->config = const_cast(this); } diff --git a/src/cpp/common/tls_credentials_options.cc b/src/cpp/common/tls_credentials_options.cc index 39a1cf3a39d..7c16241717b 100644 --- a/src/cpp/common/tls_credentials_options.cc +++ b/src/cpp/common/tls_credentials_options.cc @@ -19,6 +19,8 @@ #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 { diff --git a/test/cpp/client/credentials_test.cc b/test/cpp/client/credentials_test.cc index 5c6b78a2b57..4d216d1609d 100644 --- a/test/cpp/client/credentials_test.cc +++ b/test/cpp/client/credentials_test.cc @@ -314,12 +314,16 @@ typedef class ::grpc_impl::experimental::TlsCredentialReloadConfig TlsCredentialReloadConfig; TEST_F(CredentialsTest, TlsCredentialReloadArgCallback) { - grpc_tls_credential_reload_arg c_arg; - c_arg.cb = tls_credential_reload_callback; - 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); + grpc_tls_credential_reload_arg* c_arg = new grpc_tls_credential_reload_arg; + c_arg->cb = tls_credential_reload_callback; + TlsCredentialReloadArg* arg = new TlsCredentialReloadArg(c_arg); + arg->set_status(GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_NEW); + arg->OnCredentialReloadDoneCallback(); + EXPECT_EQ(arg->status(), GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_UNCHANGED); + + // Cleanup. + delete arg; + delete c_arg; } TEST_F(CredentialsTest, TlsCredentialReloadConfigSchedule) { @@ -427,35 +431,39 @@ typedef class ::grpc_impl::experimental::TlsServerAuthorizationCheckConfig 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); - 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()); - EXPECT_EQ(arg.success(), 1); - EXPECT_STREQ(arg.target_name().c_str(), "callback_target_name"); - 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"); + grpc_tls_server_authorization_check_arg* c_arg = + new grpc_tls_server_authorization_check_arg; + c_arg->cb = tls_server_authorization_check_callback; + TlsServerAuthorizationCheckArg* arg = + new TlsServerAuthorizationCheckArg(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()); + EXPECT_EQ(arg->success(), 1); + EXPECT_STREQ(arg->target_name().c_str(), "callback_target_name"); + 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)); + gpr_free(const_cast(c_arg->target_name)); + gpr_free(const_cast(c_arg->peer_cert)); + gpr_free(const_cast(c_arg->error_details)); + delete arg; + delete c_arg; } TEST_F(CredentialsTest, TlsServerAuthorizationCheckConfigSchedule) { @@ -654,6 +662,62 @@ TEST_F(CredentialsTest, LoadSpiffeChannelCredentials) { GPR_ASSERT(channel_credentials != nullptr); } +TEST_F(CredentialsTest, TlsCredentialReloadConfigErrorMessages) { + std::shared_ptr config( + new TlsCredentialReloadConfig(nullptr)); + grpc_tls_credential_reload_arg* c_arg = new grpc_tls_credential_reload_arg; + TlsCredentialReloadArg* arg = new TlsCredentialReloadArg(c_arg); + int schedule_output = config->Schedule(arg); + + EXPECT_EQ(schedule_output, 1); + EXPECT_EQ(arg->status(), GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_FAIL); + EXPECT_STREQ(arg->error_details().c_str(), + "the interface of the credential reload config is nullptr"); + gpr_free(const_cast(c_arg->error_details)); + + arg->set_status(GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_UNCHANGED); + config->Cancel(arg); + EXPECT_EQ(arg->status(), GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_FAIL); + EXPECT_STREQ(arg->error_details().c_str(), + "the interface of the credential reload config is nullptr"); + + // Cleanup. + gpr_free(const_cast(c_arg->error_details)); + delete arg; + delete c_arg; + gpr_free(config->c_config()); +} + +TEST_F(CredentialsTest, TlsServerAuthorizationCheckConfigErrorMessages) { + std::shared_ptr config( + new TlsServerAuthorizationCheckConfig(nullptr)); + grpc_tls_server_authorization_check_arg* c_arg = + new grpc_tls_server_authorization_check_arg; + TlsServerAuthorizationCheckArg* arg = + new TlsServerAuthorizationCheckArg(c_arg); + int schedule_output = config->Schedule(arg); + + EXPECT_EQ(schedule_output, 1); + EXPECT_EQ(arg->status(), GRPC_STATUS_NOT_FOUND); + EXPECT_STREQ( + arg->error_details().c_str(), + "the interface of the server authorization check config is nullptr"); + gpr_free(const_cast(c_arg->error_details)); + + arg->set_status(GRPC_STATUS_OK); + config->Cancel(arg); + EXPECT_EQ(arg->status(), GRPC_STATUS_NOT_FOUND); + EXPECT_STREQ( + arg->error_details().c_str(), + "the interface of the server authorization check config is nullptr"); + + // Cleanup. + gpr_free(const_cast(c_arg->error_details)); + delete arg; + delete c_arg; + gpr_free(config->c_config()); +} + } // namespace testing } // namespace grpc