From 967b911f85c4259d047b90509522d718d1235337 Mon Sep 17 00:00:00 2001 From: Matthew Stevenson Date: Wed, 21 Aug 2019 09:08:15 -0700 Subject: [PATCH] Add forgotten callback function for server authorization check. --- .../grpcpp/security/tls_credentials_options.h | 16 ++-- src/cpp/common/tls_credentials_options.cc | 45 +++++----- test/cpp/client/credentials_test.cc | 89 ++++++++++++------- 3 files changed, 91 insertions(+), 59 deletions(-) diff --git a/include/grpcpp/security/tls_credentials_options.h b/include/grpcpp/security/tls_credentials_options.h index 7db070d7e8f..9cba5aa27bc 100644 --- a/include/grpcpp/security/tls_credentials_options.h +++ b/include/grpcpp/security/tls_credentials_options.h @@ -48,7 +48,7 @@ class TlsKeyMaterialsConfig { * transfers ownership of the arguments to the config. **/ void set_key_materials(grpc::string pem_root_certs, std::vector pem_key_cert_pair_list); - void set_version(int version) { version_ = version;}; + void set_version(int version) { version_ = version; }; private: int version_; @@ -63,7 +63,6 @@ grpc_tls_key_materials_config* c_key_materials( std::shared_ptr tls_key_materials_c_to_cpp( const grpc_tls_key_materials_config* config); - /** TLS credential reload arguments, wraps grpc_tls_credential_reload_arg. **/ class TlsCredentialReloadArg { public: @@ -80,7 +79,7 @@ class TlsCredentialReloadArg { /** Setters for member fields. **/ void set_cb_user_data(void* cb_user_data); void set_key_materials_config( - std::shared_ptr key_materials_config); + const std::shared_ptr& key_materials_config); void set_status(grpc_ssl_certificate_config_reload_status status); void set_error_details(const grpc::string& error_details); @@ -167,9 +166,8 @@ class TlsServerAuthorizationCheckArg { // Exposed for testing purposes. int tls_server_authorization_check_config_c_schedule( void* config_user_data, grpc_tls_server_authorization_check_arg* arg); -void tls_server_authorization_check_config_c_cancel(void* config_user_data, - grpc_tls_server_authorization_check_arg* arg); - +void tls_server_authorization_check_config_c_cancel( + void* config_user_data, grpc_tls_server_authorization_check_arg* arg); /** TLS server authorization check config, wraps * grps_tls_server_authorization_check_config. **/ @@ -197,7 +195,8 @@ class TlsServerAuthorizationCheckConfig { } /** Creates C struct for the credential reload config. **/ - grpc_tls_server_authorization_check_config* c_server_authorization_check() const { + grpc_tls_server_authorization_check_config* c_server_authorization_check() + const { return c_config_; } @@ -219,8 +218,7 @@ class TlsCredentialsOptions { std::shared_ptr key_materials_config() const { return key_materials_config_; } - std::shared_ptr credential_reload_config() - const { + std::shared_ptr credential_reload_config() const { return credential_reload_config_; } std::shared_ptr diff --git a/src/cpp/common/tls_credentials_options.cc b/src/cpp/common/tls_credentials_options.cc index a48e9f94ba6..4d42affed07 100644 --- a/src/cpp/common/tls_credentials_options.cc +++ b/src/cpp/common/tls_credentials_options.cc @@ -32,13 +32,15 @@ void TlsKeyMaterialsConfig::set_key_materials( } /** Creates a new C struct for the key materials. **/ -grpc_tls_key_materials_config* c_key_materials(const std::shared_ptr& config) { +grpc_tls_key_materials_config* c_key_materials( + const std::shared_ptr& config) { grpc_tls_key_materials_config* c_config = grpc_tls_key_materials_config_create(); ::grpc_core::InlinedVector<::grpc_core::PemKeyCertPair, 1> c_pem_key_cert_pair_list; for (auto key_cert_pair = config->pem_key_cert_pair_list().begin(); - key_cert_pair != config->pem_key_cert_pair_list().end(); key_cert_pair++) { + key_cert_pair != config->pem_key_cert_pair_list().end(); + 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)); @@ -48,7 +50,8 @@ grpc_tls_key_materials_config* c_key_materials(const std::shared_ptr c_pem_root_certs(gpr_strdup(config->pem_root_certs().c_str())); + ::grpc_core::UniquePtr c_pem_root_certs( + gpr_strdup(config->pem_root_certs().c_str())); c_config->set_key_materials(std::move(c_pem_root_certs), std::move(c_pem_key_cert_pair_list)); c_config->set_version(config->version()); @@ -60,8 +63,7 @@ std::shared_ptr tls_key_materials_c_to_cpp( const grpc_tls_key_materials_config* config) { std::shared_ptr cpp_config( new TlsKeyMaterialsConfig()); - std::vector - cpp_pem_key_cert_pair_list; + std::vector cpp_pem_key_cert_pair_list; grpc_tls_key_materials_config::PemKeyCertPairList pem_key_cert_pair_list = config->pem_key_cert_pair_list(); for (size_t i = 0; i < pem_key_cert_pair_list.size(); i++) { @@ -71,9 +73,8 @@ std::shared_ptr tls_key_materials_c_to_cpp( gpr_strdup(key_cert_pair.cert_chain())}; cpp_pem_key_cert_pair_list.push_back(::std::move(p)); } - cpp_config->set_key_materials( - std::move(gpr_strdup(config->pem_root_certs())), - std::move(cpp_pem_key_cert_pair_list)); + cpp_config->set_key_materials(std::move(gpr_strdup(config->pem_root_certs())), + std::move(cpp_pem_key_cert_pair_list)); cpp_config->set_version(config->version()); return cpp_config; } @@ -95,16 +96,19 @@ void* TlsCredentialReloadArg::cb_user_data() const { /** This function creates a new TlsKeyMaterialsConfig instance whose fields are * not shared with the corresponding key materials config fields of the * TlsCredentialReloadArg instance. **/ -std::shared_ptr TlsCredentialReloadArg::key_materials_config() const { +std::shared_ptr +TlsCredentialReloadArg::key_materials_config() const { return tls_key_materials_c_to_cpp(c_arg_.key_materials_config); } -grpc_ssl_certificate_config_reload_status TlsCredentialReloadArg::status() const { +grpc_ssl_certificate_config_reload_status TlsCredentialReloadArg::status() + const { return c_arg_.status; } std::shared_ptr TlsCredentialReloadArg::error_details() const { - std::shared_ptr cpp_error_details(new grpc::string(c_arg_.error_details)); + std::shared_ptr cpp_error_details( + new grpc::string(c_arg_.error_details)); return cpp_error_details; } @@ -113,7 +117,7 @@ void TlsCredentialReloadArg::set_cb_user_data(void* cb_user_data) { } void TlsCredentialReloadArg::set_key_materials_config( - std::shared_ptr key_materials_config) { + const std::shared_ptr& key_materials_config) { c_arg_.key_materials_config = c_key_materials(key_materials_config); } @@ -122,13 +126,12 @@ void TlsCredentialReloadArg::set_status( c_arg_.status = status; } -void TlsCredentialReloadArg::set_error_details(const grpc::string& error_details) { +void TlsCredentialReloadArg::set_error_details( + const grpc::string& error_details) { c_arg_.error_details = gpr_strdup(error_details.c_str()); } -void TlsCredentialReloadArg::callback() { - c_arg_.cb(&c_arg_); -} +void TlsCredentialReloadArg::callback() { c_arg_.cb(&c_arg_); } /** The C schedule and cancel functions for the credential reload config. **/ int tls_credential_reload_config_c_schedule( @@ -211,7 +214,7 @@ grpc_status_code TlsServerAuthorizationCheckArg::status() const { std::shared_ptr TlsServerAuthorizationCheckArg::error_details() const { std::shared_ptr cpp_error_details( -new grpc::string(c_arg_.error_details)); + new grpc::string(c_arg_.error_details)); return cpp_error_details; } @@ -242,6 +245,8 @@ void TlsServerAuthorizationCheckArg::set_error_details( c_arg_.error_details = gpr_strdup(error_details.c_str()); } +void TlsServerAuthorizationCheckArg::callback() { c_arg_.cb(&c_arg_); } + /** The C schedule and cancel functions for the credential reload config. **/ int tls_server_authorization_check_config_c_schedule( void* config_user_data, grpc_tls_server_authorization_check_arg* arg) { @@ -282,14 +287,13 @@ TlsServerAuthorizationCheckConfig::TlsServerAuthorizationCheckConfig( config_user_data_ = const_cast(config_user_data); schedule_ = schedule; cancel_ = cancel; -destruct_ = destruct; + destruct_ = destruct; c_config_ = grpc_tls_server_authorization_check_config_create( config_user_data_, &tls_server_authorization_check_config_c_schedule, &tls_server_authorization_check_config_c_cancel, destruct_); c_config_->set_context(static_cast(this)); } - TlsServerAuthorizationCheckConfig::~TlsServerAuthorizationCheckConfig() {} /** gRPC TLS credential options API implementation **/ @@ -299,7 +303,8 @@ grpc_tls_credentials_options* TlsCredentialsOptions::c_credentials_options() grpc_tls_credentials_options_create(); c_options->set_cert_request_type(cert_request_type_); c_options->set_key_materials_config( - ::grpc_core::RefCountedPtr(c_key_materials(key_materials_config_))); + ::grpc_core::RefCountedPtr( + c_key_materials(key_materials_config_))); c_options->set_credential_reload_config( ::grpc_core::RefCountedPtr( credential_reload_config_->c_credential_reload())); diff --git a/test/cpp/client/credentials_test.cc b/test/cpp/client/credentials_test.cc index 92d2782b9b4..e205f195aec 100644 --- a/test/cpp/client/credentials_test.cc +++ b/test/cpp/client/credentials_test.cc @@ -80,7 +80,8 @@ static void tls_server_authorization_check_callback( arg->error_details = "callback_error_details"; } -static int tls_server_authorization_check_sync(void* config_user_data, TlsServerAuthorizationCheckArg* arg) { +static int tls_server_authorization_check_sync( + void* config_user_data, TlsServerAuthorizationCheckArg* arg) { GPR_ASSERT(arg != nullptr); grpc::string cb_user_data = "cb_user_data"; arg->set_cb_user_data(static_cast(gpr_strdup(cb_user_data.c_str()))); @@ -92,7 +93,8 @@ static int tls_server_authorization_check_sync(void* config_user_data, TlsServer return 1; } -static void tls_server_authorization_check_cancel(void* config_user_data, TlsServerAuthorizationCheckArg* arg) { +static void tls_server_authorization_check_cancel( + void* config_user_data, TlsServerAuthorizationCheckArg* arg) { GPR_ASSERT(arg != nullptr); arg->set_status(GRPC_STATUS_PERMISSION_DENIED); arg->set_error_details("cancelled"); @@ -314,8 +316,10 @@ TEST_F(CredentialsTest, TlsKeyMaterialsCtoCpp) { EXPECT_STREQ("cert_chain", cpp_pair_list[0].cert_chain.c_str()); } -typedef class ::grpc_impl::experimental::TlsCredentialReloadArg TlsCredentialReloadArg; -typedef class ::grpc_impl::experimental::TlsCredentialReloadConfig TlsCredentialReloadConfig; +typedef class ::grpc_impl::experimental::TlsCredentialReloadArg + TlsCredentialReloadArg; +typedef class ::grpc_impl::experimental::TlsCredentialReloadConfig + TlsCredentialReloadConfig; TEST_F(CredentialsTest, TlsCredentialReloadArgCallback) { grpc_tls_credential_reload_arg c_arg; @@ -337,8 +341,7 @@ TEST_F(CredentialsTest, TlsCredentialReloadConfigSchedule) { "cert_chain1"}; struct TlsKeyMaterialsConfig::PemKeyCertPair pair2 = {"private_key2", "cert_chain2"}; - std::vector pair_list = {pair1, - pair2}; + 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_status(GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_NEW); @@ -435,7 +438,8 @@ TEST_F(CredentialsTest, TlsServerAuthorizationCheckArgCallback) { } TEST_F(CredentialsTest, TlsServerAuthorizationCheckConfigSchedule) { - TlsServerAuthorizationCheckConfig config = TlsServerAuthorizationCheckConfig(nullptr, &tls_server_authorization_check_sync, nullptr, nullptr); + TlsServerAuthorizationCheckConfig config = TlsServerAuthorizationCheckConfig( + nullptr, &tls_server_authorization_check_sync, nullptr, nullptr); TlsServerAuthorizationCheckArg arg; arg.set_cb_user_data(nullptr); arg.set_success(0); @@ -455,7 +459,8 @@ TEST_F(CredentialsTest, TlsServerAuthorizationCheckConfigSchedule) { TEST_F(CredentialsTest, TlsServerAuthorizationCheckConfigCppToC) { TlsServerAuthorizationCheckConfig config = TlsServerAuthorizationCheckConfig( - nullptr, &tls_server_authorization_check_sync, &tls_server_authorization_check_cancel, nullptr); + nullptr, &tls_server_authorization_check_sync, + &tls_server_authorization_check_cancel, nullptr); grpc_tls_server_authorization_check_arg c_arg; c_arg.cb = tls_server_authorization_check_callback; c_arg.cb_user_data = nullptr; @@ -465,7 +470,8 @@ TEST_F(CredentialsTest, TlsServerAuthorizationCheckConfigCppToC) { c_arg.status = GRPC_STATUS_UNAUTHENTICATED; c_arg.error_details = "error_details"; - grpc_tls_server_authorization_check_config* c_config = config.c_server_authorization_check(); + grpc_tls_server_authorization_check_config* c_config = + config.c_server_authorization_check(); c_arg.config = c_config; int c_schedule_output = c_config->Schedule(&c_arg); EXPECT_EQ(c_schedule_output, 1); @@ -495,25 +501,33 @@ TEST_F(CredentialsTest, TlsCredentialsOptionsCppToC) { 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)); + 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); + 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); grpc_tls_credentials_options* c_options = options.c_credentials_options(); 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 = c_options->credential_reload_config(); + grpc_tls_key_materials_config* c_key_materials_config = + c_options->key_materials_config(); + 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.key_materials_config = c_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(); - grpc_tls_server_authorization_check_config* c_server_authorization_check_config = c_options->server_authorization_check_config(); + grpc_tls_server_authorization_check_config* + 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.cb = tls_server_authorization_check_callback; c_server_authorization_check_arg.cb_user_data = nullptr; @@ -524,14 +538,21 @@ TEST_F(CredentialsTest, TlsCredentialsOptionsCppToC) { 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()), 1); - EXPECT_STREQ(c_key_materials_config->pem_key_cert_pair_list()[0].private_key(), "private_key"); - EXPECT_STREQ(c_key_materials_config->pem_key_cert_pair_list()[0].cert_chain(), "cert_chain"); - - int c_credential_reload_schedule_output = c_credential_reload_config->Schedule(&c_credential_reload_arg); + EXPECT_EQ( + static_cast(c_key_materials_config->pem_key_cert_pair_list().size()), + 1); + EXPECT_STREQ( + c_key_materials_config->pem_key_cert_pair_list()[0].private_key(), + "private_key"); + EXPECT_STREQ(c_key_materials_config->pem_key_cert_pair_list()[0].cert_chain(), + "cert_chain"); + + int c_credential_reload_schedule_output = + c_credential_reload_config->Schedule(&c_credential_reload_arg); EXPECT_EQ(c_credential_reload_schedule_output, 0); EXPECT_EQ(c_credential_reload_arg.cb_user_data, nullptr); - EXPECT_STREQ(c_credential_reload_arg.key_materials_config->pem_root_certs(), "new_pem_root_certs"); + EXPECT_STREQ(c_credential_reload_arg.key_materials_config->pem_root_certs(), + "new_pem_root_certs"); ::grpc_core::InlinedVector<::grpc_core::PemKeyCertPair, 1> c_pair_list = c_credential_reload_arg.key_materials_config->pem_key_cert_pair_list(); EXPECT_EQ(static_cast(c_pair_list.size()), 2); @@ -539,17 +560,25 @@ TEST_F(CredentialsTest, TlsCredentialsOptionsCppToC) { EXPECT_STREQ(c_pair_list[0].cert_chain(), "cert_chain"); EXPECT_STREQ(c_pair_list[1].private_key(), "private_key3"); EXPECT_STREQ(c_pair_list[1].cert_chain(), "cert_chain3"); - EXPECT_EQ(c_credential_reload_arg.status, GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_NEW); - EXPECT_STREQ(c_credential_reload_arg.error_details, test_error_details.c_str()); - - int c_server_authorization_check_schedule_output = c_server_authorization_check_config->Schedule(&c_server_authorization_check_arg); + EXPECT_EQ(c_credential_reload_arg.status, + GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_NEW); + EXPECT_STREQ(c_credential_reload_arg.error_details, + test_error_details.c_str()); + + int c_server_authorization_check_schedule_output = + c_server_authorization_check_config->Schedule( + &c_server_authorization_check_arg); EXPECT_EQ(c_server_authorization_check_schedule_output, 1); - EXPECT_STREQ(static_cast(c_server_authorization_check_arg.cb_user_data), "cb_user_data"); + EXPECT_STREQ( + static_cast(c_server_authorization_check_arg.cb_user_data), + "cb_user_data"); EXPECT_EQ(c_server_authorization_check_arg.success, 1); - EXPECT_STREQ(c_server_authorization_check_arg.target_name, "sync_target_name"); + EXPECT_STREQ(c_server_authorization_check_arg.target_name, + "sync_target_name"); EXPECT_STREQ(c_server_authorization_check_arg.peer_cert, "sync_peer_cert"); EXPECT_EQ(c_server_authorization_check_arg.status, GRPC_STATUS_OK); - EXPECT_STREQ(c_server_authorization_check_arg.error_details, "sync_error_details"); + EXPECT_STREQ(c_server_authorization_check_arg.error_details, + "sync_error_details"); gpr_free(c_options); }