Fixing #3680

The server auth filter needs a reference on the server credentials so
that the processor that belongs to the creds is not destroyed when the
server auth filter is still using it.

The server auth filter also does not need the security connector but
just the auth context.

6 files changed, 91 insertions, 36 deletions

diff --git a/src/core/security/credentials.c b/src/core/security/credentials.c
index 398db20e8c..5e155d83b9 100644
--- a/src/core/security/credentials.c
+++ b/src/core/security/credentials.c
@@ -181,6 +181,48 @@ void grpc_server_credentials_set_auth_metadata_processor(
   creds->processor = processor;
 }
 
+static void server_credentials_pointer_arg_destroy(void *p) {
+  grpc_server_credentials_unref(p);
+}
+
+static void *server_credentials_pointer_arg_copy(void *p) {
+  return grpc_server_credentials_ref(p);
+}
+
+grpc_arg grpc_server_credentials_to_arg(grpc_server_credentials *p) {
+  grpc_arg arg;
+  memset(&arg, 0, sizeof(grpc_arg));
+  arg.type = GRPC_ARG_POINTER;
+  arg.key = GRPC_SERVER_CREDENTIALS_ARG;
+  arg.value.pointer.p = p;
+  arg.value.pointer.copy = server_credentials_pointer_arg_copy;
+  arg.value.pointer.destroy = server_credentials_pointer_arg_destroy;
+  return arg;
+}
+
+grpc_server_credentials *grpc_server_credentials_from_arg(
+    const grpc_arg *arg) {
+  if (strcmp(arg->key, GRPC_SERVER_CREDENTIALS_ARG) != 0) return NULL;
+  if (arg->type != GRPC_ARG_POINTER) {
+    gpr_log(GPR_ERROR, "Invalid type %d for arg %s", arg->type,
+            GRPC_SERVER_CREDENTIALS_ARG);
+    return NULL;
+  }
+  return arg->value.pointer.p;
+}
+
+grpc_server_credentials *grpc_find_server_credentials_in_args(
+    const grpc_channel_args *args) {
+  size_t i;
+  if (args == NULL) return NULL;
+  for (i = 0; i < args->num_args; i++) {
+    grpc_server_credentials *p =
+        grpc_server_credentials_from_arg(&args->args[i]);
+    if (p != NULL) return p;
+  }
+  return NULL;
+}
+
 /* -- Ssl credentials. -- */
 
 static void ssl_destruct(grpc_credentials *creds) {
diff --git a/src/core/security/credentials.h b/src/core/security/credentials.h
index b213e052d3..01203b08f1 100644
--- a/src/core/security/credentials.h
+++ b/src/core/security/credentials.h
@@ -215,7 +215,6 @@ typedef struct {
       grpc_server_credentials *c, grpc_security_connector **sc);
 } grpc_server_credentials_vtable;
 
