Chiebot-Mirror
/
grpc
mirror of https://github.com/grpc/grpc.git
8
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge pull request #22370 from grpc/zhen_error_details_fix

Browse Source 
[ImproveTLS] add a wrapper to error_detail in C core args
pull/22414/head
ZhenLian 5 years ago committed by GitHub
parent 260d688809 738272f3a8
commit c97492faf0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 77 additions and 44 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 8
      include/grpc/grpc_security.h
  2. 25
      src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h
  3. 14
      src/core/lib/security/security_connector/tls/tls_security_connector.cc
  4. 10
      src/cpp/common/tls_credentials_options.cc
  5. 7
      test/core/security/grpc_tls_credentials_options_test.cc
  6. 57
      test/cpp/client/credentials_test.cc

8
include/grpc/grpc_security.h
Unescape View File

@ -714,6 +714,10 @@ GRPCAPI grpc_server_credentials* grpc_local_server_credentials_create(
/** --- TLS channel/server credentials ---
* It is used for experimental purpose for now and subject to change. */
/** Struct for indicating errors. It is used for
* experimental purpose for now and subject to change. */
typedef struct grpc_tls_error_details grpc_tls_error_details;
/** Config for TLS key materials. It is used for
* experimental purpose for now and subject to change. */
typedef struct grpc_tls_key_materials_config grpc_tls_key_materials_config;
@ -857,7 +861,7 @@ struct grpc_tls_credential_reload_arg {
void* cb_user_data;
grpc_tls_key_materials_config* key_materials_config;
grpc_ssl_certificate_config_reload_status status;
const char* error_details;
grpc_tls_error_details* error_details;
grpc_tls_credential_reload_config* config;
void* context;
void (*destroy_context)(void* ctx);
@ -935,7 +939,7 @@ struct grpc_tls_server_authorization_check_arg {
const char* peer_cert;
const char* peer_cert_full_chain;
grpc_status_code status;
const char* error_details;
grpc_tls_error_details* error_details;
grpc_tls_server_authorization_check_config* config;
void* context;
void (*destroy_context)(void* ctx);

25
src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h
Unescape View File

@ -27,6 +27,19 @@
#include "src/core/lib/gprpp/ref_counted.h"
#include "src/core/lib/security/security_connector/ssl_utils.h"
struct grpc_tls_error_details
: public grpc_core::RefCounted<grpc_tls_error_details> {
public:
grpc_tls_error_details() : error_details_("") {}
void set_error_details(const char* err_details) {
error_details_ = err_details;
}
const std::string& error_details() { return error_details_; }
private:
std::string error_details_;
};
/** TLS key materials config. **/
struct grpc_tls_key_materials_config
: public grpc_core::RefCounted<grpc_tls_key_materials_config> {
@ -93,8 +106,8 @@ struct grpc_tls_credential_reload_config
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");
arg->error_details->set_error_details(
"schedule API in credential reload config is nullptr");
}
return 1;
}
@ -108,8 +121,8 @@ struct grpc_tls_credential_reload_config
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");
arg->error_details->set_error_details(
"cancel API in credential reload config is nullptr");
}
return;
}
@ -169,7 +182,7 @@ struct grpc_tls_server_authorization_check_config
gpr_log(GPR_ERROR, "schedule API is nullptr");
if (arg != nullptr) {
arg->status = GRPC_STATUS_NOT_FOUND;
arg->error_details = gpr_strdup(
arg->error_details->set_error_details(
"schedule API in server authorization check config is nullptr");
}
return 1;
@ -185,7 +198,7 @@ struct grpc_tls_server_authorization_check_config
gpr_log(GPR_ERROR, "cancel API is nullptr.");
if (arg != nullptr) {
arg->status = GRPC_STATUS_NOT_FOUND;
arg->error_details = gpr_strdup(
arg->error_details->set_error_details(
"schedule API in server authorization check config is nullptr");
}
return;

14
src/core/lib/security/security_connector/tls/tls_security_connector.cc
Unescape View File

@ -88,6 +88,7 @@ grpc_status_code TlsFetchKeyMaterials(
if (credential_reload_config != nullptr) {
grpc_tls_credential_reload_arg* arg = new grpc_tls_credential_reload_arg();
arg->key_materials_config = key_materials_config.get();
arg->error_details = new grpc_tls_error_details();
int result = credential_reload_config->Schedule(arg);
if (result) {
/** Credential reloading is performed async. This is not yet supported.
@ -105,13 +106,13 @@ grpc_status_code TlsFetchKeyMaterials(
} else if (arg->status == GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_FAIL) {
gpr_log(GPR_ERROR, "Credential reload failed with an error:");
if (arg->error_details != nullptr) {
gpr_log(GPR_ERROR, "%s", arg->error_details);
gpr_log(GPR_ERROR, "%s", arg->error_details->error_details().c_str());
}
reload_status =
is_key_materials_empty ? GRPC_STATUS_INTERNAL : GRPC_STATUS_OK;
}
}
gpr_free((void*)arg->error_details);
delete arg->error_details;
/** If the credential reload config was constructed via a wrapped language,
* then |arg->context| and |arg->destroy_context| will not be nullptr. In
* this case, we must destroy |arg->context|, which stores the wrapped
@ -406,14 +407,14 @@ grpc_error* TlsChannelSecurityConnector::ProcessServerAuthorizationCheckResult(
gpr_asprintf(&msg,
"Server authorization check is cancelled by the caller with "
"error: %s",
arg->error_details);
arg->error_details->error_details().c_str());
error = GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg);
} else if (arg->status == GRPC_STATUS_OK) {
/* Server authorization check completed successfully but returned check
* failure. */
if (!arg->success) {
gpr_asprintf(&msg, "Server authorization check failed with error: %s",
arg->error_details);
arg->error_details->error_details().c_str());
error = GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg);
}
/* Server authorization check did not complete correctly. */
@ -421,7 +422,7 @@ grpc_error* TlsChannelSecurityConnector::ProcessServerAuthorizationCheckResult(
gpr_asprintf(
&msg,
"Server authorization check did not finish correctly with error: %s",
arg->error_details);
arg->error_details->error_details().c_str());
error = GRPC_ERROR_CREATE_FROM_COPIED_STRING(msg);
}
gpr_free(msg);
@ -433,6 +434,7 @@ TlsChannelSecurityConnector::ServerAuthorizationCheckArgCreate(
void* user_data) {
grpc_tls_server_authorization_check_arg* arg =
new grpc_tls_server_authorization_check_arg();
arg->error_details = new grpc_tls_error_details();
arg->cb = ServerAuthorizationCheckDone;
arg->cb_user_data = user_data;
arg->status = GRPC_STATUS_OK;
@ -447,7 +449,7 @@ void TlsChannelSecurityConnector::ServerAuthorizationCheckArgDestroy(
gpr_free((void*)arg->target_name);
gpr_free((void*)arg->peer_cert);
if (arg->peer_cert_full_chain) gpr_free((void*)arg->peer_cert_full_chain);
gpr_free((void*)arg->error_details);
delete arg->error_details;
if (arg->destroy_context != nullptr) {
arg->destroy_context(arg->context);
}

10
src/cpp/common/tls_credentials_options.cc
Unescape View File

@ -68,8 +68,7 @@ grpc_ssl_certificate_config_reload_status TlsCredentialReloadArg::status()
}
grpc::string TlsCredentialReloadArg::error_details() const {
grpc::string cpp_error_details(c_arg_->error_details);
return cpp_error_details;
return c_arg_->error_details->error_details();
}
void TlsCredentialReloadArg::set_cb_user_data(void* cb_user_data) {
@ -159,7 +158,7 @@ void TlsCredentialReloadArg::set_status(
void TlsCredentialReloadArg::set_error_details(
const grpc::string& error_details) {
c_arg_->error_details = gpr_strdup(error_details.c_str());
c_arg_->error_details->set_error_details(error_details.c_str());
}
void TlsCredentialReloadArg::OnCredentialReloadDoneCallback() {
@ -221,8 +220,7 @@ grpc_status_code TlsServerAuthorizationCheckArg::status() const {
}
grpc::string TlsServerAuthorizationCheckArg::error_details() const {
grpc::string cpp_error_details(c_arg_->error_details);
return cpp_error_details;
return c_arg_->error_details->error_details();
}
void TlsServerAuthorizationCheckArg::set_cb_user_data(void* cb_user_data) {
@ -254,7 +252,7 @@ void TlsServerAuthorizationCheckArg::set_status(grpc_status_code status) {
void TlsServerAuthorizationCheckArg::set_error_details(
const grpc::string& error_details) {
c_arg_->error_details = gpr_strdup(error_details.c_str());
c_arg_->error_details->set_error_details(error_details.c_str());
}
void TlsServerAuthorizationCheckArg::OnServerAuthorizationCheckDoneCallback() {

7
test/core/security/grpc_tls_credentials_options_test.cc
Unescape View File

@ -50,6 +50,13 @@ TEST(GrpcTlsCredentialsOptionsTest, SetKeyMaterials) {
delete config;
}
TEST(GrpcTlsCredentialsOptionsTest, ErrorDetails) {
grpc_tls_error_details error_details;
EXPECT_STREQ(error_details.error_details().c_str(), "");
error_details.set_error_details("test error details");
EXPECT_STREQ(error_details.error_details().c_str(), "test error details");
}
} // namespace testing
int main(int argc, char** argv) {

57
test/cpp/client/credentials_test.cc
Unescape View File

@ -78,7 +78,7 @@ static void tls_server_authorization_check_callback(
arg->target_name = gpr_strdup("callback_target_name");
arg->peer_cert = gpr_strdup("callback_peer_cert");
arg->status = GRPC_STATUS_OK;
arg->error_details = gpr_strdup("callback_error_details");
arg->error_details->set_error_details("callback_error_details");
}
class TestTlsServerAuthorizationCheck
@ -342,6 +342,7 @@ TEST_F(CredentialsTest, TlsCredentialReloadConfigSchedule) {
std::shared_ptr<TlsCredentialReloadConfig> config(
new TlsCredentialReloadConfig(test_credential_reload));
grpc_tls_credential_reload_arg* c_arg = new grpc_tls_credential_reload_arg();
c_arg->error_details = new grpc_tls_error_details();
c_arg->context = nullptr;
TlsCredentialReloadArg* arg = new TlsCredentialReloadArg(c_arg);
struct TlsKeyMaterialsConfig::PemKeyCertPair pair1 = {"private_key1",
@ -352,7 +353,6 @@ TEST_F(CredentialsTest, TlsCredentialReloadConfigSchedule) {
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;
int schedule_output = config->Schedule(arg);
EXPECT_EQ(schedule_output, 0);
@ -372,11 +372,11 @@ TEST_F(CredentialsTest, TlsCredentialReloadConfigSchedule) {
EXPECT_STREQ(arg->error_details().c_str(), "error_details");
// Cleanup.
gpr_free(const_cast<char*>(error_details_before_schedule));
delete c_arg->key_materials_config;
if (c_arg->destroy_context != nullptr) {
c_arg->destroy_context(c_arg->context);
}
delete c_arg->error_details;
delete c_arg;
delete config->c_config();
}
@ -386,6 +386,7 @@ TEST_F(CredentialsTest, TlsCredentialReloadConfigCppToC) {
new TestTlsCredentialReload());
TlsCredentialReloadConfig config(test_credential_reload);
grpc_tls_credential_reload_arg c_arg;
c_arg.error_details = new grpc_tls_error_details();
c_arg.context = nullptr;
c_arg.cb_user_data = static_cast<void*>(nullptr);
grpc_tls_key_materials_config c_key_materials;
@ -407,7 +408,7 @@ TEST_F(CredentialsTest, TlsCredentialReloadConfigCppToC) {
c_arg.key_materials_config = &c_key_materials;
c_arg.status = GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_UNCHANGED;
grpc::string test_error_details = "error_details";
c_arg.error_details = test_error_details.c_str();
c_arg.error_details->set_error_details(test_error_details.c_str());
grpc_tls_credential_reload_config* c_config = config.c_config();
c_arg.config = c_config;
@ -424,10 +425,12 @@ TEST_F(CredentialsTest, TlsCredentialReloadConfigCppToC) {
EXPECT_STREQ(pair_list[1].private_key(), "private_key3");
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());
EXPECT_STREQ(c_arg.error_details->error_details().c_str(),
test_error_details.c_str());
// Cleanup.
c_arg.destroy_context(c_arg.context);
delete c_arg.error_details;
delete config.c_config();
}
@ -441,6 +444,7 @@ TEST_F(CredentialsTest, TlsServerAuthorizationCheckArgCallback) {
new grpc_tls_server_authorization_check_arg;
c_arg->cb = tls_server_authorization_check_callback;
c_arg->context = nullptr;
c_arg->error_details = new grpc_tls_error_details();
TlsServerAuthorizationCheckArg* arg =
new TlsServerAuthorizationCheckArg(c_arg);
arg->set_cb_user_data(nullptr);
@ -451,7 +455,6 @@ TEST_F(CredentialsTest, TlsServerAuthorizationCheckArgCallback) {
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<char*>(arg->cb_user_data()), "cb_user_data");
@ -465,10 +468,9 @@ TEST_F(CredentialsTest, TlsServerAuthorizationCheckArgCallback) {
// Cleanup.
gpr_free(const_cast<char*>(target_name_before_callback));
gpr_free(const_cast<char*>(peer_cert_before_callback));
gpr_free(const_cast<char*>(error_details_before_callback));
gpr_free(const_cast<char*>(c_arg->target_name));
gpr_free(const_cast<char*>(c_arg->peer_cert));
gpr_free(const_cast<char*>(c_arg->error_details));
delete c_arg->error_details;
delete arg;
delete c_arg;
}
@ -479,6 +481,7 @@ TEST_F(CredentialsTest, TlsServerAuthorizationCheckConfigSchedule) {
TlsServerAuthorizationCheckConfig config(test_server_authorization_check);
grpc_tls_server_authorization_check_arg* c_arg =
new grpc_tls_server_authorization_check_arg();
c_arg->error_details = new grpc_tls_error_details();
c_arg->context = nullptr;
TlsServerAuthorizationCheckArg* arg =
new TlsServerAuthorizationCheckArg(c_arg);
@ -490,7 +493,6 @@ TEST_F(CredentialsTest, TlsServerAuthorizationCheckConfigSchedule) {
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);
@ -505,10 +507,9 @@ TEST_F(CredentialsTest, TlsServerAuthorizationCheckConfigSchedule) {
gpr_free(arg->cb_user_data());
gpr_free(const_cast<char*>(target_name_before_schedule));
gpr_free(const_cast<char*>(peer_cert_before_schedule));
gpr_free(const_cast<char*>(error_details_before_schedule));
gpr_free(const_cast<char*>(c_arg->target_name));
gpr_free(const_cast<char*>(c_arg->peer_cert));
gpr_free(const_cast<char*>(c_arg->error_details));
delete c_arg->error_details;
if (c_arg->destroy_context != nullptr) {
c_arg->destroy_context(c_arg->context);
}
@ -527,7 +528,8 @@ TEST_F(CredentialsTest, TlsServerAuthorizationCheckConfigCppToC) {
c_arg.target_name = "target_name";
c_arg.peer_cert = "peer_cert";
c_arg.status = GRPC_STATUS_UNAUTHENTICATED;
c_arg.error_details = "error_details";
c_arg.error_details = new grpc_tls_error_details();
c_arg.error_details->set_error_details("error_details");
c_arg.config = config.c_config();
c_arg.context = nullptr;
int c_schedule_output = (c_arg.config)->Schedule(&c_arg);
@ -537,12 +539,13 @@ TEST_F(CredentialsTest, TlsServerAuthorizationCheckConfigCppToC) {
EXPECT_STREQ(c_arg.target_name, "sync_target_name");
EXPECT_STREQ(c_arg.peer_cert, "sync_peer_cert");
EXPECT_EQ(c_arg.status, GRPC_STATUS_OK);
EXPECT_STREQ(c_arg.error_details, "sync_error_details");
EXPECT_STREQ(c_arg.error_details->error_details().c_str(),
"sync_error_details");
// Cleanup.
gpr_free(c_arg.cb_user_data);
c_arg.destroy_context(c_arg.context);
gpr_free(const_cast<char*>(c_arg.error_details));
delete c_arg.error_details;
gpr_free(const_cast<char*>(c_arg.target_name));
gpr_free(const_cast<char*>(c_arg.peer_cert));
delete config.c_config();
@ -587,7 +590,9 @@ TEST_F(CredentialsTest, TlsCredentialsOptionsCppToC) {
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();
c_credential_reload_arg.error_details = new grpc_tls_error_details();
c_credential_reload_arg.error_details->set_error_details(
test_error_details.c_str());
c_credential_reload_arg.context = nullptr;
grpc_tls_server_authorization_check_config*
c_server_authorization_check_config =
@ -599,7 +604,9 @@ TEST_F(CredentialsTest, TlsCredentialsOptionsCppToC) {
c_server_authorization_check_arg.target_name = "target_name";
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";
c_server_authorization_check_arg.error_details = new grpc_tls_error_details();
c_server_authorization_check_arg.error_details->set_error_details(
"error_details");
c_server_authorization_check_arg.context = nullptr;
EXPECT_STREQ(c_key_materials_config->pem_root_certs(), "pem_root_certs");
EXPECT_EQ(
@ -627,7 +634,7 @@ TEST_F(CredentialsTest, TlsCredentialsOptionsCppToC) {
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,
EXPECT_STREQ(c_credential_reload_arg.error_details->error_details().c_str(),
test_error_details.c_str());
int c_server_authorization_check_schedule_output =
@ -642,17 +649,19 @@ TEST_F(CredentialsTest, TlsCredentialsOptionsCppToC) {
"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->error_details().c_str(),
"sync_error_details");
// Cleanup.
c_credential_reload_arg.destroy_context(c_credential_reload_arg.context);
delete c_credential_reload_arg.error_details;
c_server_authorization_check_arg.destroy_context(
c_server_authorization_check_arg.context);
gpr_free(c_server_authorization_check_arg.cb_user_data);
gpr_free(const_cast<char*>(c_server_authorization_check_arg.target_name));
gpr_free(const_cast<char*>(c_server_authorization_check_arg.peer_cert));
gpr_free(const_cast<char*>(c_server_authorization_check_arg.error_details));
delete c_server_authorization_check_arg.error_details;
delete c_options;
}
@ -698,6 +707,7 @@ TEST_F(CredentialsTest, TlsCredentialReloadConfigErrorMessages) {
std::shared_ptr<TlsCredentialReloadConfig> config(
new TlsCredentialReloadConfig(nullptr));
grpc_tls_credential_reload_arg* c_arg = new grpc_tls_credential_reload_arg;
c_arg->error_details = new grpc_tls_error_details();
c_arg->context = nullptr;
TlsCredentialReloadArg* arg = new TlsCredentialReloadArg(c_arg);
int schedule_output = config->Schedule(arg);
@ -706,7 +716,6 @@ TEST_F(CredentialsTest, TlsCredentialReloadConfigErrorMessages) {
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<char*>(c_arg->error_details));
arg->set_status(GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_UNCHANGED);
config->Cancel(arg);
@ -715,10 +724,10 @@ TEST_F(CredentialsTest, TlsCredentialReloadConfigErrorMessages) {
"the interface of the credential reload config is nullptr");
// Cleanup.
gpr_free(const_cast<char*>(c_arg->error_details));
if (c_arg->destroy_context != nullptr) {
c_arg->destroy_context(c_arg->context);
}
delete c_arg->error_details;
delete c_arg;
delete config->c_config();
}
@ -728,6 +737,7 @@ TEST_F(CredentialsTest, TlsServerAuthorizationCheckConfigErrorMessages) {
new TlsServerAuthorizationCheckConfig(nullptr));
grpc_tls_server_authorization_check_arg* c_arg =
new grpc_tls_server_authorization_check_arg;
c_arg->error_details = new grpc_tls_error_details();
c_arg->context = nullptr;
TlsServerAuthorizationCheckArg* arg =
new TlsServerAuthorizationCheckArg(c_arg);
@ -738,7 +748,6 @@ TEST_F(CredentialsTest, TlsServerAuthorizationCheckConfigErrorMessages) {
EXPECT_STREQ(
arg->error_details().c_str(),
"the interface of the server authorization check config is nullptr");
gpr_free(const_cast<char*>(c_arg->error_details));
arg->set_status(GRPC_STATUS_OK);
config->Cancel(arg);
@ -748,7 +757,7 @@ TEST_F(CredentialsTest, TlsServerAuthorizationCheckConfigErrorMessages) {
"the interface of the server authorization check config is nullptr");
// Cleanup.
gpr_free(const_cast<char*>(c_arg->error_details));
delete c_arg->error_details;
if (c_arg->destroy_context != nullptr) {
c_arg->destroy_context(c_arg->context);
}

Write Preview
Loading…
Cancel
Save