From 87047d7e932246eef44c92598361dc60351a0bab Mon Sep 17 00:00:00 2001 From: Julien Boeuf Date: Fri, 21 Aug 2015 14:30:33 -0700 Subject: [PATCH 01/17] Security connector is now responsible for the handshake. --- BUILD | 8 +- Makefile | 4 +- build.json | 4 +- gRPC.podspec | 6 +- src/core/httpcli/httpcli_security_connector.c | 28 ++- .../{secure_transport_setup.c => handshake.c} | 176 +++++++++--------- .../{secure_transport_setup.h => handshake.h} | 20 +- src/core/security/security_connector.c | 93 ++++++--- src/core/security/security_connector.h | 20 +- src/core/security/server_secure_chttp2.c | 12 +- src/core/surface/secure_channel_create.c | 14 +- tools/doxygen/Doxyfile.core.internal | 4 +- tools/run_tests/sources_and_headers.json | 6 +- tools/run_tests/tests.json | 1 + vsprojects/grpc/grpc.vcxproj | 6 +- vsprojects/grpc/grpc.vcxproj.filters | 12 +- 16 files changed, 224 insertions(+), 190 deletions(-) rename src/core/security/{secure_transport_setup.c => handshake.c} (58%) rename src/core/security/{secure_transport_setup.h => handshake.h} (70%) diff --git a/BUILD b/BUILD index 7eb59797d43..3c747d4de64 100644 --- a/BUILD +++ b/BUILD @@ -133,10 +133,10 @@ cc_library( "src/core/security/auth_filters.h", "src/core/security/base64.h", "src/core/security/credentials.h", + "src/core/security/handshake.h", "src/core/security/json_token.h", "src/core/security/jwt_verifier.h", "src/core/security/secure_endpoint.h", - "src/core/security/secure_transport_setup.h", "src/core/security/security_connector.h", "src/core/security/security_context.h", "src/core/tsi/fake_transport_security.h", @@ -253,10 +253,10 @@ cc_library( "src/core/security/credentials_posix.c", "src/core/security/credentials_win32.c", "src/core/security/google_default_credentials.c", + "src/core/security/handshake.c", "src/core/security/json_token.c", "src/core/security/jwt_verifier.c", "src/core/security/secure_endpoint.c", - "src/core/security/secure_transport_setup.c", "src/core/security/security_connector.c", "src/core/security/security_context.c", "src/core/security/server_auth_filter.c", @@ -1003,10 +1003,10 @@ objc_library( "src/core/security/credentials_posix.c", "src/core/security/credentials_win32.c", "src/core/security/google_default_credentials.c", + "src/core/security/handshake.c", "src/core/security/json_token.c", "src/core/security/jwt_verifier.c", "src/core/security/secure_endpoint.c", - "src/core/security/secure_transport_setup.c", "src/core/security/security_connector.c", "src/core/security/security_context.c", "src/core/security/server_auth_filter.c", @@ -1146,10 +1146,10 @@ objc_library( "src/core/security/auth_filters.h", "src/core/security/base64.h", "src/core/security/credentials.h", + "src/core/security/handshake.h", "src/core/security/json_token.h", "src/core/security/jwt_verifier.h", "src/core/security/secure_endpoint.h", - "src/core/security/secure_transport_setup.h", "src/core/security/security_connector.h", "src/core/security/security_context.h", "src/core/tsi/fake_transport_security.h", diff --git a/Makefile b/Makefile index 31628b44122..9530d9374e1 100644 --- a/Makefile +++ b/Makefile @@ -4067,10 +4067,10 @@ LIBGRPC_SRC = \ src/core/security/credentials_posix.c \ src/core/security/credentials_win32.c \ src/core/security/google_default_credentials.c \ + src/core/security/handshake.c \ src/core/security/json_token.c \ src/core/security/jwt_verifier.c \ src/core/security/secure_endpoint.c \ - src/core/security/secure_transport_setup.c \ src/core/security/security_connector.c \ src/core/security/security_context.c \ src/core/security/server_auth_filter.c \ @@ -20637,10 +20637,10 @@ src/core/security/credentials_metadata.c: $(OPENSSL_DEP) src/core/security/credentials_posix.c: $(OPENSSL_DEP) src/core/security/credentials_win32.c: $(OPENSSL_DEP) src/core/security/google_default_credentials.c: $(OPENSSL_DEP) +src/core/security/handshake.c: $(OPENSSL_DEP) src/core/security/json_token.c: $(OPENSSL_DEP) src/core/security/jwt_verifier.c: $(OPENSSL_DEP) src/core/security/secure_endpoint.c: $(OPENSSL_DEP) -src/core/security/secure_transport_setup.c: $(OPENSSL_DEP) src/core/security/security_connector.c: $(OPENSSL_DEP) src/core/security/security_context.c: $(OPENSSL_DEP) src/core/security/server_auth_filter.c: $(OPENSSL_DEP) diff --git a/build.json b/build.json index bd707d2e34c..5d561a72739 100644 --- a/build.json +++ b/build.json @@ -470,10 +470,10 @@ "src/core/security/auth_filters.h", "src/core/security/base64.h", "src/core/security/credentials.h", + "src/core/security/handshake.h", "src/core/security/json_token.h", "src/core/security/jwt_verifier.h", "src/core/security/secure_endpoint.h", - "src/core/security/secure_transport_setup.h", "src/core/security/security_connector.h", "src/core/security/security_context.h", "src/core/tsi/fake_transport_security.h", @@ -490,10 +490,10 @@ "src/core/security/credentials_posix.c", "src/core/security/credentials_win32.c", "src/core/security/google_default_credentials.c", + "src/core/security/handshake.c", "src/core/security/json_token.c", "src/core/security/jwt_verifier.c", "src/core/security/secure_endpoint.c", - "src/core/security/secure_transport_setup.c", "src/core/security/security_connector.c", "src/core/security/security_context.c", "src/core/security/server_auth_filter.c", diff --git a/gRPC.podspec b/gRPC.podspec index d945c299b51..ece8ea22840 100644 --- a/gRPC.podspec +++ b/gRPC.podspec @@ -135,10 +135,10 @@ Pod::Spec.new do |s| 'src/core/security/auth_filters.h', 'src/core/security/base64.h', 'src/core/security/credentials.h', + 'src/core/security/handshake.h', 'src/core/security/json_token.h', 'src/core/security/jwt_verifier.h', 'src/core/security/secure_endpoint.h', - 'src/core/security/secure_transport_setup.h', 'src/core/security/security_connector.h', 'src/core/security/security_context.h', 'src/core/tsi/fake_transport_security.h', @@ -262,10 +262,10 @@ Pod::Spec.new do |s| 'src/core/security/credentials_posix.c', 'src/core/security/credentials_win32.c', 'src/core/security/google_default_credentials.c', + 'src/core/security/handshake.c', 'src/core/security/json_token.c', 'src/core/security/jwt_verifier.c', 'src/core/security/secure_endpoint.c', - 'src/core/security/secure_transport_setup.c', 'src/core/security/security_connector.c', 'src/core/security/security_context.c', 'src/core/security/server_auth_filter.c', @@ -404,10 +404,10 @@ Pod::Spec.new do |s| 'src/core/security/auth_filters.h', 'src/core/security/base64.h', 'src/core/security/credentials.h', + 'src/core/security/handshake.h', 'src/core/security/json_token.h', 'src/core/security/jwt_verifier.h', 'src/core/security/secure_endpoint.h', - 'src/core/security/secure_transport_setup.h', 'src/core/security/security_connector.h', 'src/core/security/security_context.h', 'src/core/tsi/fake_transport_security.h', diff --git a/src/core/httpcli/httpcli_security_connector.c b/src/core/httpcli/httpcli_security_connector.c index 7887f9d5304..a4cd00d9a07 100644 --- a/src/core/httpcli/httpcli_security_connector.c +++ b/src/core/httpcli/httpcli_security_connector.c @@ -35,7 +35,7 @@ #include -#include "src/core/security/secure_transport_setup.h" +#include "src/core/security/handshake.h" #include "src/core/support/string.h" #include #include @@ -55,23 +55,31 @@ static void httpcli_ssl_destroy(grpc_security_connector *sc) { tsi_ssl_handshaker_factory_destroy(c->handshaker_factory); } if (c->secure_peer_name != NULL) gpr_free(c->secure_peer_name); + tsi_handshaker_destroy(sc->handshaker); gpr_free(sc); } -static grpc_security_status httpcli_ssl_create_handshaker( - grpc_security_connector *sc, tsi_handshaker **handshaker) { +static void httpcli_ssl_do_handshake( + grpc_security_connector *sc, grpc_endpoint *nonsecure_endpoint, + grpc_security_handshake_done_cb cb, void *user_data) { grpc_httpcli_ssl_channel_security_connector *c = (grpc_httpcli_ssl_channel_security_connector *)sc; tsi_result result = TSI_OK; - if (c->handshaker_factory == NULL) return GRPC_SECURITY_ERROR; + if (c->handshaker_factory == NULL) { + cb(user_data, GRPC_SECURITY_ERROR, nonsecure_endpoint, NULL); + return; + } + tsi_handshaker_destroy(sc->handshaker); + sc->handshaker = NULL; result = tsi_ssl_handshaker_factory_create_handshaker( - c->handshaker_factory, c->secure_peer_name, handshaker); + c->handshaker_factory, c->secure_peer_name, &sc->handshaker); if (result != TSI_OK) { gpr_log(GPR_ERROR, "Handshaker creation failed with error %s.", tsi_result_to_string(result)); - return GRPC_SECURITY_ERROR; + cb(user_data, GRPC_SECURITY_ERROR, nonsecure_endpoint, NULL); + } else { + grpc_do_security_handshake(sc, nonsecure_endpoint, cb, user_data); } - return GRPC_SECURITY_OK; } static grpc_security_status httpcli_ssl_check_peer(grpc_security_connector *sc, @@ -94,7 +102,7 @@ static grpc_security_status httpcli_ssl_check_peer(grpc_security_connector *sc, } static grpc_security_connector_vtable httpcli_ssl_vtable = { - httpcli_ssl_destroy, httpcli_ssl_create_handshaker, httpcli_ssl_check_peer}; + httpcli_ssl_destroy, httpcli_ssl_do_handshake, httpcli_ssl_check_peer}; static grpc_security_status httpcli_ssl_channel_security_connector_create( const unsigned char *pem_root_certs, size_t pem_root_certs_size, @@ -169,8 +177,8 @@ static void ssl_handshake(void *arg, grpc_endpoint *tcp, const char *host, GPR_ASSERT(httpcli_ssl_channel_security_connector_create( pem_root_certs, pem_root_certs_size, host, &sc) == GRPC_SECURITY_OK); - grpc_setup_secure_transport(&sc->base, tcp, on_secure_transport_setup_done, - c); + grpc_security_connector_do_handshake(&sc->base, tcp, + on_secure_transport_setup_done, c); GRPC_SECURITY_CONNECTOR_UNREF(&sc->base, "httpcli"); } diff --git a/src/core/security/secure_transport_setup.c b/src/core/security/handshake.c similarity index 58% rename from src/core/security/secure_transport_setup.c rename to src/core/security/handshake.c index 0c3572b53c4..23830ed5a1a 100644 --- a/src/core/security/secure_transport_setup.c +++ b/src/core/security/handshake.c @@ -31,7 +31,7 @@ * */ -#include "src/core/security/secure_transport_setup.h" +#include "src/core/security/handshake.h" #include @@ -44,126 +44,125 @@ typedef struct { grpc_security_connector *connector; - tsi_handshaker *handshaker; unsigned char *handshake_buffer; size_t handshake_buffer_size; grpc_endpoint *wrapped_endpoint; grpc_endpoint *secure_endpoint; gpr_slice_buffer left_overs; - grpc_secure_transport_setup_done_cb cb; + grpc_security_handshake_done_cb cb; void *user_data; -} grpc_secure_transport_setup; +} grpc_security_handshake; -static void on_handshake_data_received_from_peer(void *setup, gpr_slice *slices, +static void on_handshake_data_received_from_peer(void *handshake, + gpr_slice *slices, size_t nslices, grpc_endpoint_cb_status error); -static void on_handshake_data_sent_to_peer(void *setup, +static void on_handshake_data_sent_to_peer(void *handshake, grpc_endpoint_cb_status error); -static void secure_transport_setup_done(grpc_secure_transport_setup *s, - int is_success) { +static void security_handshake_done(grpc_security_handshake *h, + int is_success) { if (is_success) { - s->cb(s->user_data, GRPC_SECURITY_OK, s->wrapped_endpoint, - s->secure_endpoint); + h->cb(h->user_data, GRPC_SECURITY_OK, h->wrapped_endpoint, + h->secure_endpoint); } else { - if (s->secure_endpoint != NULL) { - grpc_endpoint_shutdown(s->secure_endpoint); - grpc_endpoint_destroy(s->secure_endpoint); + if (h->secure_endpoint != NULL) { + grpc_endpoint_shutdown(h->secure_endpoint); + grpc_endpoint_destroy(h->secure_endpoint); } else { - grpc_endpoint_destroy(s->wrapped_endpoint); + grpc_endpoint_destroy(h->wrapped_endpoint); } - s->cb(s->user_data, GRPC_SECURITY_ERROR, s->wrapped_endpoint, NULL); + h->cb(h->user_data, GRPC_SECURITY_ERROR, h->wrapped_endpoint, NULL); } - if (s->handshaker != NULL) tsi_handshaker_destroy(s->handshaker); - if (s->handshake_buffer != NULL) gpr_free(s->handshake_buffer); - gpr_slice_buffer_destroy(&s->left_overs); - GRPC_SECURITY_CONNECTOR_UNREF(s->connector, "secure_transport_setup"); - gpr_free(s); + if (h->handshake_buffer != NULL) gpr_free(h->handshake_buffer); + gpr_slice_buffer_destroy(&h->left_overs); + gpr_free(h); } static void on_peer_checked(void *user_data, grpc_security_status status) { - grpc_secure_transport_setup *s = user_data; + grpc_security_handshake *h = user_data; tsi_frame_protector *protector; tsi_result result; if (status != GRPC_SECURITY_OK) { gpr_log(GPR_ERROR, "Error checking peer."); - secure_transport_setup_done(s, 0); + security_handshake_done(h, 0); return; } - result = - tsi_handshaker_create_frame_protector(s->handshaker, NULL, &protector); + result = tsi_handshaker_create_frame_protector(h->connector->handshaker, NULL, + &protector); if (result != TSI_OK) { gpr_log(GPR_ERROR, "Frame protector creation failed with error %s.", tsi_result_to_string(result)); - secure_transport_setup_done(s, 0); + security_handshake_done(h, 0); return; } - s->secure_endpoint = - grpc_secure_endpoint_create(protector, s->wrapped_endpoint, - s->left_overs.slices, s->left_overs.count); - secure_transport_setup_done(s, 1); + h->secure_endpoint = + grpc_secure_endpoint_create(protector, h->wrapped_endpoint, + h->left_overs.slices, h->left_overs.count); + security_handshake_done(h, 1); return; } -static void check_peer(grpc_secure_transport_setup *s) { +static void check_peer(grpc_security_handshake *h) { grpc_security_status peer_status; tsi_peer peer; - tsi_result result = tsi_handshaker_extract_peer(s->handshaker, &peer); + tsi_result result = + tsi_handshaker_extract_peer(h->connector->handshaker, &peer); if (result != TSI_OK) { gpr_log(GPR_ERROR, "Peer extraction failed with error %s", tsi_result_to_string(result)); - secure_transport_setup_done(s, 0); + security_handshake_done(h, 0); return; } - peer_status = grpc_security_connector_check_peer(s->connector, peer, - on_peer_checked, s); + 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."); - secure_transport_setup_done(s, 0); + security_handshake_done(h, 0); return; } else if (peer_status == GRPC_SECURITY_OK) { - on_peer_checked(s, peer_status); + on_peer_checked(h, peer_status); } } -static void send_handshake_bytes_to_peer(grpc_secure_transport_setup *s) { +static void send_handshake_bytes_to_peer(grpc_security_handshake *h) { size_t offset = 0; tsi_result result = TSI_OK; gpr_slice to_send; grpc_endpoint_write_status write_status; do { - size_t to_send_size = s->handshake_buffer_size - offset; + size_t to_send_size = h->handshake_buffer_size - offset; result = tsi_handshaker_get_bytes_to_send_to_peer( - s->handshaker, s->handshake_buffer + offset, &to_send_size); + h->connector->handshaker, h->handshake_buffer + offset, &to_send_size); offset += to_send_size; if (result == TSI_INCOMPLETE_DATA) { - s->handshake_buffer_size *= 2; - s->handshake_buffer = - gpr_realloc(s->handshake_buffer, s->handshake_buffer_size); + h->handshake_buffer_size *= 2; + h->handshake_buffer = + gpr_realloc(h->handshake_buffer, h->handshake_buffer_size); } } while (result == TSI_INCOMPLETE_DATA); if (result != TSI_OK) { gpr_log(GPR_ERROR, "Handshake failed with error %s", tsi_result_to_string(result)); - secure_transport_setup_done(s, 0); + security_handshake_done(h, 0); return; } to_send = - gpr_slice_from_copied_buffer((const char *)s->handshake_buffer, offset); + gpr_slice_from_copied_buffer((const char *)h->handshake_buffer, offset); /* TODO(klempner,jboeuf): This should probably use the client setup deadline */ - write_status = grpc_endpoint_write(s->wrapped_endpoint, &to_send, 1, - on_handshake_data_sent_to_peer, s); + write_status = grpc_endpoint_write(h->wrapped_endpoint, &to_send, 1, + on_handshake_data_sent_to_peer, h); if (write_status == GRPC_ENDPOINT_WRITE_ERROR) { gpr_log(GPR_ERROR, "Could not send handshake data to peer."); - secure_transport_setup_done(s, 0); + security_handshake_done(h, 0); } else if (write_status == GRPC_ENDPOINT_WRITE_DONE) { - on_handshake_data_sent_to_peer(s, GRPC_ENDPOINT_CB_OK); + on_handshake_data_sent_to_peer(h, GRPC_ENDPOINT_CB_OK); } } @@ -175,9 +174,9 @@ static void cleanup_slices(gpr_slice *slices, size_t num_slices) { } static void on_handshake_data_received_from_peer( - void *setup, gpr_slice *slices, size_t nslices, + void *handshake, gpr_slice *slices, size_t nslices, grpc_endpoint_cb_status error) { - grpc_secure_transport_setup *s = setup; + grpc_security_handshake *h = handshake; size_t consumed_slice_size = 0; tsi_result result = TSI_OK; size_t i; @@ -187,28 +186,29 @@ static void on_handshake_data_received_from_peer( if (error != GRPC_ENDPOINT_CB_OK) { gpr_log(GPR_ERROR, "Read failed."); cleanup_slices(slices, nslices); - secure_transport_setup_done(s, 0); + security_handshake_done(h, 0); return; } for (i = 0; i < nslices; i++) { consumed_slice_size = GPR_SLICE_LENGTH(slices[i]); result = tsi_handshaker_process_bytes_from_peer( - s->handshaker, GPR_SLICE_START_PTR(slices[i]), &consumed_slice_size); - if (!tsi_handshaker_is_in_progress(s->handshaker)) break; + h->connector->handshaker, GPR_SLICE_START_PTR(slices[i]), + &consumed_slice_size); + if (!tsi_handshaker_is_in_progress(h->connector->handshaker)) break; } - if (tsi_handshaker_is_in_progress(s->handshaker)) { + if (tsi_handshaker_is_in_progress(h->connector->handshaker)) { /* We may need more data. */ if (result == TSI_INCOMPLETE_DATA) { /* TODO(klempner,jboeuf): This should probably use the client setup deadline */ - grpc_endpoint_notify_on_read(s->wrapped_endpoint, - on_handshake_data_received_from_peer, setup); + grpc_endpoint_notify_on_read( + h->wrapped_endpoint, on_handshake_data_received_from_peer, handshake); cleanup_slices(slices, nslices); return; } else { - send_handshake_bytes_to_peer(s); + send_handshake_bytes_to_peer(h); cleanup_slices(slices, nslices); return; } @@ -218,7 +218,7 @@ static void on_handshake_data_received_from_peer( gpr_log(GPR_ERROR, "Handshake failed with error %s", tsi_result_to_string(result)); cleanup_slices(slices, nslices); - secure_transport_setup_done(s, 0); + security_handshake_done(h, 0); return; } @@ -228,66 +228,58 @@ static void on_handshake_data_received_from_peer( num_left_overs = (has_left_overs_in_current_slice ? 1 : 0) + nslices - i - 1; if (num_left_overs == 0) { cleanup_slices(slices, nslices); - check_peer(s); + check_peer(h); return; } cleanup_slices(slices, nslices - num_left_overs); /* Put the leftovers in our buffer (ownership transfered). */ if (has_left_overs_in_current_slice) { - gpr_slice_buffer_add(&s->left_overs, + gpr_slice_buffer_add(&h->left_overs, gpr_slice_split_tail(&slices[i], consumed_slice_size)); gpr_slice_unref(slices[i]); /* split_tail above increments refcount. */ } gpr_slice_buffer_addn( - &s->left_overs, &slices[i + 1], + &h->left_overs, &slices[i + 1], num_left_overs - (size_t)has_left_overs_in_current_slice); - check_peer(s); + check_peer(h); } -/* If setup is NULL, the setup is done. */ -static void on_handshake_data_sent_to_peer(void *setup, +/* If handshake is NULL, the handshake is done. */ +static void on_handshake_data_sent_to_peer(void *handshake, grpc_endpoint_cb_status error) { - grpc_secure_transport_setup *s = setup; + grpc_security_handshake *h = handshake; /* Make sure that write is OK. */ if (error != GRPC_ENDPOINT_CB_OK) { gpr_log(GPR_ERROR, "Write failed with error %d.", error); - if (setup != NULL) secure_transport_setup_done(s, 0); + if (handshake != NULL) security_handshake_done(h, 0); return; } /* We may be done. */ - if (tsi_handshaker_is_in_progress(s->handshaker)) { + if (tsi_handshaker_is_in_progress(h->connector->handshaker)) { /* TODO(klempner,jboeuf): This should probably use the client setup deadline */ - grpc_endpoint_notify_on_read(s->wrapped_endpoint, - on_handshake_data_received_from_peer, setup); + grpc_endpoint_notify_on_read( + h->wrapped_endpoint, on_handshake_data_received_from_peer, handshake); } else { - check_peer(s); + check_peer(h); } } -void grpc_setup_secure_transport(grpc_security_connector *connector, - grpc_endpoint *nonsecure_endpoint, - grpc_secure_transport_setup_done_cb cb, - void *user_data) { - grpc_security_status result = GRPC_SECURITY_OK; - grpc_secure_transport_setup *s = - gpr_malloc(sizeof(grpc_secure_transport_setup)); - memset(s, 0, sizeof(grpc_secure_transport_setup)); - result = grpc_security_connector_create_handshaker(connector, &s->handshaker); - if (result != GRPC_SECURITY_OK) { - secure_transport_setup_done(s, 0); - return; - } - s->connector = - GRPC_SECURITY_CONNECTOR_REF(connector, "secure_transport_setup"); - s->handshake_buffer_size = GRPC_INITIAL_HANDSHAKE_BUFFER_SIZE; - s->handshake_buffer = gpr_malloc(s->handshake_buffer_size); - s->wrapped_endpoint = nonsecure_endpoint; - s->user_data = user_data; - s->cb = cb; - gpr_slice_buffer_init(&s->left_overs); - send_handshake_bytes_to_peer(s); +void grpc_do_security_handshake(grpc_security_connector *connector, + grpc_endpoint *nonsecure_endpoint, + grpc_security_handshake_done_cb cb, + void *user_data) { + grpc_security_handshake *h = gpr_malloc(sizeof(grpc_security_handshake)); + memset(h, 0, sizeof(grpc_security_handshake)); + h->connector = connector; + h->handshake_buffer_size = GRPC_INITIAL_HANDSHAKE_BUFFER_SIZE; + h->handshake_buffer = gpr_malloc(h->handshake_buffer_size); + h->wrapped_endpoint = nonsecure_endpoint; + h->user_data = user_data; + h->cb = cb; + gpr_slice_buffer_init(&h->left_overs); + send_handshake_bytes_to_peer(h); } diff --git a/src/core/security/secure_transport_setup.h b/src/core/security/handshake.h similarity index 70% rename from src/core/security/secure_transport_setup.h rename to src/core/security/handshake.h index d9b802556de..1739855d69b 100644 --- a/src/core/security/secure_transport_setup.h +++ b/src/core/security/handshake.h @@ -31,23 +31,17 @@ * */ -#ifndef GRPC_INTERNAL_CORE_SECURITY_SECURE_TRANSPORT_SETUP_H -#define GRPC_INTERNAL_CORE_SECURITY_SECURE_TRANSPORT_SETUP_H +#ifndef GRPC_INTERNAL_CORE_SECURITY_HANDSHAKE_H +#define GRPC_INTERNAL_CORE_SECURITY_HANDSHAKE_H #include "src/core/iomgr/endpoint.h" #include "src/core/security/security_connector.h" -/* --- Secure transport setup --- */ - -/* Ownership of the secure_endpoint is transfered. */ -typedef void (*grpc_secure_transport_setup_done_cb)( - void *user_data, grpc_security_status status, - grpc_endpoint *wrapped_endpoint, grpc_endpoint *secure_endpoint); /* Calls the callback upon completion. */ -void grpc_setup_secure_transport(grpc_security_connector *connector, - grpc_endpoint *nonsecure_endpoint, - grpc_secure_transport_setup_done_cb cb, - void *user_data); +void grpc_do_security_handshake(grpc_security_connector *connector, + grpc_endpoint *nonsecure_endpoint, + grpc_security_handshake_done_cb cb, + void *user_data); -#endif /* GRPC_INTERNAL_CORE_SECURITY_SECURE_TRANSPORT_SETUP_H */ +#endif /* GRPC_INTERNAL_CORE_SECURITY_HANDSHAKE_H */ diff --git a/src/core/security/security_connector.c b/src/core/security/security_connector.c index a354536dcd3..922b92adeaa 100644 --- a/src/core/security/security_connector.c +++ b/src/core/security/security_connector.c @@ -36,6 +36,7 @@ #include #include "src/core/security/credentials.h" +#include "src/core/security/handshake.h" #include "src/core/security/secure_endpoint.h" #include "src/core/security/security_context.h" #include "src/core/support/env.h" @@ -101,10 +102,15 @@ const tsi_peer_property *tsi_peer_get_property_by_name(const tsi_peer *peer, return NULL; } -grpc_security_status grpc_security_connector_create_handshaker( - grpc_security_connector *sc, tsi_handshaker **handshaker) { - if (sc == NULL || handshaker == NULL) return GRPC_SECURITY_ERROR; - return sc->vtable->create_handshaker(sc, handshaker); +void grpc_security_connector_do_handshake(grpc_security_connector *sc, + grpc_endpoint *nonsecure_endpoint, + grpc_security_handshake_done_cb cb, + void *user_data) { + if (sc == NULL || nonsecure_endpoint == NULL) { + cb(user_data, GRPC_SECURITY_ERROR, nonsecure_endpoint, NULL); + } else { + sc->vtable->do_handshake(sc, nonsecure_endpoint, cb, user_data); + } } grpc_security_status grpc_security_connector_check_peer( @@ -216,27 +222,17 @@ typedef struct { static void fake_channel_destroy(grpc_security_connector *sc) { grpc_channel_security_connector *c = (grpc_channel_security_connector *)sc; grpc_credentials_unref(c->request_metadata_creds); + tsi_handshaker_destroy(sc->handshaker); GRPC_AUTH_CONTEXT_UNREF(sc->auth_context, "connector"); gpr_free(sc); } static void fake_server_destroy(grpc_security_connector *sc) { + tsi_handshaker_destroy(sc->handshaker); GRPC_AUTH_CONTEXT_UNREF(sc->auth_context, "connector"); gpr_free(sc); } -static grpc_security_status fake_channel_create_handshaker( - grpc_security_connector *sc, tsi_handshaker **handshaker) { - *handshaker = tsi_create_fake_handshaker(1); - return GRPC_SECURITY_OK; -} - -static grpc_security_status fake_server_create_handshaker( - grpc_security_connector *sc, tsi_handshaker **handshaker) { - *handshaker = tsi_create_fake_handshaker(0); - return GRPC_SECURITY_OK; -} - static grpc_security_status fake_check_peer(grpc_security_connector *sc, tsi_peer peer, grpc_security_check_cb cb, @@ -286,11 +282,29 @@ static grpc_security_status fake_channel_check_call_host( } } +static void fake_channel_do_handshake(grpc_security_connector *sc, + grpc_endpoint *nonsecure_endpoint, + grpc_security_handshake_done_cb cb, + void *user_data) { + tsi_handshaker_destroy(sc->handshaker); + sc->handshaker = tsi_create_fake_handshaker(1); + grpc_do_security_handshake(sc, nonsecure_endpoint, cb, user_data); +} + +static void fake_server_do_handshake(grpc_security_connector *sc, + grpc_endpoint *nonsecure_endpoint, + grpc_security_handshake_done_cb cb, + void *user_data) { + tsi_handshaker_destroy(sc->handshaker); + sc->handshaker = tsi_create_fake_handshaker(0); + grpc_do_security_handshake(sc, nonsecure_endpoint, cb, user_data); +} + static grpc_security_connector_vtable fake_channel_vtable = { - fake_channel_destroy, fake_channel_create_handshaker, fake_check_peer}; + fake_channel_destroy, fake_channel_do_handshake, fake_check_peer}; static grpc_security_connector_vtable fake_server_vtable = { - fake_server_destroy, fake_server_create_handshaker, fake_check_peer}; + fake_server_destroy, fake_server_do_handshake, fake_check_peer}; grpc_channel_security_connector *grpc_fake_channel_security_connector_create( grpc_credentials *request_metadata_creds, int call_host_check_is_async) { @@ -344,6 +358,7 @@ static void ssl_channel_destroy(grpc_security_connector *sc) { if (c->overridden_target_name != NULL) gpr_free(c->overridden_target_name); tsi_peer_destruct(&c->peer); GRPC_AUTH_CONTEXT_UNREF(sc->auth_context, "connector"); + tsi_handshaker_destroy(sc->handshaker); gpr_free(sc); } @@ -354,6 +369,7 @@ static void ssl_server_destroy(grpc_security_connector *sc) { tsi_ssl_handshaker_factory_destroy(c->handshaker_factory); } GRPC_AUTH_CONTEXT_UNREF(sc->auth_context, "connector"); + tsi_handshaker_destroy(sc->handshaker); gpr_free(sc); } @@ -362,6 +378,8 @@ static grpc_security_status ssl_create_handshaker( const char *peer_name, tsi_handshaker **handshaker) { tsi_result result = TSI_OK; if (handshaker_factory == NULL) return GRPC_SECURITY_ERROR; + tsi_handshaker_destroy(*handshaker); + *handshaker = NULL; result = tsi_ssl_handshaker_factory_create_handshaker( handshaker_factory, is_client ? peer_name : NULL, handshaker); if (result != TSI_OK) { @@ -372,22 +390,37 @@ static grpc_security_status ssl_create_handshaker( return GRPC_SECURITY_OK; } -static grpc_security_status ssl_channel_create_handshaker( - grpc_security_connector *sc, tsi_handshaker **handshaker) { +static void ssl_channel_do_handshake(grpc_security_connector *sc, + grpc_endpoint *nonsecure_endpoint, + grpc_security_handshake_done_cb cb, + void *user_data) { grpc_ssl_channel_security_connector *c = (grpc_ssl_channel_security_connector *)sc; - return ssl_create_handshaker(c->handshaker_factory, 1, - c->overridden_target_name != NULL - ? c->overridden_target_name - : c->target_name, - handshaker); + grpc_security_status status = ssl_create_handshaker( + c->handshaker_factory, 1, + c->overridden_target_name != NULL ? c->overridden_target_name + : c->target_name, + &sc->handshaker); + if (status != GRPC_SECURITY_OK) { + cb(user_data, status, nonsecure_endpoint, NULL); + } else { + grpc_do_security_handshake(sc, nonsecure_endpoint, cb, user_data); + } } -static grpc_security_status ssl_server_create_handshaker( - grpc_security_connector *sc, tsi_handshaker **handshaker) { +static void ssl_server_do_handshake(grpc_security_connector *sc, + grpc_endpoint *nonsecure_endpoint, + grpc_security_handshake_done_cb cb, + void *user_data) { grpc_ssl_server_security_connector *c = (grpc_ssl_server_security_connector *)sc; - return ssl_create_handshaker(c->handshaker_factory, 0, NULL, handshaker); + grpc_security_status status = + ssl_create_handshaker(c->handshaker_factory, 0, NULL, &sc->handshaker); + if (status != GRPC_SECURITY_OK) { + cb(user_data, status, nonsecure_endpoint, NULL); + } else { + grpc_do_security_handshake(sc, nonsecure_endpoint, cb, user_data); + } } static int ssl_host_matches_name(const tsi_peer *peer, const char *peer_name) { @@ -512,10 +545,10 @@ static grpc_security_status ssl_channel_check_call_host( } static grpc_security_connector_vtable ssl_channel_vtable = { - ssl_channel_destroy, ssl_channel_create_handshaker, ssl_channel_check_peer}; + ssl_channel_destroy, ssl_channel_do_handshake, ssl_channel_check_peer}; static grpc_security_connector_vtable ssl_server_vtable = { - ssl_server_destroy, ssl_server_create_handshaker, ssl_server_check_peer}; + ssl_server_destroy, ssl_server_do_handshake, ssl_server_check_peer}; static gpr_slice default_pem_root_certs; diff --git a/src/core/security/security_connector.h b/src/core/security/security_connector.h index 2c9aa1c5a4e..76d860d277f 100644 --- a/src/core/security/security_connector.h +++ b/src/core/security/security_connector.h @@ -63,10 +63,17 @@ typedef struct grpc_security_connector grpc_security_connector; typedef void (*grpc_security_check_cb)(void *user_data, grpc_security_status status); + +/* Ownership of the secure_endpoint is transfered. */ +typedef void (*grpc_security_handshake_done_cb)( + void *user_data, grpc_security_status status, + grpc_endpoint *wrapped_endpoint, grpc_endpoint *secure_endpoint); + typedef struct { void (*destroy)(grpc_security_connector *sc); - grpc_security_status (*create_handshaker)(grpc_security_connector *sc, - tsi_handshaker **handshaker); + void (*do_handshake)(grpc_security_connector *sc, + grpc_endpoint *nonsecure_endpoint, + grpc_security_handshake_done_cb cb, void *user_data); grpc_security_status (*check_peer)(grpc_security_connector *sc, tsi_peer peer, grpc_security_check_cb cb, void *user_data); @@ -77,6 +84,7 @@ struct grpc_security_connector { gpr_refcount refcount; int is_client_side; const char *url_scheme; + tsi_handshaker *handshaker; grpc_auth_context *auth_context; /* Populated after the peer is checked. */ }; @@ -100,9 +108,11 @@ grpc_security_connector *grpc_security_connector_ref( void grpc_security_connector_unref(grpc_security_connector *policy); #endif -/* Handshake creation. */ -grpc_security_status grpc_security_connector_create_handshaker( - grpc_security_connector *sc, tsi_handshaker **handshaker); +/* Handshake. */ +void grpc_security_connector_do_handshake(grpc_security_connector *connector, + grpc_endpoint *nonsecure_endpoint, + grpc_security_handshake_done_cb cb, + void *user_data); /* Check the peer. Implementations can choose to check the peer either synchronously or diff --git a/src/core/security/server_secure_chttp2.c b/src/core/security/server_secure_chttp2.c index 8d9d036d808..a3976ac2391 100644 --- a/src/core/security/server_secure_chttp2.c +++ b/src/core/security/server_secure_chttp2.c @@ -44,7 +44,6 @@ #include "src/core/security/credentials.h" #include "src/core/security/security_connector.h" #include "src/core/security/security_context.h" -#include "src/core/security/secure_transport_setup.h" #include "src/core/surface/server.h" #include "src/core/transport/chttp2_transport.h" #include @@ -121,10 +120,9 @@ static int remove_tcp_from_list_locked(grpc_server_secure_state *state, return -1; } -static void on_secure_transport_setup_done(void *statep, - grpc_security_status status, - grpc_endpoint *wrapped_endpoint, - grpc_endpoint *secure_endpoint) { +static void on_secure_handshake_done(void *statep, grpc_security_status status, + grpc_endpoint *wrapped_endpoint, + grpc_endpoint *secure_endpoint) { grpc_server_secure_state *state = statep; grpc_transport *transport; grpc_mdctx *mdctx; @@ -163,8 +161,8 @@ static void on_accept(void *statep, grpc_endpoint *tcp) { node->next = state->handshaking_tcp_endpoints; state->handshaking_tcp_endpoints = node; gpr_mu_unlock(&state->mu); - grpc_setup_secure_transport(state->sc, tcp, on_secure_transport_setup_done, - state); + grpc_security_connector_do_handshake(state->sc, tcp, on_secure_handshake_done, + state); } /* Server callback: start listening on our ports */ diff --git a/src/core/surface/secure_channel_create.c b/src/core/surface/secure_channel_create.c index 5b03ba95a79..43b8c97e021 100644 --- a/src/core/surface/secure_channel_create.c +++ b/src/core/surface/secure_channel_create.c @@ -46,7 +46,6 @@ #include "src/core/iomgr/tcp_client.h" #include "src/core/security/auth_filters.h" #include "src/core/security/credentials.h" -#include "src/core/security/secure_transport_setup.h" #include "src/core/surface/channel.h" #include "src/core/transport/chttp2_transport.h" #include "src/core/tsi/transport_security_interface.h" @@ -74,14 +73,13 @@ static void connector_unref(grpc_connector *con) { } } -static void on_secure_transport_setup_done(void *arg, - grpc_security_status status, - grpc_endpoint *wrapped_endpoint, - grpc_endpoint *secure_endpoint) { +static void on_secure_handshake_done(void *arg, grpc_security_status status, + grpc_endpoint *wrapped_endpoint, + grpc_endpoint *secure_endpoint) { connector *c = arg; grpc_iomgr_closure *notify; if (status != GRPC_SECURITY_OK) { - gpr_log(GPR_ERROR, "Secure transport setup failed with error %d.", status); + gpr_log(GPR_ERROR, "Secure handshake failed with error %d.", status); memset(c->result, 0, sizeof(*c->result)); } else { c->result->transport = grpc_create_chttp2_transport( @@ -101,8 +99,8 @@ static void connected(void *arg, grpc_endpoint *tcp) { connector *c = arg; grpc_iomgr_closure *notify; if (tcp != NULL) { - grpc_setup_secure_transport(&c->security_connector->base, tcp, - on_secure_transport_setup_done, c); + grpc_security_connector_do_handshake(&c->security_connector->base, tcp, + on_secure_handshake_done, c); } else { memset(c->result, 0, sizeof(*c->result)); notify = c->notify; diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 325a293e04e..9d303d14020 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -770,10 +770,10 @@ include/grpc/census.h \ src/core/security/auth_filters.h \ src/core/security/base64.h \ src/core/security/credentials.h \ +src/core/security/handshake.h \ src/core/security/json_token.h \ src/core/security/jwt_verifier.h \ src/core/security/secure_endpoint.h \ -src/core/security/secure_transport_setup.h \ src/core/security/security_connector.h \ src/core/security/security_context.h \ src/core/tsi/fake_transport_security.h \ @@ -890,10 +890,10 @@ src/core/security/credentials_metadata.c \ src/core/security/credentials_posix.c \ src/core/security/credentials_win32.c \ src/core/security/google_default_credentials.c \ +src/core/security/handshake.c \ src/core/security/json_token.c \ src/core/security/jwt_verifier.c \ src/core/security/secure_endpoint.c \ -src/core/security/secure_transport_setup.c \ src/core/security/security_connector.c \ src/core/security/security_context.c \ src/core/security/server_auth_filter.c \ diff --git a/tools/run_tests/sources_and_headers.json b/tools/run_tests/sources_and_headers.json index 50f078586d1..5f78949d56e 100644 --- a/tools/run_tests/sources_and_headers.json +++ b/tools/run_tests/sources_and_headers.json @@ -12340,10 +12340,10 @@ "src/core/security/auth_filters.h", "src/core/security/base64.h", "src/core/security/credentials.h", + "src/core/security/handshake.h", "src/core/security/json_token.h", "src/core/security/jwt_verifier.h", "src/core/security/secure_endpoint.h", - "src/core/security/secure_transport_setup.h", "src/core/security/security_connector.h", "src/core/security/security_context.h", "src/core/surface/byte_buffer_queue.h", @@ -12547,14 +12547,14 @@ "src/core/security/credentials_posix.c", "src/core/security/credentials_win32.c", "src/core/security/google_default_credentials.c", + "src/core/security/handshake.c", + "src/core/security/handshake.h", "src/core/security/json_token.c", "src/core/security/json_token.h", "src/core/security/jwt_verifier.c", "src/core/security/jwt_verifier.h", "src/core/security/secure_endpoint.c", "src/core/security/secure_endpoint.h", - "src/core/security/secure_transport_setup.c", - "src/core/security/secure_transport_setup.h", "src/core/security/security_connector.c", "src/core/security/security_connector.h", "src/core/security/security_context.c", diff --git a/tools/run_tests/tests.json b/tools/run_tests/tests.json index 6c06d74834b..127b1dfc408 100644 --- a/tools/run_tests/tests.json +++ b/tools/run_tests/tests.json @@ -1538,6 +1538,7 @@ "posix", "windows" ], + "exclude_configs": [], "flaky": false, "language": "c++", "name": "status_test", diff --git a/vsprojects/grpc/grpc.vcxproj b/vsprojects/grpc/grpc.vcxproj index ebdc926ee7e..0fe0eb5cf91 100644 --- a/vsprojects/grpc/grpc.vcxproj +++ b/vsprojects/grpc/grpc.vcxproj @@ -232,10 +232,10 @@ + - @@ -362,14 +362,14 @@ + + - - diff --git a/vsprojects/grpc/grpc.vcxproj.filters b/vsprojects/grpc/grpc.vcxproj.filters index baec0db4f92..a0713561a3b 100644 --- a/vsprojects/grpc/grpc.vcxproj.filters +++ b/vsprojects/grpc/grpc.vcxproj.filters @@ -25,6 +25,9 @@ src\core\security + + src\core\security + src\core\security @@ -34,9 +37,6 @@ src\core\security - - src\core\security - src\core\security @@ -452,6 +452,9 @@ src\core\security + + src\core\security + src\core\security @@ -461,9 +464,6 @@ src\core\security - - src\core\security - src\core\security From db5282b2dca2bfd08b5fe7c31660faf80eb9fd59 Mon Sep 17 00:00:00 2001 From: Julien Boeuf Date: Fri, 21 Aug 2015 15:23:52 -0700 Subject: [PATCH 02/17] Transfer the ownership of the handshaker. --- src/core/httpcli/httpcli_security_connector.c | 9 +++--- src/core/security/handshake.c | 27 ++++++++++-------- src/core/security/handshake.h | 5 ++-- src/core/security/security_connector.c | 28 ++++++++----------- src/core/security/security_connector.h | 1 - 5 files changed, 34 insertions(+), 36 deletions(-) diff --git a/src/core/httpcli/httpcli_security_connector.c b/src/core/httpcli/httpcli_security_connector.c index a4cd00d9a07..86f34db1d0b 100644 --- a/src/core/httpcli/httpcli_security_connector.c +++ b/src/core/httpcli/httpcli_security_connector.c @@ -55,7 +55,6 @@ static void httpcli_ssl_destroy(grpc_security_connector *sc) { tsi_ssl_handshaker_factory_destroy(c->handshaker_factory); } if (c->secure_peer_name != NULL) gpr_free(c->secure_peer_name); - tsi_handshaker_destroy(sc->handshaker); gpr_free(sc); } @@ -65,20 +64,20 @@ static void httpcli_ssl_do_handshake( grpc_httpcli_ssl_channel_security_connector *c = (grpc_httpcli_ssl_channel_security_connector *)sc; tsi_result result = TSI_OK; + tsi_handshaker *handshaker; if (c->handshaker_factory == NULL) { cb(user_data, GRPC_SECURITY_ERROR, nonsecure_endpoint, NULL); return; } - tsi_handshaker_destroy(sc->handshaker); - sc->handshaker = NULL; result = tsi_ssl_handshaker_factory_create_handshaker( - c->handshaker_factory, c->secure_peer_name, &sc->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(user_data, GRPC_SECURITY_ERROR, nonsecure_endpoint, NULL); } else { - grpc_do_security_handshake(sc, nonsecure_endpoint, cb, user_data); + grpc_do_security_handshake(handshaker, sc, nonsecure_endpoint, cb, + user_data); } } diff --git a/src/core/security/handshake.c b/src/core/security/handshake.c index 23830ed5a1a..62208ca8f23 100644 --- a/src/core/security/handshake.c +++ b/src/core/security/handshake.c @@ -44,6 +44,7 @@ typedef struct { grpc_security_connector *connector; + tsi_handshaker *handshaker; unsigned char *handshake_buffer; size_t handshake_buffer_size; grpc_endpoint *wrapped_endpoint; @@ -77,6 +78,8 @@ static void security_handshake_done(grpc_security_handshake *h, } if (h->handshake_buffer != NULL) gpr_free(h->handshake_buffer); gpr_slice_buffer_destroy(&h->left_overs); + tsi_handshaker_destroy(h->handshaker); + GRPC_SECURITY_CONNECTOR_UNREF(h->connector, "handshake"); gpr_free(h); } @@ -89,8 +92,8 @@ static void on_peer_checked(void *user_data, grpc_security_status status) { security_handshake_done(h, 0); return; } - result = tsi_handshaker_create_frame_protector(h->connector->handshaker, NULL, - &protector); + result = + tsi_handshaker_create_frame_protector(h->handshaker, NULL, &protector); if (result != TSI_OK) { gpr_log(GPR_ERROR, "Frame protector creation failed with error %s.", tsi_result_to_string(result)); @@ -107,8 +110,7 @@ static void on_peer_checked(void *user_data, grpc_security_status status) { static void check_peer(grpc_security_handshake *h) { grpc_security_status peer_status; tsi_peer peer; - tsi_result result = - tsi_handshaker_extract_peer(h->connector->handshaker, &peer); + tsi_result result = tsi_handshaker_extract_peer(h->handshaker, &peer); if (result != TSI_OK) { gpr_log(GPR_ERROR, "Peer extraction failed with error %s", @@ -136,7 +138,7 @@ static void send_handshake_bytes_to_peer(grpc_security_handshake *h) { do { size_t to_send_size = h->handshake_buffer_size - offset; result = tsi_handshaker_get_bytes_to_send_to_peer( - h->connector->handshaker, h->handshake_buffer + offset, &to_send_size); + h->handshaker, h->handshake_buffer + offset, &to_send_size); offset += to_send_size; if (result == TSI_INCOMPLETE_DATA) { h->handshake_buffer_size *= 2; @@ -193,12 +195,11 @@ static void on_handshake_data_received_from_peer( for (i = 0; i < nslices; i++) { consumed_slice_size = GPR_SLICE_LENGTH(slices[i]); result = tsi_handshaker_process_bytes_from_peer( - h->connector->handshaker, GPR_SLICE_START_PTR(slices[i]), - &consumed_slice_size); - if (!tsi_handshaker_is_in_progress(h->connector->handshaker)) break; + h->handshaker, GPR_SLICE_START_PTR(slices[i]), &consumed_slice_size); + if (!tsi_handshaker_is_in_progress(h->handshaker)) break; } - if (tsi_handshaker_is_in_progress(h->connector->handshaker)) { + if (tsi_handshaker_is_in_progress(h->handshaker)) { /* We may need more data. */ if (result == TSI_INCOMPLETE_DATA) { /* TODO(klempner,jboeuf): This should probably use the client setup @@ -258,7 +259,7 @@ static void on_handshake_data_sent_to_peer(void *handshake, } /* We may be done. */ - if (tsi_handshaker_is_in_progress(h->connector->handshaker)) { + if (tsi_handshaker_is_in_progress(h->handshaker)) { /* TODO(klempner,jboeuf): This should probably use the client setup deadline */ grpc_endpoint_notify_on_read( @@ -268,13 +269,15 @@ static void on_handshake_data_sent_to_peer(void *handshake, } } -void grpc_do_security_handshake(grpc_security_connector *connector, +void grpc_do_security_handshake(tsi_handshaker *handshaker, + grpc_security_connector *connector, grpc_endpoint *nonsecure_endpoint, grpc_security_handshake_done_cb cb, void *user_data) { grpc_security_handshake *h = gpr_malloc(sizeof(grpc_security_handshake)); memset(h, 0, sizeof(grpc_security_handshake)); - h->connector = connector; + h->handshaker = handshaker; + h->connector = GRPC_SECURITY_CONNECTOR_REF(connector, "handshake"); h->handshake_buffer_size = GRPC_INITIAL_HANDSHAKE_BUFFER_SIZE; h->handshake_buffer = gpr_malloc(h->handshake_buffer_size); h->wrapped_endpoint = nonsecure_endpoint; diff --git a/src/core/security/handshake.h b/src/core/security/handshake.h index 1739855d69b..d7e4a305800 100644 --- a/src/core/security/handshake.h +++ b/src/core/security/handshake.h @@ -38,8 +38,9 @@ #include "src/core/security/security_connector.h" -/* Calls the callback upon completion. */ -void grpc_do_security_handshake(grpc_security_connector *connector, +/* Calls the callback upon completion. Takes owership of handshaker. */ +void grpc_do_security_handshake(tsi_handshaker *handshaker, + grpc_security_connector *connector, grpc_endpoint *nonsecure_endpoint, grpc_security_handshake_done_cb cb, void *user_data); diff --git a/src/core/security/security_connector.c b/src/core/security/security_connector.c index 922b92adeaa..983ac7635da 100644 --- a/src/core/security/security_connector.c +++ b/src/core/security/security_connector.c @@ -222,13 +222,11 @@ typedef struct { static void fake_channel_destroy(grpc_security_connector *sc) { grpc_channel_security_connector *c = (grpc_channel_security_connector *)sc; grpc_credentials_unref(c->request_metadata_creds); - tsi_handshaker_destroy(sc->handshaker); GRPC_AUTH_CONTEXT_UNREF(sc->auth_context, "connector"); gpr_free(sc); } static void fake_server_destroy(grpc_security_connector *sc) { - tsi_handshaker_destroy(sc->handshaker); GRPC_AUTH_CONTEXT_UNREF(sc->auth_context, "connector"); gpr_free(sc); } @@ -286,18 +284,16 @@ static void fake_channel_do_handshake(grpc_security_connector *sc, grpc_endpoint *nonsecure_endpoint, grpc_security_handshake_done_cb cb, void *user_data) { - tsi_handshaker_destroy(sc->handshaker); - sc->handshaker = tsi_create_fake_handshaker(1); - grpc_do_security_handshake(sc, nonsecure_endpoint, cb, user_data); + grpc_do_security_handshake(tsi_create_fake_handshaker(1), sc, + nonsecure_endpoint, cb, user_data); } static void fake_server_do_handshake(grpc_security_connector *sc, grpc_endpoint *nonsecure_endpoint, grpc_security_handshake_done_cb cb, void *user_data) { - tsi_handshaker_destroy(sc->handshaker); - sc->handshaker = tsi_create_fake_handshaker(0); - grpc_do_security_handshake(sc, nonsecure_endpoint, cb, user_data); + grpc_do_security_handshake(tsi_create_fake_handshaker(0), sc, + nonsecure_endpoint, cb, user_data); } static grpc_security_connector_vtable fake_channel_vtable = { @@ -358,7 +354,6 @@ static void ssl_channel_destroy(grpc_security_connector *sc) { if (c->overridden_target_name != NULL) gpr_free(c->overridden_target_name); tsi_peer_destruct(&c->peer); GRPC_AUTH_CONTEXT_UNREF(sc->auth_context, "connector"); - tsi_handshaker_destroy(sc->handshaker); gpr_free(sc); } @@ -369,7 +364,6 @@ static void ssl_server_destroy(grpc_security_connector *sc) { tsi_ssl_handshaker_factory_destroy(c->handshaker_factory); } GRPC_AUTH_CONTEXT_UNREF(sc->auth_context, "connector"); - tsi_handshaker_destroy(sc->handshaker); gpr_free(sc); } @@ -378,8 +372,6 @@ static grpc_security_status ssl_create_handshaker( const char *peer_name, tsi_handshaker **handshaker) { tsi_result result = TSI_OK; if (handshaker_factory == NULL) return GRPC_SECURITY_ERROR; - tsi_handshaker_destroy(*handshaker); - *handshaker = NULL; result = tsi_ssl_handshaker_factory_create_handshaker( handshaker_factory, is_client ? peer_name : NULL, handshaker); if (result != TSI_OK) { @@ -396,15 +388,17 @@ static void ssl_channel_do_handshake(grpc_security_connector *sc, void *user_data) { grpc_ssl_channel_security_connector *c = (grpc_ssl_channel_security_connector *)sc; + tsi_handshaker *handshaker; grpc_security_status status = ssl_create_handshaker( c->handshaker_factory, 1, c->overridden_target_name != NULL ? c->overridden_target_name : c->target_name, - &sc->handshaker); + &handshaker); if (status != GRPC_SECURITY_OK) { cb(user_data, status, nonsecure_endpoint, NULL); } else { - grpc_do_security_handshake(sc, nonsecure_endpoint, cb, user_data); + grpc_do_security_handshake(handshaker, sc, nonsecure_endpoint, cb, + user_data); } } @@ -414,12 +408,14 @@ static void ssl_server_do_handshake(grpc_security_connector *sc, void *user_data) { grpc_ssl_server_security_connector *c = (grpc_ssl_server_security_connector *)sc; + tsi_handshaker *handshaker; grpc_security_status status = - ssl_create_handshaker(c->handshaker_factory, 0, NULL, &sc->handshaker); + ssl_create_handshaker(c->handshaker_factory, 0, NULL, &handshaker); if (status != GRPC_SECURITY_OK) { cb(user_data, status, nonsecure_endpoint, NULL); } else { - grpc_do_security_handshake(sc, nonsecure_endpoint, cb, user_data); + grpc_do_security_handshake(handshaker, sc, nonsecure_endpoint, cb, + user_data); } } diff --git a/src/core/security/security_connector.h b/src/core/security/security_connector.h index 76d860d277f..5fc1db382e5 100644 --- a/src/core/security/security_connector.h +++ b/src/core/security/security_connector.h @@ -84,7 +84,6 @@ struct grpc_security_connector { gpr_refcount refcount; int is_client_side; const char *url_scheme; - tsi_handshaker *handshaker; grpc_auth_context *auth_context; /* Populated after the peer is checked. */ }; From 5600a6aaac8b1582b029157b6549c24c755ff619 Mon Sep 17 00:00:00 2001 From: Julien Boeuf Date: Tue, 15 Sep 2015 23:01:39 -0700 Subject: [PATCH 03/17] Changing build.yaml: s/secure_tranport_setup/handshake --- build.yaml | 12 ++++++------ gRPC.podspec | 10 +++++----- vsprojects/vcxproj/grpc/grpc.vcxproj | 6 +++--- vsprojects/vcxproj/grpc/grpc.vcxproj.filters | 12 ++++++------ 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/build.yaml b/build.yaml index 444ee626f40..322903e82cb 100644 --- a/build.yaml +++ b/build.yaml @@ -178,15 +178,15 @@ libs: language: c public_headers: [include/grpc/grpc_security.h] headers: [src/core/security/auth_filters.h, src/core/security/base64.h, src/core/security/credentials.h, - src/core/security/json_token.h, src/core/security/jwt_verifier.h, src/core/security/secure_endpoint.h, - src/core/security/secure_transport_setup.h, src/core/security/security_connector.h, - src/core/security/security_context.h, src/core/tsi/fake_transport_security.h, - src/core/tsi/ssl_transport_security.h, src/core/tsi/transport_security.h, src/core/tsi/transport_security_interface.h] + src/core/security/handshake.h, src/core/security/json_token.h, src/core/security/jwt_verifier.h, + src/core/security/secure_endpoint.h, src/core/security/security_connector.h, src/core/security/security_context.h, + src/core/tsi/fake_transport_security.h, src/core/tsi/ssl_transport_security.h, + src/core/tsi/transport_security.h, src/core/tsi/transport_security_interface.h] src: [src/core/httpcli/httpcli_security_connector.c, src/core/security/base64.c, src/core/security/client_auth_filter.c, src/core/security/credentials.c, src/core/security/credentials_metadata.c, src/core/security/credentials_posix.c, src/core/security/credentials_win32.c, - src/core/security/google_default_credentials.c, src/core/security/json_token.c, - src/core/security/jwt_verifier.c, src/core/security/secure_endpoint.c, src/core/security/secure_transport_setup.c, + src/core/security/google_default_credentials.c, src/core/security/handshake.c, + src/core/security/json_token.c, src/core/security/jwt_verifier.c, src/core/security/secure_endpoint.c, src/core/security/security_connector.c, src/core/security/security_context.c, src/core/security/server_auth_filter.c, src/core/security/server_secure_chttp2.c, src/core/surface/init_secure.c, src/core/surface/secure_channel_create.c, src/core/tsi/fake_transport_security.c, diff --git a/gRPC.podspec b/gRPC.podspec index 997b1172d04..673f6decb97 100644 --- a/gRPC.podspec +++ b/gRPC.podspec @@ -36,17 +36,17 @@ Pod::Spec.new do |s| s.name = 'gRPC' - s.version = '0.7.0' + s.version = '0.11.0' s.summary = 'gRPC client library for iOS/OSX' s.homepage = 'http://www.grpc.io' s.license = 'New BSD' s.authors = { 'The gRPC contributors' => 'grpc-packages@google.com' } # s.source = { :git => 'https://github.com/grpc/grpc.git', - # :tag => 'release-0_10_0-objectivec-0.6.0' } + # :tag => 'release-0_11_0-objectivec-0.11.0' } - s.ios.deployment_target = '6.0' - s.osx.deployment_target = '10.8' + s.ios.deployment_target = '7.1' + s.osx.deployment_target = '10.9' s.requires_arc = true objc_dir = 'src/objective-c' @@ -591,6 +591,6 @@ Pod::Spec.new do |s| ss.dependency 'gRPC/GRPCClient' ss.dependency 'gRPC/RxLibrary' - ss.dependency 'Protobuf', '~> 3.0.0-alpha-3' + ss.dependency 'Protobuf', '~> 3.0.0-alpha-4' end end diff --git a/vsprojects/vcxproj/grpc/grpc.vcxproj b/vsprojects/vcxproj/grpc/grpc.vcxproj index 2eda9e32e67..1ff4d6ad4ab 100644 --- a/vsprojects/vcxproj/grpc/grpc.vcxproj +++ b/vsprojects/vcxproj/grpc/grpc.vcxproj @@ -232,10 +232,10 @@ + - @@ -366,14 +366,14 @@ + + - - diff --git a/vsprojects/vcxproj/grpc/grpc.vcxproj.filters b/vsprojects/vcxproj/grpc/grpc.vcxproj.filters index 7026da89abd..c69f49de245 100644 --- a/vsprojects/vcxproj/grpc/grpc.vcxproj.filters +++ b/vsprojects/vcxproj/grpc/grpc.vcxproj.filters @@ -25,6 +25,9 @@ src\core\security + + src\core\security + src\core\security @@ -34,9 +37,6 @@ src\core\security - - src\core\security - src\core\security @@ -464,6 +464,9 @@ src\core\security + + src\core\security + src\core\security @@ -473,9 +476,6 @@ src\core\security - - src\core\security - src\core\security From 4a47c2ad442b509fc8130bf26d1af242fcfb0b49 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Thu, 17 Sep 2015 18:48:58 -0700 Subject: [PATCH 04/17] Revert "Swallow content-type at the client. Fixes #3368" This reverts commit 784db97ad2991330b8a7d8e1fe53bcdd06203cf3. --- src/core/surface/call.c | 2 -- src/core/surface/channel.c | 8 -------- src/core/surface/channel.h | 1 - 3 files changed, 11 deletions(-) diff --git a/src/core/surface/call.c b/src/core/surface/call.c index 1636998f594..4168c2ef0cc 100644 --- a/src/core/surface/call.c +++ b/src/core/surface/call.c @@ -1485,8 +1485,6 @@ static void recv_metadata(grpc_call *call, grpc_metadata_batch *md) { } else if (key == grpc_channel_get_encodings_accepted_by_peer_string( call->channel)) { set_encodings_accepted_by_peer(call, md->value->slice); - } else if (key == grpc_channel_get_content_type_string(call->channel)) { - continue; /* swallow "content-type" header */ } else { dest = &call->buffered_metadata[is_trailing]; if (dest->count == dest->capacity) { diff --git a/src/core/surface/channel.c b/src/core/surface/channel.c index cc9d44f45d6..a89523b3ab6 100644 --- a/src/core/surface/channel.c +++ b/src/core/surface/channel.c @@ -69,7 +69,6 @@ struct grpc_channel { grpc_mdstr *grpc_compression_algorithm_string; grpc_mdstr *grpc_encodings_accepted_by_peer_string; grpc_mdstr *grpc_message_string; - grpc_mdstr *content_type_string; grpc_mdstr *path_string; grpc_mdstr *authority_string; grpc_mdelem *default_authority; @@ -112,8 +111,6 @@ grpc_channel *grpc_channel_create_from_filters( grpc_mdstr_from_string(mdctx, "grpc-accept-encoding", 0); channel->grpc_message_string = grpc_mdstr_from_string(mdctx, "grpc-message", 0); - channel->content_type_string = - grpc_mdstr_from_string(mdctx, "content-type", 0); for (i = 0; i < NUM_CACHED_STATUS_ELEMS; i++) { char buf[GPR_LTOA_MIN_BUFSIZE]; gpr_ltoa((long)i, buf); @@ -284,7 +281,6 @@ static void destroy_channel(void *p, int ok) { GRPC_MDSTR_UNREF(channel->grpc_compression_algorithm_string); GRPC_MDSTR_UNREF(channel->grpc_encodings_accepted_by_peer_string); GRPC_MDSTR_UNREF(channel->grpc_message_string); - GRPC_MDSTR_UNREF(channel->content_type_string); GRPC_MDSTR_UNREF(channel->path_string); GRPC_MDSTR_UNREF(channel->authority_string); while (channel->registered_calls) { @@ -368,10 +364,6 @@ grpc_mdstr *grpc_channel_get_message_string(grpc_channel *channel) { return channel->grpc_message_string; } -grpc_mdstr *grpc_channel_get_content_type_string(grpc_channel *channel) { - return channel->content_type_string; -} - gpr_uint32 grpc_channel_get_max_message_length(grpc_channel *channel) { return channel->max_message_length; } diff --git a/src/core/surface/channel.h b/src/core/surface/channel.h index 05fbc8d75cd..f271616f606 100644 --- a/src/core/surface/channel.h +++ b/src/core/surface/channel.h @@ -59,7 +59,6 @@ grpc_mdstr *grpc_channel_get_compression_algorithm_string( grpc_mdstr *grpc_channel_get_encodings_accepted_by_peer_string( grpc_channel *channel); grpc_mdstr *grpc_channel_get_message_string(grpc_channel *channel); -grpc_mdstr *grpc_channel_get_content_type_string(grpc_channel *channel); gpr_uint32 grpc_channel_get_max_message_length(grpc_channel *channel); #ifdef GRPC_CHANNEL_REF_COUNT_DEBUG From a63fe4e259d572b591ec8b4ea21101d242be4f62 Mon Sep 17 00:00:00 2001 From: yang-g Date: Fri, 18 Sep 2015 00:02:59 -0700 Subject: [PATCH 05/17] remove interested party early --- src/core/surface/channel_connectivity.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/core/surface/channel_connectivity.c b/src/core/surface/channel_connectivity.c index 88a7c165985..e89c80795ae 100644 --- a/src/core/surface/channel_connectivity.c +++ b/src/core/surface/channel_connectivity.c @@ -67,6 +67,7 @@ typedef struct { gpr_mu mu; callback_phase phase; int success; + int removed; grpc_iomgr_closure on_complete; grpc_alarm alarm; grpc_connectivity_state state; @@ -77,11 +78,6 @@ typedef struct { } state_watcher; static void delete_state_watcher(state_watcher *w) { - grpc_channel_element *client_channel_elem = grpc_channel_stack_last_element( - grpc_channel_get_channel_stack(w->channel)); - grpc_client_channel_del_interested_party(client_channel_elem, - grpc_cq_pollset(w->cq)); - GRPC_CHANNEL_INTERNAL_UNREF(w->channel, "watch_connectivity"); gpr_mu_destroy(&w->mu); gpr_free(w); } @@ -112,7 +108,18 @@ static void finished_completion(void *pw, grpc_cq_completion *ignored) { static void partly_done(state_watcher *w, int due_to_completion) { int delete = 0; + grpc_channel_element *client_channel_elem = NULL; + gpr_mu_lock(&w->mu); + if (w->removed == 0) { + w->removed = 1; + client_channel_elem = grpc_channel_stack_last_element( + grpc_channel_get_channel_stack(w->channel)); + grpc_client_channel_del_interested_party(client_channel_elem, + grpc_cq_pollset(w->cq)); + GRPC_CHANNEL_INTERNAL_UNREF(w->channel, "watch_connectivity"); + } + gpr_mu_unlock(&w->mu); if (due_to_completion) { gpr_mu_lock(&w->mu); w->success = 1; @@ -163,6 +170,7 @@ void grpc_channel_watch_connectivity_state( w->phase = WAITING; w->state = last_observed_state; w->success = 0; + w->removed = 0; w->cq = cq; w->tag = tag; w->channel = channel; From d886f339399e0936e26a55fd71519b2db588d2fc Mon Sep 17 00:00:00 2001 From: yang-g Date: Fri, 18 Sep 2015 01:24:14 -0700 Subject: [PATCH 06/17] add a test --- src/core/surface/channel_connectivity.c | 2 +- test/cpp/end2end/end2end_test.cc | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/core/surface/channel_connectivity.c b/src/core/surface/channel_connectivity.c index e89c80795ae..5c55ad36550 100644 --- a/src/core/surface/channel_connectivity.c +++ b/src/core/surface/channel_connectivity.c @@ -78,6 +78,7 @@ typedef struct { } state_watcher; static void delete_state_watcher(state_watcher *w) { + GRPC_CHANNEL_INTERNAL_UNREF(w->channel, "watch_connectivity"); gpr_mu_destroy(&w->mu); gpr_free(w); } @@ -117,7 +118,6 @@ static void partly_done(state_watcher *w, int due_to_completion) { grpc_channel_get_channel_stack(w->channel)); grpc_client_channel_del_interested_party(client_channel_elem, grpc_cq_pollset(w->cq)); - GRPC_CHANNEL_INTERNAL_UNREF(w->channel, "watch_connectivity"); } gpr_mu_unlock(&w->mu); if (due_to_completion) { diff --git a/test/cpp/end2end/end2end_test.cc b/test/cpp/end2end/end2end_test.cc index bd829d96e1d..c294d594541 100644 --- a/test/cpp/end2end/end2end_test.cc +++ b/test/cpp/end2end/end2end_test.cc @@ -1076,6 +1076,24 @@ TEST_F(End2endTest, ChannelState) { EXPECT_EQ(GRPC_CHANNEL_CONNECTING, channel_->GetState(false)); } +// Takes 10s. +TEST_F(End2endTest, ChannelStateTimeout) { + int port = grpc_pick_unused_port_or_die(); + std::ostringstream server_address; + server_address << "127.0.0.1:" << port; + // Channel to non-existing server + auto channel = CreateChannel(server_address.str(), InsecureCredentials()); + // Start IDLE + EXPECT_EQ(GRPC_CHANNEL_IDLE, channel->GetState(true)); + + auto state = GRPC_CHANNEL_IDLE; + for (int i = 0; i < 10; i++) { + channel->WaitForStateChange(state, std::chrono::system_clock::now() + + std::chrono::milliseconds(1000)); + state = channel->GetState(false); + } +} + // Talking to a non-existing service. TEST_F(End2endTest, NonExistingService) { ResetChannel(); From 5e8abedd6110d58067980a2b089aab48311317a0 Mon Sep 17 00:00:00 2001 From: yang-g Date: Fri, 18 Sep 2015 01:31:57 -0700 Subject: [PATCH 07/17] use seconds --- test/cpp/end2end/end2end_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cpp/end2end/end2end_test.cc b/test/cpp/end2end/end2end_test.cc index c294d594541..111a1a4f2f5 100644 --- a/test/cpp/end2end/end2end_test.cc +++ b/test/cpp/end2end/end2end_test.cc @@ -1089,7 +1089,7 @@ TEST_F(End2endTest, ChannelStateTimeout) { auto state = GRPC_CHANNEL_IDLE; for (int i = 0; i < 10; i++) { channel->WaitForStateChange(state, std::chrono::system_clock::now() + - std::chrono::milliseconds(1000)); + std::chrono::seconds(1)); state = channel->GetState(false); } } From 13a71120f7cc1c1bc2811dc269bceff2f1f93047 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Fri, 18 Sep 2015 12:43:52 -0700 Subject: [PATCH 08/17] Client now swallows content-type on receive --- src/core/channel/http_client_filter.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/core/channel/http_client_filter.c b/src/core/channel/http_client_filter.c index ec832a03672..5f20f8c16d5 100644 --- a/src/core/channel/http_client_filter.c +++ b/src/core/channel/http_client_filter.c @@ -70,7 +70,7 @@ typedef struct channel_data { /* used to silence 'variable not used' warnings */ static void ignore_unused(void *ignored) {} -static grpc_mdelem *client_filter(void *user_data, grpc_mdelem *md) { +static grpc_mdelem *client_recv_filter(void *user_data, grpc_mdelem *md) { grpc_call_element *elem = user_data; channel_data *channeld = elem->channel_data; if (md == channeld->status) { @@ -78,6 +78,8 @@ static grpc_mdelem *client_filter(void *user_data, grpc_mdelem *md) { } else if (md->key == channeld->status->key) { grpc_call_element_send_cancel(elem); return NULL; + } else if (md->key == channeld->content_type->key) { + return NULL; } return md; } @@ -92,11 +94,13 @@ static void hc_on_recv(void *user_data, int success) { grpc_stream_op *op = &ops[i]; if (op->type != GRPC_OP_METADATA) continue; calld->got_initial_metadata = 1; - grpc_metadata_batch_filter(&op->data.metadata, client_filter, elem); + grpc_metadata_batch_filter(&op->data.metadata, client_recv_filter, elem); } calld->on_done_recv->cb(calld->on_done_recv->cb_arg, success); } + + static grpc_mdelem *client_strip_filter(void *user_data, grpc_mdelem *md) { grpc_call_element *elem = user_data; channel_data *channeld = elem->channel_data; From c95eead416aa1cad9f7b08c87975a3ca9a597421 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 18 Sep 2015 13:03:50 -0700 Subject: [PATCH 09/17] use_docker support for run_tests.py --- tools/jenkins/build_docker_and_run_tests.sh | 81 +++++++++++++++++++ ...ker_run_jenkins.sh => docker_run_tests.sh} | 7 +- tools/jenkins/run_jenkins.sh | 40 +-------- tools/run_tests/run_tests.py | 47 +++++++++-- 4 files changed, 126 insertions(+), 49 deletions(-) create mode 100755 tools/jenkins/build_docker_and_run_tests.sh rename tools/jenkins/{docker_run_jenkins.sh => docker_run_tests.sh} (88%) diff --git a/tools/jenkins/build_docker_and_run_tests.sh b/tools/jenkins/build_docker_and_run_tests.sh new file mode 100755 index 00000000000..f554b65f709 --- /dev/null +++ b/tools/jenkins/build_docker_and_run_tests.sh @@ -0,0 +1,81 @@ +#!/bin/bash +# Copyright 2015, Google Inc. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +# +# This script is invoked by run_tests.py to accommodate "test under docker" +# scenario. You should never need to call this script on your own. + +set -ex + +cd `dirname $0`/../.. +git_root=`pwd` +cd - + +mkdir -p /tmp/ccache + +# Create a local branch so the child Docker script won't complain +git branch -f jenkins-docker + +# Use image name based on Dockerfile checksum +DOCKER_IMAGE_NAME=grpc_jenkins_slave${docker_suffix}_`sha1sum tools/jenkins/grpc_jenkins_slave/Dockerfile | cut -f1 -d\ ` + +# Make sure docker image has been built. Should be instantaneous if so. +docker build -t $DOCKER_IMAGE_NAME tools/jenkins/grpc_jenkins_slave$docker_suffix + +# Make sure the CID file is gone. +rm -f docker.cid + +# Run tests inside docker +docker run \ + -e "RUN_TESTS_COMMAND=$RUN_TESTS_COMMAND" \ + -e "config=$config" \ + -e "arch=$arch" \ + -e CCACHE_DIR=/tmp/ccache \ + -it \ + -v "$git_root:/var/local/jenkins/grpc" \ + -v /tmp/ccache:/tmp/ccache \ + -w /var/local/git/grpc \ + --cidfile=docker.cid \ + $DOCKER_IMAGE_NAME \ + bash -l /var/local/jenkins/grpc/tools/jenkins/docker_run_tests.sh || DOCKER_FAILED="true" + +DOCKER_CID=`cat docker.cid` + +if [ "$XML_REPORT" != "" ] +then + docker cp "$DOCKER_CID:/var/local/git/grpc/$XML_REPORT" $git_root +fi + +# remove the container, possibly killing it first +docker rm -f $DOCKER_CID || true + +if [ "$DOCKER_FAILED" != "" ] && [ "$XML_REPORT" == "" ] +then + exit 1 +fi diff --git a/tools/jenkins/docker_run_jenkins.sh b/tools/jenkins/docker_run_tests.sh similarity index 88% rename from tools/jenkins/docker_run_jenkins.sh rename to tools/jenkins/docker_run_tests.sh index 0a5516c30df..781bff26b93 100755 --- a/tools/jenkins/docker_run_jenkins.sh +++ b/tools/jenkins/docker_run_tests.sh @@ -28,18 +28,17 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. # -# This script is invoked by run_jekins.sh when piggy-backing into docker. +# This script is invoked by build_docker_and_run_tests.py inside a docker +# container. You should never need to call this script on your own. set -e export CONFIG=$config export ASAN_SYMBOLIZER_PATH=/usr/bin/llvm-symbolizer-3.5 -export CPPFLAGS=-I/tmp/prebuilt/include mkdir -p /var/local/git git clone --recursive /var/local/jenkins/grpc /var/local/git/grpc -cd /var/local/git/grpc nvm use 0.12 rvm use ruby-2.1 -setarch $arch tools/run_tests/run_tests.py -t -c $config -l $language -x report.xml +$RUN_TESTS_COMMAND diff --git a/tools/jenkins/run_jenkins.sh b/tools/jenkins/run_jenkins.sh index 9ff588e8fa4..a55f1de9960 100755 --- a/tools/jenkins/run_jenkins.sh +++ b/tools/jenkins/run_jenkins.sh @@ -56,46 +56,8 @@ if [ "$platform" == "linux" ] then echo "building $language on Linux" - cd `dirname $0`/../.. - git_root=`pwd` - cd - + ./tools/run_tests/run_tests.py --use_docker -t -l $language -c $config -x report.xml || true - mkdir -p /tmp/ccache - - # Use image name based on Dockerfile checksum - DOCKER_IMAGE_NAME=grpc_jenkins_slave$docker_suffix_`sha1sum tools/jenkins/grpc_jenkins_slave/Dockerfile | cut -f1 -d\ ` - - # Make sure docker image has been built. Should be instantaneous if so. - docker build -t $DOCKER_IMAGE_NAME tools/jenkins/grpc_jenkins_slave$docker_suffix - - # Create a local branch so the child Docker script won't complain - git branch jenkins-docker - - # Make sure the CID file is gone. - rm -f docker.cid - - # Run tests inside docker - docker run \ - -e "config=$config" \ - -e "language=$language" \ - -e "arch=$arch" \ - -e CCACHE_DIR=/tmp/ccache \ - -i \ - -v "$git_root:/var/local/jenkins/grpc" \ - -v /tmp/ccache:/tmp/ccache \ - --cidfile=docker.cid \ - $DOCKER_IMAGE_NAME \ - bash -l /var/local/jenkins/grpc/tools/jenkins/docker_run_jenkins.sh || DOCKER_FAILED="true" - - DOCKER_CID=`cat docker.cid` - # forcefully kill the instance if it's still running, otherwise - # continue - # (failure to kill something that's already dead => things are dead) - docker kill $DOCKER_CID || true - docker cp $DOCKER_CID:/var/local/git/grpc/report.xml $git_root - # TODO(ctiller): why? - sleep 4 - docker rm $DOCKER_CID || true elif [ "$platform" == "interop" ] then python tools/run_tests/run_interops.py --language=$language diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index 8f6511b1f88..4774a317053 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -397,12 +397,6 @@ _WINDOWS_CONFIG = { 'opt': 'Release', } -# parse command line -argp = argparse.ArgumentParser(description='Run grpc tests.') -argp.add_argument('-c', '--config', - choices=['all'] + sorted(_CONFIGS.keys()), - nargs='+', - default=_DEFAULT) def runs_per_test_type(arg_str): """Auxilary function to parse the "runs_per_test" flag. @@ -423,6 +417,13 @@ def runs_per_test_type(arg_str): except: msg = "'{}' isn't a positive integer or 'inf'".format(arg_str) raise argparse.ArgumentTypeError(msg) + +# parse command line +argp = argparse.ArgumentParser(description='Run grpc tests.') +argp.add_argument('-c', '--config', + choices=['all'] + sorted(_CONFIGS.keys()), + nargs='+', + default=_DEFAULT) argp.add_argument('-n', '--runs_per_test', default=1, type=runs_per_test_type, help='A positive integer or "inf". If "inf", all tests will run in an ' 'infinite loop. Especially useful in combination with "-f"') @@ -449,11 +450,45 @@ argp.add_argument('-S', '--stop_on_failure', default=False, action='store_const', const=True) +argp.add_argument('--use_docker', + default=False, + action='store_const', + const=True, + help="Run all the tests under docker. That provides " + + "additional isolation and prevents the need to installs " + + "language specific prerequisites. Only available on Linux.") argp.add_argument('-a', '--antagonists', default=0, type=int) argp.add_argument('-x', '--xml_report', default=None, type=str, help='Generates a JUnit-compatible XML report') args = argp.parse_args() +if args.use_docker and not args.travis: + print 'Seen --use_docker flag, will run tests under docker.' + print + print 'IMPORTANT: The changes you are testing need to be locally committed' + print 'because only the committed changes in the current branch will be' + print 'copied to the docker environment.' + time.sleep(5) + + child_argv = [ arg for arg in sys.argv if not arg == '--use_docker' ] + run_tests_cmd = 'tools/run_tests/run_tests.py %s' % " ".join(child_argv[1:]) + + # TODO(jtattermusch): revisit if we need special handling for arch here + # set arch command prefix in case we are working with different arch. + arch_env = os.getenv('arch') + if arch_env: + run_test_cmd = 'arch %s %s' % (arch_env, run_test_cmd) + + env = os.environ.copy() + env['RUN_TESTS_COMMAND'] = run_tests_cmd + if args.xml_report: + env['XML_REPORT'] = args.xml_report + + subprocess.check_call(['tools/jenkins/build_docker_and_run_tests.sh'], + shell=True, + env=env) + sys.exit(0) + # grab config run_configs = set(_CONFIGS[cfg] for cfg in itertools.chain.from_iterable( From c96b9eb7503ba6ac12f50bc5ae69395510040362 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 18 Sep 2015 16:01:21 -0700 Subject: [PATCH 10/17] correct behavior for travis flag --- tools/run_tests/run_tests.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index 4774a317053..1491b1c41c2 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -462,13 +462,14 @@ argp.add_argument('-x', '--xml_report', default=None, type=str, help='Generates a JUnit-compatible XML report') args = argp.parse_args() -if args.use_docker and not args.travis: - print 'Seen --use_docker flag, will run tests under docker.' - print - print 'IMPORTANT: The changes you are testing need to be locally committed' - print 'because only the committed changes in the current branch will be' - print 'copied to the docker environment.' - time.sleep(5) +if args.use_docker: + if not args.travis: + print 'Seen --use_docker flag, will run tests under docker.' + print + print 'IMPORTANT: The changes you are testing need to be locally committed' + print 'because only the committed changes in the current branch will be' + print 'copied to the docker environment.' + time.sleep(5) child_argv = [ arg for arg in sys.argv if not arg == '--use_docker' ] run_tests_cmd = 'tools/run_tests/run_tests.py %s' % " ".join(child_argv[1:]) From 261b58ca804e9703d79e46990e86f48a094d0637 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 18 Sep 2015 17:15:48 -0700 Subject: [PATCH 11/17] using tty option is not ok on jenkins --- tools/jenkins/build_docker_and_run_tests.sh | 2 +- tools/run_tests/run_tests.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/jenkins/build_docker_and_run_tests.sh b/tools/jenkins/build_docker_and_run_tests.sh index f554b65f709..fa6bd44e180 100755 --- a/tools/jenkins/build_docker_and_run_tests.sh +++ b/tools/jenkins/build_docker_and_run_tests.sh @@ -57,7 +57,7 @@ docker run \ -e "config=$config" \ -e "arch=$arch" \ -e CCACHE_DIR=/tmp/ccache \ - -it \ + -i $TTY_FLAG \ -v "$git_root:/var/local/jenkins/grpc" \ -v /tmp/ccache:/tmp/ccache \ -w /var/local/git/grpc \ diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index 1491b1c41c2..f7e9805393d 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -484,6 +484,8 @@ if args.use_docker: env['RUN_TESTS_COMMAND'] = run_tests_cmd if args.xml_report: env['XML_REPORT'] = args.xml_report + if not args.travis: + env['TTY_FLAG'] = '-t' # enables Ctrl-C when not on Jenkins. subprocess.check_call(['tools/jenkins/build_docker_and_run_tests.sh'], shell=True, From 13e65dfd4f5a61d453dc42f0a0e8b1bf1885fbcb Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Mon, 21 Sep 2015 09:30:49 -0700 Subject: [PATCH 12/17] import yaml module lazily --- tools/run_tests/port_server.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/run_tests/port_server.py b/tools/run_tests/port_server.py index d14c829fe06..acb6deaae67 100755 --- a/tools/run_tests/port_server.py +++ b/tools/run_tests/port_server.py @@ -37,7 +37,6 @@ import os import socket import sys import time -import yaml argp = argparse.ArgumentParser(description='Server for httpcli_test') argp.add_argument('-p', '--port', default=12345, type=int) @@ -118,6 +117,9 @@ class Handler(BaseHTTPServer.BaseHTTPRequestHandler): self.end_headers() self.wfile.write(_MY_VERSION) elif self.path == '/dump': + # yaml module is not installed on Macs and Windows machines by default + # so we import it lazily (/dump action is only used for debugging) + import yaml self.send_response(200) self.send_header('Content-Type', 'text/plain') self.end_headers() From 34f398edf67da03a90076b1cc48dfb94c6b86067 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Mon, 21 Sep 2015 09:49:00 -0700 Subject: [PATCH 13/17] remove leftover field initialization --- src/core/iomgr/tcp_server_windows.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/core/iomgr/tcp_server_windows.c b/src/core/iomgr/tcp_server_windows.c index a043baafae2..63d90cd4258 100644 --- a/src/core/iomgr/tcp_server_windows.c +++ b/src/core/iomgr/tcp_server_windows.c @@ -96,7 +96,6 @@ grpc_tcp_server *grpc_tcp_server_create(void) { grpc_tcp_server *s = gpr_malloc(sizeof(grpc_tcp_server)); gpr_mu_init(&s->mu); s->active_ports = 0; - s->iomgr_callbacks_pending = 0; s->on_accept_cb = NULL; s->on_accept_cb_arg = NULL; s->ports = gpr_malloc(sizeof(server_port) * INIT_PORT_CAP); From 48e57fabca1e7b9a5a4566a5b496b54e4a9ab4e2 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Mon, 21 Sep 2015 13:43:54 -0700 Subject: [PATCH 14/17] make function definitions match declaration in header --- src/core/iomgr/tcp_client_windows.c | 4 ++-- src/core/iomgr/tcp_server_windows.c | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/core/iomgr/tcp_client_windows.c b/src/core/iomgr/tcp_client_windows.c index a42ec7cf11a..6f57de02899 100644 --- a/src/core/iomgr/tcp_client_windows.c +++ b/src/core/iomgr/tcp_client_windows.c @@ -121,7 +121,7 @@ static void on_connect(void *acp, int from_iocp) { notification request for the connection, and one timeout alert. */ void grpc_tcp_client_connect(void (*cb)(void *arg, grpc_endpoint *tcp), void *arg, grpc_pollset_set *interested_parties, - const struct sockaddr *addr, int addr_len, + const struct sockaddr *addr, size_t addr_len, gpr_timespec deadline) { SOCKET sock = INVALID_SOCKET; BOOL success; @@ -176,7 +176,7 @@ void grpc_tcp_client_connect(void (*cb)(void *arg, grpc_endpoint *tcp), socket = grpc_winsocket_create(sock, "client"); info = &socket->write_info; - success = ConnectEx(sock, addr, addr_len, NULL, 0, NULL, &info->overlapped); + success = ConnectEx(sock, addr, (int)addr_len, NULL, 0, NULL, &info->overlapped); /* It wouldn't be unusual to get a success immediately. But we'll still get an IOCP notification, so let's ignore it. */ diff --git a/src/core/iomgr/tcp_server_windows.c b/src/core/iomgr/tcp_server_windows.c index a043baafae2..1adeec44ce7 100644 --- a/src/core/iomgr/tcp_server_windows.c +++ b/src/core/iomgr/tcp_server_windows.c @@ -156,7 +156,7 @@ void grpc_tcp_server_destroy(grpc_tcp_server *s, /* Prepare (bind) a recently-created socket for listening. */ static int prepare_socket(SOCKET sock, const struct sockaddr *addr, - int addr_len) { + size_t addr_len) { struct sockaddr_storage sockname_temp; socklen_t sockname_len; @@ -169,7 +169,7 @@ static int prepare_socket(SOCKET sock, const struct sockaddr *addr, goto error; } - if (bind(sock, addr, addr_len) == SOCKET_ERROR) { + if (bind(sock, addr, (int)addr_len) == SOCKET_ERROR) { char *addr_str; char *utf8_message = gpr_format_message(WSAGetLastError()); grpc_sockaddr_to_string(&addr_str, addr, 0); @@ -355,7 +355,7 @@ static void on_accept(void *arg, int from_iocp) { } static int add_socket_to_server(grpc_tcp_server *s, SOCKET sock, - const struct sockaddr *addr, int addr_len) { + const struct sockaddr *addr, size_t addr_len) { server_port *sp; int port; int status; @@ -402,7 +402,7 @@ static int add_socket_to_server(grpc_tcp_server *s, SOCKET sock, } int grpc_tcp_server_add_port(grpc_tcp_server *s, const void *addr, - int addr_len) { + size_t addr_len) { int allocated_port = -1; unsigned i; SOCKET sock; From af970edf2e969172ccd2dd183d7c50d9bf43a44f Mon Sep 17 00:00:00 2001 From: "David G. Quintas" Date: Mon, 21 Sep 2015 18:16:46 -0700 Subject: [PATCH 15/17] Fixed typo in channel_args.h docstring --- src/core/channel/channel_args.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/channel/channel_args.h b/src/core/channel/channel_args.h index 06a6012dee8..1a6be91359e 100644 --- a/src/core/channel/channel_args.h +++ b/src/core/channel/channel_args.h @@ -71,7 +71,7 @@ grpc_channel_args *grpc_channel_args_set_compression_algorithm( * compression algorithms are enabled. It's an error to disable an algorithm set * by grpc_channel_args_set_compression_algorithm. * - * Returns an instance will the updated algorithm states. The \a a pointer is + * Returns an instance with the updated algorithm states. The \a a pointer is * modified to point to the returned instance (which may be different from the * input value of \a a). */ grpc_channel_args *grpc_channel_args_compression_algorithm_set_state( From f065c744e3396afd5ef34f602c4d5cec1341c96d Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Mon, 21 Sep 2015 18:21:55 -0700 Subject: [PATCH 16/17] Removed spurious predicate from if branch the channeld->content_type->key is already consumed by the preceding if. --- src/core/channel/http_server_filter.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/channel/http_server_filter.c b/src/core/channel/http_server_filter.c index 2f061946a1f..9898efd608e 100644 --- a/src/core/channel/http_server_filter.c +++ b/src/core/channel/http_server_filter.c @@ -111,8 +111,7 @@ static grpc_mdelem *server_filter(void *user_data, grpc_mdelem *md) { return NULL; } else if (md->key == channeld->te_trailers->key || md->key == channeld->method_post->key || - md->key == channeld->http_scheme->key || - md->key == channeld->content_type->key) { + md->key == channeld->http_scheme->key) { gpr_log(GPR_ERROR, "Invalid %s: header: '%s'", grpc_mdstr_as_c_string(md->key), grpc_mdstr_as_c_string(md->value)); /* swallow it and error everything out. */ From 14929d4e35a6c4760fe9376e4aa8cb10d9411539 Mon Sep 17 00:00:00 2001 From: Julien Boeuf Date: Mon, 21 Sep 2015 20:57:26 -0700 Subject: [PATCH 17/17] Credentials plugin fixes (after the branch was merged). --- include/grpc++/security/credentials.h | 2 +- src/cpp/client/secure_credentials.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/grpc++/security/credentials.h b/include/grpc++/security/credentials.h index 01b9710f6b3..fafcfdc9061 100644 --- a/include/grpc++/security/credentials.h +++ b/include/grpc++/security/credentials.h @@ -177,7 +177,7 @@ class MetadataCredentialsPlugin { // a different thread from the one processing the call. virtual bool IsBlocking() const { return true; } - // Gets the auth metatada produced by this plugin. */ + // Gets the auth metatada produced by this plugin. virtual Status GetMetadata( grpc::string_ref service_url, std::multimap* metadata) = 0; diff --git a/src/cpp/client/secure_credentials.cc b/src/cpp/client/secure_credentials.cc index 8333b01f29b..99b7468e865 100644 --- a/src/cpp/client/secure_credentials.cc +++ b/src/cpp/client/secure_credentials.cc @@ -183,7 +183,7 @@ void MetadataCredentialsPluginWrapper::InvokePlugin( 0, {{nullptr, nullptr, nullptr, nullptr}}}); } - cb(user_data, &md[0], md.size(), + cb(user_data, md.empty() ? nullptr : &md[0], md.size(), static_cast(status.error_code()), status.error_message().c_str()); }