-/* TODO(jboeuf): Add a refcount. */
 struct grpc_server_credentials {
   const grpc_server_credentials_vtable *vtable;
   const char *type;
@@ -231,6 +230,13 @@ grpc_server_credentials *grpc_server_credentials_ref(
 
 void grpc_server_credentials_unref(grpc_server_credentials *creds);
 
+#define GRPC_SERVER_CREDENTIALS_ARG "grpc.server_credentials"
+
+grpc_arg grpc_server_credentials_to_arg(grpc_server_credentials *c);
+grpc_server_credentials *grpc_server_credentials_from_arg(const grpc_arg *arg);
+grpc_server_credentials *grpc_find_server_credentials_in_args(
+    const grpc_channel_args *args);
+
 /* -- Ssl credentials. -- */
 
 typedef struct {
diff --git a/src/core/security/security_context.c b/src/core/security/security_context.c
index fb905e0b22..f544c1d943 100644
--- a/src/core/security/security_context.c
+++ b/src/core/security/security_context.c
@@ -305,33 +305,43 @@ 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) {
+static void auth_context_pointer_arg_destroy(void *p) {
+  GRPC_AUTH_CONTEXT_UNREF(p, "auth_context_pointer_arg");
+}
+
+static void *auth_context_pointer_arg_copy(void *p) {
+  return GRPC_AUTH_CONTEXT_REF(p, "auth_context_pointer_arg");
+}
+
+grpc_arg grpc_auth_context_to_arg(grpc_auth_context *p) {
   grpc_arg arg;
   memset(&arg, 0, sizeof(grpc_arg));
   arg.type = GRPC_ARG_POINTER;
-  arg.key = GRPC_AUTH_METADATA_PROCESSOR_ARG;
+  arg.key = GRPC_AUTH_CONTEXT_ARG;
   arg.value.pointer.p = p;
+  arg.value.pointer.copy = auth_context_pointer_arg_copy;
+  arg.value.pointer.destroy = auth_context_pointer_arg_destroy;
   return arg;
 }
 
-grpc_auth_metadata_processor *grpc_auth_metadata_processor_from_arg(
+grpc_auth_context *grpc_auth_context_from_arg(
     const grpc_arg *arg) {
-  if (strcmp(arg->key, GRPC_AUTH_METADATA_PROCESSOR_ARG) != 0) return NULL;
+  if (strcmp(arg->key, GRPC_AUTH_CONTEXT_ARG) != 0) return NULL;
   if (arg->type != GRPC_ARG_POINTER) {
     gpr_log(GPR_ERROR, "Invalid type %d for arg %s", arg->type,
-            GRPC_AUTH_METADATA_PROCESSOR_ARG);
+            GRPC_AUTH_CONTEXT_ARG);
     return NULL;
   }
   return arg->value.pointer.p;
 }
 
-grpc_auth_metadata_processor *grpc_find_auth_metadata_processor_in_args(
+grpc_auth_context *grpc_find_auth_context_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]);
+    grpc_auth_context *p =
+        grpc_auth_context_from_arg(&args->args[i]);
     if (p != NULL) return p;
   }
   return NULL;
diff --git a/src/core/security/security_context.h b/src/core/security/security_context.h
index a9a0306410..2bbdc4be97 100644
--- a/src/core/security/security_context.h
+++ b/src/core/security/security_context.h
@@ -103,13 +103,12 @@ typedef struct {
 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"
+/* --- Channel args for auth context --- */
+#define GRPC_AUTH_CONTEXT_ARG "grpc.auth_context"
 
-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);
-grpc_auth_metadata_processor *grpc_find_auth_metadata_processor_in_args(
+grpc_arg grpc_auth_context_to_arg(grpc_auth_context *c);
+grpc_auth_context *grpc_auth_context_from_arg(const grpc_arg *arg);
+grpc_auth_context *grpc_find_auth_context_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 30ca9f57a2..2e18369fe8 100644
--- a/src/core/security/server_auth_filter.c
+++ b/src/core/security/server_auth_filter.c
@@ -34,7 +34,7 @@
 #include <string.h>
 
 #include "src/core/security/auth_filters.h"
-#include "src/core/security/security_connector.h"
+#include "src/core/security/credentials.h"
 #include "src/core/security/security_context.h"
 
 #include <grpc/support/alloc.h>
@@ -58,8 +58,8 @@ typedef struct call_data {
 } call_data;
 
 typedef struct channel_data {
-  grpc_security_connector *security_connector;
-  grpc_auth_metadata_processor processor;
+  grpc_auth_context *auth_context;
+  grpc_server_credentials *creds;
   grpc_mdctx *mdctx;
 } channel_data;
 
@@ -160,12 +160,12 @@ static void auth_on_recv(grpc_exec_ctx *exec_ctx, void *user_data,
       grpc_stream_op *op = &ops[i];
       if (op->type != GRPC_OP_METADATA || calld->got_client_metadata) continue;
       calld->got_client_metadata = 1;
-      if (chand->processor.process == NULL) continue;
+      if (chand->creds->processor.process == NULL) continue;
       calld->md_op = op;
       calld->md = metadata_batch_to_md_array(&op->data.metadata);
-      chand->processor.process(chand->processor.state, calld->auth_context,
-                               calld->md.metadata, calld->md.count,
-                               on_md_processing_done, elem);
+      chand->creds->processor.process(
+          chand->creds->processor.state, calld->auth_context,
+          calld->md.metadata, calld->md.count, on_md_processing_done, elem);
       return;
     }
   }
@@ -221,7 +221,7 @@ static void init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem,
   }
   server_ctx = grpc_server_security_context_create();
   server_ctx->auth_context =
