Fixing API (thanks Craig for the comments) and associated tests.

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;
 

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 ---

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);

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;

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);
 }
 

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);
 }
 

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);
 }
 

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);
 }

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);
   }
 }