From 24778bf8ea57943bd5dfb07e5c989e478261b8e3 Mon Sep 17 00:00:00 2001 From: Vignesh Babu Date: Thu, 23 Dec 2021 21:20:31 +0000 Subject: [PATCH] Revert "Enable CRL checking in gRPC core (#26287)" (#28416) This reverts commit 7d8c9ae890c96abd959aae4e0c3191fa9eb0472b. --- CMakeLists.txt | 41 --- build_autogenerated.yaml | 15 - grpc.def | 1 - include/grpc/grpc_security.h | 10 - .../tls/grpc_tls_credentials_options.cc | 6 - .../tls/grpc_tls_credentials_options.h | 7 - .../security/security_connector/ssl_utils.cc | 4 - .../security/security_connector/ssl_utils.h | 2 - .../tls/tls_security_connector.cc | 4 +- src/core/tsi/ssl_transport_security.cc | 46 +-- src/core/tsi/ssl_transport_security.h | 18 +- src/ruby/ext/grpc/rb_grpc_imports.generated.c | 2 - src/ruby/ext/grpc/rb_grpc_imports.generated.h | 3 - .../core/surface/public_headers_must_be_c89.c | 1 - test/core/tsi/BUILD | 25 -- .../tsi/crl_ssl_transport_security_test.cc | 279 ------------------ .../crl_data/demoCA/index.txt.attr.old | 1 - test/core/tsi/test_creds/demoCA/crlnumber | 1 - test/core/tsi/test_creds/demoCA/index.txt | 1 - .../core/tsi/test_creds/demoCA/index.txt.attr | 1 - test/core/tsi/test_creds/demoCA/index.txt.old | 0 test/core/tsi/transport_security_test_lib.cc | 1 - tools/run_tests/generated/tests.json | 22 -- 23 files changed, 6 insertions(+), 485 deletions(-) delete mode 100644 test/core/tsi/crl_ssl_transport_security_test.cc delete mode 100644 test/core/tsi/test_creds/crl_data/demoCA/index.txt.attr.old delete mode 100644 test/core/tsi/test_creds/demoCA/crlnumber delete mode 100644 test/core/tsi/test_creds/demoCA/index.txt delete mode 100644 test/core/tsi/test_creds/demoCA/index.txt.attr delete mode 100644 test/core/tsi/test_creds/demoCA/index.txt.old diff --git a/CMakeLists.txt b/CMakeLists.txt index 2c41936c0f2..5f275b33c5b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -825,9 +825,6 @@ if(gRPC_BUILD_TESTS) add_dependencies(buildtests_cxx context_test) add_dependencies(buildtests_cxx core_configuration_test) add_dependencies(buildtests_cxx cpp_impl_of_test) - if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) - add_dependencies(buildtests_cxx crl_ssl_transport_security_test) - endif() add_dependencies(buildtests_cxx delegating_channel_test) add_dependencies(buildtests_cxx destroy_grpclb_channel_with_active_connect_stress_test) add_dependencies(buildtests_cxx dual_ref_counted_test) @@ -9734,44 +9731,6 @@ target_link_libraries(cpp_impl_of_test ) -endif() -if(gRPC_BUILD_TESTS) -if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) - - add_executable(crl_ssl_transport_security_test - test/core/tsi/crl_ssl_transport_security_test.cc - test/core/tsi/transport_security_test_lib.cc - third_party/googletest/googletest/src/gtest-all.cc - third_party/googletest/googlemock/src/gmock-all.cc - ) - - target_include_directories(crl_ssl_transport_security_test - PRIVATE - ${CMAKE_CURRENT_SOURCE_DIR} - ${CMAKE_CURRENT_SOURCE_DIR}/include - ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR} - ${_gRPC_RE2_INCLUDE_DIR} - ${_gRPC_SSL_INCLUDE_DIR} - ${_gRPC_UPB_GENERATED_DIR} - ${_gRPC_UPB_GRPC_GENERATED_DIR} - ${_gRPC_UPB_INCLUDE_DIR} - ${_gRPC_XXHASH_INCLUDE_DIR} - ${_gRPC_ZLIB_INCLUDE_DIR} - third_party/googletest/googletest/include - third_party/googletest/googletest - third_party/googletest/googlemock/include - third_party/googletest/googlemock - ${_gRPC_PROTO_GENS_DIR} - ) - - target_link_libraries(crl_ssl_transport_security_test - ${_gRPC_PROTOBUF_LIBRARIES} - ${_gRPC_ALLTARGETS_LIBRARIES} - grpc_test_util - ) - - -endif() endif() if(gRPC_BUILD_TESTS) diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 8c6eaf92265..6e660f87924 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -5392,21 +5392,6 @@ targets: - test/core/gprpp/cpp_impl_of_test.cc deps: [] uses_polling: false -- name: crl_ssl_transport_security_test - gtest: true - build: test - language: c++ - headers: - - test/core/tsi/transport_security_test_lib.h - src: - - test/core/tsi/crl_ssl_transport_security_test.cc - - test/core/tsi/transport_security_test_lib.cc - deps: - - grpc_test_util - platforms: - - linux - - posix - - mac - name: delegating_channel_test gtest: true build: test diff --git a/grpc.def b/grpc.def index 58979d814b9..caba5e2aab8 100644 --- a/grpc.def +++ b/grpc.def @@ -155,7 +155,6 @@ EXPORTS grpc_tls_credentials_options_watch_identity_key_cert_pairs grpc_tls_credentials_options_set_identity_cert_name grpc_tls_credentials_options_set_cert_request_type - grpc_tls_credentials_options_set_crl_directory grpc_tls_credentials_options_set_verify_server_cert grpc_tls_credentials_options_set_check_call_host grpc_xds_credentials_create diff --git a/include/grpc/grpc_security.h b/include/grpc/grpc_security.h index 414f2216573..48f5afe08cf 100644 --- a/include/grpc/grpc_security.h +++ b/include/grpc/grpc_security.h @@ -920,16 +920,6 @@ GRPCAPI void grpc_tls_credentials_options_set_identity_cert_name( GRPCAPI void grpc_tls_credentials_options_set_cert_request_type( grpc_tls_credentials_options* options, grpc_ssl_client_certificate_request_type type); -/** - * EXPERIMENTAL API - Subject to change - * - * If set, gRPC will read all hashed x.509 CRL files in the directory and - * enforce the CRL files on all TLS handshakes. Only supported for OpenSSL - * version > 1.1. - * It is used for experimental purpose for now and subject to change. - */ -GRPCAPI void grpc_tls_credentials_options_set_crl_directory( - grpc_tls_credentials_options* options, const char* crl_directory); /** * EXPERIMENTAL API - Subject to change 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 0617fd8ca14..8042cf26686 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 @@ -91,12 +91,6 @@ void grpc_tls_credentials_options_set_certificate_verifier( options->set_certificate_verifier(verifier->Ref()); } -void grpc_tls_credentials_options_set_crl_directory( - grpc_tls_credentials_options* options, const char* crl_directory) { - GPR_ASSERT(options != nullptr); - options->set_crl_directory(crl_directory); -} - void grpc_tls_credentials_options_set_check_call_host( grpc_tls_credentials_options* options, int check_call_host) { GPR_ASSERT(options != nullptr); 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 526a5a376d5..edaac0e438c 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 @@ -60,7 +60,6 @@ struct grpc_tls_credentials_options const std::string& root_cert_name() { return root_cert_name_; } bool watch_identity_pair() { return watch_identity_pair_; } const std::string& identity_cert_name() { return identity_cert_name_; } - const std::string& crl_directory() { return crl_directory_; } // Setters for member fields. void set_cert_request_type( @@ -113,11 +112,6 @@ struct grpc_tls_credentials_options identity_cert_name_ = std::move(identity_cert_name); } - // gRPC will enforce CRLs on all handshakes from all hashed CRL files inside - // of the crl_directory. If not set, an empty string will be used, which will - // not enable CRL checking. Only supported for OpenSSL version > 1.1. - void set_crl_directory(std::string path) { crl_directory_ = std::move(path); } - private: grpc_ssl_client_certificate_request_type cert_request_type_ = GRPC_SSL_DONT_REQUEST_CLIENT_CERTIFICATE; @@ -131,7 +125,6 @@ struct grpc_tls_credentials_options std::string root_cert_name_; bool watch_identity_pair_ = false; std::string identity_cert_name_; - std::string crl_directory_; }; #endif // GRPC_CORE_LIB_SECURITY_CREDENTIALS_TLS_GRPC_TLS_CREDENTIALS_OPTIONS_H diff --git a/src/core/lib/security/security_connector/ssl_utils.cc b/src/core/lib/security/security_connector/ssl_utils.cc index b72ce0c022b..189bd9a0f53 100644 --- a/src/core/lib/security/security_connector/ssl_utils.cc +++ b/src/core/lib/security/security_connector/ssl_utils.cc @@ -423,7 +423,6 @@ 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, bool skip_server_certificate_verification, tsi_tls_version min_tls_version, tsi_tls_version max_tls_version, tsi_ssl_session_cache* ssl_session_cache, - const char* crl_directory, tsi_ssl_client_handshaker_factory** handshaker_factory) { const char* root_certs; const tsi_ssl_root_certs_store* root_store; @@ -460,7 +459,6 @@ grpc_security_status grpc_ssl_tsi_client_handshaker_factory_init( skip_server_certificate_verification; options.min_tls_version = min_tls_version; options.max_tls_version = max_tls_version; - options.crl_directory = crl_directory; const tsi_result result = tsi_create_ssl_client_handshaker_factory_with_options(&options, handshaker_factory); @@ -478,7 +476,6 @@ grpc_security_status grpc_ssl_tsi_server_handshaker_factory_init( const char* pem_root_certs, grpc_ssl_client_certificate_request_type client_certificate_request, tsi_tls_version min_tls_version, tsi_tls_version max_tls_version, - const char* crl_directory, tsi_ssl_server_handshaker_factory** handshaker_factory) { size_t num_alpn_protocols = 0; const char** alpn_protocol_strings = @@ -494,7 +491,6 @@ grpc_security_status grpc_ssl_tsi_server_handshaker_factory_init( options.num_alpn_protocols = static_cast(num_alpn_protocols); options.min_tls_version = min_tls_version; options.max_tls_version = max_tls_version; - options.crl_directory = crl_directory; const tsi_result result = tsi_create_ssl_server_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 324137c08a5..f7e747c460d 100644 --- a/src/core/lib/security/security_connector/ssl_utils.h +++ b/src/core/lib/security/security_connector/ssl_utils.h @@ -91,7 +91,6 @@ grpc_security_status grpc_ssl_tsi_client_handshaker_factory_init( tsi_ssl_pem_key_cert_pair* key_cert_pair, const char* pem_root_certs, bool skip_server_certificate_verification, tsi_tls_version min_tls_version, tsi_tls_version max_tls_version, tsi_ssl_session_cache* ssl_session_cache, - const char* crl_directory, tsi_ssl_client_handshaker_factory** handshaker_factory); grpc_security_status grpc_ssl_tsi_server_handshaker_factory_init( @@ -99,7 +98,6 @@ grpc_security_status grpc_ssl_tsi_server_handshaker_factory_init( const char* pem_root_certs, grpc_ssl_client_certificate_request_type client_certificate_request, tsi_tls_version min_tls_version, tsi_tls_version max_tls_version, - const char* crl_directory, tsi_ssl_server_handshaker_factory** handshaker_factory); /* Exposed for testing only. */ 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 7720d96bada..2f7dd7de870 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 @@ -538,7 +538,7 @@ TlsChannelSecurityConnector::UpdateHandshakerFactoryLocked() { skip_server_certificate_verification, grpc_get_tsi_tls_version(options_->min_tls_version()), grpc_get_tsi_tls_version(options_->max_tls_version()), ssl_session_cache_, - options_->crl_directory().c_str(), &client_handshaker_factory_); + &client_handshaker_factory_); /* Free memory. */ if (pem_key_cert_pair != nullptr) { grpc_tsi_ssl_pem_key_cert_pairs_destroy(pem_key_cert_pair, 1); @@ -806,7 +806,7 @@ TlsServerSecurityConnector::UpdateHandshakerFactoryLocked() { options_->cert_request_type(), grpc_get_tsi_tls_version(options_->min_tls_version()), grpc_get_tsi_tls_version(options_->max_tls_version()), - options_->crl_directory().c_str(), &server_handshaker_factory_); + &server_handshaker_factory_); /* Free memory. */ grpc_tsi_ssl_pem_key_cert_pairs_destroy(pem_key_cert_pairs, num_key_cert_pairs); diff --git a/src/core/tsi/ssl_transport_security.cc b/src/core/tsi/ssl_transport_security.cc index 0dfb1bb5172..a991aea79d6 100644 --- a/src/core/tsi/ssl_transport_security.cc +++ b/src/core/tsi/ssl_transport_security.cc @@ -1921,14 +1921,6 @@ static int server_handshaker_factory_new_session_callback( return 1; } -static int verify_cb(int ok, X509_STORE_CTX* ctx) { - int cert_error = X509_STORE_CTX_get_error(ctx); - if (cert_error != 0) { - gpr_log(GPR_ERROR, "Certificate verify failed with code %d", cert_error); - } - return ok; -} - /* --- tsi_ssl_handshaker_factory constructors. --- */ static tsi_ssl_handshaker_factory_vtable client_handshaker_factory_vtable = { @@ -2049,24 +2041,7 @@ tsi_result tsi_create_ssl_client_handshaker_factory_with_options( } else { SSL_CTX_set_verify(ssl_context, SSL_VERIFY_PEER, nullptr); } - -#if OPENSSL_VERSION_NUMBER >= 0x10100000 - if (options->crl_directory != nullptr && - strcmp(options->crl_directory, "") != 0) { - gpr_log(GPR_INFO, "enabling client CRL checking with path: %s", - options->crl_directory); - X509_STORE* cert_store = SSL_CTX_get_cert_store(ssl_context); - X509_STORE_set_verify_cb(cert_store, verify_cb); - if (!X509_STORE_load_locations(cert_store, nullptr, - options->crl_directory)) { - gpr_log(GPR_ERROR, "Failed to load CRL File from directory."); - } else { - X509_VERIFY_PARAM* param = X509_STORE_get0_param(cert_store); - X509_VERIFY_PARAM_set_flags(param, X509_V_FLAG_CRL_CHECK); - gpr_log(GPR_INFO, "enabled client side CRL checking."); - } - } -#endif + /* TODO(jboeuf): Add revocation verification. */ *factory = impl; return TSI_OK; @@ -2228,24 +2203,7 @@ tsi_result tsi_create_ssl_server_handshaker_factory_with_options( nullptr); break; } - -#if OPENSSL_VERSION_NUMBER >= 0x10100000 - if (options->crl_directory != nullptr && - strcmp(options->crl_directory, "") != 0) { - gpr_log(GPR_INFO, "enabling server CRL checking with path %s", - options->crl_directory); - X509_STORE* cert_store = SSL_CTX_get_cert_store(impl->ssl_contexts[i]); - X509_STORE_set_verify_cb(cert_store, verify_cb); - if (!X509_STORE_load_locations(cert_store, nullptr, - options->crl_directory)) { - gpr_log(GPR_ERROR, "Failed to load CRL File from directory."); - } else { - X509_VERIFY_PARAM* param = X509_STORE_get0_param(cert_store); - X509_VERIFY_PARAM_set_flags(param, X509_V_FLAG_CRL_CHECK); - gpr_log(GPR_INFO, "enabled server CRL checking."); - } - } -#endif + /* TODO(jboeuf): Add revocation verification. */ result = tsi_ssl_extract_x509_subject_names_from_pem_cert( options->pem_key_cert_pairs[i].cert_chain, diff --git a/src/core/tsi/ssl_transport_security.h b/src/core/tsi/ssl_transport_security.h index c911c9b5ea0..4b03e2ab912 100644 --- a/src/core/tsi/ssl_transport_security.h +++ b/src/core/tsi/ssl_transport_security.h @@ -157,12 +157,6 @@ struct tsi_ssl_client_handshaker_options { tsi_tls_version min_tls_version; tsi_tls_version max_tls_version; - /* The directory where all hashed CRL files enforced by the handshaker are - located. If the directory is invalid, CRL checking will fail open and just - log. An empty directory will not enable crl checking. Only OpenSSL version - > 1.1 is supported for CRL checking*/ - const char* crl_directory; - tsi_ssl_client_handshaker_options() : pem_key_cert_pair(nullptr), pem_root_certs(nullptr), @@ -173,8 +167,7 @@ struct tsi_ssl_client_handshaker_options { session_cache(nullptr), skip_server_certificate_verification(false), min_tls_version(tsi_tls_version::TSI_TLS1_2), - max_tls_version(tsi_tls_version::TSI_TLS1_3), - crl_directory(nullptr) {} + max_tls_version(tsi_tls_version::TSI_TLS1_3) {} }; /* Creates a client handshaker factory. @@ -294,12 +287,6 @@ struct tsi_ssl_server_handshaker_options { tsi_tls_version min_tls_version; tsi_tls_version max_tls_version; - /* The directory where all hashed CRL files are cached in the x.509 store and - * enforced by the handshaker are located. If the directory is invalid, CRL - * checking will fail open and just log. An empty directory will not enable - * crl checking. Only OpenSSL version > 1.1 is supported for CRL checking */ - const char* crl_directory; - tsi_ssl_server_handshaker_options() : pem_key_cert_pairs(nullptr), num_key_cert_pairs(0), @@ -311,8 +298,7 @@ struct tsi_ssl_server_handshaker_options { session_ticket_key(nullptr), session_ticket_key_size(0), min_tls_version(tsi_tls_version::TSI_TLS1_2), - max_tls_version(tsi_tls_version::TSI_TLS1_3), - crl_directory(nullptr) {} + max_tls_version(tsi_tls_version::TSI_TLS1_3) {} }; /* Creates a server handshaker factory. diff --git a/src/ruby/ext/grpc/rb_grpc_imports.generated.c b/src/ruby/ext/grpc/rb_grpc_imports.generated.c index 1f2e3748e74..7558727a68c 100644 --- a/src/ruby/ext/grpc/rb_grpc_imports.generated.c +++ b/src/ruby/ext/grpc/rb_grpc_imports.generated.c @@ -178,7 +178,6 @@ grpc_tls_credentials_options_set_root_cert_name_type grpc_tls_credentials_option grpc_tls_credentials_options_watch_identity_key_cert_pairs_type grpc_tls_credentials_options_watch_identity_key_cert_pairs_import; grpc_tls_credentials_options_set_identity_cert_name_type grpc_tls_credentials_options_set_identity_cert_name_import; grpc_tls_credentials_options_set_cert_request_type_type grpc_tls_credentials_options_set_cert_request_type_import; -grpc_tls_credentials_options_set_crl_directory_type grpc_tls_credentials_options_set_crl_directory_import; grpc_tls_credentials_options_set_verify_server_cert_type grpc_tls_credentials_options_set_verify_server_cert_import; grpc_tls_credentials_options_set_check_call_host_type grpc_tls_credentials_options_set_check_call_host_import; grpc_xds_credentials_create_type grpc_xds_credentials_create_import; @@ -467,7 +466,6 @@ void grpc_rb_load_imports(HMODULE library) { grpc_tls_credentials_options_watch_identity_key_cert_pairs_import = (grpc_tls_credentials_options_watch_identity_key_cert_pairs_type) GetProcAddress(library, "grpc_tls_credentials_options_watch_identity_key_cert_pairs"); grpc_tls_credentials_options_set_identity_cert_name_import = (grpc_tls_credentials_options_set_identity_cert_name_type) GetProcAddress(library, "grpc_tls_credentials_options_set_identity_cert_name"); 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_crl_directory_import = (grpc_tls_credentials_options_set_crl_directory_type) GetProcAddress(library, "grpc_tls_credentials_options_set_crl_directory"); grpc_tls_credentials_options_set_verify_server_cert_import = (grpc_tls_credentials_options_set_verify_server_cert_type) GetProcAddress(library, "grpc_tls_credentials_options_set_verify_server_cert"); grpc_tls_credentials_options_set_check_call_host_import = (grpc_tls_credentials_options_set_check_call_host_type) GetProcAddress(library, "grpc_tls_credentials_options_set_check_call_host"); grpc_xds_credentials_create_import = (grpc_xds_credentials_create_type) GetProcAddress(library, "grpc_xds_credentials_create"); diff --git a/src/ruby/ext/grpc/rb_grpc_imports.generated.h b/src/ruby/ext/grpc/rb_grpc_imports.generated.h index ba60c8aff35..a9535823f87 100644 --- a/src/ruby/ext/grpc/rb_grpc_imports.generated.h +++ b/src/ruby/ext/grpc/rb_grpc_imports.generated.h @@ -509,9 +509,6 @@ extern grpc_tls_credentials_options_set_identity_cert_name_type grpc_tls_credent typedef void(*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 void(*grpc_tls_credentials_options_set_crl_directory_type)(grpc_tls_credentials_options* options, const char* crl_directory); -extern grpc_tls_credentials_options_set_crl_directory_type grpc_tls_credentials_options_set_crl_directory_import; -#define grpc_tls_credentials_options_set_crl_directory grpc_tls_credentials_options_set_crl_directory_import typedef void(*grpc_tls_credentials_options_set_verify_server_cert_type)(grpc_tls_credentials_options* options, int verify_server_cert); extern grpc_tls_credentials_options_set_verify_server_cert_type grpc_tls_credentials_options_set_verify_server_cert_import; #define grpc_tls_credentials_options_set_verify_server_cert grpc_tls_credentials_options_set_verify_server_cert_import diff --git a/test/core/surface/public_headers_must_be_c89.c b/test/core/surface/public_headers_must_be_c89.c index 96ebc19c460..e4ab05b19b0 100644 --- a/test/core/surface/public_headers_must_be_c89.c +++ b/test/core/surface/public_headers_must_be_c89.c @@ -222,7 +222,6 @@ int main(int argc, char **argv) { printf("%lx", (unsigned long) grpc_tls_credentials_options_watch_identity_key_cert_pairs); printf("%lx", (unsigned long) grpc_tls_credentials_options_set_identity_cert_name); printf("%lx", (unsigned long) grpc_tls_credentials_options_set_cert_request_type); - printf("%lx", (unsigned long) grpc_tls_credentials_options_set_crl_directory); printf("%lx", (unsigned long) grpc_tls_credentials_options_set_verify_server_cert); printf("%lx", (unsigned long) grpc_tls_credentials_options_set_check_call_host); printf("%lx", (unsigned long) grpc_xds_credentials_create); diff --git a/test/core/tsi/BUILD b/test/core/tsi/BUILD index 5b341570914..14cf42afbc7 100644 --- a/test/core/tsi/BUILD +++ b/test/core/tsi/BUILD @@ -88,31 +88,6 @@ grpc_cc_test( ], ) -grpc_cc_test( - name = "crl_ssl_transport_security_test", - srcs = ["crl_ssl_transport_security_test.cc"], - data = [ - "//test/core/tsi/test_creds/crl_data:ab06acdd.r0", - "//test/core/tsi/test_creds/crl_data:ca.pem", - "//test/core/tsi/test_creds/crl_data:revoked.key", - "//test/core/tsi/test_creds/crl_data:revoked.pem", - "//test/core/tsi/test_creds/crl_data:valid.key", - "//test/core/tsi/test_creds/crl_data:valid.pem", - ], - external_deps = [ - "gtest", - ], - language = "C++", - tags = ["no_windows"], - deps = [ - ":transport_security_test_lib", - "//:gpr", - "//:grpc", - "//:tsi", - "//test/core/util:grpc_test_util", - ], -) - grpc_cc_test( name = "transport_security_test", srcs = ["transport_security_test.cc"], diff --git a/test/core/tsi/crl_ssl_transport_security_test.cc b/test/core/tsi/crl_ssl_transport_security_test.cc deleted file mode 100644 index 7fd0f56aa5b..00000000000 --- a/test/core/tsi/crl_ssl_transport_security_test.cc +++ /dev/null @@ -1,279 +0,0 @@ -// Copyright 2021 gRPC authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include -#include -#include - -#include -#include - -#include -#include -#include -#include - -#include "src/core/lib/iomgr/load_file.h" -#include "src/core/lib/security/security_connector/security_connector.h" -#include "src/core/tsi/ssl_transport_security.h" -#include "src/core/tsi/transport_security.h" -#include "src/core/tsi/transport_security_interface.h" -#include "test/core/tsi/transport_security_test_lib.h" -#include "test/core/util/test_config.h" - -extern "C" { -#include -#include -} - -namespace { - -const int kSslTsiTestRevokedKeyCertPairsNum = 1; -const int kSslTsiTestValidKeyCertPairsNum = 1; -const char* kSslTsiTestCrlSupportedCredentialsDir = - "test/core/tsi/test_creds/crl_data/"; - -class CrlSslTransportSecurityTest - : public testing::TestWithParam { - protected: - // A tsi_test_fixture implementation. - class SslTsiTestFixture { - public: - static SslTsiTestFixture* Create(bool use_revoked_server_cert, - bool use_revoked_client_cert) { - return new SslTsiTestFixture(use_revoked_server_cert, - use_revoked_client_cert); - } - - void Run() { - tsi_test_do_handshake(&base_); - tsi_test_fixture_destroy(&base_); - } - - private: - SslTsiTestFixture(bool use_revoked_server_cert, - bool use_revoked_client_cert) - : use_revoked_server_cert_(use_revoked_server_cert), - use_revoked_client_cert_(use_revoked_client_cert) { - tsi_test_fixture_init(&base_); - base_.test_unused_bytes = true; - base_.vtable = &kVtable; - // Load cert data. - revoked_pem_key_cert_pairs_ = static_cast( - gpr_malloc(sizeof(tsi_ssl_pem_key_cert_pair) * - kSslTsiTestRevokedKeyCertPairsNum)); - revoked_pem_key_cert_pairs_[0].private_key = LoadFile( - absl::StrCat(kSslTsiTestCrlSupportedCredentialsDir, "revoked.key")); - revoked_pem_key_cert_pairs_[0].cert_chain = LoadFile( - absl::StrCat(kSslTsiTestCrlSupportedCredentialsDir, "revoked.pem")); - valid_pem_key_cert_pairs_ = static_cast( - gpr_malloc(sizeof(tsi_ssl_pem_key_cert_pair) * - kSslTsiTestValidKeyCertPairsNum)); - valid_pem_key_cert_pairs_[0].private_key = LoadFile( - absl::StrCat(kSslTsiTestCrlSupportedCredentialsDir, "valid.key")); - valid_pem_key_cert_pairs_[0].cert_chain = LoadFile( - absl::StrCat(kSslTsiTestCrlSupportedCredentialsDir, "valid.pem")); - root_cert_ = LoadFile( - absl::StrCat(kSslTsiTestCrlSupportedCredentialsDir, "ca.pem")); - root_store_ = tsi_ssl_root_certs_store_create(root_cert_); - GPR_ASSERT(root_store_ != nullptr); - } - - ~SslTsiTestFixture() { - for (size_t i = 0; i < kSslTsiTestValidKeyCertPairsNum; i++) { - PemKeyCertPairDestroy(valid_pem_key_cert_pairs_[i]); - } - gpr_free(valid_pem_key_cert_pairs_); - for (size_t i = 0; i < kSslTsiTestRevokedKeyCertPairsNum; i++) { - PemKeyCertPairDestroy(revoked_pem_key_cert_pairs_[i]); - } - gpr_free(revoked_pem_key_cert_pairs_); - gpr_free(root_cert_); - tsi_ssl_root_certs_store_destroy(root_store_); - tsi_ssl_server_handshaker_factory_unref(server_handshaker_factory_); - tsi_ssl_client_handshaker_factory_unref(client_handshaker_factory_); - } - - static void SetupHandshakers(tsi_test_fixture* fixture) { - GPR_ASSERT(fixture != nullptr); - auto* self = reinterpret_cast(fixture); - self->SetupHandshakers(); - } - - void SetupHandshakers() { - // Create client handshaker factory. - tsi_ssl_client_handshaker_options client_options; - client_options.pem_root_certs = root_cert_; - if (use_revoked_client_cert_) { - client_options.pem_key_cert_pair = revoked_pem_key_cert_pairs_; - } else { - client_options.pem_key_cert_pair = valid_pem_key_cert_pairs_; - } - client_options.crl_directory = kSslTsiTestCrlSupportedCredentialsDir; - client_options.root_store = root_store_; - client_options.min_tls_version = GetParam(); - client_options.max_tls_version = GetParam(); - EXPECT_EQ(tsi_create_ssl_client_handshaker_factory_with_options( - &client_options, &client_handshaker_factory_), - TSI_OK); - // Create server handshaker factory. - tsi_ssl_server_handshaker_options server_options; - if (use_revoked_server_cert_) { - server_options.pem_key_cert_pairs = revoked_pem_key_cert_pairs_; - server_options.num_key_cert_pairs = kSslTsiTestRevokedKeyCertPairsNum; - } else { - server_options.pem_key_cert_pairs = valid_pem_key_cert_pairs_; - server_options.num_key_cert_pairs = kSslTsiTestValidKeyCertPairsNum; - } - server_options.pem_client_root_certs = root_cert_; - server_options.crl_directory = kSslTsiTestCrlSupportedCredentialsDir; - server_options.client_certificate_request = - TSI_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_AND_VERIFY; - server_options.session_ticket_key = nullptr; - server_options.session_ticket_key_size = 0; - server_options.min_tls_version = GetParam(); - server_options.max_tls_version = GetParam(); - EXPECT_EQ(tsi_create_ssl_server_handshaker_factory_with_options( - &server_options, &server_handshaker_factory_), - TSI_OK); - // Create server and client handshakers. - EXPECT_EQ( - tsi_ssl_client_handshaker_factory_create_handshaker( - client_handshaker_factory_, nullptr, &base_.client_handshaker), - TSI_OK); - EXPECT_EQ(tsi_ssl_server_handshaker_factory_create_handshaker( - server_handshaker_factory_, &base_.server_handshaker), - TSI_OK); - } - - static void CheckHandshakerPeers(tsi_test_fixture* fixture) { - GPR_ASSERT(fixture != nullptr); - auto* self = reinterpret_cast(fixture); - self->CheckHandshakerPeers(); - } - - void CheckHandshakerPeers() { - // In TLS 1.3, the client-side handshake succeeds even if the client - // sends a revoked certificate. In such a case, the server would fail - // the TLS handshake and send an alert to the client as the first - // application data message. In TLS 1.2, the client-side handshake will - // fail if the client sends a revoked certificate. - // - // For OpenSSL versions < 1.1, TLS 1.3 is not supported, so the - // client-side handshake should succeed precisely when the server-side - // handshake succeeds. - bool expect_server_success = - !(use_revoked_server_cert_ || use_revoked_client_cert_); -#if OPENSSL_VERSION_NUMBER >= 0x10100000 - bool expect_client_success = GetParam() == tsi_tls_version::TSI_TLS1_2 - ? expect_server_success - : !use_revoked_server_cert_; -#else - bool expect_client_success = expect_server_success; -#endif - tsi_peer peer; - if (expect_client_success) { - EXPECT_EQ( - tsi_handshaker_result_extract_peer(base_.client_result, &peer), - TSI_OK); - tsi_peer_destruct(&peer); - } else { - EXPECT_EQ(base_.client_result, nullptr); - } - if (expect_server_success) { - EXPECT_EQ( - tsi_handshaker_result_extract_peer(base_.server_result, &peer), - TSI_OK); - tsi_peer_destruct(&peer); - } else { - EXPECT_EQ(base_.server_result, nullptr); - } - } - - static void PemKeyCertPairDestroy(tsi_ssl_pem_key_cert_pair kp) { - gpr_free(const_cast(kp.private_key)); - gpr_free(const_cast(kp.cert_chain)); - } - - static void Destruct(tsi_test_fixture* fixture) { - auto* self = reinterpret_cast(fixture); - delete self; - } - - static char* LoadFile(absl::string_view file_path) { - grpc_slice slice; - GPR_ASSERT(grpc_load_file(file_path.data(), 1, &slice) == - GRPC_ERROR_NONE); - char* data = grpc_slice_to_c_string(slice); - grpc_slice_unref(slice); - return data; - } - - static struct tsi_test_fixture_vtable kVtable; - - tsi_test_fixture base_; - bool use_revoked_server_cert_; - bool use_revoked_client_cert_; - char* root_cert_; - tsi_ssl_root_certs_store* root_store_; - tsi_ssl_pem_key_cert_pair* revoked_pem_key_cert_pairs_; - tsi_ssl_pem_key_cert_pair* valid_pem_key_cert_pairs_; - tsi_ssl_server_handshaker_factory* server_handshaker_factory_; - tsi_ssl_client_handshaker_factory* client_handshaker_factory_; - }; -}; - -struct tsi_test_fixture_vtable - CrlSslTransportSecurityTest::SslTsiTestFixture::kVtable = { - &CrlSslTransportSecurityTest::SslTsiTestFixture::SetupHandshakers, - &CrlSslTransportSecurityTest::SslTsiTestFixture::CheckHandshakerPeers, - &CrlSslTransportSecurityTest::SslTsiTestFixture::Destruct}; - -TEST_P(CrlSslTransportSecurityTest, RevokedServerCert) { - auto* fixture = SslTsiTestFixture::Create(/*use_revoked_server_cert=*/true, - /*use_revoked_client_cert=*/false); - fixture->Run(); -} - -TEST_P(CrlSslTransportSecurityTest, RevokedClientCert) { - auto* fixture = SslTsiTestFixture::Create(/*use_revoked_server_cert=*/false, - /*use_revoked_client_cert=*/true); - fixture->Run(); -} - -TEST_P(CrlSslTransportSecurityTest, ValidCerts) { - auto* fixture = SslTsiTestFixture::Create(/*use_revoked_server_cert=*/false, - /*use_revoked_client_cert=*/false); - fixture->Run(); -} - -std::string TestNameSuffix( - const ::testing::TestParamInfo& version) { - if (version.param == tsi_tls_version::TSI_TLS1_2) return "TLS_1_2"; - GPR_ASSERT(version.param == tsi_tls_version::TSI_TLS1_3); - return "TLS_1_3"; -} - -INSTANTIATE_TEST_SUITE_P(TLSVersionsTest, CrlSslTransportSecurityTest, - testing::Values(tsi_tls_version::TSI_TLS1_2, - tsi_tls_version::TSI_TLS1_3), - &TestNameSuffix); - -} // namespace - -int main(int argc, char** argv) { - grpc::testing::TestEnvironment env(argc, argv); - ::testing::InitGoogleTest(&argc, argv); - return RUN_ALL_TESTS(); -} diff --git a/test/core/tsi/test_creds/crl_data/demoCA/index.txt.attr.old b/test/core/tsi/test_creds/crl_data/demoCA/index.txt.attr.old deleted file mode 100644 index 8f7e63a3475..00000000000 --- a/test/core/tsi/test_creds/crl_data/demoCA/index.txt.attr.old +++ /dev/null @@ -1 +0,0 @@ -unique_subject = yes diff --git a/test/core/tsi/test_creds/demoCA/crlnumber b/test/core/tsi/test_creds/demoCA/crlnumber deleted file mode 100644 index dd11724042e..00000000000 --- a/test/core/tsi/test_creds/demoCA/crlnumber +++ /dev/null @@ -1 +0,0 @@ -1001 diff --git a/test/core/tsi/test_creds/demoCA/index.txt b/test/core/tsi/test_creds/demoCA/index.txt deleted file mode 100644 index e184fd7b63e..00000000000 --- a/test/core/tsi/test_creds/demoCA/index.txt +++ /dev/null @@ -1 +0,0 @@ -R 310920052448Z 211102205058Z 8B24B0063265547C unknown /CN=revoked diff --git a/test/core/tsi/test_creds/demoCA/index.txt.attr b/test/core/tsi/test_creds/demoCA/index.txt.attr deleted file mode 100644 index 8f7e63a3475..00000000000 --- a/test/core/tsi/test_creds/demoCA/index.txt.attr +++ /dev/null @@ -1 +0,0 @@ -unique_subject = yes diff --git a/test/core/tsi/test_creds/demoCA/index.txt.old b/test/core/tsi/test_creds/demoCA/index.txt.old deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/test/core/tsi/transport_security_test_lib.cc b/test/core/tsi/transport_security_test_lib.cc index 95ddfb9aafe..875919ca984 100644 --- a/test/core/tsi/transport_security_test_lib.cc +++ b/test/core/tsi/transport_security_test_lib.cc @@ -605,7 +605,6 @@ static void tsi_test_channel_destroy(tsi_test_channel* channel) { } void tsi_test_fixture_init(tsi_test_fixture* fixture) { - memset(fixture, 0, sizeof(tsi_test_fixture)); fixture->config = tsi_test_frame_protector_config_create( true, true, true, true, true, true, true); fixture->handshake_buffer_size = TSI_TEST_DEFAULT_BUFFER_SIZE; diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index 08c0a8bd725..3e8c641d75d 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -3951,28 +3951,6 @@ ], "uses_polling": false }, - { - "args": [], - "benchmark": false, - "ci_platforms": [ - "linux", - "mac", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "gtest": true, - "language": "c++", - "name": "crl_ssl_transport_security_test", - "platforms": [ - "linux", - "mac", - "posix" - ], - "uses_polling": true - }, { "args": [], "benchmark": false,