From c580af37e0bae96b5ef5ae13960742afe22d949e Mon Sep 17 00:00:00 2001
From: yang-g <yangg@google.com>
Date: Thu, 15 Sep 2016 15:28:38 -0700
Subject: [PATCH] Add a check for metadata from auth plugin
---
.../credentials/plugin/plugin_credentials.c | 25 ++++++++--
test/cpp/end2end/end2end_test.cc | 46 +++++++++++++++----
2 files changed, 59 insertions(+), 12 deletions(-)
diff --git a/src/core/lib/security/credentials/plugin/plugin_credentials.c b/src/core/lib/security/credentials/plugin/plugin_credentials.c
index 824ff081dc..905de3723e 100644
--- a/src/core/lib/security/credentials/plugin/plugin_credentials.c
+++ b/src/core/lib/security/credentials/plugin/plugin_credentials.c
@@ -37,6 +37,7 @@
#include "src/core/lib/surface/api_trace.h"
+#include <grpc/grpc.h>
#include <grpc/support/alloc.h>
#include <grpc/support/log.h>
#include <grpc/support/string_util.h>
@@ -71,17 +72,33 @@ static void plugin_md_request_metadata_ready(void *request,
error_details);
} else {
size_t i;
+ bool seen_illegal_header = false;
grpc_credentials_md *md_array = NULL;
- if (num_md > 0) {
+ for (i = 0; i < num_md; i++) {
+ if (!grpc_header_key_is_legal(md[i].key, strlen(md[i].key))) {
+ gpr_log(GPR_ERROR, "Plugin added invalid metadata key: %s", md[i].key);
+ seen_illegal_header = true;
+ break;
+ } else if (!grpc_is_binary_header(md[i].key, strlen(md[i].key)) &&
+ !grpc_header_nonbin_value_is_legal(md[i].value,
+ md[i].value_length)) {
+ gpr_log(GPR_ERROR, "Plugin added invalid metadata value.");
+ seen_illegal_header = true;
+ break;
+ }
+ }
+ if (seen_illegal_header) {
+ r->cb(&exec_ctx, r->user_data, NULL, 0, GRPC_CREDENTIALS_ERROR,
+ "Illegal metadata");
+ } else if (num_md > 0) {
md_array = gpr_malloc(num_md * sizeof(grpc_credentials_md));
for (i = 0; i < num_md; i++) {
md_array[i].key = gpr_slice_from_copied_string(md[i].key);
md_array[i].value =
gpr_slice_from_copied_buffer(md[i].value, md[i].value_length);
}
- }
- r->cb(&exec_ctx, r->user_data, md_array, num_md, GRPC_CREDENTIALS_OK, NULL);
- if (md_array != NULL) {
+ r->cb(&exec_ctx, r->user_data, md_array, num_md, GRPC_CREDENTIALS_OK,
+ NULL);
for (i = 0; i < num_md; i++) {
gpr_slice_unref(md_array[i].key);
gpr_slice_unref(md_array[i].value);
diff --git a/test/cpp/end2end/end2end_test.cc b/test/cpp/end2end/end2end_test.cc
index 66614922f1..1d78f4a911 100644
--- a/test/cpp/end2end/end2end_test.cc
+++ b/test/cpp/end2end/end2end_test.cc
@@ -80,11 +80,14 @@ const char kTestCredsPluginErrorMsg[] = "Could not find plugin metadata.";
class TestMetadataCredentialsPlugin : public MetadataCredentialsPlugin {
public:
- static const char kMetadataKey[];
+ static const char kGoodMetadataKey[];
+ static const char kBadMetadataKey[];
- TestMetadataCredentialsPlugin(grpc::string_ref metadata_value,
+ TestMetadataCredentialsPlugin(grpc::string_ref metadata_key,
+ grpc::string_ref metadata_value,
bool is_blocking, bool is_successful)
- : metadata_value_(metadata_value.data(), metadata_value.length()),
+ : metadata_key_(metadata_key.data(), metadata_key.length()),
+ metadata_value_(metadata_value.data(), metadata_value.length()),
is_blocking_(is_blocking),
is_successful_(is_successful) {}
@@ -99,7 +102,7 @@ class TestMetadataCredentialsPlugin : public MetadataCredentialsPlugin {
EXPECT_TRUE(channel_auth_context.IsPeerAuthenticated());
EXPECT_TRUE(metadata != nullptr);
if (is_successful_) {
- metadata->insert(std::make_pair(kMetadataKey, metadata_value_));
+ metadata->insert(std::make_pair(metadata_key_, metadata_value_));
return Status::OK;
} else {
return Status(StatusCode::NOT_FOUND, kTestCredsPluginErrorMsg);
@@ -107,12 +110,16 @@ class TestMetadataCredentialsPlugin : public MetadataCredentialsPlugin {
}
private:
+ grpc::string metadata_key_;
grpc::string metadata_value_;
bool is_blocking_;
bool is_successful_;
};
-const char TestMetadataCredentialsPlugin::kMetadataKey[] = "TestPluginMetadata";
+const char TestMetadataCredentialsPlugin::kBadMetadataKey[] =
+ "TestPluginMetadata";
+const char TestMetadataCredentialsPlugin::kGoodMetadataKey[] =
+ "test-plugin-metadata";
class TestAuthMetadataProcessor : public AuthMetadataProcessor {
public:
@@ -123,13 +130,17 @@ class TestAuthMetadataProcessor : public AuthMetadataProcessor {
std::shared_ptr<CallCredentials> GetCompatibleClientCreds() {
return MetadataCredentialsFromPlugin(
std::unique_ptr<MetadataCredentialsPlugin>(
- new TestMetadataCredentialsPlugin(kGoodGuy, is_blocking_, true)));
+ new TestMetadataCredentialsPlugin(
+ TestMetadataCredentialsPlugin::kGoodMetadataKey, kGoodGuy,
+ is_blocking_, true)));
}
std::shared_ptr<CallCredentials> GetIncompatibleClientCreds() {
return MetadataCredentialsFromPlugin(
std::unique_ptr<MetadataCredentialsPlugin>(
- new TestMetadataCredentialsPlugin("Mr Hyde", is_blocking_, true)));
+ new TestMetadataCredentialsPlugin(
+ TestMetadataCredentialsPlugin::kGoodMetadataKey, "Mr Hyde",
+ is_blocking_, true)));
}
// Interface implementation
@@ -142,7 +153,7 @@ class TestAuthMetadataProcessor : public AuthMetadataProcessor {
EXPECT_TRUE(context != nullptr);
EXPECT_TRUE(response_metadata != nullptr);
auto auth_md =
- auth_metadata.find(TestMetadataCredentialsPlugin::kMetadataKey);
+ auth_metadata.find(TestMetadataCredentialsPlugin::kGoodMetadataKey);
EXPECT_NE(auth_md, auth_metadata.end());
string_ref auth_md_value = auth_md->second;
if (auth_md_value == kGoodGuy) {
@@ -1322,6 +1333,23 @@ TEST_P(SecureEnd2endTest, OverridePerCallCredentials) {
EXPECT_TRUE(s.ok());
}
+TEST_P(SecureEnd2endTest, AuthMetadataPluginKeyFailure) {
+ ResetStub();
+ EchoRequest request;
+ EchoResponse response;
+ ClientContext context;
+ context.set_credentials(
+ MetadataCredentialsFromPlugin(std::unique_ptr<MetadataCredentialsPlugin>(
+ new TestMetadataCredentialsPlugin(
+ TestMetadataCredentialsPlugin::kBadMetadataKey,
+ "Does not matter, will fail the key is invalid.", false, true))));
+ request.set_message("Hello");
+
+ Status s = stub_->Echo(&context, request, &response);
+ EXPECT_FALSE(s.ok());
+ EXPECT_EQ(s.error_code(), StatusCode::UNAUTHENTICATED);
+}
+
TEST_P(SecureEnd2endTest, NonBlockingAuthMetadataPluginFailure) {
ResetStub();
EchoRequest request;
@@ -1330,6 +1358,7 @@ TEST_P(SecureEnd2endTest, NonBlockingAuthMetadataPluginFailure) {
context.set_credentials(
MetadataCredentialsFromPlugin(std::unique_ptr<MetadataCredentialsPlugin>(
new TestMetadataCredentialsPlugin(
+ TestMetadataCredentialsPlugin::kGoodMetadataKey,
"Does not matter, will fail anyway (see 3rd param)", false,
false))));
request.set_message("Hello");
@@ -1388,6 +1417,7 @@ TEST_P(SecureEnd2endTest, BlockingAuthMetadataPluginFailure) {
context.set_credentials(
MetadataCredentialsFromPlugin(std::unique_ptr<MetadataCredentialsPlugin>(
new TestMetadataCredentialsPlugin(
+ TestMetadataCredentialsPlugin::kGoodMetadataKey,
"Does not matter, will fail anyway (see 3rd param)", true,
false))));
request.set_message("Hello");
--
GitLab