From db11b94f255ac15596f4feee77485a443d74859e Mon Sep 17 00:00:00 2001 From: Akshay Kumar Date: Sun, 8 Dec 2019 12:08:52 -0800 Subject: [PATCH 1/7] FullChainExperimental-01-200103 --- grpc.def | 1 + include/grpc/grpc_security.h | 14 +++++ include/grpc/grpc_security_constants.h | 12 +++++ .../grpcpp/security/tls_credentials_options.h | 9 ++++ .../tls/grpc_tls_credentials_options.cc | 20 +++++++ .../tls/grpc_tls_credentials_options.h | 8 +++ .../security/security_connector/ssl_utils.cc | 24 +++++++++ .../security/security_connector/ssl_utils.h | 6 +++ .../tls/tls_security_connector.cc | 37 +++++++++---- .../tls/tls_security_connector.h | 2 +- src/core/tsi/ssl_transport_security.cc | 44 +++++++++++++-- src/core/tsi/ssl_transport_security.h | 15 +++++- src/core/tsi/transport_security_interface.h | 11 ++++ src/cpp/common/tls_credentials_options.cc | 14 +++++ test/core/security/security_connector_test.cc | 54 ++++++++++++++++--- .../security/tls_security_connector_test.cc | 20 +++---- test/core/tsi/ssl_transport_security_test.cc | 32 +++++++++++ test/cpp/client/credentials_test.cc | 8 +-- 18 files changed, 295 insertions(+), 36 deletions(-) diff --git a/grpc.def b/grpc.def index ff68b965eb2..2f753b29939 100644 --- a/grpc.def +++ b/grpc.def @@ -136,6 +136,7 @@ EXPORTS grpc_local_server_credentials_create grpc_tls_credentials_options_create grpc_tls_credentials_options_set_cert_request_type + grpc_tls_credentials_options_set_server_verification_option grpc_tls_credentials_options_set_key_materials_config grpc_tls_credentials_options_set_credential_reload_config grpc_tls_credentials_options_set_server_authorization_check_config diff --git a/include/grpc/grpc_security.h b/include/grpc/grpc_security.h index 164c6dacdb0..31496e96e74 100644 --- a/include/grpc/grpc_security.h +++ b/include/grpc/grpc_security.h @@ -731,6 +731,19 @@ GRPCAPI int grpc_tls_credentials_options_set_cert_request_type( grpc_tls_credentials_options* options, grpc_ssl_client_certificate_request_type type); +/** Set grpc_tls_server_verification_option field in credentials options + with the provided server_verification_option. options should not be NULL. + This should be called only on the client side. + If grpc_tls_server_verification_option is not + GRPC_TLS_SERVER_VERIFICATION, use of a customer server + authorization check (grpc_tls_server_authorization_check_config) + will be mandatory. + It returns 1 on success and 0 on failure. It is used for + experimental purpose for now and subject to change. */ +GRPCAPI int grpc_tls_credentials_options_set_server_verification_option( + grpc_tls_credentials_options* options, + grpc_tls_server_verification_option server_verification_option); + /** Set grpc_tls_key_materials_config field in credentials options with the provided config struct whose ownership is transferred. Both parameters should not be NULL. @@ -902,6 +915,7 @@ struct grpc_tls_server_authorization_check_arg { int success; const char* target_name; const char* peer_cert; + const char* peer_cert_full_chain; grpc_status_code status; const char* error_details; grpc_tls_server_authorization_check_config* config; diff --git a/include/grpc/grpc_security_constants.h b/include/grpc/grpc_security_constants.h index 13d6e9ff8e0..63900e41cb3 100644 --- a/include/grpc/grpc_security_constants.h +++ b/include/grpc/grpc_security_constants.h @@ -29,6 +29,7 @@ extern "C" { #define GRPC_X509_CN_PROPERTY_NAME "x509_common_name" #define GRPC_X509_SAN_PROPERTY_NAME "x509_subject_alternative_name" #define GRPC_X509_PEM_CERT_PROPERTY_NAME "x509_pem_cert" +#define GRPC_X509_PEM_CERT_CHAIN_PROPERTY_NAME "x509_pem_cert_chain" #define GRPC_SSL_SESSION_REUSED_PROPERTY "ssl_session_reused" /** Environment variable that points to the default SSL roots file. This file @@ -114,6 +115,17 @@ typedef enum { GRPC_SECURITY_MAX = GRPC_PRIVACY_AND_INTEGRITY, } grpc_security_level; +typedef enum { + /** Default option: performs server certificate verification and hostname + verification */ + GRPC_TLS_SERVER_VERIFICATION, + /** Performs server certificate verification, but skips hostname verification + */ + GRPC_TLS_SKIP_HOSTNAME_VERIFICATION, + /** Skips both server certificate and hostname verification */ + GRPC_TLS_SKIP_ALL_SERVER_VERIFICATION +} grpc_tls_server_verification_option; + /** * Type of local connections for which local channel/server credentials will be * applied. It supports UDS and local TCP connections. diff --git a/include/grpcpp/security/tls_credentials_options.h b/include/grpcpp/security/tls_credentials_options.h index 3e9b037ec25..b5bb7c78b7f 100644 --- a/include/grpcpp/security/tls_credentials_options.h +++ b/include/grpcpp/security/tls_credentials_options.h @@ -193,6 +193,7 @@ class TlsServerAuthorizationCheckArg { int success() const; grpc::string target_name() const; grpc::string peer_cert() const; + grpc::string peer_cert_full_chain() const; grpc_status_code status() const; grpc::string error_details() const; @@ -206,6 +207,7 @@ class TlsServerAuthorizationCheckArg { void set_success(int success); void set_target_name(const grpc::string& target_name); void set_peer_cert(const grpc::string& peer_cert); + void set_peer_cert_full_chain(const grpc::string& peer_cert_full_chain); void set_status(grpc_status_code status); void set_error_details(const grpc::string& error_details); @@ -287,6 +289,7 @@ class TlsCredentialsOptions { public: TlsCredentialsOptions( grpc_ssl_client_certificate_request_type cert_request_type, + grpc_tls_server_verification_option server_verification_option, std::shared_ptr key_materials_config, std::shared_ptr credential_reload_config, std::shared_ptr @@ -297,6 +300,9 @@ class TlsCredentialsOptions { grpc_ssl_client_certificate_request_type cert_request_type() const { return cert_request_type_; } + grpc_tls_server_verification_option server_verification_option() const { + return server_verification_option_; + } std::shared_ptr key_materials_config() const { return key_materials_config_; } @@ -317,6 +323,9 @@ class TlsCredentialsOptions { * goes unused when creating channel credentials, and the user can set it to * GRPC_SSL_DONT_REQUEST_CLIENT_CERTIFICATE. **/ grpc_ssl_client_certificate_request_type cert_request_type_; + /** The server_verification_option_ flag is only relevant when the + * TlsCredentialsOptions are used to instantiate client credentials; **/ + grpc_tls_server_verification_option server_verification_option_; std::shared_ptr key_materials_config_; std::shared_ptr credential_reload_config_; std::shared_ptr diff --git a/src/core/lib/security/credentials/tls/grpc_tls_credentials_options.cc b/src/core/lib/security/credentials/tls/grpc_tls_credentials_options.cc index 199a15b6354..937dfacfa79 100644 --- a/src/core/lib/security/credentials/tls/grpc_tls_credentials_options.cc +++ b/src/core/lib/security/credentials/tls/grpc_tls_credentials_options.cc @@ -92,6 +92,26 @@ int grpc_tls_credentials_options_set_cert_request_type( return 1; } +int grpc_tls_credentials_options_set_server_verification_option( + grpc_tls_credentials_options* options, + grpc_tls_server_verification_option server_verification_option) { + if (options == nullptr) { + gpr_log(GPR_ERROR, + "Invalid nullptr arguments to " + "grpc_tls_credentials_options_set_server_verification_option()"); + return 0; + } + if (server_verification_option != GRPC_TLS_SERVER_VERIFICATION && + options->server_authorization_check_config() == nullptr) { + gpr_log(GPR_ERROR, + "server_authorization_check_config needs to be specified when" + "server_verification_option is not GRPC_TLS_SERVER_VERIFICATION"); + return 0; + } + options->set_server_verification_option(server_verification_option); + return 1; +} + int grpc_tls_credentials_options_set_key_materials_config( grpc_tls_credentials_options* options, grpc_tls_key_materials_config* config) { diff --git a/src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h b/src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h index a7f99822121..ca82a294928 100644 --- a/src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h +++ b/src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h @@ -234,6 +234,9 @@ struct grpc_tls_credentials_options grpc_ssl_client_certificate_request_type cert_request_type() const { return cert_request_type_; } + grpc_tls_server_verification_option server_verification_option() const { + return server_verification_option_; + } grpc_tls_key_materials_config* key_materials_config() const { return key_materials_config_.get(); } @@ -250,6 +253,10 @@ struct grpc_tls_credentials_options const grpc_ssl_client_certificate_request_type type) { cert_request_type_ = type; } + void set_server_verification_option( + const grpc_tls_server_verification_option server_verification_option) { + server_verification_option_ = server_verification_option; + } void set_key_materials_config( grpc_core::RefCountedPtr config) { key_materials_config_ = std::move(config); @@ -266,6 +273,7 @@ struct grpc_tls_credentials_options private: grpc_ssl_client_certificate_request_type cert_request_type_; + grpc_tls_server_verification_option server_verification_option_; grpc_core::RefCountedPtr key_materials_config_; grpc_core::RefCountedPtr credential_reload_config_; diff --git a/src/core/lib/security/security_connector/ssl_utils.cc b/src/core/lib/security/security_connector/ssl_utils.cc index f4bb70bef8d..e34c66b35d6 100644 --- a/src/core/lib/security/security_connector/ssl_utils.cc +++ b/src/core/lib/security/security_connector/ssl_utils.cc @@ -108,6 +108,20 @@ grpc_get_tsi_client_certificate_request_type( } } +tsi_server_verification_option grpc_get_tsi_server_verification_option( + grpc_tls_server_verification_option server_verification_option) { + switch (server_verification_option) { + case GRPC_TLS_SERVER_VERIFICATION: + return TSI_SERVER_VERIFICATION; + case GRPC_TLS_SKIP_HOSTNAME_VERIFICATION: + return TSI_SKIP_HOSTNAME_VERIFICATION; + case GRPC_TLS_SKIP_ALL_SERVER_VERIFICATION: + return TSI_SKIP_ALL_SERVER_VERIFICATION; + default: + return TSI_SERVER_VERIFICATION; + } +} + grpc_error* grpc_ssl_check_alpn(const tsi_peer* peer) { #if TSI_OPENSSL_ALPN_SUPPORT /* Check the ALPN if ALPN is supported. */ @@ -225,6 +239,10 @@ grpc_core::RefCountedPtr grpc_ssl_peer_to_auth_context( grpc_auth_context_add_property(ctx.get(), GRPC_X509_PEM_CERT_PROPERTY_NAME, prop->value.data, prop->value.length); + } else if (strcmp(prop->name, TSI_X509_PEM_CERT_CHAIN_PROPERTY) == 0) { + grpc_auth_context_add_property(ctx.get(), + GRPC_X509_PEM_CERT_CHAIN_PROPERTY_NAME, + prop->value.data, prop->value.length); } else if (strcmp(prop->name, TSI_SSL_SESSION_REUSED_PEER_PROPERTY) == 0) { grpc_auth_context_add_property(ctx.get(), GRPC_SSL_SESSION_REUSED_PROPERTY, @@ -272,6 +290,10 @@ tsi_peer grpc_shallow_peer_from_ssl_auth_context( } else if (strcmp(prop->name, GRPC_X509_PEM_CERT_PROPERTY_NAME) == 0) { add_shallow_auth_property_to_peer(&peer, prop, TSI_X509_PEM_CERT_PROPERTY); + } else if (strcmp(prop->name, GRPC_X509_PEM_CERT_CHAIN_PROPERTY_NAME) == + 0) { + add_shallow_auth_property_to_peer(&peer, prop, + TSI_X509_PEM_CERT_CHAIN_PROPERTY); } } } @@ -284,6 +306,7 @@ void grpc_shallow_peer_destruct(tsi_peer* peer) { grpc_security_status grpc_ssl_tsi_client_handshaker_factory_init( tsi_ssl_pem_key_cert_pair* pem_key_cert_pair, const char* pem_root_certs, + tsi_server_verification_option server_verification_option, tsi_ssl_session_cache* ssl_session_cache, tsi_ssl_client_handshaker_factory** handshaker_factory) { const char* root_certs; @@ -314,6 +337,7 @@ grpc_security_status grpc_ssl_tsi_client_handshaker_factory_init( } options.cipher_suites = grpc_get_ssl_cipher_suites(); options.session_cache = ssl_session_cache; + options.server_verification_option = server_verification_option; const tsi_result result = tsi_create_ssl_client_handshaker_factory_with_options(&options, handshaker_factory); diff --git a/src/core/lib/security/security_connector/ssl_utils.h b/src/core/lib/security/security_connector/ssl_utils.h index 6ee2c3c7248..e6370db9768 100644 --- a/src/core/lib/security/security_connector/ssl_utils.h +++ b/src/core/lib/security/security_connector/ssl_utils.h @@ -68,12 +68,18 @@ tsi_client_certificate_request_type grpc_get_tsi_client_certificate_request_type( grpc_ssl_client_certificate_request_type grpc_request_type); +/* Map from grpc_tls_server_verification_option to + * tsi_server_verification_option. */ +tsi_server_verification_option grpc_get_tsi_server_verification_option( + grpc_tls_server_verification_option server_verification_option); + /* Return an array of strings containing alpn protocols. */ const char** grpc_fill_alpn_protocol_strings(size_t* num_alpn_protocols); /* Initialize TSI SSL server/client handshaker factory. */ grpc_security_status grpc_ssl_tsi_client_handshaker_factory_init( tsi_ssl_pem_key_cert_pair* key_cert_pair, const char* pem_root_certs, + tsi_server_verification_option server_verification_option, tsi_ssl_session_cache* ssl_session_cache, tsi_ssl_client_handshaker_factory** handshaker_factory); diff --git a/src/core/lib/security/security_connector/tls/tls_security_connector.cc b/src/core/lib/security/security_connector/tls/tls_security_connector.cc index 62948eff57a..205f1dda7ff 100644 --- a/src/core/lib/security/security_connector/tls/tls_security_connector.cc +++ b/src/core/lib/security/security_connector/tls/tls_security_connector.cc @@ -66,12 +66,13 @@ tsi_ssl_pem_key_cert_pair* ConvertToTsiPemKeyCertPair( grpc_status_code TlsFetchKeyMaterials( const grpc_core::RefCountedPtr& key_materials_config, - const grpc_tls_credentials_options& options, + const grpc_tls_credentials_options& options, bool server_config, grpc_ssl_certificate_config_reload_status* reload_status) { GPR_ASSERT(key_materials_config != nullptr); bool is_key_materials_empty = key_materials_config->pem_key_cert_pair_list().empty(); - if (options.credential_reload_config() == nullptr && is_key_materials_empty) { + if (options.credential_reload_config() == nullptr && is_key_materials_empty && + server_config) { gpr_log(GPR_ERROR, "Either credential reload config or key materials should be " "provisioned."); @@ -190,9 +191,8 @@ void TlsChannelSecurityConnector::check_peer( error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( "Cannot check peer: missing pem cert property."); } else { - char* peer_pem = static_cast(gpr_malloc(p->value.length + 1)); + char* peer_pem = static_cast(gpr_zalloc(p->value.length + 1)); memcpy(peer_pem, p->value.data, p->value.length); - peer_pem[p->value.length] = '\0'; GPR_ASSERT(check_arg_ != nullptr); check_arg_->peer_cert = check_arg_->peer_cert == nullptr ? gpr_strdup(peer_pem) @@ -202,6 +202,18 @@ void TlsChannelSecurityConnector::check_peer( : check_arg_->target_name; on_peer_checked_ = on_peer_checked; gpr_free(peer_pem); + const tsi_peer_property* chain = tsi_peer_get_property_by_name( + &peer, TSI_X509_PEM_CERT_CHAIN_PROPERTY); + if (chain != nullptr) { + char* peer_pem_chain = + static_cast(gpr_zalloc(chain->value.length + 1)); + memcpy(peer_pem_chain, chain->value.data, chain->value.length); + check_arg_->peer_cert_full_chain = + check_arg_->peer_cert_full_chain == nullptr + ? gpr_strdup(peer_pem_chain) + : check_arg_->peer_cert_full_chain; + gpr_free(peer_pem_chain); + } int callback_status = config->Schedule(check_arg_); /* Server authorization check is handled asynchronously. */ if (callback_status) { @@ -272,16 +284,21 @@ TlsChannelSecurityConnector::CreateTlsChannelSecurityConnector( grpc_security_status TlsChannelSecurityConnector::ReplaceHandshakerFactory( tsi_ssl_session_cache* ssl_session_cache) { + const TlsCredentials* creds = + static_cast(channel_creds()); + tsi_server_verification_option server_verification_option = + grpc_get_tsi_server_verification_option( + creds->options().server_verification_option()); /* Free the client handshaker factory if exists. */ if (client_handshaker_factory_) { tsi_ssl_client_handshaker_factory_unref(client_handshaker_factory_); } - GPR_ASSERT(!key_materials_config_->pem_key_cert_pair_list().empty()); tsi_ssl_pem_key_cert_pair* pem_key_cert_pair = ConvertToTsiPemKeyCertPair( key_materials_config_->pem_key_cert_pair_list()); grpc_security_status status = grpc_ssl_tsi_client_handshaker_factory_init( pem_key_cert_pair, key_materials_config_->pem_root_certs(), - ssl_session_cache, &client_handshaker_factory_); + server_verification_option, ssl_session_cache, + &client_handshaker_factory_); /* Free memory. */ grpc_tsi_ssl_pem_key_cert_pairs_destroy(pem_key_cert_pair, 1); return status; @@ -305,7 +322,7 @@ grpc_security_status TlsChannelSecurityConnector::InitializeHandshakerFactory( } grpc_ssl_certificate_config_reload_status reload_status = GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_UNCHANGED; - if (TlsFetchKeyMaterials(key_materials_config_, creds->options(), + if (TlsFetchKeyMaterials(key_materials_config_, creds->options(), false, &reload_status) != GRPC_STATUS_OK) { /* Raise an error if key materials are not populated. */ return GRPC_SECURITY_ERROR; @@ -319,7 +336,7 @@ grpc_security_status TlsChannelSecurityConnector::RefreshHandshakerFactory() { static_cast(channel_creds()); grpc_ssl_certificate_config_reload_status reload_status = GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_UNCHANGED; - if (TlsFetchKeyMaterials(key_materials_config_, creds->options(), + if (TlsFetchKeyMaterials(key_materials_config_, creds->options(), false, &reload_status) != GRPC_STATUS_OK) { return GRPC_SECURITY_ERROR; } @@ -507,7 +524,7 @@ grpc_security_status TlsServerSecurityConnector::InitializeHandshakerFactory() { } grpc_ssl_certificate_config_reload_status reload_status = GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_UNCHANGED; - if (TlsFetchKeyMaterials(key_materials_config_, creds->options(), + if (TlsFetchKeyMaterials(key_materials_config_, creds->options(), true, &reload_status) != GRPC_STATUS_OK) { /* Raise an error if key materials are not populated. */ return GRPC_SECURITY_ERROR; @@ -521,7 +538,7 @@ grpc_security_status TlsServerSecurityConnector::RefreshHandshakerFactory() { static_cast(server_creds()); grpc_ssl_certificate_config_reload_status reload_status = GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_UNCHANGED; - if (TlsFetchKeyMaterials(key_materials_config_, creds->options(), + if (TlsFetchKeyMaterials(key_materials_config_, creds->options(), true, &reload_status) != GRPC_STATUS_OK) { return GRPC_SECURITY_ERROR; } diff --git a/src/core/lib/security/security_connector/tls/tls_security_connector.h b/src/core/lib/security/security_connector/tls/tls_security_connector.h index c669c6b9b75..825ffe7b1f7 100644 --- a/src/core/lib/security/security_connector/tls/tls_security_connector.h +++ b/src/core/lib/security/security_connector/tls/tls_security_connector.h @@ -148,7 +148,7 @@ class TlsServerSecurityConnector final : public grpc_server_security_connector { grpc_status_code TlsFetchKeyMaterials( const grpc_core::RefCountedPtr& key_materials_config, - const grpc_tls_credentials_options& options, + const grpc_tls_credentials_options& options, bool server_config, grpc_ssl_certificate_config_reload_status* status); } // namespace grpc_core diff --git a/src/core/tsi/ssl_transport_security.cc b/src/core/tsi/ssl_transport_security.cc index 8b6d9f39dd4..2063ef0dfc9 100644 --- a/src/core/tsi/ssl_transport_security.cc +++ b/src/core/tsi/ssl_transport_security.cc @@ -35,6 +35,7 @@ #include #endif +#include #include #include #include @@ -1024,6 +1025,29 @@ static void tsi_ssl_handshaker_factory_init( gpr_ref_init(&factory->refcount, 1); } +/* Gets the X509 cert chain in PEM format as a tsi_peer_property. */ +tsi_result tsi_ssl_get_cert_chain_contents(STACK_OF(X509) * peer_chain, + tsi_peer_property* property) { + BIO* bio = BIO_new(BIO_s_mem()); + for (int i = 0; i < sk_X509_num(peer_chain); i++) { + if (!PEM_write_bio_X509(bio, sk_X509_value(peer_chain, i))) { + BIO_free(bio); + return TSI_INTERNAL_ERROR; + } + } + char* contents; + long len = BIO_get_mem_data(bio, &contents); + if (len <= 0) { + BIO_free(bio); + return TSI_INTERNAL_ERROR; + } + tsi_result result = tsi_construct_string_peer_property( + TSI_X509_PEM_CERT_CHAIN_PROPERTY, (const char*)contents, + static_cast(len), property); + BIO_free(bio); + return result; +} + /* --- tsi_handshaker_result methods implementation. ---*/ static tsi_result ssl_handshaker_result_extract_peer( const tsi_handshaker_result* self, tsi_peer* peer) { @@ -1032,7 +1056,6 @@ static tsi_result ssl_handshaker_result_extract_peer( unsigned int alpn_selected_len; const tsi_ssl_handshaker_result* impl = reinterpret_cast(self); - // TODO(yihuazhang): Return a full certificate chain as a peer property. X509* peer_cert = SSL_get_peer_certificate(impl->ssl); if (peer_cert != nullptr) { result = peer_from_x509(peer_cert, 1, peer); @@ -1047,10 +1070,14 @@ static tsi_result ssl_handshaker_result_extract_peer( SSL_get0_next_proto_negotiated(impl->ssl, &alpn_selected, &alpn_selected_len); } - + // When called on the client side, the stack also contains the + // peer's certificate; When called on the server side, + // the peer's certificate is not present in the stack + STACK_OF(X509)* peer_chain = SSL_get_peer_cert_chain(impl->ssl); // 1 is for session reused property. size_t new_property_count = peer->property_count + 1; if (alpn_selected != nullptr) new_property_count++; + if (peer_chain != nullptr) new_property_count++; tsi_peer_property* new_properties = static_cast( gpr_zalloc(sizeof(*new_properties) * new_property_count)); for (size_t i = 0; i < peer->property_count; i++) { @@ -1058,7 +1085,12 @@ static tsi_result ssl_handshaker_result_extract_peer( } if (peer->properties != nullptr) gpr_free(peer->properties); peer->properties = new_properties; - + // Add peer chain if available + if (peer_chain != nullptr) { + result = tsi_ssl_get_cert_chain_contents( + peer_chain, &peer->properties[peer->property_count]); + if (result == TSI_OK) peer->property_count++; + } if (alpn_selected != nullptr) { result = tsi_construct_string_peer_property( TSI_SSL_ALPN_SELECTED_PROTOCOL, @@ -1733,7 +1765,11 @@ tsi_result tsi_create_ssl_client_handshaker_factory_with_options( tsi_ssl_handshaker_factory_unref(&impl->base); return result; } - SSL_CTX_set_verify(ssl_context, SSL_VERIFY_PEER, nullptr); + if (options->server_verification_option == TSI_SKIP_ALL_SERVER_VERIFICATION) { + SSL_CTX_set_verify(ssl_context, SSL_VERIFY_PEER, NullVerifyCallback); + } else { + SSL_CTX_set_verify(ssl_context, SSL_VERIFY_PEER, nullptr); + } /* TODO(jboeuf): Add revocation verification. */ *factory = impl; diff --git a/src/core/tsi/ssl_transport_security.h b/src/core/tsi/ssl_transport_security.h index dba711088a3..c5daba01779 100644 --- a/src/core/tsi/ssl_transport_security.h +++ b/src/core/tsi/ssl_transport_security.h @@ -20,6 +20,9 @@ #define GRPC_CORE_TSI_SSL_TRANSPORT_SECURITY_H #include +extern "C" { +#include +} #include "src/core/lib/gprpp/string_view.h" #include "src/core/tsi/transport_security_interface.h" @@ -35,6 +38,8 @@ #define TSI_X509_PEM_CERT_PROPERTY "x509_pem_cert" +#define TSI_X509_PEM_CERT_CHAIN_PROPERTY "x509_pem_cert_chain" + #define TSI_SSL_ALPN_SELECTED_PROTOCOL "ssl_alpn_selected_protocol" /* --- tsi_ssl_root_certs_store object --- @@ -142,6 +147,9 @@ struct tsi_ssl_client_handshaker_options { /* ssl_session_cache is a cache for reusable client-side sessions. */ tsi_ssl_session_cache* session_cache; + /* Server verification option */ + tsi_server_verification_option server_verification_option; + tsi_ssl_client_handshaker_options() : pem_key_cert_pair(nullptr), pem_root_certs(nullptr), @@ -149,7 +157,8 @@ struct tsi_ssl_client_handshaker_options { cipher_suites(nullptr), alpn_protocols(nullptr), num_alpn_protocols(0), - session_cache(nullptr) {} + session_cache(nullptr), + server_verification_option(TSI_SERVER_VERIFICATION) {} }; /* Creates a client handshaker factory. @@ -336,4 +345,8 @@ const tsi_ssl_handshaker_factory_vtable* tsi_ssl_handshaker_factory_swap_vtable( tsi_result tsi_ssl_extract_x509_subject_names_from_pem_cert( const char* pem_cert, tsi_peer* peer); +/* Exposed for testing only. */ +tsi_result tsi_ssl_get_cert_chain_contents(STACK_OF(X509) * peer_chain, + tsi_peer_property* property); + #endif /* GRPC_CORE_TSI_SSL_TRANSPORT_SECURITY_H */ diff --git a/src/core/tsi/transport_security_interface.h b/src/core/tsi/transport_security_interface.h index 7a0cdc3453a..6d597e4bdf7 100644 --- a/src/core/tsi/transport_security_interface.h +++ b/src/core/tsi/transport_security_interface.h @@ -55,6 +55,17 @@ typedef enum { TSI_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_AND_VERIFY, } tsi_client_certificate_request_type; +typedef enum { + /** Default option: performs server certificate verification and hostname + verification */ + TSI_SERVER_VERIFICATION, + /** Performs server certificate verification, but skips hostname verification + */ + TSI_SKIP_HOSTNAME_VERIFICATION, + /** Skips both server certificate and hostname verification */ + TSI_SKIP_ALL_SERVER_VERIFICATION, +} tsi_server_verification_option; + const char* tsi_result_to_string(tsi_result result); /* --- tsi tracing --- */ diff --git a/src/cpp/common/tls_credentials_options.cc b/src/cpp/common/tls_credentials_options.cc index 60d6a23fee1..d5692c6effd 100644 --- a/src/cpp/common/tls_credentials_options.cc +++ b/src/cpp/common/tls_credentials_options.cc @@ -186,6 +186,11 @@ grpc::string TlsServerAuthorizationCheckArg::peer_cert() const { return cpp_peer_cert; } +grpc::string TlsServerAuthorizationCheckArg::peer_cert_full_chain() const { + grpc::string cpp_peer_cert_full_chain(c_arg_->peer_cert_full_chain); + return cpp_peer_cert_full_chain; +} + grpc_status_code TlsServerAuthorizationCheckArg::status() const { return c_arg_->status; } @@ -213,6 +218,11 @@ void TlsServerAuthorizationCheckArg::set_peer_cert( c_arg_->peer_cert = gpr_strdup(peer_cert.c_str()); } +void TlsServerAuthorizationCheckArg::set_peer_cert_full_chain( + const grpc::string& peer_cert_full_chain) { + c_arg_->peer_cert_full_chain = gpr_strdup(peer_cert_full_chain.c_str()); +} + void TlsServerAuthorizationCheckArg::set_status(grpc_status_code status) { c_arg_->status = status; } @@ -247,11 +257,13 @@ TlsServerAuthorizationCheckConfig::~TlsServerAuthorizationCheckConfig() {} /** gRPC TLS credential options API implementation **/ TlsCredentialsOptions::TlsCredentialsOptions( grpc_ssl_client_certificate_request_type cert_request_type, + grpc_tls_server_verification_option server_verification_option, std::shared_ptr key_materials_config, std::shared_ptr credential_reload_config, std::shared_ptr server_authorization_check_config) : cert_request_type_(cert_request_type), + server_verification_option_(server_verification_option), key_materials_config_(std::move(key_materials_config)), credential_reload_config_(std::move(credential_reload_config)), server_authorization_check_config_( @@ -272,6 +284,8 @@ TlsCredentialsOptions::TlsCredentialsOptions( grpc_tls_credentials_options_set_server_authorization_check_config( c_credentials_options_, server_authorization_check_config_->c_config()); } + grpc_tls_credentials_options_set_server_verification_option( + c_credentials_options_, server_verification_option); } TlsCredentialsOptions::~TlsCredentialsOptions() {} diff --git a/test/core/security/security_connector_test.cc b/test/core/security/security_connector_test.cc index a1404a05688..1a0e9ee9c4b 100644 --- a/test/core/security/security_connector_test.cc +++ b/test/core/security/security_connector_test.cc @@ -176,12 +176,34 @@ static int check_x509_pem_cert(const grpc_auth_context* ctx, return 1; } +static int check_x509_pem_cert_chain(const grpc_auth_context* ctx, + const char* expected_pem_cert_chain) { + grpc_auth_property_iterator it = grpc_auth_context_find_properties_by_name( + ctx, GRPC_X509_PEM_CERT_CHAIN_PROPERTY_NAME); + const grpc_auth_property* prop = grpc_auth_property_iterator_next(&it); + if (prop == nullptr) { + gpr_log(GPR_ERROR, "Pem certificate chain property not found."); + return 0; + } + if (strncmp(prop->value, expected_pem_cert_chain, prop->value_length) != 0) { + gpr_log(GPR_ERROR, "Expected pem cert chain %s and got %s", + expected_pem_cert_chain, prop->value); + return 0; + } + if (grpc_auth_property_iterator_next(&it) != nullptr) { + gpr_log(GPR_ERROR, "Expected only one property for pem cert chain."); + return 0; + } + return 1; +} + static void test_cn_only_ssl_peer_to_auth_context(void) { tsi_peer peer; tsi_peer rpeer; const char* expected_cn = "cn1"; const char* expected_pem_cert = "pem_cert1"; - GPR_ASSERT(tsi_construct_peer(3, &peer) == TSI_OK); + const char* expected_pem_cert_chain = "pem_cert1_chain"; + GPR_ASSERT(tsi_construct_peer(4, &peer) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( TSI_CERTIFICATE_TYPE_PEER_PROPERTY, TSI_X509_CERTIFICATE_TYPE, &peer.properties[0]) == TSI_OK); @@ -191,6 +213,9 @@ static void test_cn_only_ssl_peer_to_auth_context(void) { GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( TSI_X509_PEM_CERT_PROPERTY, expected_pem_cert, &peer.properties[2]) == TSI_OK); + GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( + TSI_X509_PEM_CERT_CHAIN_PROPERTY, expected_pem_cert_chain, + &peer.properties[3]) == TSI_OK); grpc_core::RefCountedPtr ctx = grpc_ssl_peer_to_auth_context(&peer, GRPC_SSL_TRANSPORT_SECURITY_TYPE); GPR_ASSERT(ctx != nullptr); @@ -200,6 +225,7 @@ static void test_cn_only_ssl_peer_to_auth_context(void) { GPR_ASSERT(check_transport_security_type(ctx.get())); GPR_ASSERT(check_x509_cn(ctx.get(), expected_cn)); GPR_ASSERT(check_x509_pem_cert(ctx.get(), expected_pem_cert)); + GPR_ASSERT(check_x509_pem_cert_chain(ctx.get(), expected_pem_cert_chain)); rpeer = grpc_shallow_peer_from_ssl_auth_context(ctx.get()); GPR_ASSERT(check_ssl_peer_equivalence(&peer, &rpeer)); @@ -215,7 +241,8 @@ static void test_cn_and_one_san_ssl_peer_to_auth_context(void) { const char* expected_cn = "cn1"; const char* expected_san = "san1"; const char* expected_pem_cert = "pem_cert1"; - GPR_ASSERT(tsi_construct_peer(4, &peer) == TSI_OK); + const char* expected_pem_cert_chain = "pem_cert1_chain"; + GPR_ASSERT(tsi_construct_peer(5, &peer) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( TSI_CERTIFICATE_TYPE_PEER_PROPERTY, TSI_X509_CERTIFICATE_TYPE, &peer.properties[0]) == TSI_OK); @@ -228,6 +255,9 @@ static void test_cn_and_one_san_ssl_peer_to_auth_context(void) { GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( TSI_X509_PEM_CERT_PROPERTY, expected_pem_cert, &peer.properties[3]) == TSI_OK); + GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( + TSI_X509_PEM_CERT_CHAIN_PROPERTY, expected_pem_cert_chain, + &peer.properties[4]) == TSI_OK); grpc_core::RefCountedPtr ctx = grpc_ssl_peer_to_auth_context(&peer, GRPC_SSL_TRANSPORT_SECURITY_TYPE); @@ -238,6 +268,7 @@ static void test_cn_and_one_san_ssl_peer_to_auth_context(void) { GPR_ASSERT(check_transport_security_type(ctx.get())); GPR_ASSERT(check_x509_cn(ctx.get(), expected_cn)); GPR_ASSERT(check_x509_pem_cert(ctx.get(), expected_pem_cert)); + GPR_ASSERT(check_x509_pem_cert_chain(ctx.get(), expected_pem_cert_chain)); rpeer = grpc_shallow_peer_from_ssl_auth_context(ctx.get()); GPR_ASSERT(check_ssl_peer_equivalence(&peer, &rpeer)); @@ -253,8 +284,9 @@ static void test_cn_and_multiple_sans_ssl_peer_to_auth_context(void) { const char* expected_cn = "cn1"; const char* expected_sans[] = {"san1", "san2", "san3"}; const char* expected_pem_cert = "pem_cert1"; + const char* expected_pem_cert_chain = "pem_cert1_chain"; size_t i; - GPR_ASSERT(tsi_construct_peer(3 + GPR_ARRAY_SIZE(expected_sans), &peer) == + GPR_ASSERT(tsi_construct_peer(4 + GPR_ARRAY_SIZE(expected_sans), &peer) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( TSI_CERTIFICATE_TYPE_PEER_PROPERTY, TSI_X509_CERTIFICATE_TYPE, @@ -265,10 +297,13 @@ static void test_cn_and_multiple_sans_ssl_peer_to_auth_context(void) { GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( TSI_X509_PEM_CERT_PROPERTY, expected_pem_cert, &peer.properties[2]) == TSI_OK); + GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( + TSI_X509_PEM_CERT_CHAIN_PROPERTY, expected_pem_cert_chain, + &peer.properties[3]) == TSI_OK); for (i = 0; i < GPR_ARRAY_SIZE(expected_sans); i++) { GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( TSI_X509_SUBJECT_ALTERNATIVE_NAME_PEER_PROPERTY, - expected_sans[i], &peer.properties[3 + i]) == TSI_OK); + expected_sans[i], &peer.properties[4 + i]) == TSI_OK); } grpc_core::RefCountedPtr ctx = grpc_ssl_peer_to_auth_context(&peer, GRPC_SSL_TRANSPORT_SECURITY_TYPE); @@ -279,6 +314,7 @@ static void test_cn_and_multiple_sans_ssl_peer_to_auth_context(void) { GPR_ASSERT(check_transport_security_type(ctx.get())); GPR_ASSERT(check_x509_cn(ctx.get(), expected_cn)); GPR_ASSERT(check_x509_pem_cert(ctx.get(), expected_pem_cert)); + GPR_ASSERT(check_x509_pem_cert_chain(ctx.get(), expected_pem_cert_chain)); rpeer = grpc_shallow_peer_from_ssl_auth_context(ctx.get()); GPR_ASSERT(check_ssl_peer_equivalence(&peer, &rpeer)); @@ -294,9 +330,10 @@ static void test_cn_and_multiple_sans_and_others_ssl_peer_to_auth_context( tsi_peer rpeer; const char* expected_cn = "cn1"; const char* expected_pem_cert = "pem_cert1"; + const char* expected_pem_cert_chain = "pem_cert1_chain"; const char* expected_sans[] = {"san1", "san2", "san3"}; size_t i; - GPR_ASSERT(tsi_construct_peer(5 + GPR_ARRAY_SIZE(expected_sans), &peer) == + GPR_ASSERT(tsi_construct_peer(6 + GPR_ARRAY_SIZE(expected_sans), &peer) == TSI_OK); GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( TSI_CERTIFICATE_TYPE_PEER_PROPERTY, TSI_X509_CERTIFICATE_TYPE, @@ -311,10 +348,13 @@ static void test_cn_and_multiple_sans_and_others_ssl_peer_to_auth_context( GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( TSI_X509_PEM_CERT_PROPERTY, expected_pem_cert, &peer.properties[4]) == TSI_OK); + GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( + TSI_X509_PEM_CERT_CHAIN_PROPERTY, expected_pem_cert_chain, + &peer.properties[5]) == TSI_OK); for (i = 0; i < GPR_ARRAY_SIZE(expected_sans); i++) { GPR_ASSERT(tsi_construct_string_peer_property_from_cstring( TSI_X509_SUBJECT_ALTERNATIVE_NAME_PEER_PROPERTY, - expected_sans[i], &peer.properties[5 + i]) == TSI_OK); + expected_sans[i], &peer.properties[6 + i]) == TSI_OK); } grpc_core::RefCountedPtr ctx = grpc_ssl_peer_to_auth_context(&peer, GRPC_SSL_TRANSPORT_SECURITY_TYPE); @@ -325,6 +365,7 @@ static void test_cn_and_multiple_sans_and_others_ssl_peer_to_auth_context( GPR_ASSERT(check_transport_security_type(ctx.get())); GPR_ASSERT(check_x509_cn(ctx.get(), expected_cn)); GPR_ASSERT(check_x509_pem_cert(ctx.get(), expected_pem_cert)); + GPR_ASSERT(check_x509_pem_cert_chain(ctx.get(), expected_pem_cert_chain)); rpeer = grpc_shallow_peer_from_ssl_auth_context(ctx.get()); GPR_ASSERT(check_ssl_peer_equivalence(&peer, &rpeer)); @@ -476,7 +517,6 @@ static void test_peer_alpn_check(void) { int main(int argc, char** argv) { grpc::testing::TestEnvironment env(argc, argv); grpc_init(); - test_unauthenticated_ssl_peer(); test_cn_only_ssl_peer_to_auth_context(); test_cn_and_one_san_ssl_peer_to_auth_context(); diff --git a/test/core/security/tls_security_connector_test.cc b/test/core/security/tls_security_connector_test.cc index bab6575a19e..475157e37be 100644 --- a/test/core/security/tls_security_connector_test.cc +++ b/test/core/security/tls_security_connector_test.cc @@ -118,7 +118,7 @@ class TlsSecurityConnectorTest : public ::testing::Test { TEST_F(TlsSecurityConnectorTest, NoKeysAndConfig) { grpc_ssl_certificate_config_reload_status reload_status; grpc_status_code status = - TlsFetchKeyMaterials(config_, *options_, &reload_status); + TlsFetchKeyMaterials(config_, *options_, true, &reload_status); EXPECT_EQ(status, GRPC_STATUS_FAILED_PRECONDITION); options_->Unref(); } @@ -127,7 +127,7 @@ TEST_F(TlsSecurityConnectorTest, NoKeySuccessReload) { grpc_ssl_certificate_config_reload_status reload_status; SetOptions(SUCCESS); grpc_status_code status = - TlsFetchKeyMaterials(config_, *options_, &reload_status); + TlsFetchKeyMaterials(config_, *options_, true, &reload_status); EXPECT_EQ(status, GRPC_STATUS_OK); EXPECT_EQ(reload_status, GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_NEW); options_->Unref(); @@ -137,7 +137,7 @@ TEST_F(TlsSecurityConnectorTest, NoKeyFailReload) { grpc_ssl_certificate_config_reload_status reload_status; SetOptions(FAIL); grpc_status_code status = - TlsFetchKeyMaterials(config_, *options_, &reload_status); + TlsFetchKeyMaterials(config_, *options_, true, &reload_status); EXPECT_EQ(status, GRPC_STATUS_INTERNAL); EXPECT_EQ(reload_status, GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_FAIL); options_->Unref(); @@ -148,7 +148,7 @@ TEST_F(TlsSecurityConnectorTest, NoKeyAsyncReload) { GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_UNCHANGED; SetOptions(ASYNC); grpc_status_code status = - TlsFetchKeyMaterials(config_, *options_, &reload_status); + TlsFetchKeyMaterials(config_, *options_, true, &reload_status); EXPECT_EQ(status, GRPC_STATUS_UNIMPLEMENTED); EXPECT_EQ(reload_status, GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_UNCHANGED); options_->Unref(); @@ -159,7 +159,7 @@ TEST_F(TlsSecurityConnectorTest, NoKeyUnchangedReload) { GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_UNCHANGED; SetOptions(UNCHANGED); grpc_status_code status = - TlsFetchKeyMaterials(config_, *options_, &reload_status); + TlsFetchKeyMaterials(config_, *options_, true, &reload_status); EXPECT_EQ(status, GRPC_STATUS_OK); EXPECT_EQ(reload_status, GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_UNCHANGED); options_->Unref(); @@ -170,7 +170,7 @@ TEST_F(TlsSecurityConnectorTest, WithKeyNoReload) { GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_UNCHANGED; SetKeyMaterialsConfig(); grpc_status_code status = - TlsFetchKeyMaterials(config_, *options_, &reload_status); + TlsFetchKeyMaterials(config_, *options_, true, &reload_status); EXPECT_EQ(status, GRPC_STATUS_OK); options_->Unref(); } @@ -180,7 +180,7 @@ TEST_F(TlsSecurityConnectorTest, WithKeySuccessReload) { SetOptions(SUCCESS); SetKeyMaterialsConfig(); grpc_status_code status = - TlsFetchKeyMaterials(config_, *options_, &reload_status); + TlsFetchKeyMaterials(config_, *options_, true, &reload_status); EXPECT_EQ(status, GRPC_STATUS_OK); EXPECT_EQ(reload_status, GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_NEW); options_->Unref(); @@ -191,7 +191,7 @@ TEST_F(TlsSecurityConnectorTest, WithKeyFailReload) { SetOptions(FAIL); SetKeyMaterialsConfig(); grpc_status_code status = - TlsFetchKeyMaterials(config_, *options_, &reload_status); + TlsFetchKeyMaterials(config_, *options_, true, &reload_status); EXPECT_EQ(status, GRPC_STATUS_OK); EXPECT_EQ(reload_status, GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_FAIL); options_->Unref(); @@ -203,7 +203,7 @@ TEST_F(TlsSecurityConnectorTest, WithKeyAsyncReload) { SetOptions(ASYNC); SetKeyMaterialsConfig(); grpc_status_code status = - TlsFetchKeyMaterials(config_, *options_, &reload_status); + TlsFetchKeyMaterials(config_, *options_, true, &reload_status); EXPECT_EQ(status, GRPC_STATUS_OK); EXPECT_EQ(reload_status, GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_UNCHANGED); options_->Unref(); @@ -215,7 +215,7 @@ TEST_F(TlsSecurityConnectorTest, WithKeyUnchangedReload) { SetOptions(UNCHANGED); SetKeyMaterialsConfig(); grpc_status_code status = - TlsFetchKeyMaterials(config_, *options_, &reload_status); + TlsFetchKeyMaterials(config_, *options_, true, &reload_status); EXPECT_EQ(status, GRPC_STATUS_OK); EXPECT_EQ(reload_status, GRPC_SSL_CERTIFICATE_CONFIG_RELOAD_UNCHANGED); options_->Unref(); diff --git a/test/core/tsi/ssl_transport_security_test.cc b/test/core/tsi/ssl_transport_security_test.cc index 8a5dd6e4a0a..3f85bb1e88d 100644 --- a/test/core/tsi/ssl_transport_security_test.cc +++ b/test/core/tsi/ssl_transport_security_test.cc @@ -35,6 +35,7 @@ extern "C" { #include +#include } #define SSL_TSI_TEST_ALPN1 "foo" @@ -855,6 +856,36 @@ void ssl_tsi_test_extract_x509_subject_names() { tsi_peer_destruct(&peer); } +void ssl_tsi_test_extract_cert_chain() { + gpr_log(GPR_INFO, "ssl_tsi_test_extract_cert_chain"); + char* cert = load_file(SSL_TSI_TEST_CREDENTIALS_DIR, "server1.pem"); + char* ca = load_file(SSL_TSI_TEST_CREDENTIALS_DIR, "ca.pem"); + char* chain = static_cast( + gpr_zalloc(sizeof(char) * (strlen(cert) + strlen(ca) + 1))); + memcpy(chain, cert, strlen(cert)); + memcpy(chain + strlen(cert), ca, strlen(ca)); + STACK_OF(X509)* cert_chain = sk_X509_new_null(); + GPR_ASSERT(cert_chain != nullptr); + BIO* bio = BIO_new_mem_buf(chain, strlen(chain)); + GPR_ASSERT(bio != nullptr); + STACK_OF(X509_INFO)* certInfos = + PEM_X509_INFO_read_bio(bio, nullptr, nullptr, nullptr); + GPR_ASSERT(certInfos != nullptr); + for (int i = 0; i < sk_X509_INFO_num(certInfos); i++) { + X509_INFO* certInfo = sk_X509_INFO_value(certInfos, i); + if (certInfo->x509 != nullptr) { + GPR_ASSERT(sk_X509_push(cert_chain, certInfo->x509) != 0); + X509_up_ref(certInfo->x509); + } + } + tsi_peer_property chain_property; + GPR_ASSERT(tsi_ssl_get_cert_chain_contents(cert_chain, &chain_property) == + TSI_OK); + GPR_ASSERT(memcmp(chain, chain_property.value.data, + chain_property.value.length) == 0); + gpr_free(chain); +} + int main(int argc, char** argv) { grpc::testing::TestEnvironment env(argc, argv); grpc_init(); @@ -881,6 +912,7 @@ int main(int argc, char** argv) { ssl_tsi_test_handshaker_factory_internals(); ssl_tsi_test_duplicate_root_certificates(); ssl_tsi_test_extract_x509_subject_names(); + ssl_tsi_test_extract_cert_chain(); grpc_shutdown(); return 0; } diff --git a/test/cpp/client/credentials_test.cc b/test/cpp/client/credentials_test.cc index 07c428de084..3cc0ebfb8e9 100644 --- a/test/cpp/client/credentials_test.cc +++ b/test/cpp/client/credentials_test.cc @@ -563,7 +563,8 @@ TEST_F(CredentialsTest, TlsCredentialsOptionsCppToC) { test_server_authorization_check)); TlsCredentialsOptions options = TlsCredentialsOptions( - GRPC_SSL_REQUEST_CLIENT_CERTIFICATE_AND_VERIFY, key_materials_config, + GRPC_SSL_REQUEST_CLIENT_CERTIFICATE_AND_VERIFY, + GRPC_TLS_SERVER_VERIFICATION, key_materials_config, credential_reload_config, server_authorization_check_config); grpc_tls_credentials_options* c_options = options.c_credentials_options(); EXPECT_EQ(c_options->cert_request_type(), @@ -661,8 +662,9 @@ TEST_F(CredentialsTest, LoadTlsChannelCredentials) { test_server_authorization_check)); TlsCredentialsOptions options = TlsCredentialsOptions( - GRPC_SSL_REQUEST_CLIENT_CERTIFICATE_AND_VERIFY, nullptr, - credential_reload_config, server_authorization_check_config); + GRPC_SSL_REQUEST_CLIENT_CERTIFICATE_AND_VERIFY, + GRPC_TLS_SERVER_VERIFICATION, nullptr, credential_reload_config, + server_authorization_check_config); std::shared_ptr channel_credentials = grpc::experimental::TlsCredentials(options); GPR_ASSERT(channel_credentials != nullptr); From b50280a61a98a4c6a834aa1b6ea7d42464be252f Mon Sep 17 00:00:00 2001 From: Akshay Kumar Date: Mon, 6 Jan 2020 09:12:21 -0800 Subject: [PATCH 2/7] FullChainExperimental-01-200106-generateprojects --- src/core/tsi/ssl_transport_security.h | 7 ++++--- test/core/surface/public_headers_must_be_c89.c | 1 + 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/core/tsi/ssl_transport_security.h b/src/core/tsi/ssl_transport_security.h index c5daba01779..2ccd15088c9 100644 --- a/src/core/tsi/ssl_transport_security.h +++ b/src/core/tsi/ssl_transport_security.h @@ -20,13 +20,14 @@ #define GRPC_CORE_TSI_SSL_TRANSPORT_SECURITY_H #include -extern "C" { -#include -} #include "src/core/lib/gprpp/string_view.h" #include "src/core/tsi/transport_security_interface.h" +extern "C" { +#include +} + /* Value for the TSI_CERTIFICATE_TYPE_PEER_PROPERTY property for X509 certs. */ #define TSI_X509_CERTIFICATE_TYPE "X509" diff --git a/test/core/surface/public_headers_must_be_c89.c b/test/core/surface/public_headers_must_be_c89.c index 58a2d7a4d9f..3c20e55620e 100644 --- a/test/core/surface/public_headers_must_be_c89.c +++ b/test/core/surface/public_headers_must_be_c89.c @@ -201,6 +201,7 @@ int main(int argc, char **argv) { printf("%lx", (unsigned long) grpc_local_server_credentials_create); printf("%lx", (unsigned long) grpc_tls_credentials_options_create); printf("%lx", (unsigned long) grpc_tls_credentials_options_set_cert_request_type); + printf("%lx", (unsigned long) grpc_tls_credentials_options_set_server_verification_option); printf("%lx", (unsigned long) grpc_tls_credentials_options_set_key_materials_config); printf("%lx", (unsigned long) grpc_tls_credentials_options_set_credential_reload_config); printf("%lx", (unsigned long) grpc_tls_credentials_options_set_server_authorization_check_config); From 3eadaa1aa811cc5b0ca905686c6d4b7c160b156a Mon Sep 17 00:00:00 2001 From: Akshay Kumar Date: Mon, 6 Jan 2020 11:19:40 -0800 Subject: [PATCH 3/7] FullChainExperimental-01-200106-rb --- .../security/security_connector/tls/tls_security_connector.cc | 1 + src/ruby/ext/grpc/rb_grpc_imports.generated.c | 2 ++ src/ruby/ext/grpc/rb_grpc_imports.generated.h | 3 +++ 3 files changed, 6 insertions(+) diff --git a/src/core/lib/security/security_connector/tls/tls_security_connector.cc b/src/core/lib/security/security_connector/tls/tls_security_connector.cc index 205f1dda7ff..702a6352ef3 100644 --- a/src/core/lib/security/security_connector/tls/tls_security_connector.cc +++ b/src/core/lib/security/security_connector/tls/tls_security_connector.cc @@ -407,6 +407,7 @@ void TlsChannelSecurityConnector::ServerAuthorizationCheckArgDestroy( } gpr_free((void*)arg->target_name); gpr_free((void*)arg->peer_cert); + if (arg->peer_cert_full_chain) gpr_free((void*)arg->peer_cert_full_chain); gpr_free((void*)arg->error_details); if (arg->destroy_context != nullptr) { arg->destroy_context(arg->context); diff --git a/src/ruby/ext/grpc/rb_grpc_imports.generated.c b/src/ruby/ext/grpc/rb_grpc_imports.generated.c index e75b83ba17e..4e50a5c3ea7 100644 --- a/src/ruby/ext/grpc/rb_grpc_imports.generated.c +++ b/src/ruby/ext/grpc/rb_grpc_imports.generated.c @@ -159,6 +159,7 @@ grpc_local_credentials_create_type grpc_local_credentials_create_import; grpc_local_server_credentials_create_type grpc_local_server_credentials_create_import; grpc_tls_credentials_options_create_type grpc_tls_credentials_options_create_import; grpc_tls_credentials_options_set_cert_request_type_type grpc_tls_credentials_options_set_cert_request_type_import; +grpc_tls_credentials_options_set_server_verification_option_type grpc_tls_credentials_options_set_server_verification_option_import; grpc_tls_credentials_options_set_key_materials_config_type grpc_tls_credentials_options_set_key_materials_config_import; grpc_tls_credentials_options_set_credential_reload_config_type grpc_tls_credentials_options_set_credential_reload_config_import; grpc_tls_credentials_options_set_server_authorization_check_config_type grpc_tls_credentials_options_set_server_authorization_check_config_import; @@ -430,6 +431,7 @@ void grpc_rb_load_imports(HMODULE library) { grpc_local_server_credentials_create_import = (grpc_local_server_credentials_create_type) GetProcAddress(library, "grpc_local_server_credentials_create"); grpc_tls_credentials_options_create_import = (grpc_tls_credentials_options_create_type) GetProcAddress(library, "grpc_tls_credentials_options_create"); grpc_tls_credentials_options_set_cert_request_type_import = (grpc_tls_credentials_options_set_cert_request_type_type) GetProcAddress(library, "grpc_tls_credentials_options_set_cert_request_type"); + grpc_tls_credentials_options_set_server_verification_option_import = (grpc_tls_credentials_options_set_server_verification_option_type) GetProcAddress(library, "grpc_tls_credentials_options_set_server_verification_option"); grpc_tls_credentials_options_set_key_materials_config_import = (grpc_tls_credentials_options_set_key_materials_config_type) GetProcAddress(library, "grpc_tls_credentials_options_set_key_materials_config"); grpc_tls_credentials_options_set_credential_reload_config_import = (grpc_tls_credentials_options_set_credential_reload_config_type) GetProcAddress(library, "grpc_tls_credentials_options_set_credential_reload_config"); grpc_tls_credentials_options_set_server_authorization_check_config_import = (grpc_tls_credentials_options_set_server_authorization_check_config_type) GetProcAddress(library, "grpc_tls_credentials_options_set_server_authorization_check_config"); diff --git a/src/ruby/ext/grpc/rb_grpc_imports.generated.h b/src/ruby/ext/grpc/rb_grpc_imports.generated.h index 0205f60d06f..32aef58ee0b 100644 --- a/src/ruby/ext/grpc/rb_grpc_imports.generated.h +++ b/src/ruby/ext/grpc/rb_grpc_imports.generated.h @@ -452,6 +452,9 @@ extern grpc_tls_credentials_options_create_type grpc_tls_credentials_options_cre typedef int(*grpc_tls_credentials_options_set_cert_request_type_type)(grpc_tls_credentials_options* options, grpc_ssl_client_certificate_request_type type); extern grpc_tls_credentials_options_set_cert_request_type_type grpc_tls_credentials_options_set_cert_request_type_import; #define grpc_tls_credentials_options_set_cert_request_type grpc_tls_credentials_options_set_cert_request_type_import +typedef int(*grpc_tls_credentials_options_set_server_verification_option_type)(grpc_tls_credentials_options* options, grpc_tls_server_verification_option server_verification_option); +extern grpc_tls_credentials_options_set_server_verification_option_type grpc_tls_credentials_options_set_server_verification_option_import; +#define grpc_tls_credentials_options_set_server_verification_option grpc_tls_credentials_options_set_server_verification_option_import typedef int(*grpc_tls_credentials_options_set_key_materials_config_type)(grpc_tls_credentials_options* options, grpc_tls_key_materials_config* config); extern grpc_tls_credentials_options_set_key_materials_config_type grpc_tls_credentials_options_set_key_materials_config_import; #define grpc_tls_credentials_options_set_key_materials_config grpc_tls_credentials_options_set_key_materials_config_import From ef376e35d77b04d8108613f5d4cdbe06c9d66ebc Mon Sep 17 00:00:00 2001 From: Akshay Kumar Date: Mon, 6 Jan 2020 13:43:14 -0800 Subject: [PATCH 4/7] FullChainExperimental-01-200106-ssl_transport_security_test --- test/core/tsi/ssl_transport_security_test.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/core/tsi/ssl_transport_security_test.cc b/test/core/tsi/ssl_transport_security_test.cc index 3f85bb1e88d..37ad7025912 100644 --- a/test/core/tsi/ssl_transport_security_test.cc +++ b/test/core/tsi/ssl_transport_security_test.cc @@ -883,7 +883,12 @@ void ssl_tsi_test_extract_cert_chain() { TSI_OK); GPR_ASSERT(memcmp(chain, chain_property.value.data, chain_property.value.length) == 0); + BIO_free(bio); gpr_free(chain); + gpr_free(cert); + gpr_free(ca); + tsi_peer_property_destruct(chain_property); + sk_X509_INFO_pop_free(inf, X509_INFO_free); } int main(int argc, char** argv) { From 48d51b24e4072908ce80c555c9c625a5153acaf2 Mon Sep 17 00:00:00 2001 From: Akshay Kumar Date: Mon, 6 Jan 2020 14:23:42 -0800 Subject: [PATCH 5/7] FullChainExperimental-01-200106-ssl_transport_security_test-2 --- test/core/tsi/ssl_transport_security_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/core/tsi/ssl_transport_security_test.cc b/test/core/tsi/ssl_transport_security_test.cc index 37ad7025912..2acfcd03dfd 100644 --- a/test/core/tsi/ssl_transport_security_test.cc +++ b/test/core/tsi/ssl_transport_security_test.cc @@ -887,8 +887,8 @@ void ssl_tsi_test_extract_cert_chain() { gpr_free(chain); gpr_free(cert); gpr_free(ca); - tsi_peer_property_destruct(chain_property); - sk_X509_INFO_pop_free(inf, X509_INFO_free); + tsi_peer_property_destruct(&chain_property); + sk_X509_INFO_pop_free(certInfos, X509_INFO_free); } int main(int argc, char** argv) { From 3be7b4362f4737665fc4ecbee38f90e943a8e5d1 Mon Sep 17 00:00:00 2001 From: Akshay Kumar Date: Mon, 6 Jan 2020 15:22:40 -0800 Subject: [PATCH 6/7] FullChainExperimental-01-200106-ssl_transport_security_test-3 --- test/core/tsi/ssl_transport_security_test.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/test/core/tsi/ssl_transport_security_test.cc b/test/core/tsi/ssl_transport_security_test.cc index 2acfcd03dfd..69e226759f8 100644 --- a/test/core/tsi/ssl_transport_security_test.cc +++ b/test/core/tsi/ssl_transport_security_test.cc @@ -889,6 +889,7 @@ void ssl_tsi_test_extract_cert_chain() { gpr_free(ca); tsi_peer_property_destruct(&chain_property); sk_X509_INFO_pop_free(certInfos, X509_INFO_free); + sk_X509_pop_free(cert_chain, X509_free); } int main(int argc, char** argv) { From 02371b7569d789e7ab6dded54ea6dfd4d8a4728d Mon Sep 17 00:00:00 2001 From: Akshay Kumar Date: Tue, 7 Jan 2020 11:22:00 -0800 Subject: [PATCH 7/7] FullChainExperimental-01-200106-ssl_transport_security_test-4 --- include/grpc/grpc_security_constants.h | 9 ++++++--- .../security/security_connector/ssl_utils.cc | 19 +++---------------- .../security/security_connector/ssl_utils.h | 7 +------ .../tls/tls_security_connector.cc | 8 ++++---- src/core/tsi/ssl_transport_security.cc | 2 +- src/core/tsi/ssl_transport_security.h | 6 +++--- src/core/tsi/transport_security_interface.h | 11 ----------- 7 files changed, 18 insertions(+), 44 deletions(-) diff --git a/include/grpc/grpc_security_constants.h b/include/grpc/grpc_security_constants.h index 63900e41cb3..c414101b66d 100644 --- a/include/grpc/grpc_security_constants.h +++ b/include/grpc/grpc_security_constants.h @@ -117,12 +117,15 @@ typedef enum { typedef enum { /** Default option: performs server certificate verification and hostname - verification */ + verification. */ GRPC_TLS_SERVER_VERIFICATION, /** Performs server certificate verification, but skips hostname verification - */ + Client is responsible for verifying server's identity via + server authorization check callback. */ GRPC_TLS_SKIP_HOSTNAME_VERIFICATION, - /** Skips both server certificate and hostname verification */ + /** Skips both server certificate and hostname verification. + Client is responsible for verifying server's identity and + server's certificate via server authorization check callback. */ GRPC_TLS_SKIP_ALL_SERVER_VERIFICATION } grpc_tls_server_verification_option; diff --git a/src/core/lib/security/security_connector/ssl_utils.cc b/src/core/lib/security/security_connector/ssl_utils.cc index e34c66b35d6..8b99850d048 100644 --- a/src/core/lib/security/security_connector/ssl_utils.cc +++ b/src/core/lib/security/security_connector/ssl_utils.cc @@ -108,20 +108,6 @@ grpc_get_tsi_client_certificate_request_type( } } -tsi_server_verification_option grpc_get_tsi_server_verification_option( - grpc_tls_server_verification_option server_verification_option) { - switch (server_verification_option) { - case GRPC_TLS_SERVER_VERIFICATION: - return TSI_SERVER_VERIFICATION; - case GRPC_TLS_SKIP_HOSTNAME_VERIFICATION: - return TSI_SKIP_HOSTNAME_VERIFICATION; - case GRPC_TLS_SKIP_ALL_SERVER_VERIFICATION: - return TSI_SKIP_ALL_SERVER_VERIFICATION; - default: - return TSI_SERVER_VERIFICATION; - } -} - grpc_error* grpc_ssl_check_alpn(const tsi_peer* peer) { #if TSI_OPENSSL_ALPN_SUPPORT /* Check the ALPN if ALPN is supported. */ @@ -306,7 +292,7 @@ void grpc_shallow_peer_destruct(tsi_peer* peer) { grpc_security_status grpc_ssl_tsi_client_handshaker_factory_init( tsi_ssl_pem_key_cert_pair* pem_key_cert_pair, const char* pem_root_certs, - tsi_server_verification_option server_verification_option, + bool skip_server_certificate_verification, tsi_ssl_session_cache* ssl_session_cache, tsi_ssl_client_handshaker_factory** handshaker_factory) { const char* root_certs; @@ -337,7 +323,8 @@ grpc_security_status grpc_ssl_tsi_client_handshaker_factory_init( } options.cipher_suites = grpc_get_ssl_cipher_suites(); options.session_cache = ssl_session_cache; - options.server_verification_option = server_verification_option; + options.skip_server_certificate_verification = + skip_server_certificate_verification; const tsi_result result = tsi_create_ssl_client_handshaker_factory_with_options(&options, handshaker_factory); diff --git a/src/core/lib/security/security_connector/ssl_utils.h b/src/core/lib/security/security_connector/ssl_utils.h index e6370db9768..bf9607a3e27 100644 --- a/src/core/lib/security/security_connector/ssl_utils.h +++ b/src/core/lib/security/security_connector/ssl_utils.h @@ -68,18 +68,13 @@ tsi_client_certificate_request_type grpc_get_tsi_client_certificate_request_type( grpc_ssl_client_certificate_request_type grpc_request_type); -/* Map from grpc_tls_server_verification_option to - * tsi_server_verification_option. */ -tsi_server_verification_option grpc_get_tsi_server_verification_option( - grpc_tls_server_verification_option server_verification_option); - /* Return an array of strings containing alpn protocols. */ const char** grpc_fill_alpn_protocol_strings(size_t* num_alpn_protocols); /* Initialize TSI SSL server/client handshaker factory. */ grpc_security_status grpc_ssl_tsi_client_handshaker_factory_init( tsi_ssl_pem_key_cert_pair* key_cert_pair, const char* pem_root_certs, - tsi_server_verification_option server_verification_option, + bool skip_server_certificate_verification, tsi_ssl_session_cache* ssl_session_cache, tsi_ssl_client_handshaker_factory** handshaker_factory); diff --git a/src/core/lib/security/security_connector/tls/tls_security_connector.cc b/src/core/lib/security/security_connector/tls/tls_security_connector.cc index 702a6352ef3..3cd83ae1a80 100644 --- a/src/core/lib/security/security_connector/tls/tls_security_connector.cc +++ b/src/core/lib/security/security_connector/tls/tls_security_connector.cc @@ -286,9 +286,9 @@ grpc_security_status TlsChannelSecurityConnector::ReplaceHandshakerFactory( tsi_ssl_session_cache* ssl_session_cache) { const TlsCredentials* creds = static_cast(channel_creds()); - tsi_server_verification_option server_verification_option = - grpc_get_tsi_server_verification_option( - creds->options().server_verification_option()); + bool skip_server_certificate_verification = + creds->options().server_verification_option() == + GRPC_TLS_SKIP_ALL_SERVER_VERIFICATION; /* Free the client handshaker factory if exists. */ if (client_handshaker_factory_) { tsi_ssl_client_handshaker_factory_unref(client_handshaker_factory_); @@ -297,7 +297,7 @@ grpc_security_status TlsChannelSecurityConnector::ReplaceHandshakerFactory( key_materials_config_->pem_key_cert_pair_list()); grpc_security_status status = grpc_ssl_tsi_client_handshaker_factory_init( pem_key_cert_pair, key_materials_config_->pem_root_certs(), - server_verification_option, ssl_session_cache, + skip_server_certificate_verification, ssl_session_cache, &client_handshaker_factory_); /* Free memory. */ grpc_tsi_ssl_pem_key_cert_pairs_destroy(pem_key_cert_pair, 1); diff --git a/src/core/tsi/ssl_transport_security.cc b/src/core/tsi/ssl_transport_security.cc index 2063ef0dfc9..2f85074ec89 100644 --- a/src/core/tsi/ssl_transport_security.cc +++ b/src/core/tsi/ssl_transport_security.cc @@ -1765,7 +1765,7 @@ tsi_result tsi_create_ssl_client_handshaker_factory_with_options( tsi_ssl_handshaker_factory_unref(&impl->base); return result; } - if (options->server_verification_option == TSI_SKIP_ALL_SERVER_VERIFICATION) { + if (options->skip_server_certificate_verification) { SSL_CTX_set_verify(ssl_context, SSL_VERIFY_PEER, NullVerifyCallback); } else { SSL_CTX_set_verify(ssl_context, SSL_VERIFY_PEER, nullptr); diff --git a/src/core/tsi/ssl_transport_security.h b/src/core/tsi/ssl_transport_security.h index 2ccd15088c9..ae1e413aad3 100644 --- a/src/core/tsi/ssl_transport_security.h +++ b/src/core/tsi/ssl_transport_security.h @@ -148,8 +148,8 @@ struct tsi_ssl_client_handshaker_options { /* ssl_session_cache is a cache for reusable client-side sessions. */ tsi_ssl_session_cache* session_cache; - /* Server verification option */ - tsi_server_verification_option server_verification_option; + /* skip server certificate verification. */ + bool skip_server_certificate_verification; tsi_ssl_client_handshaker_options() : pem_key_cert_pair(nullptr), @@ -159,7 +159,7 @@ struct tsi_ssl_client_handshaker_options { alpn_protocols(nullptr), num_alpn_protocols(0), session_cache(nullptr), - server_verification_option(TSI_SERVER_VERIFICATION) {} + skip_server_certificate_verification(false) {} }; /* Creates a client handshaker factory. diff --git a/src/core/tsi/transport_security_interface.h b/src/core/tsi/transport_security_interface.h index 6d597e4bdf7..7a0cdc3453a 100644 --- a/src/core/tsi/transport_security_interface.h +++ b/src/core/tsi/transport_security_interface.h @@ -55,17 +55,6 @@ typedef enum { TSI_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_AND_VERIFY, } tsi_client_certificate_request_type; -typedef enum { - /** Default option: performs server certificate verification and hostname - verification */ - TSI_SERVER_VERIFICATION, - /** Performs server certificate verification, but skips hostname verification - */ - TSI_SKIP_HOSTNAME_VERIFICATION, - /** Skips both server certificate and hostname verification */ - TSI_SKIP_ALL_SERVER_VERIFICATION, -} tsi_server_verification_option; - const char* tsi_result_to_string(tsi_result result); /* --- tsi tracing --- */