diff options
author | 2016-11-29 11:25:12 -0800 | |
---|---|---|
committer | 2016-11-29 11:25:12 -0800 | |
commit | 3beb6c52ac27febccf2aa37c87bf6a8070084093 (patch) | |
tree | 70f88be01880bcdf23a562384089c6d4ff45f4d4 | |
parent | 062ab441c422cf9f474a1df203891cc5d62bdd13 (diff) |
Add locking in security handshaker.
-rw-r--r-- | src/core/lib/http/httpcli_security_connector.c | 16 | ||||
-rw-r--r-- | src/core/lib/security/transport/security_connector.c | 88 | ||||
-rw-r--r-- | src/core/lib/security/transport/security_connector.h | 20 | ||||
-rw-r--r-- | src/core/lib/security/transport/security_handshaker.c | 177 |
4 files changed, 161 insertions, 140 deletions
diff --git a/src/core/lib/http/httpcli_security_connector.c b/src/core/lib/http/httpcli_security_connector.c index 9e7e3e2fa1..229021a345 100644 --- a/src/core/lib/http/httpcli_security_connector.c +++ b/src/core/lib/http/httpcli_security_connector.c @@ -86,20 +86,22 @@ static void httpcli_ssl_create_handshakers( 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_auth_context **auth_context, + grpc_closure *on_peer_checked) { grpc_httpcli_ssl_channel_security_connector *c = (grpc_httpcli_ssl_channel_security_connector *)sc; - grpc_security_status status = GRPC_SECURITY_OK; + grpc_error *error = GRPC_ERROR_NONE; /* Check the peer name. */ if (c->secure_peer_name != NULL && !tsi_ssl_peer_matches_name(&peer, c->secure_peer_name)) { - gpr_log(GPR_ERROR, "Peer name %s is not in peer certificate", - c->secure_peer_name); - status = GRPC_SECURITY_ERROR; + char *msg; + gpr_asprintf(&msg, "Peer name %s is not in peer certificate", + c->secure_peer_name); + error = GRPC_ERROR_CREATE(msg); + gpr_free(msg); } - cb(exec_ctx, user_data, status, NULL); + grpc_exec_ctx_sched(exec_ctx, on_peer_checked, error, NULL); tsi_peer_destruct(&peer); } diff --git a/src/core/lib/security/transport/security_connector.c b/src/core/lib/security/transport/security_connector.c index ac68ab3125..cb2e8a2d9a 100644 --- a/src/core/lib/security/transport/security_connector.c +++ b/src/core/lib/security/transport/security_connector.c @@ -130,13 +130,15 @@ void grpc_server_security_connector_create_handshakers( void grpc_security_connector_check_peer(grpc_exec_ctx *exec_ctx, grpc_security_connector *sc, tsi_peer peer, - grpc_security_peer_check_cb cb, - void *user_data) { + grpc_auth_context **auth_context, + grpc_closure *on_peer_checked) { if (sc == NULL) { - cb(exec_ctx, user_data, GRPC_SECURITY_ERROR, NULL); + grpc_exec_ctx_sched( + exec_ctx, on_peer_checked, + GRPC_ERROR_CREATE("cannot check peer -- no security connector"), NULL); tsi_peer_destruct(&peer); } else { - sc->vtable->check_peer(exec_ctx, sc, peer, cb, user_data); + sc->vtable->check_peer(exec_ctx, sc, peer, auth_context, on_peer_checked); } } @@ -242,37 +244,37 @@ static void fake_server_destroy(grpc_security_connector *sc) { static void fake_check_peer(grpc_exec_ctx *exec_ctx, grpc_security_connector *sc, tsi_peer peer, - grpc_security_peer_check_cb cb, void *user_data) { + grpc_auth_context **auth_context, + grpc_closure *on_peer_checked) { const char *prop_name; - grpc_security_status status = GRPC_SECURITY_OK; - grpc_auth_context *auth_context = NULL; + grpc_error *error = GRPC_ERROR_NONE; + *auth_context = NULL; if (peer.property_count != 1) { - gpr_log(GPR_ERROR, "Fake peers should only have 1 property."); - status = GRPC_SECURITY_ERROR; + error = GRPC_ERROR_CREATE("Fake peers should only have 1 property."); goto end; } prop_name = peer.properties[0].name; if (prop_name == NULL || strcmp(prop_name, TSI_CERTIFICATE_TYPE_PEER_PROPERTY)) { - gpr_log(GPR_ERROR, "Unexpected property in fake peer: %s.", - prop_name == NULL ? "<EMPTY>" : prop_name); - status = GRPC_SECURITY_ERROR; + char *msg; + gpr_asprintf(&msg, "Unexpected property in fake peer: %s.", + prop_name == NULL ? "<EMPTY>" : prop_name); + error = GRPC_ERROR_CREATE(msg); + gpr_free(msg); goto end; } if (strncmp(peer.properties[0].value.data, TSI_FAKE_CERTIFICATE_TYPE, peer.properties[0].value.length)) { - gpr_log(GPR_ERROR, "Invalid value for cert type property."); - status = GRPC_SECURITY_ERROR; + error = GRPC_ERROR_CREATE("Invalid value for cert type property."); goto end; } - auth_context = grpc_auth_context_create(NULL); + *auth_context = grpc_auth_context_create(NULL); grpc_auth_context_add_cstring_property( - auth_context, GRPC_TRANSPORT_SECURITY_TYPE_PROPERTY_NAME, + *auth_context, GRPC_TRANSPORT_SECURITY_TYPE_PROPERTY_NAME, GRPC_FAKE_TRANSPORT_SECURITY_TYPE); end: - cb(exec_ctx, user_data, status, auth_context); - grpc_auth_context_unref(auth_context); + grpc_exec_ctx_sched(exec_ctx, on_peer_checked, error, NULL); tsi_peer_destruct(&peer); } @@ -466,57 +468,53 @@ grpc_auth_context *tsi_ssl_peer_to_auth_context(const tsi_peer *peer) { return ctx; } -static grpc_security_status ssl_check_peer(grpc_security_connector *sc, - const char *peer_name, - const tsi_peer *peer, - grpc_auth_context **auth_context) { +static grpc_error *ssl_check_peer(grpc_security_connector *sc, + const char *peer_name, const tsi_peer *peer, + grpc_auth_context **auth_context) { /* Check the ALPN. */ const tsi_peer_property *p = tsi_peer_get_property_by_name(peer, TSI_SSL_ALPN_SELECTED_PROTOCOL); if (p == NULL) { - gpr_log(GPR_ERROR, "Missing selected ALPN property."); - return GRPC_SECURITY_ERROR; + return GRPC_ERROR_CREATE("Cannot check peer: " + "missing selected ALPN property."); } if (!grpc_chttp2_is_alpn_version_supported(p->value.data, p->value.length)) { - gpr_log(GPR_ERROR, "Invalid ALPN value."); - return GRPC_SECURITY_ERROR; + return GRPC_ERROR_CREATE("Cannot check peer: invalid ALPN value."); } /* Check the peer name if specified. */ if (peer_name != NULL && !ssl_host_matches_name(peer, peer_name)) { - gpr_log(GPR_ERROR, "Peer name %s is not in peer certificate", peer_name); - return GRPC_SECURITY_ERROR; + char *msg; + gpr_asprintf(&msg, "Peer name %s is not in peer certificate", peer_name); + grpc_error *error = GRPC_ERROR_CREATE(msg); + gpr_free(msg); + return error; } *auth_context = tsi_ssl_peer_to_auth_context(peer); - return GRPC_SECURITY_OK; + return GRPC_ERROR_NONE; } static void ssl_channel_check_peer(grpc_exec_ctx *exec_ctx, grpc_security_connector *sc, tsi_peer peer, - grpc_security_peer_check_cb cb, - void *user_data) { + grpc_auth_context **auth_context, + grpc_closure *on_peer_checked) { grpc_ssl_channel_security_connector *c = (grpc_ssl_channel_security_connector *)sc; - grpc_security_status status; - grpc_auth_context *auth_context = NULL; - status = ssl_check_peer(sc, c->overridden_target_name != NULL - ? c->overridden_target_name - : c->target_name, - &peer, &auth_context); - cb(exec_ctx, user_data, status, auth_context); - grpc_auth_context_unref(auth_context); + grpc_error *error = ssl_check_peer(sc, c->overridden_target_name != NULL + ? c->overridden_target_name + : c->target_name, + &peer, auth_context); + grpc_exec_ctx_sched(exec_ctx, on_peer_checked, error, NULL); tsi_peer_destruct(&peer); } static void ssl_server_check_peer(grpc_exec_ctx *exec_ctx, grpc_security_connector *sc, tsi_peer peer, - grpc_security_peer_check_cb cb, - void *user_data) { - grpc_auth_context *auth_context = NULL; - grpc_security_status status = ssl_check_peer(sc, NULL, &peer, &auth_context); + grpc_auth_context **auth_context, + grpc_closure *on_peer_checked) { + grpc_error *error = ssl_check_peer(sc, NULL, &peer, auth_context); tsi_peer_destruct(&peer); - cb(exec_ctx, user_data, status, auth_context); - grpc_auth_context_unref(auth_context); + grpc_exec_ctx_sched(exec_ctx, on_peer_checked, error, NULL); } static void add_shallow_auth_property_to_peer(tsi_peer *peer, diff --git a/src/core/lib/security/transport/security_connector.h b/src/core/lib/security/transport/security_connector.h index 2dffb5c24a..0e4e0aa720 100644 --- a/src/core/lib/security/transport/security_connector.h +++ b/src/core/lib/security/transport/security_connector.h @@ -59,21 +59,11 @@ typedef struct grpc_security_connector grpc_security_connector; #define GRPC_SECURITY_CONNECTOR_ARG "grpc.security_connector" -typedef void (*grpc_security_peer_check_cb)(grpc_exec_ctx *exec_ctx, - void *user_data, - grpc_security_status status, - grpc_auth_context *auth_context); - -/* Ownership of the secure_endpoint is transfered. */ -typedef void (*grpc_security_handshake_done_cb)( - grpc_exec_ctx *exec_ctx, void *user_data, grpc_security_status status, - grpc_endpoint *secure_endpoint, grpc_auth_context *auth_context); - typedef struct { void (*destroy)(grpc_security_connector *sc); void (*check_peer)(grpc_exec_ctx *exec_ctx, grpc_security_connector *sc, - tsi_peer peer, grpc_security_peer_check_cb cb, - void *user_data); + tsi_peer peer, grpc_auth_context **auth_context, + grpc_closure *on_peer_checked); } grpc_security_connector_vtable; typedef struct grpc_security_connector_handshake_list { @@ -108,12 +98,12 @@ void grpc_security_connector_unref(grpc_security_connector *policy); #endif /* Check the peer. Callee takes ownership of the peer object. - The callback will include the resulting auth_context. */ + Sets *auth_context and invokes on_peer_checked when done. */ void grpc_security_connector_check_peer(grpc_exec_ctx *exec_ctx, grpc_security_connector *sc, tsi_peer peer, - grpc_security_peer_check_cb cb, - void *user_data); + grpc_auth_context **auth_context, + grpc_closure *on_peer_checked); /* Util to encapsulate the connector in a channel arg. */ grpc_arg grpc_security_connector_to_arg(grpc_security_connector *sc); diff --git a/src/core/lib/security/transport/security_handshaker.c b/src/core/lib/security/transport/security_handshaker.c index 4b04f58bfd..1f9a4dae4a 100644 --- a/src/core/lib/security/transport/security_handshaker.c +++ b/src/core/lib/security/transport/security_handshaker.c @@ -56,7 +56,8 @@ typedef struct { grpc_closure* on_handshake_done; grpc_security_connector *connector; tsi_handshaker *handshaker; -// FIXME: add locking + gpr_mu mu; + gpr_refcount refs; unsigned char *handshake_buffer; size_t handshake_buffer_size; // FIXME: use args->endpoint instead @@ -66,8 +67,8 @@ typedef struct { grpc_slice_buffer outgoing; grpc_closure on_handshake_data_sent_to_peer; grpc_closure on_handshake_data_received_from_peer; + grpc_closure on_peer_checked; grpc_auth_context *auth_context; - gpr_refcount refs; } security_handshaker; static void on_handshake_data_received_from_peer(grpc_exec_ctx *exec_ctx, @@ -80,6 +81,7 @@ static void on_handshake_data_sent_to_peer(grpc_exec_ctx *exec_ctx, void *setup, static void unref_handshake(security_handshaker *h) { if (gpr_unref(&h->refs)) { if (h->handshaker != NULL) tsi_handshaker_destroy(h->handshaker); + gpr_mu_destroy(&h->mu); if (h->handshake_buffer != NULL) gpr_free(h->handshake_buffer); grpc_slice_buffer_destroy(&h->left_overs); grpc_slice_buffer_destroy(&h->outgoing); @@ -89,9 +91,9 @@ static void unref_handshake(security_handshaker *h) { } } -static void security_handshake_done(grpc_exec_ctx *exec_ctx, - security_handshaker *h, - grpc_error *error) { +static void security_handshake_done_locked(grpc_exec_ctx *exec_ctx, + security_handshaker *h, + grpc_error *error) { if (error == GRPC_ERROR_NONE) { h->args->endpoint = h->secure_endpoint; grpc_arg auth_context_arg = grpc_auth_context_to_arg(h->auth_context); @@ -116,60 +118,55 @@ static void security_handshake_done(grpc_exec_ctx *exec_ctx, grpc_slice_buffer_reset_and_unref(h->args->read_buffer); h->args = NULL; grpc_exec_ctx_sched(exec_ctx, h->on_handshake_done, error, NULL); - unref_handshake(h); } -static void on_peer_checked(grpc_exec_ctx *exec_ctx, void *user_data, - grpc_security_status status, - grpc_auth_context *auth_context) { - security_handshaker *h = user_data; - tsi_frame_protector *protector; - tsi_result result; - if (status != GRPC_SECURITY_OK) { - security_handshake_done( - exec_ctx, h, - grpc_error_set_int(GRPC_ERROR_CREATE("Error checking peer."), - GRPC_ERROR_INT_SECURITY_STATUS, status)); - return; +static void on_peer_checked(grpc_exec_ctx *exec_ctx, void *arg, + grpc_error *error) { + security_handshaker *h = arg; + gpr_mu_lock(&h->mu); + if (error != GRPC_ERROR_NONE) { + // Take a new ref to pass to security_handshake_done_locked(). + GRPC_ERROR_REF(error); + goto done; } - h->auth_context = GRPC_AUTH_CONTEXT_REF(auth_context, "handshake"); - result = + // Get frame protector. + tsi_frame_protector *protector; + tsi_result result = tsi_handshaker_create_frame_protector(h->handshaker, NULL, &protector); if (result != TSI_OK) { - security_handshake_done( - exec_ctx, h, - grpc_set_tsi_error_result( - GRPC_ERROR_CREATE("Frame protector creation failed"), result)); - return; + error = grpc_set_tsi_error_result( + GRPC_ERROR_CREATE("Frame protector creation failed"), result); + goto done; } h->secure_endpoint = grpc_secure_endpoint_create(protector, h->wrapped_endpoint, h->left_overs.slices, h->left_overs.count); h->left_overs.count = 0; h->left_overs.length = 0; - security_handshake_done(exec_ctx, h, GRPC_ERROR_NONE); +done: + security_handshake_done_locked(exec_ctx, h, error); + gpr_mu_unlock(&h->mu); + unref_handshake(h); } -static void check_peer(grpc_exec_ctx *exec_ctx, security_handshaker *h) { +static grpc_error* check_peer_locked(grpc_exec_ctx *exec_ctx, + security_handshaker *h) { tsi_peer peer; tsi_result result = tsi_handshaker_extract_peer(h->handshaker, &peer); - if (result != TSI_OK) { - security_handshake_done( - exec_ctx, h, grpc_set_tsi_error_result( - GRPC_ERROR_CREATE("Peer extraction failed"), result)); - return; + return grpc_set_tsi_error_result( + GRPC_ERROR_CREATE("Peer extraction failed"), result); } grpc_security_connector_check_peer(exec_ctx, h->connector, peer, - on_peer_checked, h); + &h->auth_context, &h->on_peer_checked); + return GRPC_ERROR_NONE; } -static void send_handshake_bytes_to_peer(grpc_exec_ctx *exec_ctx, - security_handshaker *h) { - size_t offset = 0; +static grpc_error* send_handshake_bytes_to_peer_locked(grpc_exec_ctx *exec_ctx, + security_handshaker *h) { + // Get data to send. tsi_result result = TSI_OK; - grpc_slice to_send; - + size_t offset = 0; do { size_t to_send_size = h->handshake_buffer_size - offset; result = tsi_handshaker_get_bytes_to_send_to_peer( @@ -181,39 +178,37 @@ static void send_handshake_bytes_to_peer(grpc_exec_ctx *exec_ctx, gpr_realloc(h->handshake_buffer, h->handshake_buffer_size); } } while (result == TSI_INCOMPLETE_DATA); - if (result != TSI_OK) { - security_handshake_done(exec_ctx, h, - grpc_set_tsi_error_result( - GRPC_ERROR_CREATE("Handshake failed"), result)); - return; + return grpc_set_tsi_error_result(GRPC_ERROR_CREATE("Handshake failed"), + result); } - - to_send = + // Send data. + grpc_slice to_send = grpc_slice_from_copied_buffer((const char *)h->handshake_buffer, offset); grpc_slice_buffer_reset_and_unref(&h->outgoing); grpc_slice_buffer_add(&h->outgoing, to_send); grpc_endpoint_write(exec_ctx, h->wrapped_endpoint, &h->outgoing, &h->on_handshake_data_sent_to_peer); + return GRPC_ERROR_NONE; } static void on_handshake_data_received_from_peer(grpc_exec_ctx *exec_ctx, void *handshake, grpc_error *error) { security_handshaker *h = handshake; - size_t consumed_slice_size = 0; - tsi_result result = TSI_OK; - size_t i; - size_t num_left_overs; - int has_left_overs_in_current_slice = 0; - + gpr_mu_lock(&h->mu); if (error != GRPC_ERROR_NONE) { - security_handshake_done( + security_handshake_done_locked( exec_ctx, h, GRPC_ERROR_CREATE_REFERENCING("Handshake read failed", &error, 1)); + gpr_mu_unlock(&h->mu); + unref_handshake(h); return; } - + // Process received data. + tsi_result result = TSI_OK; + size_t consumed_slice_size = 0; + size_t i; for (i = 0; i < h->args->read_buffer->count; i++) { consumed_slice_size = GRPC_SLICE_LENGTH(h->args->read_buffer->slices[i]); result = tsi_handshaker_process_bytes_from_peer( @@ -221,31 +216,37 @@ static void on_handshake_data_received_from_peer(grpc_exec_ctx *exec_ctx, &consumed_slice_size); if (!tsi_handshaker_is_in_progress(h->handshaker)) break; } - if (tsi_handshaker_is_in_progress(h->handshaker)) { /* We may need more data. */ if (result == TSI_INCOMPLETE_DATA) { grpc_endpoint_read(exec_ctx, h->wrapped_endpoint, h->args->read_buffer, &h->on_handshake_data_received_from_peer); - return; + goto done; } else { - send_handshake_bytes_to_peer(exec_ctx, h); - return; + error = send_handshake_bytes_to_peer_locked(exec_ctx, h); + if (error != GRPC_ERROR_NONE) { + security_handshake_done_locked(exec_ctx, h, error); + gpr_mu_unlock(&h->mu); + unref_handshake(h); + return; + } + goto done; } } - if (result != TSI_OK) { - security_handshake_done(exec_ctx, h, - grpc_set_tsi_error_result( - GRPC_ERROR_CREATE("Handshake failed"), result)); + security_handshake_done_locked( + exec_ctx, h, + grpc_set_tsi_error_result(GRPC_ERROR_CREATE("Handshake failed"), + result)); + gpr_mu_unlock(&h->mu); + unref_handshake(h); return; } - /* Handshake is done and successful this point. */ - has_left_overs_in_current_slice = + bool has_left_overs_in_current_slice = (consumed_slice_size < GRPC_SLICE_LENGTH(h->args->read_buffer->slices[i])); - num_left_overs = + size_t num_left_overs = (has_left_overs_in_current_slice ? 1 : 0) + h->args->read_buffer->count - i - 1; if (num_left_overs > 0) { @@ -262,31 +263,49 @@ static void on_handshake_data_received_from_peer(grpc_exec_ctx *exec_ctx, &h->left_overs, &h->args->read_buffer->slices[i + 1], num_left_overs - (size_t)has_left_overs_in_current_slice); } - - check_peer(exec_ctx, h); + // Check peer. + error = check_peer_locked(exec_ctx, h); + if (error != GRPC_ERROR_NONE) { + security_handshake_done_locked(exec_ctx, h, error); + gpr_mu_unlock(&h->mu); + unref_handshake(h); + return; + } +done: + gpr_mu_unlock(&h->mu); } /* If handshake is NULL, the handshake is done. */ static void on_handshake_data_sent_to_peer(grpc_exec_ctx *exec_ctx, void *handshake, grpc_error *error) { security_handshaker *h = handshake; - /* Make sure that write is OK. */ if (error != GRPC_ERROR_NONE) { - if (handshake != NULL) - security_handshake_done( + if (handshake != NULL) { + gpr_mu_lock(&h->mu); + security_handshake_done_locked( exec_ctx, h, GRPC_ERROR_CREATE_REFERENCING("Handshake write failed", &error, 1)); + gpr_mu_unlock(&h->mu); + unref_handshake(h); + } return; } - /* We may be done. */ + gpr_mu_lock(&h->mu); if (tsi_handshaker_is_in_progress(h->handshaker)) { grpc_endpoint_read(exec_ctx, h->wrapped_endpoint, h->args->read_buffer, &h->on_handshake_data_received_from_peer); } else { - check_peer(exec_ctx, h); + error = check_peer_locked(exec_ctx, h); + if (error != GRPC_ERROR_NONE) { + security_handshake_done_locked(exec_ctx, h, error); + gpr_mu_unlock(&h->mu); + unref_handshake(h); + return; + } } + gpr_mu_unlock(&h->mu); } // @@ -302,9 +321,11 @@ static void security_handshaker_destroy(grpc_exec_ctx* exec_ctx, static void security_handshaker_shutdown(grpc_exec_ctx* exec_ctx, grpc_handshaker* handshaker) { security_handshaker *h = (security_handshaker*)handshaker; + gpr_mu_lock(&h->mu); if (h->args != NULL) { grpc_endpoint_shutdown(exec_ctx, h->wrapped_endpoint); } + gpr_mu_unlock(&h->mu); } static void security_handshaker_do_handshake( @@ -312,11 +333,19 @@ static void security_handshaker_do_handshake( grpc_tcp_server_acceptor* acceptor, grpc_closure* on_handshake_done, grpc_handshaker_args* args) { security_handshaker* h = (security_handshaker*)handshaker; + gpr_mu_lock(&h->mu); h->args = args; h->on_handshake_done = on_handshake_done; h->wrapped_endpoint = args->endpoint; // FIXME: remove? gpr_ref(&h->refs); - send_handshake_bytes_to_peer(exec_ctx, h); + grpc_error* error = send_handshake_bytes_to_peer_locked(exec_ctx, h); + if (error != GRPC_ERROR_NONE) { + security_handshake_done_locked(exec_ctx, h, error); + gpr_mu_unlock(&h->mu); + unref_handshake(h); + return; + } + gpr_mu_unlock(&h->mu); } static const grpc_handshaker_vtable security_handshaker_vtable = { @@ -331,13 +360,15 @@ static grpc_handshaker* security_handshaker_create( grpc_handshaker_init(&security_handshaker_vtable, &h->base); h->handshaker = handshaker; h->connector = GRPC_SECURITY_CONNECTOR_REF(connector, "handshake"); + gpr_mu_init(&h->mu); + gpr_ref_init(&h->refs, 1); h->handshake_buffer_size = GRPC_INITIAL_HANDSHAKE_BUFFER_SIZE; h->handshake_buffer = gpr_malloc(h->handshake_buffer_size); - gpr_ref_init(&h->refs, 1); grpc_closure_init(&h->on_handshake_data_sent_to_peer, on_handshake_data_sent_to_peer, h); grpc_closure_init(&h->on_handshake_data_received_from_peer, on_handshake_data_received_from_peer, h); + grpc_closure_init(&h->on_peer_checked, on_peer_checked, h); grpc_slice_buffer_init(&h->left_overs); grpc_slice_buffer_init(&h->outgoing); return &h->base; |