From ea456fc2bf09e1b80a3add3b898175605da3bf60 Mon Sep 17 00:00:00 2001 From: Julien Boeuf Date: Tue, 7 Jul 2015 15:23:30 -0700 Subject: [PATCH 1/9] Server auth metadata processor. - Right now it is a global function: would be better to have this per (secure) port. - Changed the interface of the auth_context slightly to make it more friendly. - Positive tests pass. Still need some work on error case (have a negative case as well). - Fixing cpp auth context tests so that they use the shiny new C API. --- include/grpc/grpc_security.h | 46 ++- src/core/security/credentials.c | 48 +-- src/core/security/credentials.h | 13 +- src/core/security/security_connector.c | 32 +- src/core/security/security_context.c | 101 ++++-- src/core/security/security_context.h | 34 +- src/core/security/server_auth_filter.c | 166 ++++++++- src/core/transport/stream_op.h | 2 +- .../chttp2_simple_ssl_with_oauth2_fullstack.c | 4 +- ...est_response_with_payload_and_call_creds.c | 322 +++++++++++++++++- test/core/security/auth_context_test.c | 73 ++-- test/core/security/credentials_test.c | 8 +- .../cpp/common/auth_property_iterator_test.cc | 15 +- test/cpp/common/secure_auth_context_test.cc | 22 +- 14 files changed, 715 insertions(+), 171 deletions(-) diff --git a/include/grpc/grpc_security.h b/include/grpc/grpc_security.h index 37d66c04ae5..9e193e697f8 100644 --- a/include/grpc/grpc_security.h +++ b/include/grpc/grpc_security.h @@ -203,8 +203,6 @@ grpc_call_error grpc_call_set_credentials(grpc_call *call, /* --- Authentication Context. --- */ -/* TODO(jboeuf): Define some well-known property names. */ - #define GRPC_TRANSPORT_SECURITY_TYPE_PROPERTY_NAME "transport_security_type" #define GRPC_FAKE_TRANSPORT_SECURITY_TYPE "fake" #define GRPC_SSL_TRANSPORT_SECURITY_TYPE "ssl" @@ -260,6 +258,50 @@ grpc_auth_context *grpc_call_auth_context(grpc_call *call); /* Releases the auth context returned from grpc_call_auth_context. */ 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). + -- */ + +/* 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); + +/* Add a C string property. */ +void grpc_auth_context_add_cstring_property(grpc_auth_context *ctx, + const char *name, + const char *value); + +/* Sets the property name. Returns 1 if successful or 0 in case of failure + (which means that no property with this name exists). */ +int grpc_auth_context_set_peer_identity_property_name(grpc_auth_context *ctx, + const char *name); + +/* --- 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. */ +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); + +/* Pluggable metadata processing function */ +typedef void (*grpc_process_auth_metadata_func)( + 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); + +/* Registration function for metadata processing. + Should be called before the server is started. */ +void grpc_server_auth_context_register_process_metadata_func( + grpc_process_auth_metadata_func func); + #ifdef __cplusplus } #endif diff --git a/src/core/security/credentials.c b/src/core/security/credentials.c index 230f0dfb85f..71513bcc25b 100644 --- a/src/core/security/credentials.c +++ b/src/core/security/credentials.c @@ -759,19 +759,19 @@ grpc_credentials *grpc_refresh_token_credentials_create( grpc_auth_refresh_token_create_from_string(json_refresh_token)); } -/* -- Fake Oauth2 credentials. -- */ +/* -- Metadata-only credentials. -- */ -static void fake_oauth2_destroy(grpc_credentials *creds) { - grpc_fake_oauth2_credentials *c = (grpc_fake_oauth2_credentials *)creds; - grpc_credentials_md_store_unref(c->access_token_md); +static void md_only_test_destroy(grpc_credentials *creds) { + grpc_md_only_test_credentials *c = (grpc_md_only_test_credentials *)creds; + grpc_credentials_md_store_unref(c->md_store); gpr_free(c); } -static int fake_oauth2_has_request_metadata(const grpc_credentials *creds) { +static int md_only_test_has_request_metadata(const grpc_credentials *creds) { return 1; } -static int fake_oauth2_has_request_metadata_only( +static int md_only_test_has_request_metadata_only( const grpc_credentials *creds) { return 1; } @@ -779,19 +779,19 @@ static int fake_oauth2_has_request_metadata_only( void on_simulated_token_fetch_done(void *user_data, int success) { grpc_credentials_metadata_request *r = (grpc_credentials_metadata_request *)user_data; - grpc_fake_oauth2_credentials *c = (grpc_fake_oauth2_credentials *)r->creds; + grpc_md_only_test_credentials *c = (grpc_md_only_test_credentials *)r->creds; GPR_ASSERT(success); - r->cb(r->user_data, c->access_token_md->entries, - c->access_token_md->num_entries, GRPC_CREDENTIALS_OK); + r->cb(r->user_data, c->md_store->entries, + c->md_store->num_entries, GRPC_CREDENTIALS_OK); grpc_credentials_metadata_request_destroy(r); } -static void fake_oauth2_get_request_metadata(grpc_credentials *creds, +static void md_only_test_get_request_metadata(grpc_credentials *creds, grpc_pollset *pollset, const char *service_url, grpc_credentials_metadata_cb cb, void *user_data) { - grpc_fake_oauth2_credentials *c = (grpc_fake_oauth2_credentials *)creds; + grpc_md_only_test_credentials *c = (grpc_md_only_test_credentials *)creds; if (c->is_async) { grpc_credentials_metadata_request *cb_arg = @@ -800,26 +800,26 @@ static void fake_oauth2_get_request_metadata(grpc_credentials *creds, on_simulated_token_fetch_done, cb_arg); grpc_iomgr_add_callback(cb_arg->on_simulated_token_fetch_done_closure); } else { - cb(user_data, c->access_token_md->entries, 1, GRPC_CREDENTIALS_OK); + cb(user_data, c->md_store->entries, 1, GRPC_CREDENTIALS_OK); } } -static grpc_credentials_vtable fake_oauth2_vtable = { - fake_oauth2_destroy, fake_oauth2_has_request_metadata, - fake_oauth2_has_request_metadata_only, fake_oauth2_get_request_metadata, +static grpc_credentials_vtable md_only_test_vtable = { + md_only_test_destroy, md_only_test_has_request_metadata, + md_only_test_has_request_metadata_only, md_only_test_get_request_metadata, NULL}; -grpc_credentials *grpc_fake_oauth2_credentials_create( - const char *token_md_value, int is_async) { - grpc_fake_oauth2_credentials *c = - gpr_malloc(sizeof(grpc_fake_oauth2_credentials)); - memset(c, 0, sizeof(grpc_fake_oauth2_credentials)); +grpc_credentials *grpc_md_only_test_credentials_create(const char *md_key, + const char *md_value, + int is_async) { + grpc_md_only_test_credentials *c = + gpr_malloc(sizeof(grpc_md_only_test_credentials)); + memset(c, 0, sizeof(grpc_md_only_test_credentials)); c->base.type = GRPC_CREDENTIALS_TYPE_OAUTH2; - c->base.vtable = &fake_oauth2_vtable; + c->base.vtable = &md_only_test_vtable; gpr_ref_init(&c->base.refcount, 1); - c->access_token_md = grpc_credentials_md_store_create(1); - grpc_credentials_md_store_add_cstrings( - c->access_token_md, GRPC_AUTHORIZATION_METADATA_KEY, token_md_value); + c->md_store = grpc_credentials_md_store_create(1); + grpc_credentials_md_store_add_cstrings(c->md_store, md_key, md_value); c->is_async = is_async; return &c->base; } diff --git a/src/core/security/credentials.h b/src/core/security/credentials.h index d988901cf74..664524522ba 100644 --- a/src/core/security/credentials.h +++ b/src/core/security/credentials.h @@ -182,9 +182,10 @@ grpc_oauth2_token_fetcher_credentials_parse_server_response( grpc_credentials_md_store **token_md, gpr_timespec *token_lifetime); void grpc_flush_cached_google_default_credentials(void); -/* Simulates an oauth2 token fetch with the specified value for testing. */ -grpc_credentials *grpc_fake_oauth2_credentials_create( - const char *token_md_value, int is_async); +/* Metadata-only credentials with the specified key and value where + asynchronicity can be simulated for testing. */ +grpc_credentials *grpc_md_only_test_credentials_create( + const char *md_key, const char *md_value, int is_async); /* Private constructor for jwt credentials from an already parsed json key. Takes ownership of the key. */ @@ -288,13 +289,13 @@ typedef struct { grpc_credentials_md_store *access_token_md; } grpc_access_token_credentials; -/* -- Fake Oauth2 credentials. -- */ +/* -- Metadata-only Test credentials. -- */ typedef struct { grpc_credentials base; - grpc_credentials_md_store *access_token_md; + grpc_credentials_md_store *md_store; int is_async; -} grpc_fake_oauth2_credentials; +} grpc_md_only_test_credentials; /* -- IAM credentials. -- */ diff --git a/src/core/security/security_connector.c b/src/core/security/security_connector.c index f6e423eb279..8aee8f3ef49 100644 --- a/src/core/security/security_connector.c +++ b/src/core/security/security_connector.c @@ -263,9 +263,9 @@ static grpc_security_status fake_check_peer(grpc_security_connector *sc, goto end; } GRPC_AUTH_CONTEXT_UNREF(sc->auth_context, "connector"); - sc->auth_context = grpc_auth_context_create(NULL, 1); - sc->auth_context->properties[0] = grpc_auth_property_init_from_cstring( - GRPC_TRANSPORT_SECURITY_TYPE_PROPERTY_NAME, + sc->auth_context = grpc_auth_context_create(NULL); + grpc_auth_context_add_cstring_property( + sc->auth_context, GRPC_TRANSPORT_SECURITY_TYPE_PROPERTY_NAME, GRPC_FAKE_TRANSPORT_SECURITY_TYPE); end: @@ -409,31 +409,35 @@ static int ssl_host_matches_name(const tsi_peer *peer, const char *peer_name) { grpc_auth_context *tsi_ssl_peer_to_auth_context(const tsi_peer *peer) { size_t i; grpc_auth_context *ctx = NULL; + const char *peer_identity_property_name = NULL; /* The caller has checked the certificate type property. */ GPR_ASSERT(peer->property_count >= 1); - ctx = grpc_auth_context_create(NULL, peer->property_count); - ctx->properties[0] = grpc_auth_property_init_from_cstring( - GRPC_TRANSPORT_SECURITY_TYPE_PROPERTY_NAME, + ctx = grpc_auth_context_create(NULL); + grpc_auth_context_add_cstring_property( + ctx, GRPC_TRANSPORT_SECURITY_TYPE_PROPERTY_NAME, GRPC_SSL_TRANSPORT_SECURITY_TYPE); - ctx->property_count = 1; for (i = 0; i < peer->property_count; i++) { const tsi_peer_property *prop = &peer->properties[i]; if (prop->name == NULL) continue; if (strcmp(prop->name, TSI_X509_SUBJECT_COMMON_NAME_PEER_PROPERTY) == 0) { /* If there is no subject alt name, have the CN as the identity. */ - if (ctx->peer_identity_property_name == NULL) { - ctx->peer_identity_property_name = GRPC_X509_CN_PROPERTY_NAME; + if (peer_identity_property_name == NULL) { + peer_identity_property_name = GRPC_X509_CN_PROPERTY_NAME; } - ctx->properties[ctx->property_count++] = grpc_auth_property_init( - GRPC_X509_CN_PROPERTY_NAME, prop->value.data, prop->value.length); + grpc_auth_context_add_property(ctx, GRPC_X509_CN_PROPERTY_NAME, + prop->value.data, prop->value.length); } else if (strcmp(prop->name, TSI_X509_SUBJECT_ALTERNATIVE_NAME_PEER_PROPERTY) == 0) { - ctx->peer_identity_property_name = GRPC_X509_SAN_PROPERTY_NAME; - ctx->properties[ctx->property_count++] = grpc_auth_property_init( - GRPC_X509_SAN_PROPERTY_NAME, prop->value.data, prop->value.length); + peer_identity_property_name = GRPC_X509_SAN_PROPERTY_NAME; + grpc_auth_context_add_property(ctx, GRPC_X509_SAN_PROPERTY_NAME, + prop->value.data, prop->value.length); } } + if (peer_identity_property_name != NULL) { + GPR_ASSERT(grpc_auth_context_set_peer_identity_property_name( + ctx, peer_identity_property_name) == 1); + } return ctx; } diff --git a/src/core/security/security_context.c b/src/core/security/security_context.c index 8ce7876bd8f..0015d5b9150 100644 --- a/src/core/security/security_context.c +++ b/src/core/security/security_context.c @@ -42,6 +42,20 @@ #include #include +/* --- grpc_process_auth_metadata_func --- */ + +static grpc_process_auth_metadata_func server_md_func = NULL; + +void grpc_server_auth_context_register_process_metadata_func( + grpc_process_auth_metadata_func func) { + server_md_func = func; +} + +grpc_process_auth_metadata_func +grpc_server_auth_context_get_process_metadata_func(void) { + return server_md_func; +} + /* --- grpc_call --- */ grpc_call_error grpc_call_set_credentials(grpc_call *call, @@ -120,15 +134,15 @@ void grpc_server_security_context_destroy(void *ctx) { static grpc_auth_property_iterator empty_iterator = {NULL, 0, NULL}; -grpc_auth_context *grpc_auth_context_create(grpc_auth_context *chained, - size_t property_count) { +grpc_auth_context *grpc_auth_context_create(grpc_auth_context *chained) { grpc_auth_context *ctx = gpr_malloc(sizeof(grpc_auth_context)); memset(ctx, 0, sizeof(grpc_auth_context)); - ctx->properties = gpr_malloc(property_count * sizeof(grpc_auth_property)); - memset(ctx->properties, 0, property_count * sizeof(grpc_auth_property)); - ctx->property_count = property_count; gpr_ref_init(&ctx->refcount, 1); - if (chained != NULL) ctx->chained = GRPC_AUTH_CONTEXT_REF(chained, "chained"); + if (chained != NULL) { + ctx->chained = GRPC_AUTH_CONTEXT_REF(chained, "chained"); + ctx->peer_identity_property_name = + ctx->chained->peer_identity_property_name; + } return ctx; } @@ -162,11 +176,11 @@ void grpc_auth_context_unref(grpc_auth_context *ctx) { if (gpr_unref(&ctx->refcount)) { size_t i; GRPC_AUTH_CONTEXT_UNREF(ctx->chained, "chained"); - if (ctx->properties != NULL) { - for (i = 0; i < ctx->property_count; i++) { - grpc_auth_property_reset(&ctx->properties[i]); + if (ctx->properties.array != NULL) { + for (i = 0; i < ctx->properties.count; i++) { + grpc_auth_property_reset(&ctx->properties.array[i]); } - gpr_free(ctx->properties); + gpr_free(ctx->properties.array); } gpr_free(ctx); } @@ -177,6 +191,20 @@ const char *grpc_auth_context_peer_identity_property_name( return ctx->peer_identity_property_name; } +int grpc_auth_context_set_peer_identity_property_name(grpc_auth_context *ctx, + const char *name) { + grpc_auth_property_iterator it = + grpc_auth_context_find_properties_by_name(ctx, name); + const grpc_auth_property *prop = grpc_auth_property_iterator_next(&it); + if (prop == NULL) { + gpr_log(GPR_ERROR, "Property name %s not found in auth context.", + name != NULL ? name : "NULL"); + return 0; + } + ctx->peer_identity_property_name = prop->name; + return 1; +} + int grpc_auth_context_peer_is_authenticated( const grpc_auth_context *ctx) { return ctx->peer_identity_property_name == NULL ? 0 : 1; @@ -193,16 +221,16 @@ grpc_auth_property_iterator grpc_auth_context_property_iterator( const grpc_auth_property *grpc_auth_property_iterator_next( grpc_auth_property_iterator *it) { if (it == NULL || it->ctx == NULL) return NULL; - while (it->index == it->ctx->property_count) { + while (it->index == it->ctx->properties.count) { if (it->ctx->chained == NULL) return NULL; it->ctx = it->ctx->chained; it->index = 0; } if (it->name == NULL) { - return &it->ctx->properties[it->index++]; + return &it->ctx->properties.array[it->index++]; } else { - while (it->index < it->ctx->property_count) { - const grpc_auth_property *prop = &it->ctx->properties[it->index++]; + while (it->index < it->ctx->properties.count) { + const grpc_auth_property *prop = &it->ctx->properties.array[it->index++]; GPR_ASSERT(prop->name != NULL); if (strcmp(it->name, prop->name) == 0) { return prop; @@ -229,24 +257,37 @@ grpc_auth_property_iterator grpc_auth_context_peer_identity( ctx, ctx->peer_identity_property_name); } -grpc_auth_property grpc_auth_property_init_from_cstring(const char *name, - const char *value) { - grpc_auth_property prop; - prop.name = gpr_strdup(name); - prop.value = gpr_strdup(value); - prop.value_length = strlen(value); - return prop; +static void ensure_auth_context_capacity(grpc_auth_context *ctx) { + if (ctx->properties.count == ctx->properties.capacity) { + ctx->properties.capacity = + GPR_MAX(ctx->properties.capacity + 8, ctx->properties.capacity * 2); + ctx->properties.array = + gpr_realloc(ctx->properties.array, + ctx->properties.capacity * sizeof(grpc_auth_property)); + } +} + +void grpc_auth_context_add_property(grpc_auth_context *ctx, const char *name, + const char *value, size_t value_length) { + grpc_auth_property *prop; + ensure_auth_context_capacity(ctx); + prop = &ctx->properties.array[ctx->properties.count++]; + prop->name = gpr_strdup(name); + prop->value = gpr_malloc(value_length + 1); + memcpy(prop->value, value, value_length); + prop->value[value_length] = '\0'; + prop->value_length = value_length; } -grpc_auth_property grpc_auth_property_init(const char *name, const char *value, - size_t value_length) { - grpc_auth_property prop; - prop.name = gpr_strdup(name); - prop.value = gpr_malloc(value_length + 1); - memcpy(prop.value, value, value_length); - prop.value[value_length] = '\0'; - prop.value_length = value_length; - return prop; +void grpc_auth_context_add_cstring_property(grpc_auth_context *ctx, + const char *name, + const char *value) { + grpc_auth_property *prop; + ensure_auth_context_capacity(ctx); + prop = &ctx->properties.array[ctx->properties.count++]; + prop->name = gpr_strdup(name); + prop->value = gpr_strdup(value); + prop->value_length = strlen(value); } void grpc_auth_property_reset(grpc_auth_property *property) { diff --git a/src/core/security/security_context.h b/src/core/security/security_context.h index 76a45910bbd..b5dfae06662 100644 --- a/src/core/security/security_context.h +++ b/src/core/security/security_context.h @@ -34,11 +34,13 @@ #ifndef GRPC_INTERNAL_CORE_SECURITY_SECURITY_CONTEXT_H #define GRPC_INTERNAL_CORE_SECURITY_SECURITY_CONTEXT_H +#include "src/core/iomgr/pollset.h" #include "src/core/security/credentials.h" -#ifdef __cplusplus -extern "C" { -#endif +/* --- grpc_auth_ticket --- */ +struct grpc_auth_ticket { + grpc_pollset *pollset; +}; /* --- grpc_auth_context --- @@ -46,18 +48,19 @@ extern "C" { /* Property names are always NULL terminated. */ +typedef struct { + grpc_auth_property *array; + size_t count; + size_t capacity; +} grpc_auth_property_array; + struct grpc_auth_context { struct grpc_auth_context *chained; - grpc_auth_property *properties; - size_t property_count; + grpc_auth_property_array properties; gpr_refcount refcount; const char *peer_identity_property_name; }; -/* Constructor. */ -grpc_auth_context *grpc_auth_context_create(grpc_auth_context *chained, - size_t property_count); - /* Refcounting. */ #ifdef GRPC_AUTH_CONTEXT_REFCOUNT_DEBUG #define GRPC_AUTH_CONTEXT_REF(p, r) \ @@ -76,12 +79,6 @@ grpc_auth_context *grpc_auth_context_ref(grpc_auth_context *policy); void grpc_auth_context_unref(grpc_auth_context *policy); #endif -grpc_auth_property grpc_auth_property_init_from_cstring(const char *name, - const char *value); - -grpc_auth_property grpc_auth_property_init(const char *name, const char *value, - size_t value_length); - void grpc_auth_property_reset(grpc_auth_property *property); /* --- grpc_client_security_context --- @@ -107,9 +104,10 @@ typedef struct { grpc_server_security_context *grpc_server_security_context_create(void); void grpc_server_security_context_destroy(void *ctx); -#ifdef __cplusplus -} -#endif +/* --- Auth metadata processing. --- */ + +grpc_process_auth_metadata_func +grpc_server_auth_context_get_process_metadata_func(void); #endif /* GRPC_INTERNAL_CORE_SECURITY_SECURITY_CONTEXT_H */ diff --git a/src/core/security/server_auth_filter.c b/src/core/security/server_auth_filter.c index 10eef6d2378..fe993e50ee9 100644 --- a/src/core/security/server_auth_filter.c +++ b/src/core/security/server_auth_filter.c @@ -31,20 +31,161 @@ * */ +#include + #include "src/core/security/auth_filters.h" #include "src/core/security/security_connector.h" #include "src/core/security/security_context.h" +#include #include typedef struct call_data { - int unused; /* C89 requires at least one struct element */ + gpr_uint8 got_client_metadata; + gpr_uint8 sent_status; + gpr_uint8 success; + grpc_linked_mdelem status; + grpc_stream_op_buffer *recv_ops; + /* Closure to call when finished with the hs_on_recv hook. */ + grpc_iomgr_closure *on_done_recv; + /* Receive closures are chained: we inject this closure as the on_done_recv + up-call on transport_op, and remember to call our on_done_recv member after + handling it. */ + grpc_iomgr_closure auth_on_recv; + 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; } call_data; typedef struct channel_data { grpc_security_connector *security_connector; + grpc_mdelem *status_auth_failure; } channel_data; +static grpc_metadata_array metadata_batch_to_md_array( + const grpc_metadata_batch *batch) { + grpc_linked_mdelem *l; + grpc_metadata_array result; + grpc_metadata_array_init(&result); + for (l = batch->list.head; l != NULL; l = l->next) { + grpc_metadata *usr_md = NULL; + grpc_mdelem *md = l->md; + grpc_mdstr *key = md->key; + grpc_mdstr *value = md->value; + if (result.count == result.capacity) { + result.capacity = GPR_MAX(result.capacity + 8, result.capacity * 2); + result.metadata = + gpr_realloc(result.metadata, result.capacity * sizeof(grpc_metadata)); + } + usr_md = &result.metadata[result.count++]; + usr_md->key = grpc_mdstr_as_c_string(key); + usr_md->value = grpc_mdstr_as_c_string(value); + usr_md->value_length = GPR_SLICE_LENGTH(value->slice); + } + return result; +} + +static grpc_mdelem *remove_consumed_md(void *user_data, grpc_mdelem *md) { + grpc_call_element *elem = user_data; + call_data *calld = elem->call_data; + size_t i; + for (i = 0; i < calld->num_consumed_md; i++) { + /* Maybe we could do a pointer comparison but we do not have any guarantee + that the metadata processor used the same pointers for consumed_md in the + callback. */ + if (memcmp(GPR_SLICE_START_PTR(md->key->slice), calld->consumed_md[i].key, + GPR_SLICE_LENGTH(md->key->slice)) == 0 && + memcmp(GPR_SLICE_START_PTR(md->value->slice), + calld->consumed_md[i].value, + GPR_SLICE_LENGTH(md->value->slice)) == 0) { + return NULL; /* Delete. */ + } + } + return 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) { + grpc_call_element *elem = user_data; + call_data *calld = elem->call_data; + + calld->success = success; + if (success) { + calld->consumed_md = consumed_md; + 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."); + } else { + grpc_call_element_send_cancel(elem); + } + + calld->on_done_recv->cb(calld->on_done_recv->cb_arg, success); +} + +static void auth_on_recv(void *user_data, int success) { + grpc_call_element *elem = 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_metadata_array md_array; + grpc_process_auth_metadata_func processor = + grpc_server_auth_context_get_process_metadata_func(); + grpc_stream_op *op = &ops[i]; + if (op->type != GRPC_OP_METADATA) continue; + calld->got_client_metadata = 1; + if (processor == NULL) continue; + calld->md_op = op; + md_array = metadata_batch_to_md_array(&op->data.metadata); + processor(&calld->ticket, chand->security_connector->auth_context, + md_array.metadata, md_array.count, on_md_processing_done, elem); + grpc_metadata_array_destroy(&md_array); + return; + } + } + calld->on_done_recv->cb(calld->on_done_recv->cb_arg, success); +} + +static void set_recv_ops_md_callbacks(grpc_call_element *elem, + grpc_transport_stream_op *op) { + call_data *calld = elem->call_data; + channel_data *chand = elem->channel_data; + + if (op->send_ops && !calld->sent_status && !calld->success) { + size_t i; + size_t nops = op->send_ops->nops; + grpc_stream_op *ops = op->send_ops->ops; + for (i = 0; i < nops; i++) { + grpc_stream_op *op = &ops[i]; + if (op->type != GRPC_OP_METADATA) continue; + calld->sent_status = 1; + grpc_metadata_batch_add_head( + &op->data.metadata, &calld->status, + GRPC_MDELEM_REF(chand->status_auth_failure)); + break; + } + } + + if (op->recv_ops && !calld->got_client_metadata) { + /* 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; + } +} + /* Called either: - in response to an API call (or similar) from above, to send something - a network event (or similar) from below, to receive something @@ -52,9 +193,7 @@ typedef struct channel_data { that is being sent or received. */ static void auth_start_transport_op(grpc_call_element *elem, grpc_transport_stream_op *op) { - /* TODO(jboeuf): Get the metadata and get a new context from it. */ - - /* pass control down the stack */ + set_recv_ops_md_callbacks(elem, op); grpc_call_next_op(elem, op); } @@ -68,11 +207,18 @@ static void init_call_elem(grpc_call_element *elem, grpc_server_security_context *server_ctx = NULL; /* initialize members */ - calld->unused = 0; + memset(calld, 0, sizeof(*calld)); + grpc_iomgr_closure_init(&calld->auth_on_recv, auth_on_recv, elem); + calld->success = 1; 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) { @@ -85,10 +231,15 @@ static void init_call_elem(grpc_call_element *elem, 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; + + /* Set the metadata callbacks. */ + set_recv_ops_md_callbacks(elem, initial_op); } /* Destructor for call_data */ -static void destroy_call_elem(grpc_call_element *elem) {} +static void destroy_call_elem(grpc_call_element *elem) { +} /* Constructor for channel_data */ static void init_channel_elem(grpc_channel_element *elem, grpc_channel *master, @@ -109,6 +260,8 @@ static void init_channel_elem(grpc_channel_element *elem, grpc_channel *master, GPR_ASSERT(!sc->is_client_side); chand->security_connector = GRPC_SECURITY_CONNECTOR_REF(sc, "server_auth_filter"); + chand->status_auth_failure = + grpc_mdelem_from_strings(mdctx, ":status", "401"); } /* Destructor for channel data */ @@ -117,6 +270,7 @@ static void destroy_channel_elem(grpc_channel_element *elem) { channel_data *chand = elem->channel_data; GRPC_SECURITY_CONNECTOR_UNREF(chand->security_connector, "server_auth_filter"); + GRPC_MDELEM_UNREF(chand->status_auth_failure); } const grpc_channel_filter grpc_server_auth_filter = { diff --git a/src/core/transport/stream_op.h b/src/core/transport/stream_op.h index 964d39d14fc..7d3024da101 100644 --- a/src/core/transport/stream_op.h +++ b/src/core/transport/stream_op.h @@ -103,7 +103,7 @@ void grpc_metadata_batch_merge(grpc_metadata_batch *target, grpc_metadata_batch *add); /** Add \a storage to the beginning of \a batch. storage->md is - assumed to be valid. + assumed to be valid. \a storage is owned by the caller and must survive for the lifetime of batch. This usually means it should be around for the lifetime of the call. */ 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 de418bf7ee0..da658a0b45a 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 @@ -100,8 +100,8 @@ static void chttp2_init_client_simple_ssl_with_oauth2_secure_fullstack( grpc_end2end_test_fixture *f, grpc_channel_args *client_args) { grpc_credentials *ssl_creds = grpc_ssl_credentials_create(test_root_cert, NULL); - grpc_credentials *oauth2_creds = - grpc_fake_oauth2_credentials_create("Bearer aaslkfjs424535asdf", 1); + grpc_credentials *oauth2_creds = grpc_md_only_test_credentials_create( + "Authorization", "Bearer aaslkfjs424535asdf", 1); grpc_credentials *ssl_oauth2_creds = grpc_composite_credentials_create(ssl_creds, oauth2_creds); grpc_arg ssl_name_override = {GRPC_ARG_STRING, 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 b5c743b4056..c0214081c5d 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 @@ -46,6 +46,11 @@ #include "src/core/security/credentials.h" #include "src/core/support/string.h" +static const char *custom_creds_md_name = "custom_creds"; +static const char *custom_creds_md_value = "custom_value"; +static const char *client_identity_property_name = "smurf_name"; +static const char *client_identity = "Brainy Smurf"; + static const char iam_token[] = "token"; static const char iam_selector[] = "selector"; static const char overridden_iam_token[] = "overridden_token"; @@ -57,15 +62,71 @@ 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, - grpc_channel_args *client_args, - grpc_channel_args *server_args) { +static const grpc_metadata *find_metadata(const grpc_metadata *md, + size_t md_count, + const char *key, + const char *value) { + size_t i; + for (i = 0; i < md_count; i++) { + if (strcmp(key, md[i].key) == 0 && strlen(value) == md[i].value_length && + memcmp(md[i].value, value, md[i].value_length) == 0) { + return &md[i]; + } + } + return NULL; +} + +static void check_peer_identity(grpc_auth_context *ctx, + const char *expected_identity) { + grpc_auth_property_iterator it = grpc_auth_context_peer_identity(ctx); + const grpc_auth_property *prop = grpc_auth_property_iterator_next(&it); + GPR_ASSERT(prop != NULL); + GPR_ASSERT(strcmp(expected_identity, prop->value) == 0); + GPR_ASSERT(grpc_auth_property_iterator_next(&it) == NULL); +} +static void process_auth_md_success(grpc_auth_ticket *t, + grpc_auth_context *channel_ctx, + const grpc_metadata *md, size_t md_count, + grpc_process_auth_metadata_done_cb cb, + void *user_data) { + grpc_auth_context *new_auth_ctx = grpc_auth_context_create(channel_ctx); + const grpc_metadata *custom_creds_md = + find_metadata(md, md_count, custom_creds_md_name, custom_creds_md_value); + GPR_ASSERT(custom_creds_md != NULL); + grpc_auth_context_add_cstring_property( + new_auth_ctx, client_identity_property_name, client_identity); + GPR_ASSERT(grpc_auth_context_set_peer_identity_property_name( + new_auth_ctx, client_identity_property_name) == 1); + cb(user_data, custom_creds_md, 1, 1, new_auth_ctx); + grpc_auth_context_release(new_auth_ctx); +} + +#if 0 +static void process_auth_md_failure(grpc_auth_ticket *t, + grpc_auth_context *channel_ctx, + const grpc_metadata *md, size_t md_count, + grpc_process_auth_metadata_done_cb cb, + void *user_data) { + const grpc_metadata *custom_creds_md = + find_metadata(md, md_count, custom_creds_md_name, custom_creds_md_value); + GPR_ASSERT(custom_creds_md != NULL); + cb(user_data, NULL, 0, 0, NULL); /* Fail. */ +} +#endif + +static grpc_end2end_test_fixture begin_test( + grpc_end2end_test_config config, const char *test_name, + grpc_process_auth_metadata_func md_func, override_mode mode) { grpc_end2end_test_fixture f; + if (mode != DESTROY) { + grpc_server_auth_context_register_process_metadata_func(md_func); + } else { + grpc_server_auth_context_register_process_metadata_func(NULL); + } gpr_log(GPR_INFO, "%s/%s", test_name, config.name); - f = config.create_fixture(client_args, server_args); - config.init_client(&f, client_args); - config.init_server(&f, server_args); + f = config.create_fixture(NULL, NULL); + config.init_client(&f, NULL); + config.init_server(&f, NULL); return f; } @@ -124,11 +185,23 @@ static void print_auth_context(int is_client, const grpc_auth_context *ctx) { } } +static grpc_credentials *iam_custom_composite_creds_create( + const char *iam_tok, const char *iam_sel) { + grpc_credentials *iam_creds = grpc_iam_credentials_create(iam_tok, iam_sel); + grpc_credentials *custom_creds = grpc_md_only_test_credentials_create( + custom_creds_md_name, custom_creds_md_value, 1); + grpc_credentials *result = + grpc_composite_credentials_create(iam_creds, custom_creds); + grpc_credentials_release(iam_creds); + grpc_credentials_release(custom_creds); + return result; +} + 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", NULL, NULL); + begin_test(config, "test_call_creds_failure", NULL, NONE); gpr_timespec deadline = five_seconds_time(); c = grpc_channel_create_call(f.client, f.cq, "/foo", "foo.test.google.fr", deadline); @@ -158,7 +231,8 @@ static void request_response_with_payload_and_call_creds( grpc_raw_byte_buffer_create(&response_payload_slice, 1); gpr_timespec deadline = five_seconds_time(); - grpc_end2end_test_fixture f = begin_test(config, test_name, NULL, NULL); + grpc_end2end_test_fixture f = + begin_test(config, test_name, process_auth_md_success, mode); cq_verifier *cqv = cq_verifier_create(f.cq); grpc_op ops[6]; grpc_op *op; @@ -174,11 +248,12 @@ static void request_response_with_payload_and_call_creds( int was_cancelled = 2; grpc_credentials *creds = NULL; grpc_auth_context *s_auth_context = NULL; + grpc_auth_context *c_auth_context = NULL; 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); + creds = iam_custom_composite_creds_create(iam_token, iam_selector); GPR_ASSERT(creds != NULL); GPR_ASSERT(grpc_call_set_credentials(c, creds) == GRPC_CALL_OK); switch (mode) { @@ -186,8 +261,8 @@ static void request_response_with_payload_and_call_creds( break; case OVERRIDE: grpc_credentials_release(creds); - creds = grpc_iam_credentials_create(overridden_iam_token, - overridden_iam_selector); + creds = iam_custom_composite_creds_create(overridden_iam_token, + overridden_iam_selector); GPR_ASSERT(creds != NULL); GPR_ASSERT(grpc_call_set_credentials(c, creds) == GRPC_CALL_OK); break; @@ -241,6 +316,11 @@ static void request_response_with_payload_and_call_creds( print_auth_context(0, s_auth_context); grpc_auth_context_release(s_auth_context); + c_auth_context = grpc_call_auth_context(c); + GPR_ASSERT(c_auth_context != NULL); + print_auth_context(1, c_auth_context); + grpc_auth_context_release(c_auth_context); + /* Cannot set creds on the server call object. */ GPR_ASSERT(grpc_call_set_credentials(s, NULL) != GRPC_CALL_OK); @@ -287,6 +367,10 @@ static void request_response_with_payload_and_call_creds( GPR_ASSERT(byte_buffer_eq_string(request_payload_recv, "hello world")); GPR_ASSERT(byte_buffer_eq_string(response_payload_recv, "hello you")); + /* Has been processed by the auth metadata processor. */ + GPR_ASSERT(!contains_metadata(&request_metadata_recv, custom_creds_md_name, + custom_creds_md_value)); + switch (mode) { case NONE: GPR_ASSERT(contains_metadata(&request_metadata_recv, @@ -295,6 +379,7 @@ static void request_response_with_payload_and_call_creds( GPR_ASSERT(contains_metadata(&request_metadata_recv, GRPC_IAM_AUTHORITY_SELECTOR_METADATA_KEY, iam_selector)); + check_peer_identity(s_auth_context, client_identity); break; case OVERRIDE: GPR_ASSERT(contains_metadata(&request_metadata_recv, @@ -303,6 +388,7 @@ static void request_response_with_payload_and_call_creds( GPR_ASSERT(contains_metadata(&request_metadata_recv, GRPC_IAM_AUTHORITY_SELECTOR_METADATA_KEY, overridden_iam_selector)); + check_peer_identity(s_auth_context, client_identity); break; case DESTROY: GPR_ASSERT(!contains_metadata(&request_metadata_recv, @@ -340,31 +426,237 @@ static void request_response_with_payload_and_call_creds( config.tear_down_data(&f); } -void test_request_response_with_payload_and_call_creds( +static void test_request_response_with_payload_and_call_creds( grpc_end2end_test_config config) { request_response_with_payload_and_call_creds( "test_request_response_with_payload_and_call_creds", config, NONE); } -void test_request_response_with_payload_and_overridden_call_creds( +static void test_request_response_with_payload_and_overridden_call_creds( grpc_end2end_test_config config) { request_response_with_payload_and_call_creds( "test_request_response_with_payload_and_overridden_call_creds", config, OVERRIDE); } -void test_request_response_with_payload_and_deleted_call_creds( +static void test_request_response_with_payload_and_deleted_call_creds( grpc_end2end_test_config config) { request_response_with_payload_and_call_creds( "test_request_response_with_payload_and_deleted_call_creds", config, DESTROY); } +static void test_request_with_bad_creds(void) { +#if 0 + grpc_call *c; + grpc_call *s; + 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); + gpr_timespec deadline = five_seconds_time(); + + grpc_end2end_test_fixture f = + begin_test(config, test_name, process_auth_md_failure, NONE); + cq_verifier *cqv = cq_verifier_create(f.cq); + grpc_op ops[6]; + grpc_op *op; + grpc_metadata_array initial_metadata_recv; + grpc_metadata_array trailing_metadata_recv; + grpc_metadata_array request_metadata_recv; + grpc_byte_buffer *request_payload_recv = NULL; + grpc_byte_buffer *response_payload_recv = NULL; + grpc_call_details call_details; + grpc_status_code status; + char *details = NULL; + size_t details_capacity = 0; + int was_cancelled = 2; + grpc_credentials *creds = NULL; + grpc_auth_context *s_auth_context = NULL; + grpc_auth_context *c_auth_context = NULL; + + c = grpc_channel_create_call(f.client, f.cq, "/foo", "foo.test.google.fr", + deadline); + GPR_ASSERT(c); + creds = iam_custom_composite_creds_create(iam_token, iam_selector); + GPR_ASSERT(creds != NULL); + GPR_ASSERT(grpc_call_set_credentials(c, creds) == GRPC_CALL_OK); + switch (mode) { + case NONE: + break; + case OVERRIDE: + grpc_credentials_release(creds); + creds = iam_custom_composite_creds_create(overridden_iam_token, + overridden_iam_selector); + GPR_ASSERT(creds != NULL); + GPR_ASSERT(grpc_call_set_credentials(c, creds) == GRPC_CALL_OK); + break; + case DESTROY: + GPR_ASSERT(grpc_call_set_credentials(c, NULL) == GRPC_CALL_OK); + break; + } + 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_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++; + 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++; + GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_batch(c, ops, op - ops, tag(1))); + + GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call( + f.server, &s, &call_details, + &request_metadata_recv, f.cq, f.cq, tag(101))); + cq_expect_completion(cqv, tag(101), 1); + cq_verify(cqv); + s_auth_context = grpc_call_auth_context(s); + GPR_ASSERT(s_auth_context != NULL); + print_auth_context(0, s_auth_context); + grpc_auth_context_release(s_auth_context); + + c_auth_context = grpc_call_auth_context(c); + GPR_ASSERT(c_auth_context != NULL); + print_auth_context(1, c_auth_context); + grpc_auth_context_release(c_auth_context); + + /* Cannot set creds on the server call object. */ + GPR_ASSERT(grpc_call_set_credentials(s, NULL) != GRPC_CALL_OK); + + op = ops; + op->op = GRPC_OP_SEND_INITIAL_METADATA; + op->data.send_initial_metadata.count = 0; + op->flags = 0; + op++; + op->op = GRPC_OP_RECV_MESSAGE; + op->data.recv_message = &request_payload_recv; + op->flags = 0; + op++; + GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_batch(s, ops, op - ops, tag(102))); + + cq_expect_completion(cqv, tag(102), 1); + cq_verify(cqv); + + op = ops; + op->op = GRPC_OP_RECV_CLOSE_ON_SERVER; + op->data.recv_close_on_server.cancelled = &was_cancelled; + op->flags = 0; + op++; + op->op = GRPC_OP_SEND_MESSAGE; + op->data.send_message = response_payload; + op->flags = 0; + op++; + op->op = GRPC_OP_SEND_STATUS_FROM_SERVER; + op->data.send_status_from_server.trailing_metadata_count = 0; + op->data.send_status_from_server.status = GRPC_STATUS_OK; + op->data.send_status_from_server.status_details = "xyz"; + op->flags = 0; + op++; + GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_batch(s, ops, op - ops, tag(103))); + + cq_expect_completion(cqv, tag(103), 1); + cq_expect_completion(cqv, tag(1), 1); + cq_verify(cqv); + + GPR_ASSERT(status == GRPC_STATUS_OK); + GPR_ASSERT(0 == strcmp(details, "xyz")); + GPR_ASSERT(0 == strcmp(call_details.method, "/foo")); + GPR_ASSERT(0 == strcmp(call_details.host, "foo.test.google.fr")); + GPR_ASSERT(was_cancelled == 0); + GPR_ASSERT(byte_buffer_eq_string(request_payload_recv, "hello world")); + GPR_ASSERT(byte_buffer_eq_string(response_payload_recv, "hello you")); + + /* Has been processed by the auth metadata processor. */ + GPR_ASSERT(!contains_metadata(&request_metadata_recv, custom_creds_md_name, + custom_creds_md_value)); + + switch (mode) { + case NONE: + GPR_ASSERT(contains_metadata(&request_metadata_recv, + GRPC_IAM_AUTHORIZATION_TOKEN_METADATA_KEY, + iam_token)); + GPR_ASSERT(contains_metadata(&request_metadata_recv, + GRPC_IAM_AUTHORITY_SELECTOR_METADATA_KEY, + iam_selector)); + check_peer_identity(s_auth_context, client_identity); + break; + case OVERRIDE: + GPR_ASSERT(contains_metadata(&request_metadata_recv, + GRPC_IAM_AUTHORIZATION_TOKEN_METADATA_KEY, + overridden_iam_token)); + GPR_ASSERT(contains_metadata(&request_metadata_recv, + GRPC_IAM_AUTHORITY_SELECTOR_METADATA_KEY, + overridden_iam_selector)); + check_peer_identity(s_auth_context, client_identity); + break; + case DESTROY: + GPR_ASSERT(!contains_metadata(&request_metadata_recv, + GRPC_IAM_AUTHORIZATION_TOKEN_METADATA_KEY, + iam_token)); + GPR_ASSERT(!contains_metadata(&request_metadata_recv, + GRPC_IAM_AUTHORITY_SELECTOR_METADATA_KEY, + iam_selector)); + GPR_ASSERT(!contains_metadata(&request_metadata_recv, + GRPC_IAM_AUTHORIZATION_TOKEN_METADATA_KEY, + overridden_iam_token)); + GPR_ASSERT(!contains_metadata(&request_metadata_recv, + GRPC_IAM_AUTHORITY_SELECTOR_METADATA_KEY, + overridden_iam_selector)); + break; + } + + gpr_free(details); + 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_call_destroy(c); + grpc_call_destroy(s); + + cq_verifier_destroy(cqv); + + grpc_byte_buffer_destroy(request_payload); + grpc_byte_buffer_destroy(response_payload); + grpc_byte_buffer_destroy(request_payload_recv); + grpc_byte_buffer_destroy(response_payload_recv); + + end_test(&f); + config.tear_down_data(&f); +#endif +} + 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_bad_creds(); } } diff --git a/test/core/security/auth_context_test.c b/test/core/security/auth_context_test.c index a30505a63ba..d785eb6064d 100644 --- a/test/core/security/auth_context_test.c +++ b/test/core/security/auth_context_test.c @@ -40,7 +40,7 @@ #include static void test_empty_context(void) { - grpc_auth_context *ctx = grpc_auth_context_create(NULL, 0); + grpc_auth_context *ctx = grpc_auth_context_create(NULL); grpc_auth_property_iterator it; gpr_log(GPR_INFO, "test_empty_context"); @@ -52,87 +52,98 @@ static void test_empty_context(void) { GPR_ASSERT(grpc_auth_property_iterator_next(&it) == NULL); it = grpc_auth_context_find_properties_by_name(ctx, "foo"); GPR_ASSERT(grpc_auth_property_iterator_next(&it) == NULL); + GPR_ASSERT(grpc_auth_context_set_peer_identity_property_name(ctx, "bar") == + 0); + GPR_ASSERT(grpc_auth_context_peer_identity_property_name(ctx) == NULL); GRPC_AUTH_CONTEXT_UNREF(ctx, "test"); } static void test_simple_context(void) { - grpc_auth_context *ctx = grpc_auth_context_create(NULL, 3); + grpc_auth_context *ctx = grpc_auth_context_create(NULL); grpc_auth_property_iterator it; size_t i; gpr_log(GPR_INFO, "test_simple_context"); GPR_ASSERT(ctx != NULL); - GPR_ASSERT(ctx->property_count == 3); - ctx->properties[0] = grpc_auth_property_init_from_cstring("name", "chapi"); - ctx->properties[1] = grpc_auth_property_init_from_cstring("name", "chapo"); - ctx->properties[2] = grpc_auth_property_init_from_cstring("foo", "bar"); - ctx->peer_identity_property_name = ctx->properties[0].name; + grpc_auth_context_add_cstring_property(ctx, "name", "chapi"); + grpc_auth_context_add_cstring_property(ctx, "name", "chapo"); + grpc_auth_context_add_cstring_property(ctx, "foo", "bar"); + GPR_ASSERT(ctx->properties.count == 3); + GPR_ASSERT(grpc_auth_context_set_peer_identity_property_name(ctx, "name") == + 1); GPR_ASSERT( strcmp(grpc_auth_context_peer_identity_property_name(ctx), "name") == 0); it = grpc_auth_context_property_iterator(ctx); - for (i = 0; i < ctx->property_count; i++) { + for (i = 0; i < ctx->properties.count; i++) { const grpc_auth_property *p = grpc_auth_property_iterator_next(&it); - GPR_ASSERT(p == &ctx->properties[i]); + GPR_ASSERT(p == &ctx->properties.array[i]); } GPR_ASSERT(grpc_auth_property_iterator_next(&it) == NULL); it = grpc_auth_context_find_properties_by_name(ctx, "foo"); - GPR_ASSERT(grpc_auth_property_iterator_next(&it) == &ctx->properties[2]); + GPR_ASSERT(grpc_auth_property_iterator_next(&it) == + &ctx->properties.array[2]); GPR_ASSERT(grpc_auth_property_iterator_next(&it) == NULL); it = grpc_auth_context_peer_identity(ctx); - GPR_ASSERT(grpc_auth_property_iterator_next(&it) == &ctx->properties[0]); - GPR_ASSERT(grpc_auth_property_iterator_next(&it) == &ctx->properties[1]); + GPR_ASSERT(grpc_auth_property_iterator_next(&it) == + &ctx->properties.array[0]); + GPR_ASSERT(grpc_auth_property_iterator_next(&it) == + &ctx->properties.array[1]); GPR_ASSERT(grpc_auth_property_iterator_next(&it) == NULL); GRPC_AUTH_CONTEXT_UNREF(ctx, "test"); } static void test_chained_context(void) { - grpc_auth_context *chained = grpc_auth_context_create(NULL, 2); - grpc_auth_context *ctx = grpc_auth_context_create(chained, 3); + grpc_auth_context *chained = grpc_auth_context_create(NULL); + grpc_auth_context *ctx = grpc_auth_context_create(chained); grpc_auth_property_iterator it; size_t i; gpr_log(GPR_INFO, "test_chained_context"); GRPC_AUTH_CONTEXT_UNREF(chained, "chained"); - chained->properties[0] = - grpc_auth_property_init_from_cstring("name", "padapo"); - chained->properties[1] = grpc_auth_property_init_from_cstring("foo", "baz"); - ctx->properties[0] = grpc_auth_property_init_from_cstring("name", "chapi"); - ctx->properties[1] = grpc_auth_property_init_from_cstring("name", "chapo"); - ctx->properties[2] = grpc_auth_property_init_from_cstring("foo", "bar"); - ctx->peer_identity_property_name = ctx->properties[0].name; + grpc_auth_context_add_cstring_property(chained, "name", "padapo"); + grpc_auth_context_add_cstring_property(chained, "foo", "baz"); + grpc_auth_context_add_cstring_property(ctx, "name", "chapi"); + grpc_auth_context_add_cstring_property(ctx, "name", "chap0"); + grpc_auth_context_add_cstring_property(ctx, "foo", "bar"); + GPR_ASSERT(grpc_auth_context_set_peer_identity_property_name(ctx, "name") == + 1); GPR_ASSERT( strcmp(grpc_auth_context_peer_identity_property_name(ctx), "name") == 0); it = grpc_auth_context_property_iterator(ctx); - for (i = 0; i < ctx->property_count; i++) { + for (i = 0; i < ctx->properties.count; i++) { const grpc_auth_property *p = grpc_auth_property_iterator_next(&it); - GPR_ASSERT(p == &ctx->properties[i]); + GPR_ASSERT(p == &ctx->properties.array[i]); } - for (i = 0; i < chained->property_count; i++) { + for (i = 0; i < chained->properties.count; i++) { const grpc_auth_property *p = grpc_auth_property_iterator_next(&it); - GPR_ASSERT(p == &chained->properties[i]); + GPR_ASSERT(p == &chained->properties.array[i]); } GPR_ASSERT(grpc_auth_property_iterator_next(&it) == NULL); it = grpc_auth_context_find_properties_by_name(ctx, "foo"); - GPR_ASSERT(grpc_auth_property_iterator_next(&it) == &ctx->properties[2]); - GPR_ASSERT(grpc_auth_property_iterator_next(&it) == &chained->properties[1]); + GPR_ASSERT(grpc_auth_property_iterator_next(&it) == + &ctx->properties.array[2]); + GPR_ASSERT(grpc_auth_property_iterator_next(&it) == + &chained->properties.array[1]); GPR_ASSERT(grpc_auth_property_iterator_next(&it) == NULL); it = grpc_auth_context_peer_identity(ctx); - GPR_ASSERT(grpc_auth_property_iterator_next(&it) == &ctx->properties[0]); - GPR_ASSERT(grpc_auth_property_iterator_next(&it) == &ctx->properties[1]); - GPR_ASSERT(grpc_auth_property_iterator_next(&it) == &chained->properties[0]); + GPR_ASSERT(grpc_auth_property_iterator_next(&it) == + &ctx->properties.array[0]); + GPR_ASSERT(grpc_auth_property_iterator_next(&it) == + &ctx->properties.array[1]); + GPR_ASSERT(grpc_auth_property_iterator_next(&it) == + &chained->properties.array[0]); GPR_ASSERT(grpc_auth_property_iterator_next(&it) == NULL); GRPC_AUTH_CONTEXT_UNREF(ctx, "test"); } - int main(int argc, char **argv) { grpc_test_init(argc, argv); test_empty_context(); diff --git a/test/core/security/credentials_test.c b/test/core/security/credentials_test.c index d3fea9680a3..96e1e396c64 100644 --- a/test/core/security/credentials_test.c +++ b/test/core/security/credentials_test.c @@ -373,8 +373,8 @@ static void test_ssl_oauth2_composite_creds(void) { grpc_credentials *ssl_creds = grpc_ssl_credentials_create(test_root_cert, NULL); const grpc_credentials_array *creds_array; - grpc_credentials *oauth2_creds = - grpc_fake_oauth2_credentials_create(test_oauth2_bearer_token, 0); + grpc_credentials *oauth2_creds = grpc_md_only_test_credentials_create( + "Authorization", test_oauth2_bearer_token, 0); grpc_credentials *composite_creds = grpc_composite_credentials_create(ssl_creds, oauth2_creds); grpc_credentials_unref(ssl_creds); @@ -424,8 +424,8 @@ static void test_ssl_oauth2_iam_composite_creds(void) { grpc_credentials *ssl_creds = grpc_ssl_credentials_create(test_root_cert, NULL); const grpc_credentials_array *creds_array; - grpc_credentials *oauth2_creds = - grpc_fake_oauth2_credentials_create(test_oauth2_bearer_token, 0); + grpc_credentials *oauth2_creds = grpc_md_only_test_credentials_create( + "Authorization", test_oauth2_bearer_token, 0); grpc_credentials *aux_creds = grpc_composite_credentials_create(ssl_creds, oauth2_creds); grpc_credentials *iam_creds = grpc_iam_credentials_create( diff --git a/test/cpp/common/auth_property_iterator_test.cc b/test/cpp/common/auth_property_iterator_test.cc index 3d983fa3109..6443e2fd85c 100644 --- a/test/cpp/common/auth_property_iterator_test.cc +++ b/test/cpp/common/auth_property_iterator_test.cc @@ -31,10 +31,10 @@ * */ +#include #include #include #include "src/cpp/common/secure_auth_context.h" -#include "src/core/security/security_context.h" namespace grpc { namespace { @@ -50,14 +50,15 @@ class TestAuthPropertyIterator : public AuthPropertyIterator { class AuthPropertyIteratorTest : public ::testing::Test { protected: void SetUp() GRPC_OVERRIDE { - ctx_ = grpc_auth_context_create(NULL, 3); - ctx_->properties[0] = grpc_auth_property_init_from_cstring("name", "chapi"); - ctx_->properties[1] = grpc_auth_property_init_from_cstring("name", "chapo"); - ctx_->properties[2] = grpc_auth_property_init_from_cstring("foo", "bar"); - ctx_->peer_identity_property_name = ctx_->properties[0].name; + ctx_ = grpc_auth_context_create(NULL); + grpc_auth_context_add_cstring_property(ctx_, "name", "chapi"); + grpc_auth_context_add_cstring_property(ctx_, "name", "chapo"); + grpc_auth_context_add_cstring_property(ctx_, "foo", "bar"); + EXPECT_EQ(1, + grpc_auth_context_set_peer_identity_property_name(ctx_, "name")); } void TearDown() GRPC_OVERRIDE { - GRPC_AUTH_CONTEXT_UNREF(ctx_, "AuthPropertyIteratorTest"); + grpc_auth_context_release(ctx_); } grpc_auth_context* ctx_; diff --git a/test/cpp/common/secure_auth_context_test.cc b/test/cpp/common/secure_auth_context_test.cc index f18a04178ef..b2eeb3fa72f 100644 --- a/test/cpp/common/secure_auth_context_test.cc +++ b/test/cpp/common/secure_auth_context_test.cc @@ -31,10 +31,10 @@ * */ +#include #include #include #include "src/cpp/common/secure_auth_context.h" -#include "src/core/security/security_context.h" namespace grpc { namespace { @@ -52,11 +52,11 @@ TEST_F(SecureAuthContextTest, EmptyContext) { } TEST_F(SecureAuthContextTest, Properties) { - grpc_auth_context* ctx = grpc_auth_context_create(NULL, 3); - ctx->properties[0] = grpc_auth_property_init_from_cstring("name", "chapi"); - ctx->properties[1] = grpc_auth_property_init_from_cstring("name", "chapo"); - ctx->properties[2] = grpc_auth_property_init_from_cstring("foo", "bar"); - ctx->peer_identity_property_name = ctx->properties[0].name; + grpc_auth_context* ctx = grpc_auth_context_create(NULL); + grpc_auth_context_add_cstring_property(ctx, "name", "chapi"); + grpc_auth_context_add_cstring_property(ctx, "name", "chapo"); + grpc_auth_context_add_cstring_property(ctx, "foo", "bar"); + EXPECT_EQ(1, grpc_auth_context_set_peer_identity_property_name(ctx, "name")); SecureAuthContext context(ctx); std::vector peer_identity = context.GetPeerIdentity(); @@ -70,11 +70,11 @@ TEST_F(SecureAuthContextTest, Properties) { } TEST_F(SecureAuthContextTest, Iterators) { - grpc_auth_context* ctx = grpc_auth_context_create(NULL, 3); - ctx->properties[0] = grpc_auth_property_init_from_cstring("name", "chapi"); - ctx->properties[1] = grpc_auth_property_init_from_cstring("name", "chapo"); - ctx->properties[2] = grpc_auth_property_init_from_cstring("foo", "bar"); - ctx->peer_identity_property_name = ctx->properties[0].name; + grpc_auth_context* ctx = grpc_auth_context_create(NULL); + grpc_auth_context_add_cstring_property(ctx, "name", "chapi"); + grpc_auth_context_add_cstring_property(ctx, "name", "chapo"); + grpc_auth_context_add_cstring_property(ctx, "foo", "bar"); + EXPECT_EQ(1, grpc_auth_context_set_peer_identity_property_name(ctx, "name")); SecureAuthContext context(ctx); AuthPropertyIterator iter = context.begin(); From a87d6c2af6a8bbad50d9ad639873357fd824b791 Mon Sep 17 00:00:00 2001 From: Julien Boeuf Date: Fri, 17 Jul 2015 15:51:46 -0700 Subject: [PATCH 2/9] Cannot figure out server filter logic for error in auth md processing. - Positive tests pass even if we will have to change the interface to add the processor to the server credentials (will be done in a separate pull request). - ASAN leaks for the error case. - The client should get a GRPC_STATUS_UNAUTHENTICATED as opposed to GPRC_STATUS_INTERNAL. --- include/grpc/grpc_security.h | 25 +- src/core/security/security_context.c | 13 +- src/core/security/security_context.h | 3 +- src/core/security/server_auth_filter.c | 55 ++-- ...est_response_with_payload_and_call_creds.c | 248 ++++++------------ 5 files changed, 118 insertions(+), 226 deletions(-) diff --git a/include/grpc/grpc_security.h b/include/grpc/grpc_security.h index 9e193e697f8..ead708b2840 100644 --- a/include/grpc/grpc_security.h +++ b/include/grpc/grpc_security.h @@ -291,16 +291,23 @@ 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); -/* Pluggable metadata processing function */ -typedef void (*grpc_process_auth_metadata_func)( - 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); - -/* Registration function for metadata processing. +/* 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); + void *state; +} grpc_auth_metadata_processor; + +/* XXXX: this is a temporarty interface. Please do NOT use. + This function will be moved to the server_credentials in a subsequent + pull request. XXXX + + Registration function for metadata processing. Should be called before the server is started. */ -void grpc_server_auth_context_register_process_metadata_func( - grpc_process_auth_metadata_func func); +void grpc_server_register_auth_metadata_processor( + grpc_auth_metadata_processor processor); #ifdef __cplusplus } diff --git a/src/core/security/security_context.c b/src/core/security/security_context.c index 0015d5b9150..8ccce89ba94 100644 --- a/src/core/security/security_context.c +++ b/src/core/security/security_context.c @@ -44,16 +44,15 @@ /* --- grpc_process_auth_metadata_func --- */ -static grpc_process_auth_metadata_func server_md_func = NULL; +static grpc_auth_metadata_processor server_processor = {NULL, NULL}; -void grpc_server_auth_context_register_process_metadata_func( - grpc_process_auth_metadata_func func) { - server_md_func = func; +grpc_auth_metadata_processor grpc_server_get_auth_metadata_processor(void) { + return server_processor; } -grpc_process_auth_metadata_func -grpc_server_auth_context_get_process_metadata_func(void) { - return server_md_func; +void grpc_server_register_auth_metadata_processor( + grpc_auth_metadata_processor processor) { + server_processor = processor; } /* --- grpc_call --- */ diff --git a/src/core/security/security_context.h b/src/core/security/security_context.h index b5dfae06662..d4351cb74c5 100644 --- a/src/core/security/security_context.h +++ b/src/core/security/security_context.h @@ -106,8 +106,7 @@ void grpc_server_security_context_destroy(void *ctx); /* --- Auth metadata processing. --- */ -grpc_process_auth_metadata_func -grpc_server_auth_context_get_process_metadata_func(void); +grpc_auth_metadata_processor grpc_server_get_auth_metadata_processor(void); #endif /* GRPC_INTERNAL_CORE_SECURITY_SECURITY_CONTEXT_H */ diff --git a/src/core/security/server_auth_filter.c b/src/core/security/server_auth_filter.c index fe993e50ee9..918cb401ebd 100644 --- a/src/core/security/server_auth_filter.c +++ b/src/core/security/server_auth_filter.c @@ -42,16 +42,14 @@ typedef struct call_data { gpr_uint8 got_client_metadata; - gpr_uint8 sent_status; - gpr_uint8 success; - grpc_linked_mdelem status; grpc_stream_op_buffer *recv_ops; - /* Closure to call when finished with the hs_on_recv hook. */ + /* Closure to call when finished with the auth_on_recv hook. */ grpc_iomgr_closure *on_done_recv; /* Receive closures are chained: we inject this closure as the on_done_recv up-call on transport_op, and remember to call our on_done_recv member after handling it. */ grpc_iomgr_closure auth_on_recv; + grpc_transport_stream_op transport_op; const grpc_metadata *consumed_md; size_t num_consumed_md; grpc_stream_op *md_op; @@ -61,7 +59,7 @@ typedef struct call_data { typedef struct channel_data { grpc_security_connector *security_connector; - grpc_mdelem *status_auth_failure; + grpc_mdctx *mdctx; } channel_data; static grpc_metadata_array metadata_batch_to_md_array( @@ -112,8 +110,8 @@ static void on_md_processing_done(void *user_data, grpc_auth_context *result) { grpc_call_element *elem = user_data; call_data *calld = elem->call_data; + channel_data *chand = elem->channel_data; - calld->success = success; if (success) { calld->consumed_md = consumed_md; calld->num_consumed_md = num_consumed_md; @@ -124,11 +122,14 @@ static void on_md_processing_done(void *user_data, "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 { - grpc_call_element_send_cancel(elem); + grpc_transport_stream_op_add_cancellation( + &calld->transport_op, GRPC_STATUS_UNAUTHENTICATED, + grpc_mdstr_from_string(chand->mdctx, + "Authentication metadata processing failed.")); + grpc_call_next_op(elem, &calld->transport_op); } - - calld->on_done_recv->cb(calld->on_done_recv->cb_arg, success); } static void auth_on_recv(void *user_data, int success) { @@ -141,16 +142,18 @@ static void auth_on_recv(void *user_data, int success) { grpc_stream_op *ops = calld->recv_ops->ops; for (i = 0; i < nops; i++) { grpc_metadata_array md_array; - grpc_process_auth_metadata_func processor = - grpc_server_auth_context_get_process_metadata_func(); + grpc_auth_metadata_processor processor = + grpc_server_get_auth_metadata_processor(); grpc_stream_op *op = &ops[i]; - if (op->type != GRPC_OP_METADATA) continue; + if (op->type != GRPC_OP_METADATA || calld->got_client_metadata) continue; calld->got_client_metadata = 1; - if (processor == NULL) continue; + if (processor.process == NULL) continue; calld->md_op = op; md_array = metadata_batch_to_md_array(&op->data.metadata); - processor(&calld->ticket, chand->security_connector->auth_context, - md_array.metadata, md_array.count, on_md_processing_done, elem); + processor.process(processor.state, &calld->ticket, + chand->security_connector->auth_context, + md_array.metadata, md_array.count, + on_md_processing_done, elem); grpc_metadata_array_destroy(&md_array); return; } @@ -161,28 +164,13 @@ static void auth_on_recv(void *user_data, int success) { static void set_recv_ops_md_callbacks(grpc_call_element *elem, grpc_transport_stream_op *op) { call_data *calld = elem->call_data; - channel_data *chand = elem->channel_data; - - if (op->send_ops && !calld->sent_status && !calld->success) { - size_t i; - size_t nops = op->send_ops->nops; - grpc_stream_op *ops = op->send_ops->ops; - for (i = 0; i < nops; i++) { - grpc_stream_op *op = &ops[i]; - if (op->type != GRPC_OP_METADATA) continue; - calld->sent_status = 1; - grpc_metadata_batch_add_head( - &op->data.metadata, &calld->status, - GRPC_MDELEM_REF(chand->status_auth_failure)); - break; - } - } if (op->recv_ops && !calld->got_client_metadata) { /* 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->transport_op = *op; } } @@ -209,7 +197,6 @@ static void init_call_elem(grpc_call_element *elem, /* initialize members */ memset(calld, 0, sizeof(*calld)); grpc_iomgr_closure_init(&calld->auth_on_recv, auth_on_recv, elem); - calld->success = 1; GPR_ASSERT(initial_op && initial_op->context != NULL && initial_op->context[GRPC_CONTEXT_SECURITY].value == NULL); @@ -260,8 +247,7 @@ static void init_channel_elem(grpc_channel_element *elem, grpc_channel *master, GPR_ASSERT(!sc->is_client_side); chand->security_connector = GRPC_SECURITY_CONNECTOR_REF(sc, "server_auth_filter"); - chand->status_auth_failure = - grpc_mdelem_from_strings(mdctx, ":status", "401"); + chand->mdctx = mdctx; } /* Destructor for channel data */ @@ -270,7 +256,6 @@ static void destroy_channel_elem(grpc_channel_element *elem) { channel_data *chand = elem->channel_data; GRPC_SECURITY_CONNECTOR_UNREF(chand->security_connector, "server_auth_filter"); - GRPC_MDELEM_UNREF(chand->status_auth_failure); } const grpc_channel_filter grpc_server_auth_filter = { 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 c0214081c5d..7facb6997b3 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 @@ -84,45 +84,51 @@ static void check_peer_identity(grpc_auth_context *ctx, GPR_ASSERT(strcmp(expected_identity, prop->value) == 0); GPR_ASSERT(grpc_auth_property_iterator_next(&it) == NULL); } -static void process_auth_md_success(grpc_auth_ticket *t, +static void process_auth_md_success(void *state, grpc_auth_ticket *t, grpc_auth_context *channel_ctx, const grpc_metadata *md, size_t md_count, grpc_process_auth_metadata_done_cb cb, void *user_data) { - grpc_auth_context *new_auth_ctx = grpc_auth_context_create(channel_ctx); - const grpc_metadata *custom_creds_md = - find_metadata(md, md_count, custom_creds_md_name, custom_creds_md_value); - GPR_ASSERT(custom_creds_md != NULL); - grpc_auth_context_add_cstring_property( - new_auth_ctx, client_identity_property_name, client_identity); - GPR_ASSERT(grpc_auth_context_set_peer_identity_property_name( - new_auth_ctx, client_identity_property_name) == 1); - cb(user_data, custom_creds_md, 1, 1, new_auth_ctx); - grpc_auth_context_release(new_auth_ctx); + override_mode *mode; + GPR_ASSERT(state != NULL); + mode = (override_mode *)state; + if (*mode != DESTROY) { + grpc_auth_context *new_auth_ctx = grpc_auth_context_create(channel_ctx); + const grpc_metadata *custom_creds_md = find_metadata( + md, md_count, custom_creds_md_name, custom_creds_md_value); + GPR_ASSERT(custom_creds_md != NULL); + grpc_auth_context_add_cstring_property( + new_auth_ctx, client_identity_property_name, client_identity); + GPR_ASSERT(grpc_auth_context_set_peer_identity_property_name( + new_auth_ctx, client_identity_property_name) == 1); + cb(user_data, custom_creds_md, 1, 1, new_auth_ctx); + grpc_auth_context_release(new_auth_ctx); + } else { + cb(user_data, NULL, 0, 1, channel_ctx); + } } -#if 0 -static void process_auth_md_failure(grpc_auth_ticket *t, +static void process_auth_md_failure(void *state, grpc_auth_ticket *t, grpc_auth_context *channel_ctx, const grpc_metadata *md, size_t md_count, grpc_process_auth_metadata_done_cb cb, void *user_data) { - const grpc_metadata *custom_creds_md = - find_metadata(md, md_count, custom_creds_md_name, custom_creds_md_value); - GPR_ASSERT(custom_creds_md != NULL); + override_mode *mode; + GPR_ASSERT(state != NULL); + mode = (override_mode *)state; + if (*mode != DESTROY) { + const grpc_metadata *custom_creds_md = find_metadata( + md, md_count, custom_creds_md_name, custom_creds_md_value); + GPR_ASSERT(custom_creds_md != NULL); + } cb(user_data, NULL, 0, 0, NULL); /* Fail. */ } -#endif static grpc_end2end_test_fixture begin_test( grpc_end2end_test_config config, const char *test_name, - grpc_process_auth_metadata_func md_func, override_mode mode) { + grpc_auth_metadata_processor processor) { grpc_end2end_test_fixture f; - if (mode != DESTROY) { - grpc_server_auth_context_register_process_metadata_func(md_func); - } else { - grpc_server_auth_context_register_process_metadata_func(NULL); - } + grpc_server_register_auth_metadata_processor(processor); gpr_log(GPR_INFO, "%s/%s", test_name, config.name); f = config.create_fixture(NULL, NULL); config.init_client(&f, NULL); @@ -200,8 +206,9 @@ static grpc_credentials *iam_custom_composite_creds_create( static void test_call_creds_failure(grpc_end2end_test_config config) { grpc_call *c; grpc_credentials *creds = NULL; + grpc_auth_metadata_processor p = {NULL, NULL}; grpc_end2end_test_fixture f = - begin_test(config, "test_call_creds_failure", NULL, NONE); + begin_test(config, "test_call_creds_failure", p); gpr_timespec deadline = five_seconds_time(); c = grpc_channel_create_call(f.client, f.cq, "/foo", "foo.test.google.fr", deadline); @@ -230,10 +237,9 @@ static void request_response_with_payload_and_call_creds( grpc_byte_buffer *response_payload = grpc_raw_byte_buffer_create(&response_payload_slice, 1); gpr_timespec deadline = five_seconds_time(); - - grpc_end2end_test_fixture f = - begin_test(config, test_name, process_auth_md_success, mode); - cq_verifier *cqv = cq_verifier_create(f.cq); + grpc_auth_metadata_processor p; + grpc_end2end_test_fixture f; + cq_verifier *cqv; grpc_op ops[6]; grpc_op *op; grpc_metadata_array initial_metadata_recv; @@ -250,6 +256,11 @@ static void request_response_with_payload_and_call_creds( grpc_auth_context *s_auth_context = NULL; grpc_auth_context *c_auth_context = NULL; + p.process = process_auth_md_success; + p.state = &mode; + f = begin_test(config, test_name, p); + cqv = cq_verifier_create(f.cq); + c = grpc_channel_create_call(f.client, f.cq, "/foo", "foo.test.google.fr", deadline); GPR_ASSERT(c); @@ -446,54 +457,41 @@ static void test_request_response_with_payload_and_deleted_call_creds( DESTROY); } -static void test_request_with_bad_creds(void) { -#if 0 - grpc_call *c; - grpc_call *s; - 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); - gpr_timespec deadline = five_seconds_time(); - - grpc_end2end_test_fixture f = - begin_test(config, test_name, process_auth_md_failure, NONE); - cq_verifier *cqv = cq_verifier_create(f.cq); +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_auth_metadata_processor p; + 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_byte_buffer *request_payload_recv = NULL; - grpc_byte_buffer *response_payload_recv = NULL; grpc_call_details call_details; grpc_status_code status; char *details = NULL; size_t details_capacity = 0; - int was_cancelled = 2; - grpc_credentials *creds = NULL; - grpc_auth_context *s_auth_context = NULL; - grpc_auth_context *c_auth_context = NULL; + 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); + override_mode mode = NONE; + grpc_credentials *creds; + + p.process = process_auth_md_failure; + p.state = &mode; + f = begin_test(config, "test_request_with_server_rejecting_client_creds", p); + 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 = iam_custom_composite_creds_create(iam_token, iam_selector); GPR_ASSERT(creds != NULL); GPR_ASSERT(grpc_call_set_credentials(c, creds) == GRPC_CALL_OK); - switch (mode) { - case NONE: - break; - case OVERRIDE: - grpc_credentials_release(creds); - creds = iam_custom_composite_creds_create(overridden_iam_token, - overridden_iam_selector); - GPR_ASSERT(creds != NULL); - GPR_ASSERT(grpc_call_set_credentials(c, creds) == GRPC_CALL_OK); - break; - case DESTROY: - GPR_ASSERT(grpc_call_set_credentials(c, NULL) == GRPC_CALL_OK); - break; - } grpc_credentials_release(creds); grpc_metadata_array_init(&initial_metadata_recv); @@ -502,6 +500,13 @@ static void test_request_with_bad_creds(void) { 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; @@ -521,134 +526,31 @@ static void test_request_with_bad_creds(void) { op->data.recv_message = &response_payload_recv; op->flags = 0; op++; - 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++; GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_batch(c, ops, op - ops, tag(1))); - GPR_ASSERT(GRPC_CALL_OK == grpc_server_request_call( - f.server, &s, &call_details, - &request_metadata_recv, f.cq, f.cq, tag(101))); - cq_expect_completion(cqv, tag(101), 1); - cq_verify(cqv); - s_auth_context = grpc_call_auth_context(s); - GPR_ASSERT(s_auth_context != NULL); - print_auth_context(0, s_auth_context); - grpc_auth_context_release(s_auth_context); - - c_auth_context = grpc_call_auth_context(c); - GPR_ASSERT(c_auth_context != NULL); - print_auth_context(1, c_auth_context); - grpc_auth_context_release(c_auth_context); - - /* Cannot set creds on the server call object. */ - GPR_ASSERT(grpc_call_set_credentials(s, NULL) != GRPC_CALL_OK); - - op = ops; - op->op = GRPC_OP_SEND_INITIAL_METADATA; - op->data.send_initial_metadata.count = 0; - op->flags = 0; - op++; - op->op = GRPC_OP_RECV_MESSAGE; - op->data.recv_message = &request_payload_recv; - op->flags = 0; - op++; - GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_batch(s, ops, op - ops, tag(102))); - - cq_expect_completion(cqv, tag(102), 1); - cq_verify(cqv); - - op = ops; - op->op = GRPC_OP_RECV_CLOSE_ON_SERVER; - op->data.recv_close_on_server.cancelled = &was_cancelled; - op->flags = 0; - op++; - op->op = GRPC_OP_SEND_MESSAGE; - op->data.send_message = response_payload; - op->flags = 0; - op++; - op->op = GRPC_OP_SEND_STATUS_FROM_SERVER; - op->data.send_status_from_server.trailing_metadata_count = 0; - op->data.send_status_from_server.status = GRPC_STATUS_OK; - op->data.send_status_from_server.status_details = "xyz"; - op->flags = 0; - op++; - GPR_ASSERT(GRPC_CALL_OK == grpc_call_start_batch(s, ops, op - ops, tag(103))); - - cq_expect_completion(cqv, tag(103), 1); cq_expect_completion(cqv, tag(1), 1); cq_verify(cqv); - GPR_ASSERT(status == GRPC_STATUS_OK); - GPR_ASSERT(0 == strcmp(details, "xyz")); - GPR_ASSERT(0 == strcmp(call_details.method, "/foo")); - GPR_ASSERT(0 == strcmp(call_details.host, "foo.test.google.fr")); - GPR_ASSERT(was_cancelled == 0); - GPR_ASSERT(byte_buffer_eq_string(request_payload_recv, "hello world")); - GPR_ASSERT(byte_buffer_eq_string(response_payload_recv, "hello you")); - - /* Has been processed by the auth metadata processor. */ - GPR_ASSERT(!contains_metadata(&request_metadata_recv, custom_creds_md_name, - custom_creds_md_value)); - - switch (mode) { - case NONE: - GPR_ASSERT(contains_metadata(&request_metadata_recv, - GRPC_IAM_AUTHORIZATION_TOKEN_METADATA_KEY, - iam_token)); - GPR_ASSERT(contains_metadata(&request_metadata_recv, - GRPC_IAM_AUTHORITY_SELECTOR_METADATA_KEY, - iam_selector)); - check_peer_identity(s_auth_context, client_identity); - break; - case OVERRIDE: - GPR_ASSERT(contains_metadata(&request_metadata_recv, - GRPC_IAM_AUTHORIZATION_TOKEN_METADATA_KEY, - overridden_iam_token)); - GPR_ASSERT(contains_metadata(&request_metadata_recv, - GRPC_IAM_AUTHORITY_SELECTOR_METADATA_KEY, - overridden_iam_selector)); - check_peer_identity(s_auth_context, client_identity); - break; - case DESTROY: - GPR_ASSERT(!contains_metadata(&request_metadata_recv, - GRPC_IAM_AUTHORIZATION_TOKEN_METADATA_KEY, - iam_token)); - GPR_ASSERT(!contains_metadata(&request_metadata_recv, - GRPC_IAM_AUTHORITY_SELECTOR_METADATA_KEY, - iam_selector)); - GPR_ASSERT(!contains_metadata(&request_metadata_recv, - GRPC_IAM_AUTHORIZATION_TOKEN_METADATA_KEY, - overridden_iam_token)); - GPR_ASSERT(!contains_metadata(&request_metadata_recv, - GRPC_IAM_AUTHORITY_SELECTOR_METADATA_KEY, - overridden_iam_selector)); - break; - } + /* XXX Should be GRPC_STATUS_UNAUTHENTICATED but it looks like there is a bug + (probably in the server_auth_context.c code) where this error on the server + does not get to the client. The current error code we are getting is + GRPC_STATUS_INTERNAL. */ + GPR_ASSERT(status != GRPC_STATUS_OK); - gpr_free(details); 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_call_destroy(c); - grpc_call_destroy(s); - - cq_verifier_destroy(cqv); - grpc_byte_buffer_destroy(request_payload); - grpc_byte_buffer_destroy(response_payload); - grpc_byte_buffer_destroy(request_payload_recv); 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); -#endif } void grpc_end2end_tests(grpc_end2end_test_config config) { @@ -657,6 +559,6 @@ void grpc_end2end_tests(grpc_end2end_test_config 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_bad_creds(); + test_request_with_server_rejecting_client_creds(config); } } From 6bdc9b47bc6dc5eeb296d66adf2d0789759aba37 Mon Sep 17 00:00:00 2001 From: Julien Boeuf Date: Sun, 19 Jul 2015 21:56:02 -0700 Subject: [PATCH 3/9] Getting started on metadata processor set on server creds. --- include/grpc/grpc_security.h | 10 ++-------- src/core/security/credentials.c | 6 ++++++ src/core/security/credentials.h | 1 + src/core/security/security_context.h | 5 ++++- src/core/security/server_secure_chttp2.c | 3 +++ 5 files changed, 16 insertions(+), 9 deletions(-) diff --git a/include/grpc/grpc_security.h b/include/grpc/grpc_security.h index ead708b2840..9b907ea3eba 100644 --- a/include/grpc/grpc_security.h +++ b/include/grpc/grpc_security.h @@ -300,14 +300,8 @@ typedef struct { void *state; } grpc_auth_metadata_processor; -/* XXXX: this is a temporarty interface. Please do NOT use. - This function will be moved to the server_credentials in a subsequent - pull request. XXXX - - Registration function for metadata processing. - Should be called before the server is started. */ -void grpc_server_register_auth_metadata_processor( - grpc_auth_metadata_processor processor); +void grpc_server_credentials_set_auth_metadata_processor( + grpc_server_credentials *creds, grpc_auth_metadata_processor processor); #ifdef __cplusplus } diff --git a/src/core/security/credentials.c b/src/core/security/credentials.c index 71513bcc25b..eb178ececba 100644 --- a/src/core/security/credentials.c +++ b/src/core/security/credentials.c @@ -149,6 +149,12 @@ grpc_security_status grpc_server_credentials_create_security_connector( return creds->vtable->create_security_connector(creds, sc); } +void grpc_server_credentials_set_auth_metadata_processor( + grpc_server_credentials *creds, grpc_auth_metadata_processor processor) { + if (creds == NULL) return; + creds->processor = processor; +} + /* -- Ssl credentials. -- */ static void ssl_destroy(grpc_credentials *creds) { diff --git a/src/core/security/credentials.h b/src/core/security/credentials.h index 664524522ba..cee04b2120c 100644 --- a/src/core/security/credentials.h +++ b/src/core/security/credentials.h @@ -208,6 +208,7 @@ typedef struct { struct grpc_server_credentials { const grpc_server_credentials_vtable *vtable; const char *type; + grpc_auth_metadata_processor processor; }; grpc_security_status grpc_server_credentials_create_security_connector( diff --git a/src/core/security/security_context.h b/src/core/security/security_context.h index d4351cb74c5..5df5311d705 100644 --- a/src/core/security/security_context.h +++ b/src/core/security/security_context.h @@ -105,8 +105,11 @@ grpc_server_security_context *grpc_server_security_context_create(void); void grpc_server_security_context_destroy(void *ctx); /* --- Auth metadata processing. --- */ +#define GRPC_AUTH_METADATA_PROCESSOR_ARG "grpc.auth_metadata_processor" -grpc_auth_metadata_processor grpc_server_get_auth_metadata_processor(void); +grpc_arg grpc_auth_metadata_processor_to_arg(grpc_auth_metadata_processor *p); +grpc_auth_metadata_processor grpc_auth_metadata_processor_from_arg( + const grpc_arg *arg); #endif /* GRPC_INTERNAL_CORE_SECURITY_SECURITY_CONTEXT_H */ diff --git a/src/core/security/server_secure_chttp2.c b/src/core/security/server_secure_chttp2.c index 3717b8989f4..5dcd7e2f92f 100644 --- a/src/core/security/server_secure_chttp2.c +++ b/src/core/security/server_secure_chttp2.c @@ -60,6 +60,7 @@ typedef struct grpc_server_secure_state { grpc_server *server; grpc_tcp_server *tcp; grpc_security_connector *sc; + grpc_auth_metadata_processor processor; tcp_endpoint_list *handshaking_tcp_endpoints; int is_shutdown; gpr_mu mu; @@ -252,9 +253,11 @@ int grpc_server_add_secure_http2_port(grpc_server *server, const char *addr, grpc_resolved_addresses_destroy(resolved); state = gpr_malloc(sizeof(*state)); + memset(state, 0, sizeof(*state)); state->server = server; state->tcp = tcp; state->sc = sc; + state->processor = creds->processor; state->handshaking_tcp_endpoints = NULL; state->is_shutdown = 0; gpr_mu_init(&state->mu); From 66a27daef6e0acc4ad9d3789580e1d3107670c9d Mon Sep 17 00:00:00 2001 From: Julien Boeuf Date: Tue, 21 Jul 2015 17:17:35 -0700 Subject: [PATCH 4/9] Putting the auth metadata processor on the server creds. --- src/core/security/security_context.c | 32 +++ src/core/security/security_context.h | 4 +- src/core/security/server_auth_filter.c | 17 +- src/core/security/server_secure_chttp2.c | 11 +- .../chttp2_simple_ssl_with_oauth2_fullstack.c | 42 +++- ...est_response_with_payload_and_call_creds.c | 201 +----------------- 6 files changed, 99 insertions(+), 208 deletions(-) diff --git a/src/core/security/security_context.c b/src/core/security/security_context.c index 8ccce89ba94..1ef0fc9255b 100644 --- a/src/core/security/security_context.c +++ b/src/core/security/security_context.c @@ -295,3 +295,35 @@ void grpc_auth_property_reset(grpc_auth_property *property) { memset(property, 0, sizeof(grpc_auth_property)); } +grpc_arg grpc_auth_metadata_processor_to_arg(grpc_auth_metadata_processor *p) { + grpc_arg arg; + memset(&arg, 0, sizeof(grpc_arg)); + arg.type = GRPC_ARG_POINTER; + arg.key = GRPC_AUTH_METADATA_PROCESSOR_ARG; + arg.value.pointer.p = p; + return arg; +} + +grpc_auth_metadata_processor *grpc_auth_metadata_processor_from_arg( + const grpc_arg *arg) { + if (strcmp(arg->key, GRPC_AUTH_METADATA_PROCESSOR_ARG) != 0) return NULL; + if (arg->type != GRPC_ARG_POINTER) { + gpr_log(GPR_ERROR, "Invalid type %d for arg %s", arg->type, + GRPC_AUTH_METADATA_PROCESSOR_ARG); + return NULL; + } + return arg->value.pointer.p; +} + +grpc_auth_metadata_processor *grpc_find_auth_metadata_processor_in_args( + const grpc_channel_args *args) { + size_t i; + if (args == NULL) return NULL; + for (i = 0; i < args->num_args; i++) { + grpc_auth_metadata_processor *p = + grpc_auth_metadata_processor_from_arg(&args->args[i]); + if (p != NULL) return p; + } + return NULL; +} + diff --git a/src/core/security/security_context.h b/src/core/security/security_context.h index 5df5311d705..ddc0a7afad6 100644 --- a/src/core/security/security_context.h +++ b/src/core/security/security_context.h @@ -108,8 +108,10 @@ void grpc_server_security_context_destroy(void *ctx); #define GRPC_AUTH_METADATA_PROCESSOR_ARG "grpc.auth_metadata_processor" grpc_arg grpc_auth_metadata_processor_to_arg(grpc_auth_metadata_processor *p); -grpc_auth_metadata_processor grpc_auth_metadata_processor_from_arg( +grpc_auth_metadata_processor *grpc_auth_metadata_processor_from_arg( const grpc_arg *arg); +grpc_auth_metadata_processor *grpc_find_auth_metadata_processor_in_args( + const grpc_channel_args *args); #endif /* GRPC_INTERNAL_CORE_SECURITY_SECURITY_CONTEXT_H */ diff --git a/src/core/security/server_auth_filter.c b/src/core/security/server_auth_filter.c index 918cb401ebd..cc260554405 100644 --- a/src/core/security/server_auth_filter.c +++ b/src/core/security/server_auth_filter.c @@ -59,6 +59,7 @@ typedef struct call_data { typedef struct channel_data { grpc_security_connector *security_connector; + grpc_auth_metadata_processor processor; grpc_mdctx *mdctx; } channel_data; @@ -142,18 +143,16 @@ static void auth_on_recv(void *user_data, int success) { grpc_stream_op *ops = calld->recv_ops->ops; for (i = 0; i < nops; i++) { grpc_metadata_array md_array; - grpc_auth_metadata_processor processor = - grpc_server_get_auth_metadata_processor(); grpc_stream_op *op = &ops[i]; if (op->type != GRPC_OP_METADATA || calld->got_client_metadata) continue; calld->got_client_metadata = 1; - if (processor.process == NULL) continue; + if (chand->processor.process == NULL) continue; calld->md_op = op; md_array = metadata_batch_to_md_array(&op->data.metadata); - processor.process(processor.state, &calld->ticket, - chand->security_connector->auth_context, - md_array.metadata, md_array.count, - on_md_processing_done, elem); + chand->processor.process(chand->processor.state, &calld->ticket, + chand->security_connector->auth_context, + md_array.metadata, md_array.count, + on_md_processing_done, elem); grpc_metadata_array_destroy(&md_array); return; } @@ -233,6 +232,8 @@ static void init_channel_elem(grpc_channel_element *elem, grpc_channel *master, const grpc_channel_args *args, grpc_mdctx *mdctx, int is_first, int is_last) { grpc_security_connector *sc = grpc_find_security_connector_in_args(args); + grpc_auth_metadata_processor *processor = + grpc_find_auth_metadata_processor_in_args(args); /* grab pointers to our data from the channel element */ channel_data *chand = elem->channel_data; @@ -242,12 +243,14 @@ static void init_channel_elem(grpc_channel_element *elem, grpc_channel *master, GPR_ASSERT(!is_first); GPR_ASSERT(!is_last); GPR_ASSERT(sc != NULL); + GPR_ASSERT(processor != NULL); /* initialize members */ GPR_ASSERT(!sc->is_client_side); chand->security_connector = GRPC_SECURITY_CONNECTOR_REF(sc, "server_auth_filter"); chand->mdctx = mdctx; + chand->processor = *processor; } /* Destructor for channel data */ diff --git a/src/core/security/server_secure_chttp2.c b/src/core/security/server_secure_chttp2.c index 5dcd7e2f92f..8d9d036d808 100644 --- a/src/core/security/server_secure_chttp2.c +++ b/src/core/security/server_secure_chttp2.c @@ -43,6 +43,7 @@ #include "src/core/security/auth_filters.h" #include "src/core/security/credentials.h" #include "src/core/security/security_connector.h" +#include "src/core/security/security_context.h" #include "src/core/security/secure_transport_setup.h" #include "src/core/surface/server.h" #include "src/core/transport/chttp2_transport.h" @@ -87,9 +88,13 @@ static void setup_transport(void *statep, grpc_transport *transport, static grpc_channel_filter const *extra_filters[] = { &grpc_server_auth_filter, &grpc_http_server_filter}; grpc_server_secure_state *state = statep; - grpc_arg connector_arg = grpc_security_connector_to_arg(state->sc); - grpc_channel_args *args_copy = grpc_channel_args_copy_and_add( - grpc_server_get_channel_args(state->server), &connector_arg, 1); + grpc_channel_args *args_copy; + grpc_arg args_to_add[2]; + args_to_add[0] = grpc_security_connector_to_arg(state->sc); + args_to_add[1] = grpc_auth_metadata_processor_to_arg(&state->processor); + args_copy = grpc_channel_args_copy_and_add( + grpc_server_get_channel_args(state->server), args_to_add, + GPR_ARRAY_SIZE(args_to_add)); grpc_server_setup_transport(state->server, transport, extra_filters, GPR_ARRAY_SIZE(extra_filters), mdctx, args_copy); grpc_channel_args_destroy(args_copy); 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 da658a0b45a..c926a4e4b74 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 @@ -46,10 +46,46 @@ #include "test/core/util/port.h" #include "test/core/end2end/data/ssl_test_data.h" +static const char oauth2_md[] = "Bearer aaslkfjs424535asdf"; +static const char *client_identity_property_name = "smurf_name"; +static const char *client_identity = "Brainy Smurf"; + typedef struct fullstack_secure_fixture_data { char *localaddr; } fullstack_secure_fixture_data; +static const grpc_metadata *find_metadata(const grpc_metadata *md, + size_t md_count, + const char *key, + const char *value) { + size_t i; + for (i = 0; i < md_count; i++) { + if (strcmp(key, md[i].key) == 0 && strlen(value) == md[i].value_length && + memcmp(md[i].value, value, md[i].value_length) == 0) { + return &md[i]; + } + } + 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) { + 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, + 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); +} + static grpc_end2end_test_fixture chttp2_create_fixture_secure_fullstack( grpc_channel_args *client_args, grpc_channel_args *server_args) { grpc_end2end_test_fixture f; @@ -100,8 +136,8 @@ static void chttp2_init_client_simple_ssl_with_oauth2_secure_fullstack( grpc_end2end_test_fixture *f, grpc_channel_args *client_args) { grpc_credentials *ssl_creds = grpc_ssl_credentials_create(test_root_cert, NULL); - grpc_credentials *oauth2_creds = grpc_md_only_test_credentials_create( - "Authorization", "Bearer aaslkfjs424535asdf", 1); + grpc_credentials *oauth2_creds = + grpc_md_only_test_credentials_create("Authorization", oauth2_md, 1); grpc_credentials *ssl_oauth2_creds = grpc_composite_credentials_create(ssl_creds, oauth2_creds); grpc_arg ssl_name_override = {GRPC_ARG_STRING, @@ -121,6 +157,8 @@ static void chttp2_init_server_simple_ssl_secure_fullstack( test_server1_cert}; grpc_server_credentials *ssl_creds = grpc_ssl_server_credentials_create(NULL, &pem_key_cert_pair, 1); + grpc_auth_metadata_processor processor = {process_oauth2, 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/tests/request_response_with_payload_and_call_creds.c b/test/core/end2end/tests/request_response_with_payload_and_call_creds.c index 7facb6997b3..e621fcbed2e 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 @@ -46,11 +46,6 @@ #include "src/core/security/credentials.h" #include "src/core/support/string.h" -static const char *custom_creds_md_name = "custom_creds"; -static const char *custom_creds_md_value = "custom_value"; -static const char *client_identity_property_name = "smurf_name"; -static const char *client_identity = "Brainy Smurf"; - static const char iam_token[] = "token"; static const char iam_selector[] = "selector"; static const char overridden_iam_token[] = "overridden_token"; @@ -62,73 +57,9 @@ enum { TIMEOUT = 200000 }; static void *tag(gpr_intptr t) { return (void *)t; } -static const grpc_metadata *find_metadata(const grpc_metadata *md, - size_t md_count, - const char *key, - const char *value) { - size_t i; - for (i = 0; i < md_count; i++) { - if (strcmp(key, md[i].key) == 0 && strlen(value) == md[i].value_length && - memcmp(md[i].value, value, md[i].value_length) == 0) { - return &md[i]; - } - } - return NULL; -} - -static void check_peer_identity(grpc_auth_context *ctx, - const char *expected_identity) { - grpc_auth_property_iterator it = grpc_auth_context_peer_identity(ctx); - const grpc_auth_property *prop = grpc_auth_property_iterator_next(&it); - GPR_ASSERT(prop != NULL); - GPR_ASSERT(strcmp(expected_identity, prop->value) == 0); - GPR_ASSERT(grpc_auth_property_iterator_next(&it) == NULL); -} -static void process_auth_md_success(void *state, grpc_auth_ticket *t, - grpc_auth_context *channel_ctx, - const grpc_metadata *md, size_t md_count, - grpc_process_auth_metadata_done_cb cb, - void *user_data) { - override_mode *mode; - GPR_ASSERT(state != NULL); - mode = (override_mode *)state; - if (*mode != DESTROY) { - grpc_auth_context *new_auth_ctx = grpc_auth_context_create(channel_ctx); - const grpc_metadata *custom_creds_md = find_metadata( - md, md_count, custom_creds_md_name, custom_creds_md_value); - GPR_ASSERT(custom_creds_md != NULL); - grpc_auth_context_add_cstring_property( - new_auth_ctx, client_identity_property_name, client_identity); - GPR_ASSERT(grpc_auth_context_set_peer_identity_property_name( - new_auth_ctx, client_identity_property_name) == 1); - cb(user_data, custom_creds_md, 1, 1, new_auth_ctx); - grpc_auth_context_release(new_auth_ctx); - } else { - cb(user_data, NULL, 0, 1, channel_ctx); - } -} - -static void process_auth_md_failure(void *state, grpc_auth_ticket *t, - grpc_auth_context *channel_ctx, - const grpc_metadata *md, size_t md_count, - grpc_process_auth_metadata_done_cb cb, - void *user_data) { - override_mode *mode; - GPR_ASSERT(state != NULL); - mode = (override_mode *)state; - if (*mode != DESTROY) { - const grpc_metadata *custom_creds_md = find_metadata( - md, md_count, custom_creds_md_name, custom_creds_md_value); - GPR_ASSERT(custom_creds_md != NULL); - } - cb(user_data, NULL, 0, 0, NULL); /* Fail. */ -} - static grpc_end2end_test_fixture begin_test( - grpc_end2end_test_config config, const char *test_name, - grpc_auth_metadata_processor processor) { + grpc_end2end_test_config config, const char *test_name) { grpc_end2end_test_fixture f; - grpc_server_register_auth_metadata_processor(processor); gpr_log(GPR_INFO, "%s/%s", test_name, config.name); f = config.create_fixture(NULL, NULL); config.init_client(&f, NULL); @@ -191,24 +122,10 @@ static void print_auth_context(int is_client, const grpc_auth_context *ctx) { } } -static grpc_credentials *iam_custom_composite_creds_create( - const char *iam_tok, const char *iam_sel) { - grpc_credentials *iam_creds = grpc_iam_credentials_create(iam_tok, iam_sel); - grpc_credentials *custom_creds = grpc_md_only_test_credentials_create( - custom_creds_md_name, custom_creds_md_value, 1); - grpc_credentials *result = - grpc_composite_credentials_create(iam_creds, custom_creds); - grpc_credentials_release(iam_creds); - grpc_credentials_release(custom_creds); - return result; -} - static void test_call_creds_failure(grpc_end2end_test_config config) { grpc_call *c; grpc_credentials *creds = NULL; - grpc_auth_metadata_processor p = {NULL, NULL}; - grpc_end2end_test_fixture f = - begin_test(config, "test_call_creds_failure", p); + grpc_end2end_test_fixture f = begin_test(config, "test_call_creds_failure"); gpr_timespec deadline = five_seconds_time(); c = grpc_channel_create_call(f.client, f.cq, "/foo", "foo.test.google.fr", deadline); @@ -237,7 +154,6 @@ static void request_response_with_payload_and_call_creds( grpc_byte_buffer *response_payload = grpc_raw_byte_buffer_create(&response_payload_slice, 1); gpr_timespec deadline = five_seconds_time(); - grpc_auth_metadata_processor p; grpc_end2end_test_fixture f; cq_verifier *cqv; grpc_op ops[6]; @@ -256,15 +172,13 @@ static void request_response_with_payload_and_call_creds( grpc_auth_context *s_auth_context = NULL; grpc_auth_context *c_auth_context = NULL; - p.process = process_auth_md_success; - p.state = &mode; - f = begin_test(config, test_name, p); + f = begin_test(config, test_name); 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 = iam_custom_composite_creds_create(iam_token, iam_selector); + creds = grpc_iam_credentials_create(iam_token, iam_selector); GPR_ASSERT(creds != NULL); GPR_ASSERT(grpc_call_set_credentials(c, creds) == GRPC_CALL_OK); switch (mode) { @@ -272,8 +186,8 @@ static void request_response_with_payload_and_call_creds( break; case OVERRIDE: grpc_credentials_release(creds); - creds = iam_custom_composite_creds_create(overridden_iam_token, - overridden_iam_selector); + creds = grpc_iam_credentials_create(overridden_iam_token, + overridden_iam_selector); GPR_ASSERT(creds != NULL); GPR_ASSERT(grpc_call_set_credentials(c, creds) == GRPC_CALL_OK); break; @@ -378,10 +292,6 @@ static void request_response_with_payload_and_call_creds( GPR_ASSERT(byte_buffer_eq_string(request_payload_recv, "hello world")); GPR_ASSERT(byte_buffer_eq_string(response_payload_recv, "hello you")); - /* Has been processed by the auth metadata processor. */ - GPR_ASSERT(!contains_metadata(&request_metadata_recv, custom_creds_md_name, - custom_creds_md_value)); - switch (mode) { case NONE: GPR_ASSERT(contains_metadata(&request_metadata_recv, @@ -390,7 +300,6 @@ static void request_response_with_payload_and_call_creds( GPR_ASSERT(contains_metadata(&request_metadata_recv, GRPC_IAM_AUTHORITY_SELECTOR_METADATA_KEY, iam_selector)); - check_peer_identity(s_auth_context, client_identity); break; case OVERRIDE: GPR_ASSERT(contains_metadata(&request_metadata_recv, @@ -399,7 +308,6 @@ static void request_response_with_payload_and_call_creds( GPR_ASSERT(contains_metadata(&request_metadata_recv, GRPC_IAM_AUTHORITY_SELECTOR_METADATA_KEY, overridden_iam_selector)); - check_peer_identity(s_auth_context, client_identity); break; case DESTROY: GPR_ASSERT(!contains_metadata(&request_metadata_recv, @@ -457,108 +365,11 @@ 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_auth_metadata_processor p; - 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); - override_mode mode = NONE; - grpc_credentials *creds; - - p.process = process_auth_md_failure; - p.state = &mode; - f = begin_test(config, "test_request_with_server_rejecting_client_creds", p); - 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 = iam_custom_composite_creds_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); - - /* XXX Should be GRPC_STATUS_UNAUTHENTICATED but it looks like there is a bug - (probably in the server_auth_context.c code) where this error on the server - does not get to the client. The current error code we are getting is - GRPC_STATUS_INTERNAL. */ - GPR_ASSERT(status != GRPC_STATUS_OK); - - 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); } } From 45ce927c7cf7abbdb452989d6d58c875a800e4ea Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 31 Jul 2015 11:22:35 -0700 Subject: [PATCH 5/9] Properly send GRPC_STATUS_UNAUTHENTICATED from server auth failures --- src/core/channel/compress_filter.c | 10 +- src/core/security/client_auth_filter.c | 6 +- src/core/security/server_auth_filter.c | 10 +- src/core/transport/chttp2/internal.h | 2 + src/core/transport/chttp2_transport.c | 113 ++++++++++++++++++ src/core/transport/metadata.c | 2 + src/core/transport/transport.c | 52 +++++++- src/core/transport/transport.h | 13 +- ...est_response_with_payload_and_call_creds.c | 2 +- 9 files changed, 189 insertions(+), 21 deletions(-) diff --git a/src/core/channel/compress_filter.c b/src/core/channel/compress_filter.c index 9fc8589fbb4..8963c13b0f5 100644 --- a/src/core/channel/compress_filter.c +++ b/src/core/channel/compress_filter.c @@ -204,7 +204,7 @@ static void process_send_ops(grpc_call_element *elem, } grpc_metadata_batch_add_tail( &(sop->data.metadata), &calld->compression_algorithm_storage, - grpc_mdelem_ref(channeld->mdelem_compression_algorithms + GRPC_MDELEM_REF(channeld->mdelem_compression_algorithms [calld->compression_algorithm])); calld->written_initial_metadata = 1; /* GPR_TRUE */ } @@ -295,7 +295,7 @@ static void init_channel_elem(grpc_channel_element *elem, grpc_channel *master, channeld->mdelem_compression_algorithms[algo_idx] = grpc_mdelem_from_metadata_strings( mdctx, - grpc_mdstr_ref(channeld->mdstr_outgoing_compression_algorithm_key), + GRPC_MDSTR_REF(channeld->mdstr_outgoing_compression_algorithm_key), grpc_mdstr_from_string(mdctx, algorithm_name, 0)); } @@ -307,11 +307,11 @@ static void destroy_channel_elem(grpc_channel_element *elem) { channel_data *channeld = elem->channel_data; grpc_compression_algorithm algo_idx; - grpc_mdstr_unref(channeld->mdstr_request_compression_algorithm_key); - grpc_mdstr_unref(channeld->mdstr_outgoing_compression_algorithm_key); + GRPC_MDSTR_UNREF(channeld->mdstr_request_compression_algorithm_key); + GRPC_MDSTR_UNREF(channeld->mdstr_outgoing_compression_algorithm_key); for (algo_idx = 0; algo_idx < GRPC_COMPRESS_ALGORITHMS_COUNT; ++algo_idx) { - grpc_mdelem_unref(channeld->mdelem_compression_algorithms[algo_idx]); + GRPC_MDELEM_UNREF(channeld->mdelem_compression_algorithms[algo_idx]); } } diff --git a/src/core/security/client_auth_filter.c b/src/core/security/client_auth_filter.c index e86b5430b2c..e2d1b6fce91 100644 --- a/src/core/security/client_auth_filter.c +++ b/src/core/security/client_auth_filter.c @@ -77,10 +77,8 @@ typedef struct { static void bubble_up_error(grpc_call_element *elem, const char *error_msg) { call_data *calld = elem->call_data; - channel_data *chand = elem->channel_data; - grpc_transport_stream_op_add_cancellation( - &calld->op, GRPC_STATUS_UNAUTHENTICATED, - grpc_mdstr_from_string(chand->md_ctx, error_msg, 0)); + grpc_transport_stream_op_add_cancellation(&calld->op, + GRPC_STATUS_UNAUTHENTICATED); grpc_call_next_op(elem, &calld->op); } diff --git a/src/core/security/server_auth_filter.c b/src/core/security/server_auth_filter.c index 10dfb09926a..fd0f94b19c3 100644 --- a/src/core/security/server_auth_filter.c +++ b/src/core/security/server_auth_filter.c @@ -110,7 +110,6 @@ static void on_md_processing_done(void *user_data, grpc_auth_context *result) { grpc_call_element *elem = user_data; call_data *calld = elem->call_data; - channel_data *chand = elem->channel_data; if (success) { calld->consumed_md = consumed_md; @@ -124,10 +123,11 @@ static void on_md_processing_done(void *user_data, GRPC_AUTH_CONTEXT_REF(result, "refing new context."); calld->on_done_recv->cb(calld->on_done_recv->cb_arg, success); } else { - grpc_transport_stream_op_add_cancellation( - &calld->transport_op, GRPC_STATUS_UNAUTHENTICATED, - grpc_mdstr_from_string(chand->mdctx, - "Authentication metadata processing failed.")); + gpr_slice message = gpr_slice_from_copied_string( + "Authentication metadata processing failed."); + grpc_sopb_reset(calld->recv_ops); + grpc_transport_stream_op_add_close(&calld->transport_op, + GRPC_STATUS_UNAUTHENTICATED, &message); grpc_call_next_op(elem, &calld->transport_op); } } diff --git a/src/core/transport/chttp2/internal.h b/src/core/transport/chttp2/internal.h index f0eeb6de505..74b3a591d56 100644 --- a/src/core/transport/chttp2/internal.h +++ b/src/core/transport/chttp2/internal.h @@ -384,6 +384,8 @@ typedef struct { gpr_uint8 in_stream_map; /** is this stream actively being written? */ gpr_uint8 writing_now; + /** has anything been written to this stream? */ + gpr_uint8 written_anything; /** stream state already published to the upper layer */ grpc_stream_state published_state; diff --git a/src/core/transport/chttp2_transport.c b/src/core/transport/chttp2_transport.c index 1ea4a82c16f..c8c4207208b 100644 --- a/src/core/transport/chttp2_transport.c +++ b/src/core/transport/chttp2_transport.c @@ -107,6 +107,11 @@ static void cancel_from_api(grpc_chttp2_transport_global *transport_global, grpc_chttp2_stream_global *stream_global, grpc_status_code status); +static void close_from_api(grpc_chttp2_transport_global *transport_global, + grpc_chttp2_stream_global *stream_global, + grpc_status_code status, + gpr_slice *optional_message); + /** Add endpoint from this transport to pollset */ static void add_to_pollset_locked(grpc_chttp2_transport *t, grpc_pollset *pollset); @@ -602,10 +607,16 @@ static void perform_stream_op_locked( cancel_from_api(transport_global, stream_global, op->cancel_with_status); } + if (op->close_with_status != GRPC_STATUS_OK) { + close_from_api(transport_global, stream_global, op->close_with_status, + op->optional_close_message); + } + if (op->send_ops) { GPR_ASSERT(stream_global->outgoing_sopb == NULL); stream_global->send_done_closure = op->on_done_send; if (!stream_global->cancelled) { + stream_global->written_anything = 1; stream_global->outgoing_sopb = op->send_ops; if (op->is_last_send && stream_global->write_state == GRPC_WRITE_STATE_OPEN) { @@ -894,6 +905,108 @@ static void cancel_from_api(grpc_chttp2_transport_global *transport_global, stream_global); } +static void close_from_api(grpc_chttp2_transport_global *transport_global, + grpc_chttp2_stream_global *stream_global, + grpc_status_code status, + gpr_slice *optional_message) { + gpr_slice hdr; + gpr_slice status_hdr; + gpr_slice message_pfx; + gpr_uint8 *p; + gpr_uint32 len = 0; + + GPR_ASSERT(status >= 0 && (int)status < 100); + + stream_global->cancelled = 1; + stream_global->cancelled_status = status; + GPR_ASSERT(stream_global->id != 0); + GPR_ASSERT(!stream_global->written_anything); + + /* Hand roll a header block. + This is unnecessarily ugly - at some point we should find a more elegant + solution. + It's complicated by the fact that our send machinery would be dead by the + time we got around to sending this, so instead we ignore HPACK compression + and just write the uncompressed bytes onto the wire. */ + status_hdr = gpr_slice_malloc(15 + (status >= 10)); + p = GPR_SLICE_START_PTR(status_hdr); + *p++ = 0x40; /* literal header */ + *p++ = 11; /* len(grpc-status) */ + *p++ = 'g'; + *p++ = 'r'; + *p++ = 'p'; + *p++ = 'c'; + *p++ = '-'; + *p++ = 's'; + *p++ = 't'; + *p++ = 'a'; + *p++ = 't'; + *p++ = 'u'; + *p++ = 's'; + if (status < 10) { + *p++ = 1; + *p++ = '0' + status; + } else { + *p++ = 2; + *p++ = '0' + (status / 10); + *p++ = '0' + (status % 10); + } + GPR_ASSERT(p == GPR_SLICE_END_PTR(status_hdr)); + len += GPR_SLICE_LENGTH(status_hdr); + + if (optional_message) { + GPR_ASSERT(GPR_SLICE_LENGTH(*optional_message) < 127); + message_pfx = gpr_slice_malloc(15); + p = GPR_SLICE_START_PTR(message_pfx); + *p++ = 0x40; + *p++ = 12; /* len(grpc-message) */ + *p++ = 'g'; + *p++ = 'r'; + *p++ = 'p'; + *p++ = 'c'; + *p++ = '-'; + *p++ = 'm'; + *p++ = 'e'; + *p++ = 's'; + *p++ = 's'; + *p++ = 'a'; + *p++ = 'g'; + *p++ = 'e'; + *p++ = GPR_SLICE_LENGTH(*optional_message); + GPR_ASSERT(p == GPR_SLICE_END_PTR(message_pfx)); + len += GPR_SLICE_LENGTH(message_pfx); + len += GPR_SLICE_LENGTH(*optional_message); + } + + hdr = gpr_slice_malloc(9); + p = GPR_SLICE_START_PTR(hdr); + *p++ = len >> 16; + *p++ = len >> 8; + *p++ = len; + *p++ = GRPC_CHTTP2_FRAME_HEADER; + *p++ = GRPC_CHTTP2_DATA_FLAG_END_STREAM | GRPC_CHTTP2_DATA_FLAG_END_HEADERS; + *p++ = stream_global->id >> 24; + *p++ = stream_global->id >> 16; + *p++ = stream_global->id >> 8; + *p++ = stream_global->id; + GPR_ASSERT(p == GPR_SLICE_END_PTR(hdr)); + + gpr_slice_buffer_add(&transport_global->qbuf, hdr); + gpr_slice_buffer_add(&transport_global->qbuf, status_hdr); + if (optional_message) { + gpr_slice_buffer_add(&transport_global->qbuf, message_pfx); + gpr_slice_buffer_add(&transport_global->qbuf, + gpr_slice_ref(*optional_message)); + } + + gpr_slice_buffer_add( + &transport_global->qbuf, + grpc_chttp2_rst_stream_create(stream_global->id, GRPC_CHTTP2_NO_ERROR)); + + grpc_chttp2_list_add_read_write_state_changed(transport_global, + stream_global); +} + static void cancel_stream_cb(grpc_chttp2_transport_global *transport_global, void *user_data, grpc_chttp2_stream_global *stream_global) { diff --git a/src/core/transport/metadata.c b/src/core/transport/metadata.c index 967fd4898c5..44d32b6cb2b 100644 --- a/src/core/transport/metadata.c +++ b/src/core/transport/metadata.c @@ -135,7 +135,9 @@ static void unlock(grpc_mdctx *ctx) { if (ctx->refs == 0) { /* uncomment if you're having trouble diagnosing an mdelem leak to make things clearer (slows down destruction a lot, however) */ +#ifdef GRPC_METADATA_REFCOUNT_DEBUG gc_mdtab(ctx); +#endif if (ctx->mdtab_count && ctx->mdtab_count == ctx->mdtab_free) { discard_metadata(ctx); } diff --git a/src/core/transport/transport.c b/src/core/transport/transport.c index 69c00b6a4fd..c0d92cf93f6 100644 --- a/src/core/transport/transport.c +++ b/src/core/transport/transport.c @@ -32,6 +32,8 @@ */ #include "src/core/transport/transport.h" +#include +#include #include "src/core/transport/transport_impl.h" size_t grpc_transport_stream_size(grpc_transport *transport) { @@ -83,12 +85,54 @@ void grpc_transport_stream_op_finish_with_failure( } void grpc_transport_stream_op_add_cancellation(grpc_transport_stream_op *op, - grpc_status_code status, - grpc_mdstr *message) { + grpc_status_code status) { + GPR_ASSERT(status != GRPC_STATUS_OK); if (op->cancel_with_status == GRPC_STATUS_OK) { op->cancel_with_status = status; } - if (message) { - GRPC_MDSTR_UNREF(message); + if (op->close_with_status != GRPC_STATUS_OK) { + op->close_with_status = GRPC_STATUS_OK; + if (op->optional_close_message != NULL) { + gpr_slice_unref(*op->optional_close_message); + op->optional_close_message = NULL; + } } } + +typedef struct { + gpr_slice message; + grpc_iomgr_closure *then_call; + grpc_iomgr_closure closure; +} close_message_data; + +static void free_message(void *p, int iomgr_success) { + close_message_data *cmd = p; + gpr_slice_unref(cmd->message); + if (cmd->then_call != NULL) { + cmd->then_call->cb(cmd->then_call->cb_arg, iomgr_success); + } + gpr_free(cmd); +} + +void grpc_transport_stream_op_add_close(grpc_transport_stream_op *op, + grpc_status_code status, + gpr_slice *optional_message) { + close_message_data *cmd; + GPR_ASSERT(status != GRPC_STATUS_OK); + if (op->cancel_with_status != GRPC_STATUS_OK || + op->close_with_status != GRPC_STATUS_OK) { + if (optional_message) { + gpr_slice_unref(*optional_message); + } + return; + } + if (optional_message) { + cmd = gpr_malloc(sizeof(*cmd)); + cmd->message = *optional_message; + cmd->then_call = op->on_consumed; + grpc_iomgr_closure_init(&cmd->closure, free_message, cmd); + op->on_consumed = &cmd->closure; + op->optional_close_message = &cmd->message; + } + op->close_with_status = status; +} diff --git a/src/core/transport/transport.h b/src/core/transport/transport.h index 7efcfcf970c..92c1f38c5ea 100644 --- a/src/core/transport/transport.h +++ b/src/core/transport/transport.h @@ -80,8 +80,14 @@ typedef struct grpc_transport_stream_op { grpc_pollset *bind_pollset; + /** If != GRPC_STATUS_OK, cancel this stream */ grpc_status_code cancel_with_status; + /** If != GRPC_STATUS_OK, send grpc-status, grpc-message, and close this + stream for both reading and writing */ + grpc_status_code close_with_status; + gpr_slice *optional_close_message; + /* Indexes correspond to grpc_context_index enum values */ grpc_call_context_element *context; } grpc_transport_stream_op; @@ -148,8 +154,11 @@ void grpc_transport_destroy_stream(grpc_transport *transport, void grpc_transport_stream_op_finish_with_failure(grpc_transport_stream_op *op); void grpc_transport_stream_op_add_cancellation(grpc_transport_stream_op *op, - grpc_status_code status, - grpc_mdstr *message); + grpc_status_code status); + +void grpc_transport_stream_op_add_close(grpc_transport_stream_op *op, + grpc_status_code status, + gpr_slice *optional_message); char *grpc_transport_stream_op_string(grpc_transport_stream_op *op); 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 7facb6997b3..48ea0a29d4b 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 @@ -535,7 +535,7 @@ static void test_request_with_server_rejecting_client_creds( (probably in the server_auth_context.c code) where this error on the server does not get to the client. The current error code we are getting is GRPC_STATUS_INTERNAL. */ - GPR_ASSERT(status != GRPC_STATUS_OK); + GPR_ASSERT(status == GRPC_STATUS_UNAUTHENTICATED); grpc_metadata_array_destroy(&initial_metadata_recv); grpc_metadata_array_destroy(&trailing_metadata_recv); From 8e9ff222999199f88344bbb9b4cee3bfc7de433f Mon Sep 17 00:00:00 2001 From: Julien Boeuf Date: Mon, 3 Aug 2015 15:49:14 -0700 Subject: [PATCH 6/9] Removing obsolete comment. --- .../tests/request_response_with_payload_and_call_creds.c | 4 ---- 1 file changed, 4 deletions(-) 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 48ea0a29d4b..2166bd41f71 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 @@ -531,10 +531,6 @@ static void test_request_with_server_rejecting_client_creds( cq_expect_completion(cqv, tag(1), 1); cq_verify(cqv); - /* XXX Should be GRPC_STATUS_UNAUTHENTICATED but it looks like there is a bug - (probably in the server_auth_context.c code) where this error on the server - does not get to the client. The current error code we are getting is - GRPC_STATUS_INTERNAL. */ GPR_ASSERT(status == GRPC_STATUS_UNAUTHENTICATED); grpc_metadata_array_destroy(&initial_metadata_recv); From 77a7b870c3e4259cc8f5cffc2b59876b42c0624a Mon Sep 17 00:00:00 2001 From: Julien Boeuf Date: Wed, 5 Aug 2015 20:11:02 -0700 Subject: [PATCH 7/9] 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); } } From 7c8d255527cbec8b261300296d61361ce94e9d18 Mon Sep 17 00:00:00 2001 From: Julien Boeuf Date: Wed, 5 Aug 2015 20:16:01 -0700 Subject: [PATCH 8/9] Cleanup. --- src/core/security/security_context.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/core/security/security_context.h b/src/core/security/security_context.h index 3ad57cadea6..7fcd438cf6f 100644 --- a/src/core/security/security_context.h +++ b/src/core/security/security_context.h @@ -78,8 +78,6 @@ 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 --- From 54b5018dd889cbc5059ff77ba083ff7020c63a9b Mon Sep 17 00:00:00 2001 From: Julien Boeuf Date: Wed, 5 Aug 2015 21:40:49 -0700 Subject: [PATCH 9/9] Fixing cpp tests. --- test/cpp/common/auth_property_iterator_test.cc | 4 ++++ test/cpp/common/secure_auth_context_test.cc | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/test/cpp/common/auth_property_iterator_test.cc b/test/cpp/common/auth_property_iterator_test.cc index 6443e2fd85c..74b18ced0d5 100644 --- a/test/cpp/common/auth_property_iterator_test.cc +++ b/test/cpp/common/auth_property_iterator_test.cc @@ -36,6 +36,10 @@ #include #include "src/cpp/common/secure_auth_context.h" +extern "C" { +#include "src/core/security/security_context.h" +} + namespace grpc { namespace { diff --git a/test/cpp/common/secure_auth_context_test.cc b/test/cpp/common/secure_auth_context_test.cc index bdc5cf73bb6..075d4ce8c98 100644 --- a/test/cpp/common/secure_auth_context_test.cc +++ b/test/cpp/common/secure_auth_context_test.cc @@ -36,6 +36,10 @@ #include #include "src/cpp/common/secure_auth_context.h" +extern "C" { +#include "src/core/security/security_context.h" +} + namespace grpc { namespace {