Skip to content

Commit

Permalink
Merge pull request grpc#4523 from jboeuf/no_auth_context_in_security_…
Browse files Browse the repository at this point in the history 
…connector

Removing the auth_context from the security connector
  • Loading branch information
@ctiller
ctiller committed Dec 18, 2015
2 parents 7bd86e1 + 5b19403 commit 3f9fe46
Show file tree
Hide file tree
Showing 10 changed files with 123 additions and 95 deletions.
3 changes: 3 additions & 0 deletions src/core/client_config/connector.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ typedef struct {
/** any additional filters (owned by the caller of connect) */
const grpc_channel_filter **filters;
size_t num_filters;

/** channel arguments (to be passed to the filters) */
const grpc_channel_args *channel_args;
} grpc_connect_out_args;

struct grpc_connector_vtable {
Expand Down
3 changes: 2 additions & 1 deletion src/core/client_config/subchannel.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,8 @@ static void publish_transport(grpc_exec_ctx *exec_ctx, grpc_subchannel *c) {
con = gpr_malloc(channel_stack_size);
stk = CHANNEL_STACK_FROM_CONNECTION(con);
grpc_channel_stack_init(exec_ctx, 1, connection_destroy, con, filters,
num_filters, c->args, "CONNECTED_SUBCHANNEL", stk);
num_filters, c->connecting_result.channel_args,
"CONNECTED_SUBCHANNEL", stk);
grpc_connected_channel_bind_transport(stk, c->connecting_result.transport);
gpr_free((void *)c->connecting_result.filters);
memset(&c->connecting_result, 0, sizeof(c->connecting_result));
Expand Down
17 changes: 9 additions & 8 deletions src/core/httpcli/httpcli_security_connector.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,25 +68,25 @@ static void httpcli_ssl_do_handshake(grpc_exec_ctx *exec_ctx,
tsi_result result = TSI_OK;
tsi_handshaker *handshaker;
if (c->handshaker_factory == NULL) {
cb(exec_ctx, user_data, GRPC_SECURITY_ERROR, NULL);
cb(exec_ctx, user_data, GRPC_SECURITY_ERROR, NULL, NULL);
return;
}
result = tsi_ssl_handshaker_factory_create_handshaker(
c->handshaker_factory, c->secure_peer_name, &handshaker);
if (result != TSI_OK) {
gpr_log(GPR_ERROR, "Handshaker creation failed with error %s.",
tsi_result_to_string(result));
cb(exec_ctx, user_data, GRPC_SECURITY_ERROR, NULL);
cb(exec_ctx, user_data, GRPC_SECURITY_ERROR, NULL, NULL);
} else {
grpc_do_security_handshake(exec_ctx, handshaker, sc, nonsecure_endpoint, cb,
user_data);
}
}

static grpc_security_status httpcli_ssl_check_peer(grpc_security_connector *sc,
tsi_peer peer,
grpc_security_check_cb cb,
void *user_data) {
static void httpcli_ssl_check_peer(grpc_exec_ctx *exec_ctx,
grpc_security_connector *sc, tsi_peer peer,
grpc_security_peer_check_cb cb,
void *user_data) {
grpc_httpcli_ssl_channel_security_connector *c =
(grpc_httpcli_ssl_channel_security_connector *)sc;
grpc_security_status status = GRPC_SECURITY_OK;
Expand All @@ -99,7 +99,7 @@ static grpc_security_status httpcli_ssl_check_peer(grpc_security_connector *sc,
status = GRPC_SECURITY_ERROR;
}
tsi_peer_destruct(&peer);
return status;
cb(exec_ctx, user_data, status, NULL);
}

static grpc_security_connector_vtable httpcli_ssl_vtable = {
Expand Down Expand Up @@ -149,7 +149,8 @@ typedef struct {

static void on_secure_transport_setup_done(grpc_exec_ctx *exec_ctx, void *rp,
grpc_security_status status,
grpc_endpoint *secure_endpoint) {
grpc_endpoint *secure_endpoint,
grpc_auth_context *auth_context) {
on_done_closure *c = rp;
if (status != GRPC_SECURITY_OK) {
gpr_log(GPR_ERROR, "Secure transport setup failed with error %d.", status);
Expand Down
22 changes: 16 additions & 6 deletions src/core/security/client_auth_filter.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ typedef struct {
/* We can have a per-channel credentials. */
typedef struct {
grpc_channel_security_connector *security_connector;
grpc_auth_context *auth_context;
} channel_data;

static void reset_auth_metadata_context(
Expand Down Expand Up @@ -122,6 +123,7 @@ static void on_credentials_metadata(grpc_exec_ctx *exec_ctx, void *user_data,
}

void build_auth_metadata_context(grpc_security_connector *sc,
grpc_auth_context *auth_context,
call_data *calld) {
char *service = gpr_strdup(grpc_mdstr_as_c_string(calld->method));
char *last_slash = strrchr(service, '/');
Expand All @@ -145,7 +147,7 @@ void build_auth_metadata_context(grpc_security_connector *sc,
calld->auth_md_context.service_url = service_url;
calld->auth_md_context.method_name = method_name;
calld->auth_md_context.channel_auth_context =
GRPC_AUTH_CONTEXT_REF(sc->auth_context, "grpc_auth_metadata_context");
GRPC_AUTH_CONTEXT_REF(auth_context, "grpc_auth_metadata_context");
gpr_free(service);
}

Expand Down Expand Up @@ -179,7 +181,8 @@ static void send_security_metadata(grpc_exec_ctx *exec_ctx,
call_creds_has_md ? ctx->creds : channel_call_creds);
}

build_auth_metadata_context(&chand->security_connector->base, calld);
build_auth_metadata_context(&chand->security_connector->base,
chand->auth_context, calld);
calld->op = *op; /* Copy op (originates from the caller's stack). */
GPR_ASSERT(calld->pollset);
grpc_call_credentials_get_request_metadata(
Expand Down Expand Up @@ -230,7 +233,7 @@ static void auth_start_transport_op(grpc_exec_ctx *exec_ctx,
sec_ctx = op->context[GRPC_CONTEXT_SECURITY].value;
GRPC_AUTH_CONTEXT_UNREF(sec_ctx->auth_context, "client auth filter");
sec_ctx->auth_context = GRPC_AUTH_CONTEXT_REF(
chand->security_connector->base.auth_context, "client_auth_filter");
chand->auth_context, "client_auth_filter");
}

if (op->send_initial_metadata != NULL) {
Expand Down Expand Up @@ -307,6 +310,9 @@ static void init_channel_elem(grpc_exec_ctx *exec_ctx,
grpc_channel_element_args *args) {
grpc_security_connector *sc =
grpc_find_security_connector_in_args(args->channel_args);
grpc_auth_context *auth_context =
grpc_find_auth_context_in_args(args->channel_args);

/* grab pointers to our data from the channel element */
channel_data *chand = elem->channel_data;

Expand All @@ -315,23 +321,27 @@ static void init_channel_elem(grpc_exec_ctx *exec_ctx,
path */
GPR_ASSERT(!args->is_last);
GPR_ASSERT(sc != NULL);
GPR_ASSERT(auth_context != NULL);

/* initialize members */
GPR_ASSERT(sc->is_client_side);
chand->security_connector =
(grpc_channel_security_connector *)GRPC_SECURITY_CONNECTOR_REF(
sc, "client_auth_filter");
chand->auth_context =
GRPC_AUTH_CONTEXT_REF(auth_context, "client_auth_filter");
}

/* Destructor for channel data */
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_channel_security_connector *ctx = chand->security_connector;
if (ctx != NULL) {
GRPC_SECURITY_CONNECTOR_UNREF(&ctx->base, "client_auth_filter");
grpc_channel_security_connector *sc = chand->security_connector;
if (sc != NULL) {
GRPC_SECURITY_CONNECTOR_UNREF(&sc->base, "client_auth_filter");
}
GRPC_AUTH_CONTEXT_UNREF(chand->auth_context, "client_auth_filter");
}

const grpc_channel_filter grpc_client_auth_filter = {
Expand Down
24 changes: 11 additions & 13 deletions src/core/security/handshake.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@

#include <string.h>

#include "src/core/security/security_context.h"
#include "src/core/security/secure_endpoint.h"
#include <grpc/support/alloc.h>
#include <grpc/support/log.h>
Expand All @@ -56,6 +57,7 @@ typedef struct {
void *user_data;
grpc_closure on_handshake_data_sent_to_peer;
grpc_closure on_handshake_data_received_from_peer;
grpc_auth_context *auth_context;
} grpc_security_handshake;

static void on_handshake_data_received_from_peer(grpc_exec_ctx *exec_ctx,
Expand Down Expand Up @@ -96,27 +98,30 @@ static void security_handshake_done(grpc_exec_ctx *exec_ctx,
security_connector_remove_handshake(h);
}
if (is_success) {
h->cb(exec_ctx, h->user_data, GRPC_SECURITY_OK, h->secure_endpoint);
h->cb(exec_ctx, h->user_data, GRPC_SECURITY_OK, h->secure_endpoint,
h->auth_context);
} else {
if (h->secure_endpoint != NULL) {
grpc_endpoint_shutdown(exec_ctx, h->secure_endpoint);
grpc_endpoint_destroy(exec_ctx, h->secure_endpoint);
} else {
grpc_endpoint_destroy(exec_ctx, h->wrapped_endpoint);
}
h->cb(exec_ctx, h->user_data, GRPC_SECURITY_ERROR, NULL);
h->cb(exec_ctx, h->user_data, GRPC_SECURITY_ERROR, NULL, NULL);
}
if (h->handshaker != NULL) tsi_handshaker_destroy(h->handshaker);
if (h->handshake_buffer != NULL) gpr_free(h->handshake_buffer);
gpr_slice_buffer_destroy(&h->left_overs);
gpr_slice_buffer_destroy(&h->outgoing);
gpr_slice_buffer_destroy(&h->incoming);
GRPC_AUTH_CONTEXT_UNREF(h->auth_context, "handshake");
GRPC_SECURITY_CONNECTOR_UNREF(h->connector, "handshake");
gpr_free(h);
}

static void on_peer_checked(grpc_exec_ctx *exec_ctx, void *user_data,
grpc_security_status status) {
grpc_security_status status,
grpc_auth_context *auth_context) {
grpc_security_handshake *h = user_data;
tsi_frame_protector *protector;
tsi_result result;
Expand All @@ -125,6 +130,7 @@ static void on_peer_checked(grpc_exec_ctx *exec_ctx, void *user_data,
security_handshake_done(exec_ctx, h, 0);
return;
}
h->auth_context = GRPC_AUTH_CONTEXT_REF(auth_context, "handshake");
result =
tsi_handshaker_create_frame_protector(h->handshaker, NULL, &protector);
if (result != TSI_OK) {
Expand All @@ -143,7 +149,6 @@ static void on_peer_checked(grpc_exec_ctx *exec_ctx, void *user_data,
}

static void check_peer(grpc_exec_ctx *exec_ctx, grpc_security_handshake *h) {
grpc_security_status peer_status;
tsi_peer peer;
tsi_result result = tsi_handshaker_extract_peer(h->handshaker, &peer);

Expand All @@ -153,15 +158,8 @@ static void check_peer(grpc_exec_ctx *exec_ctx, grpc_security_handshake *h) {
security_handshake_done(exec_ctx, h, 0);
return;
}
peer_status = grpc_security_connector_check_peer(h->connector, peer,
on_peer_checked, h);
if (peer_status == GRPC_SECURITY_ERROR) {
gpr_log(GPR_ERROR, "Peer check failed.");
security_handshake_done(exec_ctx, h, 0);
return;
} else if (peer_status == GRPC_SECURITY_OK) {
on_peer_checked(exec_ctx, h, peer_status);
}
grpc_security_connector_check_peer(exec_ctx, h->connector, peer,
on_peer_checked, h);
}

static void send_handshake_bytes_to_peer(grpc_exec_ctx *exec_ctx,
Expand Down
Loading

0 comments on commit 3f9fe46

Please sign in to comment.