diff --git a/src/core/lib/http/httpcli_security_connector.c b/src/core/lib/http/httpcli_security_connector.c index fc338342e48..9eab1360a4a 100644 --- a/src/core/lib/http/httpcli_security_connector.c +++ b/src/core/lib/http/httpcli_security_connector.c @@ -47,7 +47,7 @@ typedef struct { grpc_channel_security_connector base; - tsi_ssl_handshaker_factory *handshaker_factory; + tsi_ssl_client_handshaker_factory *handshaker_factory; char *secure_peer_name; } grpc_httpcli_ssl_channel_security_connector; @@ -56,7 +56,7 @@ static void httpcli_ssl_destroy(grpc_exec_ctx *exec_ctx, grpc_httpcli_ssl_channel_security_connector *c = (grpc_httpcli_ssl_channel_security_connector *)sc; if (c->handshaker_factory != NULL) { - tsi_ssl_handshaker_factory_destroy(c->handshaker_factory); + tsi_ssl_client_handshaker_factory_destroy(c->handshaker_factory); } if (c->secure_peer_name != NULL) gpr_free(c->secure_peer_name); gpr_free(sc); @@ -69,7 +69,7 @@ static void httpcli_ssl_add_handshakers(grpc_exec_ctx *exec_ctx, (grpc_httpcli_ssl_channel_security_connector *)sc; tsi_handshaker *handshaker = NULL; if (c->handshaker_factory != NULL) { - tsi_result result = tsi_ssl_handshaker_factory_create_handshaker( + tsi_result result = tsi_ssl_client_handshaker_factory_create_handshaker( c->handshaker_factory, c->secure_peer_name, &handshaker); if (result != TSI_OK) { gpr_log(GPR_ERROR, "Handshaker creation failed with error %s.", diff --git a/src/core/lib/security/transport/security_connector.c b/src/core/lib/security/transport/security_connector.c index 2b517061611..dbe3263f92a 100644 --- a/src/core/lib/security/transport/security_connector.c +++ b/src/core/lib/security/transport/security_connector.c @@ -448,14 +448,14 @@ grpc_server_security_connector *grpc_fake_server_security_connector_create( typedef struct { grpc_channel_security_connector base; - tsi_ssl_handshaker_factory *handshaker_factory; + tsi_ssl_client_handshaker_factory *handshaker_factory; char *target_name; char *overridden_target_name; } grpc_ssl_channel_security_connector; typedef struct { grpc_server_security_connector base; - tsi_ssl_handshaker_factory *handshaker_factory; + tsi_ssl_server_handshaker_factory *handshaker_factory; } grpc_ssl_server_security_connector; static void ssl_channel_destroy(grpc_exec_ctx *exec_ctx, @@ -464,7 +464,7 @@ static void ssl_channel_destroy(grpc_exec_ctx *exec_ctx, (grpc_ssl_channel_security_connector *)sc; grpc_call_credentials_unref(exec_ctx, c->base.request_metadata_creds); if (c->handshaker_factory != NULL) { - tsi_ssl_handshaker_factory_destroy(c->handshaker_factory); + tsi_ssl_client_handshaker_factory_destroy(c->handshaker_factory); } if (c->target_name != NULL) gpr_free(c->target_name); if (c->overridden_target_name != NULL) gpr_free(c->overridden_target_name); @@ -476,26 +476,11 @@ static void ssl_server_destroy(grpc_exec_ctx *exec_ctx, grpc_ssl_server_security_connector *c = (grpc_ssl_server_security_connector *)sc; if (c->handshaker_factory != NULL) { - tsi_ssl_handshaker_factory_destroy(c->handshaker_factory); + tsi_ssl_server_handshaker_factory_destroy(c->handshaker_factory); } gpr_free(sc); } -static grpc_security_status ssl_create_handshaker( - tsi_ssl_handshaker_factory *handshaker_factory, bool is_client, - const char *peer_name, tsi_handshaker **handshaker) { - tsi_result result = TSI_OK; - if (handshaker_factory == NULL) return GRPC_SECURITY_ERROR; - result = tsi_ssl_handshaker_factory_create_handshaker( - handshaker_factory, is_client ? peer_name : NULL, handshaker); - if (result != TSI_OK) { - gpr_log(GPR_ERROR, "Handshaker creation failed with error %s.", - tsi_result_to_string(result)); - return GRPC_SECURITY_ERROR; - } - return GRPC_SECURITY_OK; -} - static void ssl_channel_add_handshakers(grpc_exec_ctx *exec_ctx, grpc_channel_security_connector *sc, grpc_handshake_manager *handshake_mgr) { @@ -503,11 +488,17 @@ static void ssl_channel_add_handshakers(grpc_exec_ctx *exec_ctx, (grpc_ssl_channel_security_connector *)sc; // Instantiate TSI handshaker. tsi_handshaker *tsi_hs = NULL; - ssl_create_handshaker(c->handshaker_factory, true /* is_client */, - c->overridden_target_name != NULL - ? c->overridden_target_name - : c->target_name, - &tsi_hs); + tsi_result result = tsi_ssl_client_handshaker_factory_create_handshaker( + c->handshaker_factory, + c->overridden_target_name != NULL ? c->overridden_target_name + : c->target_name, + &tsi_hs); + if (result != TSI_OK) { + gpr_log(GPR_ERROR, "Handshaker creation failed with error %s.", + tsi_result_to_string(result)); + return; + } + // Create handshakers. grpc_handshake_manager_add(handshake_mgr, grpc_security_handshaker_create( exec_ctx, tsi_hs, &sc->base)); @@ -520,8 +511,14 @@ static void ssl_server_add_handshakers(grpc_exec_ctx *exec_ctx, (grpc_ssl_server_security_connector *)sc; // Instantiate TSI handshaker. tsi_handshaker *tsi_hs = NULL; - ssl_create_handshaker(c->handshaker_factory, false /* is_client */, - NULL /* peer_name */, &tsi_hs); + tsi_result result = tsi_ssl_server_handshaker_factory_create_handshaker( + c->handshaker_factory, &tsi_hs); + if (result != TSI_OK) { + gpr_log(GPR_ERROR, "Handshaker creation failed with error %s.", + tsi_result_to_string(result)); + return; + } + // Create handshakers. grpc_handshake_manager_add(handshake_mgr, grpc_security_handshaker_create( exec_ctx, tsi_hs, &sc->base)); diff --git a/src/core/tsi/ssl_transport_security.c b/src/core/tsi/ssl_transport_security.c index a0325cc1835..984f745b01d 100644 --- a/src/core/tsi/ssl_transport_security.c +++ b/src/core/tsi/ssl_transport_security.c @@ -81,23 +81,13 @@ /* --- Structure definitions. ---*/ -struct tsi_ssl_handshaker_factory { - tsi_result (*create_handshaker)(tsi_ssl_handshaker_factory *self, - const char *server_name_indication, - tsi_handshaker **handshaker); - void (*destroy)(tsi_ssl_handshaker_factory *self); -}; - -typedef struct { - tsi_ssl_handshaker_factory base; +struct tsi_ssl_client_handshaker_factory { SSL_CTX *ssl_context; unsigned char *alpn_protocol_list; size_t alpn_protocol_list_length; -} tsi_ssl_client_handshaker_factory; - -typedef struct { - tsi_ssl_handshaker_factory base; +}; +struct tsi_ssl_server_handshaker_factory { /* Several contexts to support SNI. The tsi_peer array contains the subject names of the server certificates associated with the contexts at the same index. */ @@ -106,7 +96,7 @@ typedef struct { size_t ssl_context_count; unsigned char *alpn_protocol_list; size_t alpn_protocol_list_length; -} tsi_ssl_server_handshaker_factory; +}; typedef struct { tsi_handshaker base; @@ -1053,18 +1043,6 @@ static const tsi_handshaker_vtable handshaker_vtable = { /* --- tsi_ssl_handshaker_factory common methods. --- */ -tsi_result tsi_ssl_handshaker_factory_create_handshaker( - tsi_ssl_handshaker_factory *self, const char *server_name_indication, - tsi_handshaker **handshaker) { - if (self == NULL || handshaker == NULL) return TSI_INVALID_ARGUMENT; - return self->create_handshaker(self, server_name_indication, handshaker); -} - -void tsi_ssl_handshaker_factory_destroy(tsi_ssl_handshaker_factory *self) { - if (self == NULL) return; - self->destroy(self); -} - static tsi_result create_tsi_ssl_handshaker(SSL_CTX *ctx, int is_client, const char *server_name_indication, tsi_handshaker **handshaker) { @@ -1152,24 +1130,20 @@ static int select_protocol_list(const unsigned char **out, return SSL_TLSEXT_ERR_NOACK; } -/* --- tsi_ssl__client_handshaker_factory methods implementation. --- */ +/* --- tsi_ssl_client_handshaker_factory methods implementation. --- */ -static tsi_result ssl_client_handshaker_factory_create_handshaker( - tsi_ssl_handshaker_factory *self, const char *server_name_indication, +tsi_result tsi_ssl_client_handshaker_factory_create_handshaker( + tsi_ssl_client_handshaker_factory *self, const char *server_name_indication, tsi_handshaker **handshaker) { - tsi_ssl_client_handshaker_factory *impl = - (tsi_ssl_client_handshaker_factory *)self; - return create_tsi_ssl_handshaker(impl->ssl_context, 1, server_name_indication, + return create_tsi_ssl_handshaker(self->ssl_context, 1, server_name_indication, handshaker); } -static void ssl_client_handshaker_factory_destroy( - tsi_ssl_handshaker_factory *self) { - tsi_ssl_client_handshaker_factory *impl = - (tsi_ssl_client_handshaker_factory *)self; - if (impl->ssl_context != NULL) SSL_CTX_free(impl->ssl_context); - if (impl->alpn_protocol_list != NULL) gpr_free(impl->alpn_protocol_list); - gpr_free(impl); +void tsi_ssl_client_handshaker_factory_destroy( + tsi_ssl_client_handshaker_factory *self) { + if (self->ssl_context != NULL) SSL_CTX_free(self->ssl_context); + if (self->alpn_protocol_list != NULL) gpr_free(self->alpn_protocol_list); + gpr_free(self); } static int client_handshaker_factory_npn_callback(SSL *ssl, unsigned char **out, @@ -1186,36 +1160,29 @@ static int client_handshaker_factory_npn_callback(SSL *ssl, unsigned char **out, /* --- tsi_ssl_server_handshaker_factory methods implementation. --- */ -static tsi_result ssl_server_handshaker_factory_create_handshaker( - tsi_ssl_handshaker_factory *self, const char *server_name_indication, - tsi_handshaker **handshaker) { - tsi_ssl_server_handshaker_factory *impl = - (tsi_ssl_server_handshaker_factory *)self; - if (impl->ssl_context_count == 0 || server_name_indication != NULL) { - return TSI_INVALID_ARGUMENT; - } +tsi_result tsi_ssl_server_handshaker_factory_create_handshaker( + tsi_ssl_server_handshaker_factory *self, tsi_handshaker **handshaker) { + if (self->ssl_context_count == 0) return TSI_INVALID_ARGUMENT; /* Create the handshaker with the first context. We will switch if needed because of SNI in ssl_server_handshaker_factory_servername_callback. */ - return create_tsi_ssl_handshaker(impl->ssl_contexts[0], 0, NULL, handshaker); + return create_tsi_ssl_handshaker(self->ssl_contexts[0], 0, NULL, handshaker); } -static void ssl_server_handshaker_factory_destroy( - tsi_ssl_handshaker_factory *self) { - tsi_ssl_server_handshaker_factory *impl = - (tsi_ssl_server_handshaker_factory *)self; +void tsi_ssl_server_handshaker_factory_destroy( + tsi_ssl_server_handshaker_factory *self) { size_t i; - for (i = 0; i < impl->ssl_context_count; i++) { - if (impl->ssl_contexts[i] != NULL) { - SSL_CTX_free(impl->ssl_contexts[i]); - tsi_peer_destruct(&impl->ssl_context_x509_subject_names[i]); + for (i = 0; i < self->ssl_context_count; i++) { + if (self->ssl_contexts[i] != NULL) { + SSL_CTX_free(self->ssl_contexts[i]); + tsi_peer_destruct(&self->ssl_context_x509_subject_names[i]); } } - if (impl->ssl_contexts != NULL) gpr_free(impl->ssl_contexts); - if (impl->ssl_context_x509_subject_names != NULL) { - gpr_free(impl->ssl_context_x509_subject_names); + if (self->ssl_contexts != NULL) gpr_free(self->ssl_contexts); + if (self->ssl_context_x509_subject_names != NULL) { + gpr_free(self->ssl_context_x509_subject_names); } - if (impl->alpn_protocol_list != NULL) gpr_free(impl->alpn_protocol_list); - gpr_free(impl); + if (self->alpn_protocol_list != NULL) gpr_free(self->alpn_protocol_list); + gpr_free(self); } static int does_entry_match_name(const char *entry, size_t entry_length, @@ -1317,7 +1284,7 @@ tsi_result tsi_create_ssl_client_handshaker_factory( const unsigned char *pem_root_certs, size_t pem_root_certs_size, const char *cipher_list, const unsigned char **alpn_protocols, const unsigned char *alpn_protocols_lengths, uint16_t num_alpn_protocols, - tsi_ssl_handshaker_factory **factory) { + tsi_ssl_client_handshaker_factory **factory) { SSL_CTX *ssl_context = NULL; tsi_ssl_client_handshaker_factory *impl = NULL; tsi_result result = TSI_OK; @@ -1373,16 +1340,13 @@ tsi_result tsi_create_ssl_client_handshaker_factory( } } while (0); if (result != TSI_OK) { - ssl_client_handshaker_factory_destroy(&impl->base); + tsi_ssl_client_handshaker_factory_destroy(impl); return result; } SSL_CTX_set_verify(ssl_context, SSL_VERIFY_PEER, NULL); /* TODO(jboeuf): Add revocation verification. */ - impl->base.create_handshaker = - ssl_client_handshaker_factory_create_handshaker; - impl->base.destroy = ssl_client_handshaker_factory_destroy; - *factory = &impl->base; + *factory = impl; return TSI_OK; } @@ -1394,7 +1358,7 @@ tsi_result tsi_create_ssl_server_handshaker_factory( size_t pem_client_root_certs_size, int force_client_auth, const char *cipher_list, const unsigned char **alpn_protocols, const unsigned char *alpn_protocols_lengths, uint16_t num_alpn_protocols, - tsi_ssl_handshaker_factory **factory) { + tsi_ssl_server_handshaker_factory **factory) { return tsi_create_ssl_server_handshaker_factory_ex( pem_private_keys, pem_private_keys_sizes, pem_cert_chains, pem_cert_chains_sizes, key_cert_pair_count, pem_client_root_certs, @@ -1414,7 +1378,7 @@ tsi_result tsi_create_ssl_server_handshaker_factory_ex( tsi_client_certificate_request_type client_certificate_request, const char *cipher_list, const unsigned char **alpn_protocols, const unsigned char *alpn_protocols_lengths, uint16_t num_alpn_protocols, - tsi_ssl_handshaker_factory **factory) { + tsi_ssl_server_handshaker_factory **factory) { tsi_ssl_server_handshaker_factory *impl = NULL; tsi_result result = TSI_OK; size_t i = 0; @@ -1429,15 +1393,12 @@ tsi_result tsi_create_ssl_server_handshaker_factory_ex( } impl = gpr_zalloc(sizeof(*impl)); - impl->base.create_handshaker = - ssl_server_handshaker_factory_create_handshaker; - impl->base.destroy = ssl_server_handshaker_factory_destroy; impl->ssl_contexts = gpr_zalloc(key_cert_pair_count * sizeof(SSL_CTX *)); impl->ssl_context_x509_subject_names = gpr_zalloc(key_cert_pair_count * sizeof(tsi_peer)); if (impl->ssl_contexts == NULL || impl->ssl_context_x509_subject_names == NULL) { - tsi_ssl_handshaker_factory_destroy(&impl->base); + tsi_ssl_server_handshaker_factory_destroy(impl); return TSI_OUT_OF_RESOURCES; } impl->ssl_context_count = key_cert_pair_count; @@ -1447,7 +1408,7 @@ tsi_result tsi_create_ssl_server_handshaker_factory_ex( alpn_protocols, alpn_protocols_lengths, num_alpn_protocols, &impl->alpn_protocol_list, &impl->alpn_protocol_list_length); if (result != TSI_OK) { - tsi_ssl_handshaker_factory_destroy(&impl->base); + tsi_ssl_server_handshaker_factory_destroy(impl); return result; } } @@ -1520,11 +1481,11 @@ tsi_result tsi_create_ssl_server_handshaker_factory_ex( } while (0); if (result != TSI_OK) { - tsi_ssl_handshaker_factory_destroy(&impl->base); + tsi_ssl_server_handshaker_factory_destroy(impl); return result; } } - *factory = &impl->base; + *factory = impl; return TSI_OK; } diff --git a/src/core/tsi/ssl_transport_security.h b/src/core/tsi/ssl_transport_security.h index 0a527e9021f..48dcaec1212 100644 --- a/src/core/tsi/ssl_transport_security.h +++ b/src/core/tsi/ssl_transport_security.h @@ -52,12 +52,13 @@ extern "C" { #define TSI_SSL_ALPN_SELECTED_PROTOCOL "ssl_alpn_selected_protocol" -/* --- tsi_ssl_handshaker_factory object --- +/* --- tsi_ssl_client_handshaker_factory object --- - This object creates tsi_handshaker objects implemented in terms of the - TLS 1.2 specificiation. */ + This object creates a client tsi_handshaker objects implemented in terms of + the TLS 1.2 specificiation. */ -typedef struct tsi_ssl_handshaker_factory tsi_ssl_handshaker_factory; +typedef struct tsi_ssl_client_handshaker_factory + tsi_ssl_client_handshaker_factory; /* Creates a client handshaker factory. - pem_private_key is the buffer containing the PEM encoding of the client's @@ -92,7 +93,33 @@ tsi_result tsi_create_ssl_client_handshaker_factory( const unsigned char *pem_root_certs, size_t pem_root_certs_size, const char *cipher_suites, const unsigned char **alpn_protocols, const unsigned char *alpn_protocols_lengths, uint16_t num_alpn_protocols, - tsi_ssl_handshaker_factory **factory); + tsi_ssl_client_handshaker_factory **factory); + +/* Creates a client handshaker. + - self is the factory from which the handshaker will be created. + - server_name_indication indicates the name of the server the client is + trying to connect to which will be relayed to the server using the SNI + extension. + - handshaker is the address of the handshaker pointer to be created. + + - This method returns TSI_OK on success or TSI_INVALID_PARAMETER in the case + where a parameter is invalid. */ +tsi_result tsi_ssl_client_handshaker_factory_create_handshaker( + tsi_ssl_client_handshaker_factory *self, const char *server_name_indication, + tsi_handshaker **handshaker); + +/* Destroys the handshaker factory. WARNING: it is unsafe to destroy a factory + while handshakers created with this factory are still in use. */ +void tsi_ssl_client_handshaker_factory_destroy( + tsi_ssl_client_handshaker_factory *self); + +/* --- tsi_ssl_server_handshaker_factory object --- + + This object creates a client tsi_handshaker objects implemented in terms of + the TLS 1.2 specificiation. */ + +typedef struct tsi_ssl_server_handshaker_factory + tsi_ssl_server_handshaker_factory; /* Creates a server handshaker factory. - version indicates which version of the specification to use. @@ -140,7 +167,7 @@ tsi_result tsi_create_ssl_server_handshaker_factory( size_t pem_client_root_certs_size, int force_client_auth, const char *cipher_suites, const unsigned char **alpn_protocols, const unsigned char *alpn_protocols_lengths, uint16_t num_alpn_protocols, - tsi_ssl_handshaker_factory **factory); + tsi_ssl_server_handshaker_factory **factory); /* Same as tsi_create_ssl_server_handshaker_factory method except uses tsi_client_certificate_request_type to support more ways to handle client @@ -157,25 +184,21 @@ tsi_result tsi_create_ssl_server_handshaker_factory_ex( tsi_client_certificate_request_type client_certificate_request, const char *cipher_suites, const unsigned char **alpn_protocols, const unsigned char *alpn_protocols_lengths, uint16_t num_alpn_protocols, - tsi_ssl_handshaker_factory **factory); + tsi_ssl_server_handshaker_factory **factory); -/* Creates a handshaker. +/* Creates a server handshaker. - self is the factory from which the handshaker will be created. - - server_name_indication indicates the name of the server the client is - trying to connect to which will be relayed to the server using the SNI - extension. - This parameter must be NULL for a server handshaker factory. - - handhshaker is the address of the handshaker pointer to be created. + - handshaker is the address of the handshaker pointer to be created. - This method returns TSI_OK on success or TSI_INVALID_PARAMETER in the case where a parameter is invalid. */ -tsi_result tsi_ssl_handshaker_factory_create_handshaker( - tsi_ssl_handshaker_factory *self, const char *server_name_indication, - tsi_handshaker **handshaker); +tsi_result tsi_ssl_server_handshaker_factory_create_handshaker( + tsi_ssl_server_handshaker_factory *self, tsi_handshaker **handshaker); /* Destroys the handshaker factory. WARNING: it is unsafe to destroy a factory while handshakers created with this factory are still in use. */ -void tsi_ssl_handshaker_factory_destroy(tsi_ssl_handshaker_factory *self); +void tsi_ssl_server_handshaker_factory_destroy( + tsi_ssl_server_handshaker_factory *self); /* Util that checks that an ssl peer matches a specific name. Still TODO(jboeuf): diff --git a/tools/dockerfile/grpc_clang_format/clang_format_all_the_things.sh b/tools/dockerfile/grpc_clang_format/clang_format_all_the_things.sh index c6e4aabfe63..02e811f664b 100755 --- a/tools/dockerfile/grpc_clang_format/clang_format_all_the_things.sh +++ b/tools/dockerfile/grpc_clang_format/clang_format_all_the_things.sh @@ -31,7 +31,7 @@ set -e # directories to run against -DIRS="src/core/lib src/core/ext src/cpp test/core test/cpp include src/compiler" +DIRS="src/core/lib src/core/tsi src/core/ext src/cpp test/core test/cpp include src/compiler" # file matching patterns to check GLOB="*.h *.c *.cc"