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.
diff --git a/src/core/security/credentials.c b/src/core/security/credentials.c
index 398db20..5e155d8 100644
--- a/src/core/security/credentials.c
+++ b/src/core/security/credentials.c
@@ -181,6 +181,48 @@
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 b213e05..01203b0 100644
--- a/src/core/security/credentials.h
+++ b/src/core/security/credentials.h
@@ -215,7 +215,6 @@
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 @@
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 fb905e0..f544c1d 100644
--- a/src/core/security/security_context.c
+++ b/src/core/security/security_context.c
@@ -305,33 +305,43 @@
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 a9a0306..2bbdc4b 100644
--- a/src/core/security/security_context.h
+++ b/src/core/security/security_context.h
@@ -103,13 +103,12 @@
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 30ca9f5..2e18369 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 @@
} 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 @@
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 @@
}
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 @@
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 @@
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 @@
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 881e44a..82c639e 100644
--- a/src/core/security/server_secure_chttp2.c
+++ b/src/core/security/server_secure_chttp2.c
@@ -93,9 +93,9 @@
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));