diff --git a/src/core/security/client_auth_filter.c b/src/core/security/client_auth_filter.c index 635982b252a..c780b65873c 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 5e155d83b9c..2a8ffae5d7f 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 01203b08f1f..c74beef0e4f 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 #include #include diff --git a/src/core/security/security_context.c b/src/core/security/security_context.c index f544c1d943d..ff3ad64d92d 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 2e18369fe8c..fee962b5762 100644 --- a/src/core/security/server_auth_filter.c +++ b/src/core/security/server_auth_filter.c @@ -41,8 +41,7 @@ #include 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 82c639e8301..851e0cfab31 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));