Merge pull request #19139 from yang-g/plugin_creds

Avoid accessing freed auth_metadata_context in C++ plugin.
pull/19136/head
Yang Gao 6 years ago committed by GitHub
commit 07c9f7a36b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 3
      src/core/lib/security/transport/auth_filters.h
  2. 13
      src/core/lib/security/transport/client_auth_filter.cc
  3. 27
      src/cpp/client/secure_credentials.cc
  4. 83
      test/cpp/end2end/end2end_test.cc

@ -32,6 +32,9 @@ void grpc_auth_metadata_context_build(
const grpc_slice& call_method, grpc_auth_context* auth_context,
grpc_auth_metadata_context* auth_md_context);
void grpc_auth_metadata_context_copy(grpc_auth_metadata_context* from,
grpc_auth_metadata_context* to);
void grpc_auth_metadata_context_reset(grpc_auth_metadata_context* context);
#endif /* GRPC_CORE_LIB_SECURITY_TRANSPORT_AUTH_FILTERS_H */

@ -112,6 +112,19 @@ struct call_data {
} // namespace
void grpc_auth_metadata_context_copy(grpc_auth_metadata_context* from,
grpc_auth_metadata_context* to) {
grpc_auth_metadata_context_reset(to);
to->channel_auth_context = from->channel_auth_context;
if (to->channel_auth_context != nullptr) {
const_cast<grpc_auth_context*>(to->channel_auth_context)
->Ref(DEBUG_LOCATION, "grpc_auth_metadata_context_copy")
.release();
}
to->service_url = gpr_strdup(from->service_url);
to->method_name = gpr_strdup(from->method_name);
}
void grpc_auth_metadata_context_reset(
grpc_auth_metadata_context* auth_md_context) {
if (auth_md_context->service_url != nullptr) {

@ -22,6 +22,8 @@
#include <grpcpp/channel.h>
#include <grpcpp/impl/grpc_library.h>
#include <grpcpp/support/channel_arguments.h>
#include "src/core/lib/iomgr/executor.h"
#include "src/core/lib/security/transport/auth_filters.h"
#include "src/cpp/client/create_channel_internal.h"
#include "src/cpp/common/secure_auth_context.h"
@ -223,13 +225,23 @@ std::shared_ptr<grpc_impl::CallCredentials> MetadataCredentialsFromPlugin(
} // namespace grpc_impl
namespace grpc {
void MetadataCredentialsPluginWrapper::Destroy(void* wrapper) {
if (wrapper == nullptr) return;
namespace {
void DeleteWrapper(void* wrapper, grpc_error* ignored) {
MetadataCredentialsPluginWrapper* w =
static_cast<MetadataCredentialsPluginWrapper*>(wrapper);
delete w;
}
} // namespace
void MetadataCredentialsPluginWrapper::Destroy(void* wrapper) {
if (wrapper == nullptr) return;
grpc_core::ApplicationCallbackExecCtx callback_exec_ctx;
grpc_core::ExecCtx exec_ctx;
GRPC_CLOSURE_RUN(GRPC_CLOSURE_CREATE(DeleteWrapper, wrapper,
grpc_core::Executor::Scheduler(
grpc_core::ExecutorJobType::SHORT)),
GRPC_ERROR_NONE);
}
int MetadataCredentialsPluginWrapper::GetMetadata(
void* wrapper, grpc_auth_metadata_context context,
@ -247,10 +259,15 @@ int MetadataCredentialsPluginWrapper::GetMetadata(
return true;
}
if (w->plugin_->IsBlocking()) {
// The internals of context may be destroyed if GetMetadata is cancelled.
// Make a copy for InvokePlugin.
grpc_auth_metadata_context context_copy = grpc_auth_metadata_context();
grpc_auth_metadata_context_copy(&context, &context_copy);
// Asynchronous return.
w->thread_pool_->Add([w, context, cb, user_data] {
w->thread_pool_->Add([w, context_copy, cb, user_data]() mutable {
w->MetadataCredentialsPluginWrapper::InvokePlugin(
context, cb, user_data, nullptr, nullptr, nullptr, nullptr);
context_copy, cb, user_data, nullptr, nullptr, nullptr, nullptr);
grpc_auth_metadata_context_reset(&context_copy);
});
return 0;
} else {

@ -26,6 +26,7 @@
#include <grpcpp/channel.h>
#include <grpcpp/client_context.h>
#include <grpcpp/create_channel.h>
#include <grpcpp/impl/codegen/status_code_enum.h>
#include <grpcpp/resource_quota.h>
#include <grpcpp/security/auth_metadata_processor.h>
#include <grpcpp/security/credentials.h>
@ -90,11 +91,13 @@ class TestMetadataCredentialsPlugin : public MetadataCredentialsPlugin {
TestMetadataCredentialsPlugin(const grpc::string_ref& metadata_key,
const grpc::string_ref& metadata_value,
bool is_blocking, bool is_successful)
bool is_blocking, bool is_successful,
int delay_ms)
: metadata_key_(metadata_key.data(), metadata_key.length()),
metadata_value_(metadata_value.data(), metadata_value.length()),
is_blocking_(is_blocking),
is_successful_(is_successful) {}
is_successful_(is_successful),
delay_ms_(delay_ms) {}
bool IsBlocking() const override { return is_blocking_; }
@ -102,6 +105,11 @@ class TestMetadataCredentialsPlugin : public MetadataCredentialsPlugin {
grpc::string_ref service_url, grpc::string_ref method_name,
const grpc::AuthContext& channel_auth_context,
std::multimap<grpc::string, grpc::string>* metadata) override {
if (delay_ms_ != 0) {
gpr_sleep_until(
gpr_time_add(gpr_now(GPR_CLOCK_REALTIME),
gpr_time_from_millis(delay_ms_, GPR_TIMESPAN)));
}
EXPECT_GT(service_url.length(), 0UL);
EXPECT_GT(method_name.length(), 0UL);
EXPECT_TRUE(channel_auth_context.IsPeerAuthenticated());
@ -119,6 +127,7 @@ class TestMetadataCredentialsPlugin : public MetadataCredentialsPlugin {
grpc::string metadata_value_;
bool is_blocking_;
bool is_successful_;
int delay_ms_;
};
const char TestMetadataCredentialsPlugin::kBadMetadataKey[] =
@ -137,7 +146,7 @@ class TestAuthMetadataProcessor : public AuthMetadataProcessor {
std::unique_ptr<MetadataCredentialsPlugin>(
new TestMetadataCredentialsPlugin(
TestMetadataCredentialsPlugin::kGoodMetadataKey, kGoodGuy,
is_blocking_, true)));
is_blocking_, true, 0)));
}
std::shared_ptr<CallCredentials> GetIncompatibleClientCreds() {
@ -145,7 +154,7 @@ class TestAuthMetadataProcessor : public AuthMetadataProcessor {
std::unique_ptr<MetadataCredentialsPlugin>(
new TestMetadataCredentialsPlugin(
TestMetadataCredentialsPlugin::kGoodMetadataKey, "Mr Hyde",
is_blocking_, true)));
is_blocking_, true, 0)));
}
// Interface implementation
@ -1835,7 +1844,8 @@ TEST_P(SecureEnd2endTest, AuthMetadataPluginKeyFailure) {
std::unique_ptr<MetadataCredentialsPlugin>(
new TestMetadataCredentialsPlugin(
TestMetadataCredentialsPlugin::kBadMetadataKey,
"Does not matter, will fail the key is invalid.", false, true))));
"Does not matter, will fail the key is invalid.", false, true,
0))));
request.set_message("Hello");
Status s = stub_->Echo(&context, request, &response);
@ -1853,7 +1863,7 @@ TEST_P(SecureEnd2endTest, AuthMetadataPluginValueFailure) {
std::unique_ptr<MetadataCredentialsPlugin>(
new TestMetadataCredentialsPlugin(
TestMetadataCredentialsPlugin::kGoodMetadataKey,
"With illegal \n value.", false, true))));
"With illegal \n value.", false, true, 0))));
request.set_message("Hello");
Status s = stub_->Echo(&context, request, &response);
@ -1861,6 +1871,55 @@ TEST_P(SecureEnd2endTest, AuthMetadataPluginValueFailure) {
EXPECT_EQ(s.error_code(), StatusCode::UNAVAILABLE);
}
TEST_P(SecureEnd2endTest, AuthMetadataPluginWithDeadline) {
MAYBE_SKIP_TEST;
ResetStub();
EchoRequest request;
EchoResponse response;
ClientContext context;
const int delay = 100;
std::chrono::system_clock::time_point deadline =
std::chrono::system_clock::now() + std::chrono::milliseconds(delay);
context.set_deadline(deadline);
context.set_credentials(grpc::MetadataCredentialsFromPlugin(
std::unique_ptr<MetadataCredentialsPlugin>(
new TestMetadataCredentialsPlugin("meta_key", "Does not matter", true,
true, delay))));
request.set_message("Hello");
Status s = stub_->Echo(&context, request, &response);
if (!s.ok()) {
EXPECT_TRUE(s.error_code() == StatusCode::DEADLINE_EXCEEDED ||
s.error_code() == StatusCode::UNAVAILABLE);
}
}
TEST_P(SecureEnd2endTest, AuthMetadataPluginWithCancel) {
MAYBE_SKIP_TEST;
ResetStub();
EchoRequest request;
EchoResponse response;
ClientContext context;
const int delay = 100;
context.set_credentials(grpc::MetadataCredentialsFromPlugin(
std::unique_ptr<MetadataCredentialsPlugin>(
new TestMetadataCredentialsPlugin("meta_key", "Does not matter", true,
true, delay))));
request.set_message("Hello");
std::thread cancel_thread([&] {
gpr_sleep_until(gpr_time_add(gpr_now(GPR_CLOCK_REALTIME),
gpr_time_from_millis(delay, GPR_TIMESPAN)));
context.TryCancel();
});
Status s = stub_->Echo(&context, request, &response);
if (!s.ok()) {
EXPECT_TRUE(s.error_code() == StatusCode::CANCELLED ||
s.error_code() == StatusCode::UNAVAILABLE);
}
cancel_thread.join();
}
TEST_P(SecureEnd2endTest, NonBlockingAuthMetadataPluginFailure) {
MAYBE_SKIP_TEST;
ResetStub();
@ -1871,8 +1930,8 @@ TEST_P(SecureEnd2endTest, NonBlockingAuthMetadataPluginFailure) {
std::unique_ptr<MetadataCredentialsPlugin>(
new TestMetadataCredentialsPlugin(
TestMetadataCredentialsPlugin::kGoodMetadataKey,
"Does not matter, will fail anyway (see 3rd param)", false,
false))));
"Does not matter, will fail anyway (see 3rd param)", false, false,
0))));
request.set_message("Hello");
Status s = stub_->Echo(&context, request, &response);
@ -1935,8 +1994,8 @@ TEST_P(SecureEnd2endTest, BlockingAuthMetadataPluginFailure) {
std::unique_ptr<MetadataCredentialsPlugin>(
new TestMetadataCredentialsPlugin(
TestMetadataCredentialsPlugin::kGoodMetadataKey,
"Does not matter, will fail anyway (see 3rd param)", true,
false))));
"Does not matter, will fail anyway (see 3rd param)", true, false,
0))));
request.set_message("Hello");
Status s = stub_->Echo(&context, request, &response);
@ -1962,11 +2021,11 @@ TEST_P(SecureEnd2endTest, CompositeCallCreds) {
grpc::MetadataCredentialsFromPlugin(
std::unique_ptr<MetadataCredentialsPlugin>(
new TestMetadataCredentialsPlugin(kMetadataKey1, kMetadataVal1,
true, true))),
true, true, 0))),
grpc::MetadataCredentialsFromPlugin(
std::unique_ptr<MetadataCredentialsPlugin>(
new TestMetadataCredentialsPlugin(kMetadataKey2, kMetadataVal2,
true, true)))));
true, true, 0)))));
request.set_message("Hello");
request.mutable_param()->set_echo_metadata(true);

Loading…
Cancel
Save