From 77a7b870c3e4259cc8f5cffc2b59876b42c0624a Mon Sep 17 00:00:00 2001 From: Julien Boeuf Date: Wed, 5 Aug 2015 20:11:02 -0700 Subject: [PATCH] Fixing API (thanks Craig for the comments) and associated tests. --- include/grpc/grpc_security.h | 26 ++--- src/core/security/security_context.h | 11 +- src/core/security/server_auth_filter.c | 26 ++--- test/core/end2end/end2end_tests.h | 2 + .../end2end/fixtures/chttp2_fake_security.c | 25 ++++ .../fixtures/chttp2_simple_ssl_fullstack.c | 24 ++++ .../chttp2_simple_ssl_fullstack_with_poll.c | 24 ++++ .../chttp2_simple_ssl_with_oauth2_fullstack.c | 48 ++++++-- ...est_response_with_payload_and_call_creds.c | 110 +++++++++++++++++- 9 files changed, 242 insertions(+), 54 deletions(-) diff --git a/include/grpc/grpc_security.h b/include/grpc/grpc_security.h index 65887d86ba1..640c1fda986 100644 --- a/include/grpc/grpc_security.h +++ b/include/grpc/grpc_security.h @@ -255,12 +255,9 @@ void grpc_auth_context_release(grpc_auth_context *context); /* -- The following auth context methods should only be called by a server metadata - processor that will augment the channel auth context (see below). + processor to set properties extracted from auth metadata. -- */ -/* Creates a new auth context based off a chained context. */ -grpc_auth_context *grpc_auth_context_create(grpc_auth_context *chained); - /* Add a property. */ void grpc_auth_context_add_property(grpc_auth_context *ctx, const char *name, const char *value, size_t value_length); @@ -277,21 +274,22 @@ int grpc_auth_context_set_peer_identity_property_name(grpc_auth_context *ctx, /* --- Auth Metadata Processing --- */ -/* Opaque data structure useful for processors defined in core. */ -typedef struct grpc_auth_ticket grpc_auth_ticket; - /* Callback function that is called when the metadata processing is done. - success is 1 if processing succeeded, 0 otherwise. */ + success is 1 if processing succeeded, 0 otherwise. + Consumed metadata will be removed from the set of metadata available on the + call. */ typedef void (*grpc_process_auth_metadata_done_cb)( void *user_data, const grpc_metadata *consumed_md, size_t num_consumed_md, - int success, grpc_auth_context *result); + int success); -/* Pluggable server-side metadata processor object */ +/* Pluggable server-side metadata processor object. */ typedef struct { - void (*process)(void *state, grpc_auth_ticket *ticket, - grpc_auth_context *channel_ctx, const grpc_metadata *md, - size_t md_count, grpc_process_auth_metadata_done_cb cb, - void *user_data); + /* The context object is read/write: it contains the properties of the + channel peer and it is the job of the process function to augment it with + properties derived from the passed-in metadata. */ + void (*process)(void *state, grpc_auth_context *context, + const grpc_metadata *md, size_t md_count, + grpc_process_auth_metadata_done_cb cb, void *user_data); void *state; } grpc_auth_metadata_processor; diff --git a/src/core/security/security_context.h b/src/core/security/security_context.h index ddc0a7afad6..3ad57cadea6 100644 --- a/src/core/security/security_context.h +++ b/src/core/security/security_context.h @@ -37,11 +37,6 @@ #include "src/core/iomgr/pollset.h" #include "src/core/security/credentials.h" -/* --- grpc_auth_ticket --- */ -struct grpc_auth_ticket { - grpc_pollset *pollset; -}; - /* --- grpc_auth_context --- High level authentication context object. Can optionally be chained. */ @@ -59,8 +54,12 @@ struct grpc_auth_context { grpc_auth_property_array properties; gpr_refcount refcount; const char *peer_identity_property_name; + grpc_pollset *pollset; }; +/* Creation. */ +grpc_auth_context *grpc_auth_context_create(grpc_auth_context *chained); + /* Refcounting. */ #ifdef GRPC_AUTH_CONTEXT_REFCOUNT_DEBUG #define GRPC_AUTH_CONTEXT_REF(p, r) \ @@ -79,6 +78,8 @@ grpc_auth_context *grpc_auth_context_ref(grpc_auth_context *policy); void grpc_auth_context_unref(grpc_auth_context *policy); #endif +/* Get the pollset. */ + void grpc_auth_property_reset(grpc_auth_property *property); /* --- grpc_client_security_context --- diff --git a/src/core/security/server_auth_filter.c b/src/core/security/server_auth_filter.c index 41d31100016..2fc689caecd 100644 --- a/src/core/security/server_auth_filter.c +++ b/src/core/security/server_auth_filter.c @@ -53,8 +53,7 @@ typedef struct call_data { const grpc_metadata *consumed_md; size_t num_consumed_md; grpc_stream_op *md_op; - grpc_auth_context **call_auth_context; - grpc_auth_ticket ticket; + grpc_auth_context *auth_context; } call_data; typedef struct channel_data { @@ -107,8 +106,7 @@ static grpc_mdelem *remove_consumed_md(void *user_data, grpc_mdelem *md) { static void on_md_processing_done(void *user_data, const grpc_metadata *consumed_md, - size_t num_consumed_md, int success, - grpc_auth_context *result) { + size_t num_consumed_md, int success) { grpc_call_element *elem = user_data; call_data *calld = elem->call_data; @@ -117,11 +115,6 @@ static void on_md_processing_done(void *user_data, calld->num_consumed_md = num_consumed_md; grpc_metadata_batch_filter(&calld->md_op->data.metadata, remove_consumed_md, elem); - GPR_ASSERT(calld->call_auth_context != NULL); - GRPC_AUTH_CONTEXT_UNREF(*calld->call_auth_context, - "releasing old context."); - *calld->call_auth_context = - GRPC_AUTH_CONTEXT_REF(result, "refing new context."); calld->on_done_recv->cb(calld->on_done_recv->cb_arg, success); } else { gpr_slice message = gpr_slice_from_copied_string( @@ -149,8 +142,7 @@ static void auth_on_recv(void *user_data, int success) { if (chand->processor.process == NULL) continue; calld->md_op = op; md_array = metadata_batch_to_md_array(&op->data.metadata); - chand->processor.process(chand->processor.state, &calld->ticket, - chand->security_connector->auth_context, + chand->processor.process(chand->processor.state, calld->auth_context, md_array.metadata, md_array.count, on_md_processing_done, elem); grpc_metadata_array_destroy(&md_array); @@ -200,11 +192,6 @@ static void init_call_elem(grpc_call_element *elem, GPR_ASSERT(initial_op && initial_op->context != NULL && initial_op->context[GRPC_CONTEXT_SECURITY].value == NULL); - /* Get the pollset for the ticket. */ - if (initial_op->bind_pollset) { - calld->ticket.pollset = initial_op->bind_pollset; - } - /* Create a security context for the call and reference the auth context from the channel. */ if (initial_op->context[GRPC_CONTEXT_SECURITY].value != NULL) { @@ -212,12 +199,13 @@ static void init_call_elem(grpc_call_element *elem, initial_op->context[GRPC_CONTEXT_SECURITY].value); } server_ctx = grpc_server_security_context_create(); - server_ctx->auth_context = GRPC_AUTH_CONTEXT_REF( - chand->security_connector->auth_context, "server_security_context"); + server_ctx->auth_context = + grpc_auth_context_create(chand->security_connector->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; - calld->call_auth_context = &server_ctx->auth_context; + calld->auth_context = server_ctx->auth_context; /* Set the metadata callbacks. */ set_recv_ops_md_callbacks(elem, initial_op); diff --git a/test/core/end2end/end2end_tests.h b/test/core/end2end/end2end_tests.h index a18c7029514..3f1665613c6 100644 --- a/test/core/end2end/end2end_tests.h +++ b/test/core/end2end/end2end_tests.h @@ -43,6 +43,8 @@ typedef struct grpc_end2end_test_config grpc_end2end_test_config; #define FEATURE_MASK_SUPPORTS_HOSTNAME_VERIFICATION 2 #define FEATURE_MASK_SUPPORTS_PER_CALL_CREDENTIALS 4 +#define FAIL_AUTH_CHECK_SERVER_ARG_NAME "fail_auth_check" + struct grpc_end2end_test_fixture { grpc_completion_queue *cq; grpc_server *server; diff --git a/test/core/end2end/fixtures/chttp2_fake_security.c b/test/core/end2end/fixtures/chttp2_fake_security.c index f879b43f794..78b692a45dd 100644 --- a/test/core/end2end/fixtures/chttp2_fake_security.c +++ b/test/core/end2end/fixtures/chttp2_fake_security.c @@ -65,6 +65,14 @@ static grpc_end2end_test_fixture chttp2_create_fixture_secure_fullstack( return f; } +static void process_auth_failure(void *state, grpc_auth_context *ctx, + const grpc_metadata *md, size_t md_count, + grpc_process_auth_metadata_done_cb cb, + void *user_data) { + GPR_ASSERT(state == NULL); + cb(user_data, NULL, 0, 0); +} + static void chttp2_init_client_secure_fullstack(grpc_end2end_test_fixture *f, grpc_channel_args *client_args, grpc_credentials *creds) { @@ -102,10 +110,27 @@ static void chttp2_init_client_fake_secure_fullstack( chttp2_init_client_secure_fullstack(f, client_args, fake_ts_creds); } +static int fail_server_auth_check(grpc_channel_args *server_args) { + size_t i; + if (server_args == NULL) return 0; + for (i = 0; i < server_args->num_args; i++) { + if (strcmp(server_args->args[i].key, FAIL_AUTH_CHECK_SERVER_ARG_NAME) == + 0) { + return 1; + } + } + return 0; +} + static void chttp2_init_server_fake_secure_fullstack( grpc_end2end_test_fixture *f, grpc_channel_args *server_args) { grpc_server_credentials *fake_ts_creds = grpc_fake_transport_security_server_credentials_create(); + if (fail_server_auth_check(server_args)) { + grpc_auth_metadata_processor processor = {process_auth_failure, NULL}; + grpc_server_credentials_set_auth_metadata_processor(fake_ts_creds, + processor); + } chttp2_init_server_secure_fullstack(f, server_args, fake_ts_creds); } diff --git a/test/core/end2end/fixtures/chttp2_simple_ssl_fullstack.c b/test/core/end2end/fixtures/chttp2_simple_ssl_fullstack.c index 6d5669d05a8..9850aac69b0 100644 --- a/test/core/end2end/fixtures/chttp2_simple_ssl_fullstack.c +++ b/test/core/end2end/fixtures/chttp2_simple_ssl_fullstack.c @@ -68,6 +68,14 @@ static grpc_end2end_test_fixture chttp2_create_fixture_secure_fullstack( return f; } +static void process_auth_failure(void *state, grpc_auth_context *ctx, + const grpc_metadata *md, size_t md_count, + grpc_process_auth_metadata_done_cb cb, + void *user_data) { + GPR_ASSERT(state == NULL); + cb(user_data, NULL, 0, 0); +} + static void chttp2_init_client_secure_fullstack(grpc_end2end_test_fixture *f, grpc_channel_args *client_args, grpc_credentials *creds) { @@ -110,12 +118,28 @@ static void chttp2_init_client_simple_ssl_secure_fullstack( grpc_channel_args_destroy(new_client_args); } +static int fail_server_auth_check(grpc_channel_args *server_args) { + size_t i; + if (server_args == NULL) return 0; + for (i = 0; i < server_args->num_args; i++) { + if (strcmp(server_args->args[i].key, FAIL_AUTH_CHECK_SERVER_ARG_NAME) == + 0) { + return 1; + } + } + return 0; +} + static void chttp2_init_server_simple_ssl_secure_fullstack( grpc_end2end_test_fixture *f, grpc_channel_args *server_args) { grpc_ssl_pem_key_cert_pair pem_cert_key_pair = {test_server1_key, test_server1_cert}; grpc_server_credentials *ssl_creds = grpc_ssl_server_credentials_create(NULL, &pem_cert_key_pair, 1, 0); + if (fail_server_auth_check(server_args)) { + grpc_auth_metadata_processor processor = {process_auth_failure, NULL}; + grpc_server_credentials_set_auth_metadata_processor(ssl_creds, processor); + } chttp2_init_server_secure_fullstack(f, server_args, ssl_creds); } diff --git a/test/core/end2end/fixtures/chttp2_simple_ssl_fullstack_with_poll.c b/test/core/end2end/fixtures/chttp2_simple_ssl_fullstack_with_poll.c index d0cc3dd74a2..3df2acd296b 100644 --- a/test/core/end2end/fixtures/chttp2_simple_ssl_fullstack_with_poll.c +++ b/test/core/end2end/fixtures/chttp2_simple_ssl_fullstack_with_poll.c @@ -68,6 +68,14 @@ static grpc_end2end_test_fixture chttp2_create_fixture_secure_fullstack( return f; } +static void process_auth_failure(void *state, grpc_auth_context *ctx, + const grpc_metadata *md, size_t md_count, + grpc_process_auth_metadata_done_cb cb, + void *user_data) { + GPR_ASSERT(state == NULL); + cb(user_data, NULL, 0, 0); +} + static void chttp2_init_client_secure_fullstack(grpc_end2end_test_fixture *f, grpc_channel_args *client_args, grpc_credentials *creds) { @@ -110,12 +118,28 @@ static void chttp2_init_client_simple_ssl_secure_fullstack( grpc_channel_args_destroy(new_client_args); } +static int fail_server_auth_check(grpc_channel_args *server_args) { + size_t i; + if (server_args == NULL) return 0; + for (i = 0; i < server_args->num_args; i++) { + if (strcmp(server_args->args[i].key, FAIL_AUTH_CHECK_SERVER_ARG_NAME) == + 0) { + return 1; + } + } + return 0; +} + static void chttp2_init_server_simple_ssl_secure_fullstack( grpc_end2end_test_fixture *f, grpc_channel_args *server_args) { grpc_ssl_pem_key_cert_pair pem_cert_key_pair = {test_server1_key, test_server1_cert}; grpc_server_credentials *ssl_creds = grpc_ssl_server_credentials_create(NULL, &pem_cert_key_pair, 1, 0); + if (fail_server_auth_check(server_args)) { + grpc_auth_metadata_processor processor = {process_auth_failure, NULL}; + grpc_server_credentials_set_auth_metadata_processor(ssl_creds, processor); + } chttp2_init_server_secure_fullstack(f, server_args, ssl_creds); } diff --git a/test/core/end2end/fixtures/chttp2_simple_ssl_with_oauth2_fullstack.c b/test/core/end2end/fixtures/chttp2_simple_ssl_with_oauth2_fullstack.c index 1fc988c98e5..284d5f07ae9 100644 --- a/test/core/end2end/fixtures/chttp2_simple_ssl_with_oauth2_fullstack.c +++ b/test/core/end2end/fixtures/chttp2_simple_ssl_with_oauth2_fullstack.c @@ -68,22 +68,30 @@ static const grpc_metadata *find_metadata(const grpc_metadata *md, return NULL; } -void process_oauth2(void *state, grpc_auth_ticket *ticket, - grpc_auth_context *channel_ctx, const grpc_metadata *md, - size_t md_count, grpc_process_auth_metadata_done_cb cb, - void *user_data) { +static void process_oauth2_success(void *state, grpc_auth_context *ctx, + const grpc_metadata *md, size_t md_count, + grpc_process_auth_metadata_done_cb cb, + void *user_data) { const grpc_metadata *oauth2 = find_metadata(md, md_count, "Authorization", oauth2_md); - grpc_auth_context *new_ctx; GPR_ASSERT(state == NULL); GPR_ASSERT(oauth2 != NULL); - new_ctx = grpc_auth_context_create(channel_ctx); - grpc_auth_context_add_cstring_property(new_ctx, client_identity_property_name, + grpc_auth_context_add_cstring_property(ctx, client_identity_property_name, client_identity); GPR_ASSERT(grpc_auth_context_set_peer_identity_property_name( - new_ctx, client_identity_property_name) == 1); - cb(user_data, oauth2, 1, 1, new_ctx); - grpc_auth_context_release(new_ctx); + ctx, client_identity_property_name) == 1); + cb(user_data, oauth2, 1, 1); +} + +static void process_oauth2_failure(void *state, grpc_auth_context *ctx, + const grpc_metadata *md, size_t md_count, + grpc_process_auth_metadata_done_cb cb, + void *user_data) { + const grpc_metadata *oauth2 = + find_metadata(md, md_count, "Authorization", oauth2_md); + GPR_ASSERT(state == NULL); + GPR_ASSERT(oauth2 != NULL); + cb(user_data, oauth2, 1, 0); } static grpc_end2end_test_fixture chttp2_create_fixture_secure_fullstack( @@ -151,13 +159,31 @@ static void chttp2_init_client_simple_ssl_with_oauth2_secure_fullstack( grpc_credentials_release(oauth2_creds); } +static int fail_server_auth_check(grpc_channel_args *server_args) { + size_t i; + if (server_args == NULL) return 0; + for (i = 0; i < server_args->num_args; i++) { + if (strcmp(server_args->args[i].key, FAIL_AUTH_CHECK_SERVER_ARG_NAME) == + 0) { + return 1; + } + } + return 0; +} + static void chttp2_init_server_simple_ssl_secure_fullstack( grpc_end2end_test_fixture *f, grpc_channel_args *server_args) { grpc_ssl_pem_key_cert_pair pem_key_cert_pair = {test_server1_key, test_server1_cert}; grpc_server_credentials *ssl_creds = grpc_ssl_server_credentials_create(NULL, &pem_key_cert_pair, 1, 0); - grpc_auth_metadata_processor processor = {process_oauth2, NULL}; + grpc_auth_metadata_processor processor; + processor.state = NULL; + if (fail_server_auth_check(server_args)) { + processor.process = process_oauth2_failure; + } else { + processor.process = process_oauth2_success; + } grpc_server_credentials_set_auth_metadata_processor(ssl_creds, processor); chttp2_init_server_secure_fullstack(f, server_args, ssl_creds); } diff --git a/test/core/end2end/tests/request_response_with_payload_and_call_creds.c b/test/core/end2end/tests/request_response_with_payload_and_call_creds.c index e621fcbed2e..b4ccaf82164 100644 --- a/test/core/end2end/tests/request_response_with_payload_and_call_creds.c +++ b/test/core/end2end/tests/request_response_with_payload_and_call_creds.c @@ -57,13 +57,23 @@ enum { TIMEOUT = 200000 }; static void *tag(gpr_intptr t) { return (void *)t; } -static grpc_end2end_test_fixture begin_test( - grpc_end2end_test_config config, const char *test_name) { +static grpc_end2end_test_fixture begin_test(grpc_end2end_test_config config, + const char *test_name, + int fail_server_auth_check) { grpc_end2end_test_fixture f; gpr_log(GPR_INFO, "%s/%s", test_name, config.name); f = config.create_fixture(NULL, NULL); config.init_client(&f, NULL); - config.init_server(&f, NULL); + if (fail_server_auth_check) { + grpc_arg fail_auth_arg = { + GRPC_ARG_STRING, FAIL_AUTH_CHECK_SERVER_ARG_NAME, {NULL}}; + grpc_channel_args args; + args.num_args= 1; + args.args = &fail_auth_arg; + config.init_server(&f, &args); + } else { + config.init_server(&f, NULL); + } return f; } @@ -125,7 +135,8 @@ static void print_auth_context(int is_client, const grpc_auth_context *ctx) { static void test_call_creds_failure(grpc_end2end_test_config config) { grpc_call *c; grpc_credentials *creds = NULL; - grpc_end2end_test_fixture f = begin_test(config, "test_call_creds_failure"); + grpc_end2end_test_fixture f = + begin_test(config, "test_call_creds_failure", 0); gpr_timespec deadline = five_seconds_time(); c = grpc_channel_create_call(f.client, f.cq, "/foo", "foo.test.google.fr", deadline); @@ -172,7 +183,7 @@ static void request_response_with_payload_and_call_creds( grpc_auth_context *s_auth_context = NULL; grpc_auth_context *c_auth_context = NULL; - f = begin_test(config, test_name); + f = begin_test(config, test_name, 0); cqv = cq_verifier_create(f.cq); c = grpc_channel_create_call(f.client, f.cq, "/foo", "foo.test.google.fr", @@ -365,11 +376,100 @@ static void test_request_response_with_payload_and_deleted_call_creds( DESTROY); } +static void test_request_with_server_rejecting_client_creds( + grpc_end2end_test_config config) { + grpc_op ops[6]; + grpc_op *op; + grpc_call *c; + grpc_end2end_test_fixture f; + gpr_timespec deadline = five_seconds_time(); + cq_verifier *cqv; + grpc_metadata_array initial_metadata_recv; + grpc_metadata_array trailing_metadata_recv; + grpc_metadata_array request_metadata_recv; + grpc_call_details call_details; + grpc_status_code status; + char *details = NULL; + size_t details_capacity = 0; + grpc_byte_buffer *response_payload_recv = NULL; + gpr_slice request_payload_slice = gpr_slice_from_copied_string("hello world"); + grpc_byte_buffer *request_payload = + grpc_raw_byte_buffer_create(&request_payload_slice, 1); + grpc_credentials *creds; + + f = begin_test(config, "test_request_with_server_rejecting_client_creds", 1); + cqv = cq_verifier_create(f.cq); + + c = grpc_channel_create_call(f.client, f.cq, "/foo", "foo.test.google.fr", + deadline); + GPR_ASSERT(c); + + creds = grpc_iam_credentials_create(iam_token, iam_selector); + GPR_ASSERT(creds != NULL); + GPR_ASSERT(grpc_call_set_credentials(c, creds) == GRPC_CALL_OK); + grpc_credentials_release(creds); + + grpc_metadata_array_init(&initial_metadata_recv); + grpc_metadata_array_init(&trailing_metadata_recv); + grpc_metadata_array_init(&request_metadata_recv); + grpc_call_details_init(&call_details); + + op = ops; + op->op = GRPC_OP_RECV_STATUS_ON_CLIENT; + op->data.recv_status_on_client.trailing_metadata = &trailing_metadata_recv; + op->data.recv_status_on_client.status = &status; + op->data.recv_status_on_client.status_details = &details; + op->data.recv_status_on_client.status_details_capacity = &details_capacity; + op->flags = 0; + op++; + op->op = GRPC_OP_SEND_INITIAL_METADATA; + op->data.send_initial_metadata.count = 0; + op->flags = 0; + op++; + op->op = GRPC_OP_SEND_MESSAGE; + op->data.send_message = request_payload; + op->flags = 0; + op++; + op->op = GRPC_OP_SEND_CLOSE_FROM_CLIENT; + op->flags = 0; + op++; + op->op = GRPC_OP_RECV_INITIAL_METADATA; + op->data.recv_initial_metadata = &initial_metadata_recv; + op->flags = 0; + op++; + op->op = GRPC_OP_RECV_MESSAGE; + op->data.recv_message = &response_payload_recv; + op->flags = 0; + op++; + GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_batch(c, ops, op - ops, tag(1))); + + cq_expect_completion(cqv, tag(1), 1); + cq_verify(cqv); + + GPR_ASSERT(status == GRPC_STATUS_UNAUTHENTICATED); + + grpc_metadata_array_destroy(&initial_metadata_recv); + grpc_metadata_array_destroy(&trailing_metadata_recv); + grpc_metadata_array_destroy(&request_metadata_recv); + grpc_call_details_destroy(&call_details); + + grpc_byte_buffer_destroy(request_payload); + grpc_byte_buffer_destroy(response_payload_recv); + gpr_free(details); + + grpc_call_destroy(c); + + cq_verifier_destroy(cqv); + end_test(&f); + config.tear_down_data(&f); +} + void grpc_end2end_tests(grpc_end2end_test_config config) { if (config.feature_mask & FEATURE_MASK_SUPPORTS_PER_CALL_CREDENTIALS) { test_call_creds_failure(config); test_request_response_with_payload_and_call_creds(config); test_request_response_with_payload_and_overridden_call_creds(config); test_request_response_with_payload_and_deleted_call_creds(config); + test_request_with_server_rejecting_client_creds(config); } }