From 0581d129f53930c474330627fbc99f4eda2d693b Mon Sep 17 00:00:00 2001
From: Craig Tiller <ctiller@google.com>
Date: Mon, 2 Nov 2015 14:09:40 -0800
Subject: [PATCH] stream_op cleanup: security changes
---
src/core/security/client_auth_filter.c | 109 ++++++++++-------------
src/core/security/credentials.c | 3 +-
src/core/security/credentials.h | 2 +-
src/core/security/security_context.c | 6 +-
src/core/security/server_auth_filter.c | 93 +++++++++----------
src/core/security/server_secure_chttp2.c | 3 +-
6 files changed, 94 insertions(+), 122 deletions(-)
diff --git a/src/core/security/client_auth_filter.c b/src/core/security/client_auth_filter.c
index 635982b252..c780b65873 100644
--- a/src/core/security/client_auth_filter.c
+++ b/src/core/security/client_auth_filter.c
@@ -59,8 +59,6 @@ typedef struct {
progress */
grpc_pollset *pollset;
grpc_transport_stream_op op;
- size_t op_md_idx;
- int sent_initial_metadata;
gpr_uint8 security_context_set;
grpc_linked_mdelem md_links[MAX_CREDENTIALS_METADATA_COUNT];
char *service_url;
@@ -108,9 +106,8 @@ static void on_credentials_metadata(grpc_exec_ctx *exec_ctx, void *user_data,
return;
}
GPR_ASSERT(num_md <= MAX_CREDENTIALS_METADATA_COUNT);
- GPR_ASSERT(op->send_ops && op->send_ops->nops > calld->op_md_idx &&
- op->send_ops->ops[calld->op_md_idx].type == GRPC_OP_METADATA);
- mdb = &op->send_ops->ops[calld->op_md_idx].data.metadata;
+ GPR_ASSERT(op->send_initial_metadata != NULL);
+ mdb = op->send_initial_metadata;
for (i = 0; i < num_md; i++) {
grpc_metadata_batch_add_tail(
mdb, &calld->md_links[i],
@@ -209,7 +206,6 @@ static void auth_start_transport_op(grpc_exec_ctx *exec_ctx,
call_data *calld = elem->call_data;
channel_data *chand = elem->channel_data;
grpc_linked_mdelem *l;
- size_t i;
grpc_client_security_context *sec_ctx = NULL;
if (calld->security_context_set == 0 &&
@@ -228,53 +224,41 @@ static void auth_start_transport_op(grpc_exec_ctx *exec_ctx,
chand->security_connector->base.auth_context, "client_auth_filter");
}
- if (op->bind_pollset != NULL) {
- calld->pollset = op->bind_pollset;
- }
-
- if (op->send_ops != NULL && !calld->sent_initial_metadata) {
- size_t nops = op->send_ops->nops;
- grpc_stream_op *ops = op->send_ops->ops;
- for (i = 0; i < nops; i++) {
- grpc_stream_op *sop = &ops[i];
- if (sop->type != GRPC_OP_METADATA) continue;
- calld->op_md_idx = i;
- calld->sent_initial_metadata = 1;
- for (l = sop->data.metadata.list.head; l != NULL; l = l->next) {
- grpc_mdelem *md = l->md;
- /* Pointer comparison is OK for md_elems created from the same context.
- */
- if (md->key == chand->authority_string) {
- if (calld->host != NULL) GRPC_MDSTR_UNREF(calld->host);
- calld->host = GRPC_MDSTR_REF(md->value);
- } else if (md->key == chand->path_string) {
- if (calld->method != NULL) GRPC_MDSTR_UNREF(calld->method);
- calld->method = GRPC_MDSTR_REF(md->value);
- }
+ if (op->send_initial_metadata != NULL) {
+ for (l = op->send_initial_metadata->list.head; l != NULL; l = l->next) {
+ grpc_mdelem *md = l->md;
+ /* Pointer comparison is OK for md_elems created from the same context.
+ */
+ if (md->key == chand->authority_string) {
+ if (calld->host != NULL) GRPC_MDSTR_UNREF(calld->host);
+ calld->host = GRPC_MDSTR_REF(md->value);
+ } else if (md->key == chand->path_string) {
+ if (calld->method != NULL) GRPC_MDSTR_UNREF(calld->method);
+ calld->method = GRPC_MDSTR_REF(md->value);
}
- if (calld->host != NULL) {
- grpc_security_status status;
- const char *call_host = grpc_mdstr_as_c_string(calld->host);
- calld->op = *op; /* Copy op (originates from the caller's stack). */
- status = grpc_channel_security_connector_check_call_host(
- exec_ctx, chand->security_connector, call_host, on_host_checked,
- elem);
- if (status != GRPC_SECURITY_OK) {
- if (status == GRPC_SECURITY_ERROR) {
- char *error_msg;
- gpr_asprintf(&error_msg,
- "Invalid host %s set in :authority metadata.",
- call_host);
- bubble_up_error(exec_ctx, elem, GRPC_STATUS_INVALID_ARGUMENT,
- error_msg);
- gpr_free(error_msg);
- }
- return; /* early exit */
+ }
+ if (calld->host != NULL) {
+ grpc_security_status status;
+ const char *call_host = grpc_mdstr_as_c_string(calld->host);
+ calld->op = *op; /* Copy op (originates from the caller's stack). */
+ status = grpc_channel_security_connector_check_call_host(
+ exec_ctx, chand->security_connector, call_host, on_host_checked,
+ elem);
+ if (status != GRPC_SECURITY_OK) {
+ if (status == GRPC_SECURITY_ERROR) {
+ char *error_msg;
+ gpr_asprintf(&error_msg,
+ "Invalid host %s set in :authority metadata.",
+ call_host);
+ bubble_up_error(exec_ctx, elem, GRPC_STATUS_INVALID_ARGUMENT,
+ error_msg);
+ gpr_free(error_msg);
}
+ return; /* early exit */
}
- send_security_metadata(exec_ctx, elem, op);
- return; /* early exit */
}
+ send_security_metadata(exec_ctx, elem, op);
+ return; /* early exit */
}
/* pass control down the stack */
@@ -283,11 +267,15 @@ static void auth_start_transport_op(grpc_exec_ctx *exec_ctx,
/* Constructor for call_data */
static void init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem,
- const void *server_transport_data,
- grpc_transport_stream_op *initial_op) {
+ grpc_call_element_args *args) {
call_data *calld = elem->call_data;
memset(calld, 0, sizeof(*calld));
- GPR_ASSERT(!initial_op || !initial_op->send_ops);
+}
+
+static void set_pollset(grpc_exec_ctx *exec_ctx, grpc_call_element *elem,
+ grpc_pollset *pollset) {
+ call_data *calld = elem->call_data;
+ calld->pollset = pollset;
}
/* Destructor for call_data */
@@ -306,18 +294,17 @@ static void destroy_call_elem(grpc_exec_ctx *exec_ctx,
/* Constructor for channel_data */
static void init_channel_elem(grpc_exec_ctx *exec_ctx,
- grpc_channel_element *elem, grpc_channel *master,
- const grpc_channel_args *args,
- grpc_mdctx *metadata_context, int is_first,
- int is_last) {
- grpc_security_connector *sc = grpc_find_security_connector_in_args(args);
+ grpc_channel_element *elem,
+ grpc_channel_element_args *args) {
+ grpc_security_connector *sc =
+ grpc_find_security_connector_in_args(args->channel_args);
/* grab pointers to our data from the channel element */
channel_data *chand = elem->channel_data;
/* The first and the last filters tend to be implemented differently to
handle the case that there's no 'next' filter to call on the up or down
path */
- GPR_ASSERT(!is_last);
+ GPR_ASSERT(!args->is_last);
GPR_ASSERT(sc != NULL);
/* initialize members */
@@ -325,7 +312,7 @@ static void init_channel_elem(grpc_exec_ctx *exec_ctx,
chand->security_connector =
(grpc_channel_security_connector *)GRPC_SECURITY_CONNECTOR_REF(
sc, "client_auth_filter");
- chand->md_ctx = metadata_context;
+ chand->md_ctx = args->metadata_context;
chand->authority_string = grpc_mdstr_from_string(chand->md_ctx, ":authority");
chand->path_string = grpc_mdstr_from_string(chand->md_ctx, ":path");
chand->error_msg_key = grpc_mdstr_from_string(chand->md_ctx, "grpc-message");
@@ -356,6 +343,6 @@ static void destroy_channel_elem(grpc_exec_ctx *exec_ctx,
const grpc_channel_filter grpc_client_auth_filter = {
auth_start_transport_op, grpc_channel_next_op, sizeof(call_data),
- init_call_elem, destroy_call_elem, sizeof(channel_data),
- init_channel_elem, destroy_channel_elem, grpc_call_next_get_peer,
+ init_call_elem, set_pollset, destroy_call_elem, sizeof(channel_data),
+ init_channel_elem, destroy_channel_elem, grpc_call_next_get_peer,
"client-auth"};
diff --git a/src/core/security/credentials.c b/src/core/security/credentials.c
index 5e155d83b9..2a8ffae5d7 100644
--- a/src/core/security/credentials.c
+++ b/src/core/security/credentials.c
@@ -200,8 +200,7 @@ grpc_arg grpc_server_credentials_to_arg(grpc_server_credentials *p) {
return arg;
}
-grpc_server_credentials *grpc_server_credentials_from_arg(
- const grpc_arg *arg) {
+grpc_server_credentials *grpc_server_credentials_from_arg(const grpc_arg *arg) {
if (strcmp(arg->key, GRPC_SERVER_CREDENTIALS_ARG) != 0) return NULL;
if (arg->type != GRPC_ARG_POINTER) {
gpr_log(GPR_ERROR, "Invalid type %d for arg %s", arg->type,
diff --git a/src/core/security/credentials.h b/src/core/security/credentials.h
index 01203b08f1..c74beef0e4 100644
--- a/src/core/security/credentials.h
+++ b/src/core/security/credentials.h
@@ -34,7 +34,7 @@
#ifndef GRPC_INTERNAL_CORE_SECURITY_CREDENTIALS_H
#define GRPC_INTERNAL_CORE_SECURITY_CREDENTIALS_H
-#include "src/core/transport/stream_op.h"
+#include "src/core/transport/metadata_batch.h"
#include <grpc/grpc.h>
#include <grpc/grpc_security.h>
#include <grpc/support/sync.h>
diff --git a/src/core/security/security_context.c b/src/core/security/security_context.c
index f544c1d943..ff3ad64d92 100644
--- a/src/core/security/security_context.c
+++ b/src/core/security/security_context.c
@@ -324,8 +324,7 @@ grpc_arg grpc_auth_context_to_arg(grpc_auth_context *p) {
return arg;
}
-grpc_auth_context *grpc_auth_context_from_arg(
- const grpc_arg *arg) {
+grpc_auth_context *grpc_auth_context_from_arg(const grpc_arg *arg) {
if (strcmp(arg->key, GRPC_AUTH_CONTEXT_ARG) != 0) return NULL;
if (arg->type != GRPC_ARG_POINTER) {
gpr_log(GPR_ERROR, "Invalid type %d for arg %s", arg->type,
@@ -340,8 +339,7 @@ grpc_auth_context *grpc_find_auth_context_in_args(
size_t i;
if (args == NULL) return NULL;
for (i = 0; i < args->num_args; i++) {
- grpc_auth_context *p =
- grpc_auth_context_from_arg(&args->args[i]);
+ grpc_auth_context *p = grpc_auth_context_from_arg(&args->args[i]);
if (p != NULL) return p;
}
return NULL;
diff --git a/src/core/security/server_auth_filter.c b/src/core/security/server_auth_filter.c
index 2e18369fe8..fee962b576 100644
--- a/src/core/security/server_auth_filter.c
+++ b/src/core/security/server_auth_filter.c
@@ -41,8 +41,7 @@
#include <grpc/support/log.h>
typedef struct call_data {
- gpr_uint8 got_client_metadata;
- grpc_stream_op_buffer *recv_ops;
+ grpc_metadata_batch *recv_initial_metadata;
/* Closure to call when finished with the auth_on_recv hook. */
grpc_closure *on_done_recv;
/* Receive closures are chained: we inject this closure as the on_done_recv
@@ -53,7 +52,6 @@ typedef struct call_data {
grpc_metadata_array md;
const grpc_metadata *consumed_md;
size_t num_consumed_md;
- grpc_stream_op *md_op;
grpc_auth_context *auth_context;
} call_data;
@@ -128,20 +126,28 @@ static void on_md_processing_done(
if (status == GRPC_STATUS_OK) {
calld->consumed_md = consumed_md;
calld->num_consumed_md = num_consumed_md;
- grpc_metadata_batch_filter(&calld->md_op->data.metadata, remove_consumed_md,
+ grpc_metadata_batch_filter(calld->recv_initial_metadata, remove_consumed_md,
elem);
grpc_metadata_array_destroy(&calld->md);
calld->on_done_recv->cb(&exec_ctx, calld->on_done_recv->cb_arg, 1);
} else {
gpr_slice message;
+ grpc_transport_stream_op close_op;
+ memset(&close_op, 0, sizeof(close_op));
grpc_metadata_array_destroy(&calld->md);
error_details = error_details != NULL
? error_details
: "Authentication metadata processing failed.";
message = gpr_slice_from_copied_string(error_details);
- grpc_sopb_reset(calld->recv_ops);
- grpc_transport_stream_op_add_close(&calld->transport_op, status, &message);
- grpc_call_next_op(&exec_ctx, elem, &calld->transport_op);
+ calld->transport_op.send_initial_metadata = NULL;
+ if (calld->transport_op.send_message != NULL) {
+ grpc_byte_stream_destroy(calld->transport_op.send_message);
+ calld->transport_op.send_message = NULL;
+ }
+ calld->transport_op.send_trailing_metadata = NULL;
+ grpc_transport_stream_op_add_close(&close_op, status, &message);
+ grpc_call_next_op(&exec_ctx, elem, &close_op);
+ calld->on_done_recv->cb(&exec_ctx, calld->on_done_recv->cb_arg, 0);
}
grpc_exec_ctx_finish(&exec_ctx);
@@ -153,16 +159,8 @@ static void auth_on_recv(grpc_exec_ctx *exec_ctx, void *user_data,
call_data *calld = elem->call_data;
channel_data *chand = elem->channel_data;
if (success) {
- size_t i;
- size_t nops = calld->recv_ops->nops;
- grpc_stream_op *ops = calld->recv_ops->ops;
- for (i = 0; i < nops; i++) {
- grpc_stream_op *op = &ops[i];
- if (op->type != GRPC_OP_METADATA || calld->got_client_metadata) continue;
- calld->got_client_metadata = 1;
- if (chand->creds->processor.process == NULL) continue;
- calld->md_op = op;
- calld->md = metadata_batch_to_md_array(&op->data.metadata);
+ if (chand->creds->processor.process != NULL) {
+ calld->md = metadata_batch_to_md_array(calld->recv_initial_metadata);
chand->creds->processor.process(
chand->creds->processor.state, calld->auth_context,
calld->md.metadata, calld->md.count, on_md_processing_done, elem);
@@ -176,11 +174,11 @@ static void set_recv_ops_md_callbacks(grpc_call_element *elem,
grpc_transport_stream_op *op) {
call_data *calld = elem->call_data;
- if (op->recv_ops && !calld->got_client_metadata) {
+ if (op->recv_initial_metadata != NULL) {
/* substitute our callback for the higher callback */
- calld->recv_ops = op->recv_ops;
- calld->on_done_recv = op->on_done_recv;
- op->on_done_recv = &calld->auth_on_recv;
+ calld->recv_initial_metadata = op->recv_initial_metadata;
+ calld->on_done_recv = op->on_complete;
+ op->on_complete = &calld->auth_on_recv;
calld->transport_op = *op;
}
}
@@ -199,8 +197,7 @@ static void auth_start_transport_op(grpc_exec_ctx *exec_ctx,
/* Constructor for call_data */
static void init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem,
- const void *server_transport_data,
- grpc_transport_stream_op *initial_op) {
+ grpc_call_element_args *args) {
/* grab pointers to our data from the call element */
call_data *calld = elem->call_data;
channel_data *chand = elem->channel_data;
@@ -210,47 +207,39 @@ static void init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem,
memset(calld, 0, sizeof(*calld));
grpc_closure_init(&calld->auth_on_recv, auth_on_recv, elem);
- GPR_ASSERT(initial_op && initial_op->context != NULL &&
- initial_op->context[GRPC_CONTEXT_SECURITY].value == NULL);
-
- /* Create a security context for the call and reference the auth context from
- the channel. */
- if (initial_op->context[GRPC_CONTEXT_SECURITY].value != NULL) {
- initial_op->context[GRPC_CONTEXT_SECURITY].destroy(
- initial_op->context[GRPC_CONTEXT_SECURITY].value);
+ if (args->context[GRPC_CONTEXT_SECURITY].value != NULL) {
+ args->context[GRPC_CONTEXT_SECURITY].destroy(
+ args->context[GRPC_CONTEXT_SECURITY].value);
}
+
server_ctx = grpc_server_security_context_create();
- server_ctx->auth_context =
- grpc_auth_context_create(chand->auth_context);
- server_ctx->auth_context->pollset = initial_op->bind_pollset;
- initial_op->context[GRPC_CONTEXT_SECURITY].value = server_ctx;
- initial_op->context[GRPC_CONTEXT_SECURITY].destroy =
- grpc_server_security_context_destroy;
+ server_ctx->auth_context = grpc_auth_context_create(chand->auth_context);
calld->auth_context = server_ctx->auth_context;
- /* Set the metadata callbacks. */
- set_recv_ops_md_callbacks(elem, initial_op);
+ args->context[GRPC_CONTEXT_SECURITY].value = server_ctx;
+ args->context[GRPC_CONTEXT_SECURITY].destroy =
+ grpc_server_security_context_destroy;
}
+static void set_pollset(grpc_exec_ctx *exec_ctx, grpc_call_element *elem,
+ grpc_pollset *pollset) {}
+
/* Destructor for call_data */
static void destroy_call_elem(grpc_exec_ctx *exec_ctx,
grpc_call_element *elem) {}
/* Constructor for channel_data */
static void init_channel_elem(grpc_exec_ctx *exec_ctx,
- grpc_channel_element *elem, grpc_channel *master,
- const grpc_channel_args *args, grpc_mdctx *mdctx,
- int is_first, int is_last) {
- grpc_auth_context *auth_context = grpc_find_auth_context_in_args(args);
- grpc_server_credentials *creds = grpc_find_server_credentials_in_args(args);
+ grpc_channel_element *elem,
+ grpc_channel_element_args *args) {
+ grpc_auth_context *auth_context =
+ grpc_find_auth_context_in_args(args->channel_args);
+ grpc_server_credentials *creds =
+ grpc_find_server_credentials_in_args(args->channel_args);
/* grab pointers to our data from the channel element */
channel_data *chand = elem->channel_data;
- /* The first and the last filters tend to be implemented differently to
- handle the case that there's no 'next' filter to call on the up or down
- path */
- GPR_ASSERT(!is_first);
- GPR_ASSERT(!is_last);
+ GPR_ASSERT(!args->is_last);
GPR_ASSERT(auth_context != NULL);
GPR_ASSERT(creds != NULL);
@@ -258,7 +247,7 @@ static void init_channel_elem(grpc_exec_ctx *exec_ctx,
chand->auth_context =
GRPC_AUTH_CONTEXT_REF(auth_context, "server_auth_filter");
chand->creds = grpc_server_credentials_ref(creds);
- chand->mdctx = mdctx;
+ chand->mdctx = args->metadata_context;
}
/* Destructor for channel data */
@@ -272,6 +261,6 @@ static void destroy_channel_elem(grpc_exec_ctx *exec_ctx,
const grpc_channel_filter grpc_server_auth_filter = {
auth_start_transport_op, grpc_channel_next_op, sizeof(call_data),
- init_call_elem, destroy_call_elem, sizeof(channel_data),
- init_channel_elem, destroy_channel_elem, grpc_call_next_get_peer,
+ init_call_elem, set_pollset, destroy_call_elem, sizeof(channel_data),
+ init_channel_elem, destroy_channel_elem, grpc_call_next_get_peer,
"server-auth"};
diff --git a/src/core/security/server_secure_chttp2.c b/src/core/security/server_secure_chttp2.c
index 82c639e830..851e0cfab3 100644
--- a/src/core/security/server_secure_chttp2.c
+++ b/src/core/security/server_secure_chttp2.c
@@ -94,8 +94,7 @@ static void setup_transport(grpc_exec_ctx *exec_ctx, void *statep,
grpc_channel_args *args_copy;
grpc_arg args_to_add[2];
args_to_add[0] = grpc_server_credentials_to_arg(state->creds);
- args_to_add[1] =
- grpc_auth_context_to_arg(state->sc->auth_context);
+ args_to_add[1] = grpc_auth_context_to_arg(state->sc->auth_context);
args_copy = grpc_channel_args_copy_and_add(
grpc_server_get_channel_args(state->server), args_to_add,
GPR_ARRAY_SIZE(args_to_add));
--
GitLab