-      grpc_auth_context_create(chand->security_connector->auth_context);
+      grpc_auth_context_create(chand->auth_context);
   server_ctx->auth_context->pollset = initial_op->bind_pollset;
   initial_op->context[GRPC_CONTEXT_SECURITY].value = server_ctx;
   initial_op->context[GRPC_CONTEXT_SECURITY].destroy =
@@ -241,9 +241,8 @@ static void init_channel_elem(grpc_exec_ctx *exec_ctx,
                               grpc_channel_element *elem, grpc_channel *master,
                               const grpc_channel_args *args, grpc_mdctx *mdctx,
                               int is_first, int is_last) {
-  grpc_security_connector *sc = grpc_find_security_connector_in_args(args);
-  grpc_auth_metadata_processor *processor =
-      grpc_find_auth_metadata_processor_in_args(args);
+  grpc_auth_context *auth_context = grpc_find_auth_context_in_args(args);
+  grpc_server_credentials *creds = grpc_find_server_credentials_in_args(args);
   /* grab pointers to our data from the channel element */
   channel_data *chand = elem->channel_data;
 
@@ -252,15 +251,14 @@ static void init_channel_elem(grpc_exec_ctx *exec_ctx,
      path */
   GPR_ASSERT(!is_first);
   GPR_ASSERT(!is_last);
-  GPR_ASSERT(sc != NULL);
-  GPR_ASSERT(processor != NULL);
+  GPR_ASSERT(auth_context != NULL);
+  GPR_ASSERT(creds != NULL);
 
   /* initialize members */
-  GPR_ASSERT(!sc->is_client_side);
-  chand->security_connector =
-      GRPC_SECURITY_CONNECTOR_REF(sc, "server_auth_filter");
+  chand->auth_context =
+      GRPC_AUTH_CONTEXT_REF(auth_context, "server_auth_filter");
+  chand->creds = grpc_server_credentials_ref(creds);
   chand->mdctx = mdctx;
-  chand->processor = *processor;
 }
 
 /* Destructor for channel data */
@@ -268,8 +266,8 @@ static void destroy_channel_elem(grpc_exec_ctx *exec_ctx,
                                  grpc_channel_element *elem) {
   /* grab pointers to our data from the channel element */
   channel_data *chand = elem->channel_data;
-  GRPC_SECURITY_CONNECTOR_UNREF(chand->security_connector,
-                                "server_auth_filter");
+  GRPC_AUTH_CONTEXT_UNREF(chand->auth_context, "server_auth_filter");
+  grpc_server_credentials_unref(chand->creds);
 }
 
 const grpc_channel_filter grpc_server_auth_filter = {
diff --git a/src/core/security/server_secure_chttp2.c b/src/core/security/server_secure_chttp2.c
index 881e44a3fe..82c639e830 100644
--- a/src/core/security/server_secure_chttp2.c
+++ b/src/core/security/server_secure_chttp2.c
@@ -93,9 +93,9 @@ static void setup_transport(grpc_exec_ctx *exec_ctx, void *statep,
   grpc_server_secure_state *state = statep;
   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[0] = grpc_server_credentials_to_arg(state->creds);
   args_to_add[1] =
-      grpc_auth_metadata_processor_to_arg(&state->creds->processor);
+      grpc_auth_context_to_arg(state->sc->auth_context);
   args_copy = grpc_channel_args_copy_and_add(
       grpc_server_get_channel_args(state->server), args_to_add,
       GPR_ARRAY_SIZE(args_to_add));