From 7af5efcfd33da23e237ae09eb8ac9f60711f08aa Mon Sep 17 00:00:00 2001 From: apolcyn Date: Tue, 17 Oct 2023 13:53:36 -0700 Subject: [PATCH] Revert "[TLS - Revocation] Crl Provider (#33786)" (#34713) This reverts commit 0f0396ae928d559266dfa2e096f3f24b3e4ab0e4. --- BUILD | 4 - CMakeLists.txt | 91 ----- Makefile | 4 - Package.swift | 3 - build_autogenerated.yaml | 33 -- config.m4 | 1 - config.w32 | 1 - gRPC-C++.podspec | 3 - gRPC-Core.podspec | 4 - grpc.gemspec | 3 - grpc.gyp | 1 - include/grpc/grpc_crl_provider.h | 85 ---- include/grpc/module.modulemap | 1 - .../grpcpp/security/tls_credentials_options.h | 3 - include/grpcpp/security/tls_crl_provider.h | 39 -- package.xml | 3 - src/core/BUILD | 23 -- .../tls/grpc_tls_credentials_options.cc | 8 - .../tls/grpc_tls_credentials_options.h | 5 - .../credentials/tls/grpc_tls_crl_provider.cc | 118 ------ .../credentials/tls/grpc_tls_crl_provider.h | 83 ---- .../credentials/tls/tls_credentials.cc | 9 - .../security/security_connector/ssl_utils.cc | 5 - .../security/security_connector/ssl_utils.h | 4 - .../tls/tls_security_connector.cc | 5 +- src/core/tsi/ssl_transport_security.cc | 122 +----- src/core/tsi/ssl_transport_security.h | 20 +- src/cpp/common/tls_credentials_options.cc | 8 - src/cpp/server/secure_server_credentials.cc | 9 +- src/python/grpcio/grpc_core_dependencies.py | 1 - test/core/security/BUILD | 16 - ...tls_credentials_options_comparator_test.cc | 10 - .../grpc_tls_credentials_options_test.cc | 38 -- .../security/grpc_tls_crl_provider_test.cc | 95 ----- test/core/tsi/BUILD | 2 - .../tsi/crl_ssl_transport_security_test.cc | 384 ++++++++---------- test/core/tsi/test_creds/crl_data/crls/BUILD | 2 - test/core/util/tls_utils.cc | 5 +- test/cpp/client/BUILD | 1 - test/cpp/client/credentials_test.cc | 25 -- test/cpp/end2end/BUILD | 27 -- test/cpp/end2end/crl_provider_test.cc | 221 ---------- test/cpp/server/BUILD | 1 - test/cpp/server/credentials_test.cc | 30 -- .../core/gen_grpc_tls_credentials_options.py | 10 - tools/distrib/fix_build_deps.py | 1 - tools/doxygen/Doxyfile.c++ | 2 - tools/doxygen/Doxyfile.c++.internal | 4 - tools/doxygen/Doxyfile.core | 1 - tools/doxygen/Doxyfile.core.internal | 3 - tools/run_tests/generated/tests.json | 48 --- 51 files changed, 199 insertions(+), 1426 deletions(-) delete mode 100644 include/grpc/grpc_crl_provider.h delete mode 100644 include/grpcpp/security/tls_crl_provider.h delete mode 100644 src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc delete mode 100644 src/core/lib/security/credentials/tls/grpc_tls_crl_provider.h delete mode 100644 test/core/security/grpc_tls_crl_provider_test.cc delete mode 100644 test/cpp/end2end/crl_provider_test.cc diff --git a/BUILD b/BUILD index 4b6304b76f3..3ee434b96f6 100644 --- a/BUILD +++ b/BUILD @@ -254,7 +254,6 @@ GPR_PUBLIC_HDRS = [ GRPC_PUBLIC_HDRS = [ "include/grpc/grpc_audit_logging.h", - "include/grpc/grpc_crl_provider.h", "include/grpc/byte_buffer.h", "include/grpc/byte_buffer_reader.h", "include/grpc/compression.h", @@ -422,7 +421,6 @@ GRPCXX_PUBLIC_HDRS = [ "include/grpcpp/impl/sync.h", "include/grpcpp/resource_quota.h", "include/grpcpp/security/audit_logging.h", - "include/grpcpp/security/tls_crl_provider.h", "include/grpcpp/security/auth_context.h", "include/grpcpp/security/auth_metadata_processor.h", "include/grpcpp/security/credentials.h", @@ -1968,7 +1966,6 @@ grpc_cc_library( "//src/core:gpr_manual_constructor", "//src/core:grpc_audit_logging", "//src/core:grpc_backend_metric_provider", - "//src/core:grpc_crl_provider", "//src/core:grpc_service_config", "//src/core:grpc_transport_inproc", "//src/core:json", @@ -3558,7 +3555,6 @@ grpc_cc_library( "tsi_ssl_session_cache", "//src/core:channel_args", "//src/core:error", - "//src/core:grpc_crl_provider", "//src/core:grpc_transport_chttp2_alpn", "//src/core:ref_counted", "//src/core:slice", diff --git a/CMakeLists.txt b/CMakeLists.txt index dc45fc53edb..27750c4ad36 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -969,7 +969,6 @@ if(gRPC_BUILD_TESTS) add_dependencies(buildtests_cxx core_configuration_test) add_dependencies(buildtests_cxx cpp_impl_of_test) add_dependencies(buildtests_cxx cpu_test) - add_dependencies(buildtests_cxx crl_provider_test) if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) add_dependencies(buildtests_cxx crl_ssl_transport_security_test) endif() @@ -1057,7 +1056,6 @@ if(gRPC_BUILD_TESTS) add_dependencies(buildtests_cxx grpc_tls_certificate_verifier_test) add_dependencies(buildtests_cxx grpc_tls_credentials_options_comparator_test) add_dependencies(buildtests_cxx grpc_tls_credentials_options_test) - add_dependencies(buildtests_cxx grpc_tls_crl_provider_test) if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_POSIX) add_dependencies(buildtests_cxx grpc_tool_test) endif() @@ -2422,7 +2420,6 @@ add_library(grpc src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.cc src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.cc src/core/lib/security/credentials/tls/grpc_tls_credentials_options.cc - src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc src/core/lib/security/credentials/tls/tls_credentials.cc src/core/lib/security/credentials/tls/tls_utils.cc src/core/lib/security/credentials/xds/xds_credentials.cc @@ -2602,7 +2599,6 @@ foreach(_hdr include/grpc/fork.h include/grpc/grpc.h include/grpc/grpc_audit_logging.h - include/grpc/grpc_crl_provider.h include/grpc/grpc_posix.h include/grpc/grpc_security.h include/grpc/grpc_security_constants.h @@ -3268,7 +3264,6 @@ foreach(_hdr include/grpc/fork.h include/grpc/grpc.h include/grpc/grpc_audit_logging.h - include/grpc/grpc_crl_provider.h include/grpc/grpc_posix.h include/grpc/grpc_security.h include/grpc/grpc_security_constants.h @@ -4123,7 +4118,6 @@ foreach(_hdr include/grpcpp/security/tls_certificate_provider.h include/grpcpp/security/tls_certificate_verifier.h include/grpcpp/security/tls_credentials_options.h - include/grpcpp/security/tls_crl_provider.h include/grpcpp/server.h include/grpcpp/server_builder.h include/grpcpp/server_context.h @@ -4815,7 +4809,6 @@ foreach(_hdr include/grpcpp/security/tls_certificate_provider.h include/grpcpp/security/tls_certificate_verifier.h include/grpcpp/security/tls_credentials_options.h - include/grpcpp/security/tls_crl_provider.h include/grpcpp/server.h include/grpcpp/server_builder.h include/grpcpp/server_context.h @@ -5200,7 +5193,6 @@ foreach(_hdr include/grpc/fork.h include/grpc/grpc.h include/grpc/grpc_audit_logging.h - include/grpc/grpc_crl_provider.h include/grpc/grpc_posix.h include/grpc/grpc_security.h include/grpc/grpc_security_constants.h @@ -10048,56 +10040,6 @@ target_link_libraries(cpu_test ) -endif() -if(gRPC_BUILD_TESTS) - -add_executable(crl_provider_test - ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo.pb.cc - ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo.grpc.pb.cc - ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo.pb.h - ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo.grpc.pb.h - ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo_messages.pb.cc - ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo_messages.grpc.pb.cc - ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo_messages.pb.h - ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/echo_messages.grpc.pb.h - ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/simple_messages.pb.cc - ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/simple_messages.grpc.pb.cc - ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/simple_messages.pb.h - ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/simple_messages.grpc.pb.h - ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/orca_load_report.pb.cc - ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/orca_load_report.grpc.pb.cc - ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/orca_load_report.pb.h - ${_gRPC_PROTO_GENS_DIR}/src/proto/grpc/testing/xds/v3/orca_load_report.grpc.pb.h - test/cpp/end2end/crl_provider_test.cc - test/cpp/end2end/test_service_impl.cc -) -target_compile_features(crl_provider_test PUBLIC cxx_std_14) -target_include_directories(crl_provider_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_provider_test - ${_gRPC_ALLTARGETS_LIBRARIES} - gtest - grpc++_test_util -) - - endif() if(gRPC_BUILD_TESTS) if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) @@ -13419,39 +13361,6 @@ target_link_libraries(grpc_tls_credentials_options_test ) -endif() -if(gRPC_BUILD_TESTS) - -add_executable(grpc_tls_crl_provider_test - test/core/security/grpc_tls_crl_provider_test.cc -) -target_compile_features(grpc_tls_crl_provider_test PUBLIC cxx_std_14) -target_include_directories(grpc_tls_crl_provider_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(grpc_tls_crl_provider_test - ${_gRPC_ALLTARGETS_LIBRARIES} - gtest - grpc_test_util -) - - endif() if(gRPC_BUILD_TESTS) if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_POSIX) diff --git a/Makefile b/Makefile index cab8c8dbe46..146bd9c29dc 100644 --- a/Makefile +++ b/Makefile @@ -1638,7 +1638,6 @@ LIBGRPC_SRC = \ src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.cc \ src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.cc \ src/core/lib/security/credentials/tls/grpc_tls_credentials_options.cc \ - src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc \ src/core/lib/security/credentials/tls/tls_credentials.cc \ src/core/lib/security/credentials/tls/tls_utils.cc \ src/core/lib/security/credentials/xds/xds_credentials.cc \ @@ -1753,7 +1752,6 @@ PUBLIC_HEADERS_C += \ include/grpc/fork.h \ include/grpc/grpc.h \ include/grpc/grpc_audit_logging.h \ - include/grpc/grpc_crl_provider.h \ include/grpc/grpc_posix.h \ include/grpc/grpc_security.h \ include/grpc/grpc_security_constants.h \ @@ -2272,7 +2270,6 @@ PUBLIC_HEADERS_C += \ include/grpc/fork.h \ include/grpc/grpc.h \ include/grpc/grpc_audit_logging.h \ - include/grpc/grpc_crl_provider.h \ include/grpc/grpc_posix.h \ include/grpc/grpc_security.h \ include/grpc/grpc_security_constants.h \ @@ -3692,7 +3689,6 @@ src/core/lib/security/credentials/tls/grpc_tls_certificate_match.cc: $(OPENSSL_D src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.cc: $(OPENSSL_DEP) src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.cc: $(OPENSSL_DEP) src/core/lib/security/credentials/tls/grpc_tls_credentials_options.cc: $(OPENSSL_DEP) -src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc: $(OPENSSL_DEP) src/core/lib/security/credentials/tls/tls_credentials.cc: $(OPENSSL_DEP) src/core/lib/security/credentials/xds/xds_credentials.cc: $(OPENSSL_DEP) src/core/lib/security/security_connector/alts/alts_security_connector.cc: $(OPENSSL_DEP) diff --git a/Package.swift b/Package.swift index 6aade7c00e0..a9f12191936 100644 --- a/Package.swift +++ b/Package.swift @@ -57,7 +57,6 @@ let package = Package( "include/grpc/fork.h", "include/grpc/grpc.h", "include/grpc/grpc_audit_logging.h", - "include/grpc/grpc_crl_provider.h", "include/grpc/grpc_posix.h", "include/grpc/grpc_security.h", "include/grpc/grpc_security_constants.h", @@ -1595,8 +1594,6 @@ let package = Package( "src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.h", "src/core/lib/security/credentials/tls/grpc_tls_credentials_options.cc", "src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h", - "src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc", - "src/core/lib/security/credentials/tls/grpc_tls_crl_provider.h", "src/core/lib/security/credentials/tls/tls_credentials.cc", "src/core/lib/security/credentials/tls/tls_credentials.h", "src/core/lib/security/credentials/tls/tls_utils.cc", diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index c7da3c6c396..cf2ee4a4c3b 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -158,7 +158,6 @@ libs: - include/grpc/fork.h - include/grpc/grpc.h - include/grpc/grpc_audit_logging.h - - include/grpc/grpc_crl_provider.h - include/grpc/grpc_posix.h - include/grpc/grpc_security.h - include/grpc/grpc_security_constants.h @@ -937,7 +936,6 @@ libs: - src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.h - src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.h - src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h - - src/core/lib/security/credentials/tls/grpc_tls_crl_provider.h - src/core/lib/security/credentials/tls/tls_credentials.h - src/core/lib/security/credentials/tls/tls_utils.h - src/core/lib/security/credentials/xds/xds_credentials.h @@ -1723,7 +1721,6 @@ libs: - src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.cc - src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.cc - src/core/lib/security/credentials/tls/grpc_tls_credentials_options.cc - - src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc - src/core/lib/security/credentials/tls/tls_credentials.cc - src/core/lib/security/credentials/tls/tls_utils.cc - src/core/lib/security/credentials/xds/xds_credentials.cc @@ -1923,7 +1920,6 @@ libs: - include/grpc/fork.h - include/grpc/grpc.h - include/grpc/grpc_audit_logging.h - - include/grpc/grpc_crl_provider.h - include/grpc/grpc_posix.h - include/grpc/grpc_security.h - include/grpc/grpc_security_constants.h @@ -3533,7 +3529,6 @@ libs: - include/grpcpp/security/tls_certificate_provider.h - include/grpcpp/security/tls_certificate_verifier.h - include/grpcpp/security/tls_credentials_options.h - - include/grpcpp/security/tls_crl_provider.h - include/grpcpp/server.h - include/grpcpp/server_builder.h - include/grpcpp/server_context.h @@ -3957,7 +3952,6 @@ libs: - include/grpcpp/security/tls_certificate_provider.h - include/grpcpp/security/tls_certificate_verifier.h - include/grpcpp/security/tls_credentials_options.h - - include/grpcpp/security/tls_crl_provider.h - include/grpcpp/server.h - include/grpcpp/server_builder.h - include/grpcpp/server_context.h @@ -4056,7 +4050,6 @@ libs: - include/grpc/fork.h - include/grpc/grpc.h - include/grpc/grpc_audit_logging.h - - include/grpc/grpc_crl_provider.h - include/grpc/grpc_posix.h - include/grpc/grpc_security.h - include/grpc/grpc_security_constants.h @@ -7420,22 +7413,6 @@ targets: - gtest - grpc_test_util uses_polling: false -- name: crl_provider_test - gtest: true - build: test - language: c++ - headers: - - test/cpp/end2end/test_service_impl.h - src: - - src/proto/grpc/testing/echo.proto - - src/proto/grpc/testing/echo_messages.proto - - src/proto/grpc/testing/simple_messages.proto - - src/proto/grpc/testing/xds/v3/orca_load_report.proto - - test/cpp/end2end/crl_provider_test.cc - - test/cpp/end2end/test_service_impl.cc - deps: - - gtest - - grpc++_test_util - name: crl_ssl_transport_security_test gtest: true build: test @@ -9460,16 +9437,6 @@ targets: deps: - gtest - grpc_test_util -- name: grpc_tls_crl_provider_test - gtest: true - build: test - language: c++ - headers: [] - src: - - test/core/security/grpc_tls_crl_provider_test.cc - deps: - - gtest - - grpc_test_util - name: grpc_tool_test gtest: true build: test diff --git a/config.m4 b/config.m4 index dbe3f16207e..75356b61be2 100644 --- a/config.m4 +++ b/config.m4 @@ -771,7 +771,6 @@ if test "$PHP_GRPC" != "no"; then src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.cc \ src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.cc \ src/core/lib/security/credentials/tls/grpc_tls_credentials_options.cc \ - src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc \ src/core/lib/security/credentials/tls/tls_credentials.cc \ src/core/lib/security/credentials/tls/tls_utils.cc \ src/core/lib/security/credentials/xds/xds_credentials.cc \ diff --git a/config.w32 b/config.w32 index 9ff76d86af3..f7c2c73cc12 100644 --- a/config.w32 +++ b/config.w32 @@ -736,7 +736,6 @@ if (PHP_GRPC != "no") { "src\\core\\lib\\security\\credentials\\tls\\grpc_tls_certificate_provider.cc " + "src\\core\\lib\\security\\credentials\\tls\\grpc_tls_certificate_verifier.cc " + "src\\core\\lib\\security\\credentials\\tls\\grpc_tls_credentials_options.cc " + - "src\\core\\lib\\security\\credentials\\tls\\grpc_tls_crl_provider.cc " + "src\\core\\lib\\security\\credentials\\tls\\tls_credentials.cc " + "src\\core\\lib\\security\\credentials\\tls\\tls_utils.cc " + "src\\core\\lib\\security\\credentials\\xds\\xds_credentials.cc " + diff --git a/gRPC-C++.podspec b/gRPC-C++.podspec index aa2cefb5fae..2b1542eb973 100644 --- a/gRPC-C++.podspec +++ b/gRPC-C++.podspec @@ -179,7 +179,6 @@ Pod::Spec.new do |s| 'include/grpcpp/security/tls_certificate_provider.h', 'include/grpcpp/security/tls_certificate_verifier.h', 'include/grpcpp/security/tls_credentials_options.h', - 'include/grpcpp/security/tls_crl_provider.h', 'include/grpcpp/server.h', 'include/grpcpp/server_builder.h', 'include/grpcpp/server_context.h', @@ -1034,7 +1033,6 @@ Pod::Spec.new do |s| 'src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.h', 'src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.h', 'src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h', - 'src/core/lib/security/credentials/tls/grpc_tls_crl_provider.h', 'src/core/lib/security/credentials/tls/tls_credentials.h', 'src/core/lib/security/credentials/tls/tls_utils.h', 'src/core/lib/security/credentials/xds/xds_credentials.h', @@ -2109,7 +2107,6 @@ Pod::Spec.new do |s| 'src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.h', 'src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.h', 'src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h', - 'src/core/lib/security/credentials/tls/grpc_tls_crl_provider.h', 'src/core/lib/security/credentials/tls/tls_credentials.h', 'src/core/lib/security/credentials/tls/tls_utils.h', 'src/core/lib/security/credentials/xds/xds_credentials.h', diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 19288cf41f9..b97c9cbc3ab 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -123,7 +123,6 @@ Pod::Spec.new do |s| 'include/grpc/fork.h', 'include/grpc/grpc.h', 'include/grpc/grpc_audit_logging.h', - 'include/grpc/grpc_crl_provider.h', 'include/grpc/grpc_posix.h', 'include/grpc/grpc_security.h', 'include/grpc/grpc_security_constants.h', @@ -1694,8 +1693,6 @@ Pod::Spec.new do |s| 'src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.h', 'src/core/lib/security/credentials/tls/grpc_tls_credentials_options.cc', 'src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h', - 'src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc', - 'src/core/lib/security/credentials/tls/grpc_tls_crl_provider.h', 'src/core/lib/security/credentials/tls/tls_credentials.cc', 'src/core/lib/security/credentials/tls/tls_credentials.h', 'src/core/lib/security/credentials/tls/tls_utils.cc', @@ -2868,7 +2865,6 @@ Pod::Spec.new do |s| 'src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.h', 'src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.h', 'src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h', - 'src/core/lib/security/credentials/tls/grpc_tls_crl_provider.h', 'src/core/lib/security/credentials/tls/tls_credentials.h', 'src/core/lib/security/credentials/tls/tls_utils.h', 'src/core/lib/security/credentials/xds/xds_credentials.h', diff --git a/grpc.gemspec b/grpc.gemspec index 6bbb9e64934..d59295177e4 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -63,7 +63,6 @@ Gem::Specification.new do |s| s.files += %w( include/grpc/fork.h ) s.files += %w( include/grpc/grpc.h ) s.files += %w( include/grpc/grpc_audit_logging.h ) - s.files += %w( include/grpc/grpc_crl_provider.h ) s.files += %w( include/grpc/grpc_posix.h ) s.files += %w( include/grpc/grpc_security.h ) s.files += %w( include/grpc/grpc_security_constants.h ) @@ -1597,8 +1596,6 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.h ) s.files += %w( src/core/lib/security/credentials/tls/grpc_tls_credentials_options.cc ) s.files += %w( src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h ) - s.files += %w( src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc ) - s.files += %w( src/core/lib/security/credentials/tls/grpc_tls_crl_provider.h ) s.files += %w( src/core/lib/security/credentials/tls/tls_credentials.cc ) s.files += %w( src/core/lib/security/credentials/tls/tls_credentials.h ) s.files += %w( src/core/lib/security/credentials/tls/tls_utils.cc ) diff --git a/grpc.gyp b/grpc.gyp index 82e9a2e73cb..e14da93b460 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -956,7 +956,6 @@ 'src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.cc', 'src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.cc', 'src/core/lib/security/credentials/tls/grpc_tls_credentials_options.cc', - 'src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc', 'src/core/lib/security/credentials/tls/tls_credentials.cc', 'src/core/lib/security/credentials/tls/tls_utils.cc', 'src/core/lib/security/credentials/xds/xds_credentials.cc', diff --git a/include/grpc/grpc_crl_provider.h b/include/grpc/grpc_crl_provider.h deleted file mode 100644 index a66e15e62ab..00000000000 --- a/include/grpc/grpc_crl_provider.h +++ /dev/null @@ -1,85 +0,0 @@ -// -// -// Copyright 2023 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. -// -// - -#ifndef GRPC_GRPC_CRL_PROVIDER_H -#define GRPC_GRPC_CRL_PROVIDER_H - -#include - -#include -#include - -#include "absl/status/statusor.h" -#include "absl/strings/string_view.h" - -#include -#include - -namespace grpc_core { -namespace experimental { - -// Opaque representation of a CRL. Must be thread safe. -class Crl { - public: - static absl::StatusOr> Parse( - absl::string_view crl_string); - virtual ~Crl() = default; - virtual absl::string_view Issuer() = 0; -}; - -// Information about a certificate to be used to fetch its associated CRL. Must -// be thread safe. -class CertificateInfo { - public: - virtual ~CertificateInfo() = default; - virtual absl::string_view Issuer() const = 0; -}; - -// The base class for CRL Provider implementations. -// CrlProviders can be passed in as a way to supply CRLs during handshakes. -// CrlProviders must be thread safe. They are on the critical path of gRPC -// creating a connection and doing a handshake, so the implementation of -// `GetCrl` should be very fast. It is suggested to have an in-memory map of -// CRLs for quick lookup and return, and doing expensive updates to this map -// asynchronously. -class CrlProvider { - public: - virtual ~CrlProvider() = default; - // Get the CRL associated with a certificate. Read-only. - virtual std::shared_ptr GetCrl( - const CertificateInfo& certificate_info) = 0; -}; - -absl::StatusOr> CreateStaticCrlProvider( - absl::Span crls); - -} // namespace experimental -} // namespace grpc_core - -// TODO(gtcooke94) - Mark with api macro when all wrapped langauges support C++ -// in core APIs -/** - * EXPERIMENTAL API - Subject to change - * - * Sets the crl provider in the options. - */ -void grpc_tls_credentials_options_set_crl_provider( - grpc_tls_credentials_options* options, - std::shared_ptr provider); - -#endif /* GRPC_GRPC_CRL_PROVIDER_H */ diff --git a/include/grpc/module.modulemap b/include/grpc/module.modulemap index 29c6508a5cb..a37f5dcaafc 100644 --- a/include/grpc/module.modulemap +++ b/include/grpc/module.modulemap @@ -9,7 +9,6 @@ header "byte_buffer.h" header "fork.h" header "grpc.h" header "grpc_audit_logging.h" - header "grpc_crl_provider.h" header "grpc_posix.h" header "grpc_security.h" header "grpc_security_constants.h" diff --git a/include/grpcpp/security/tls_credentials_options.h b/include/grpcpp/security/tls_credentials_options.h index 7f8ff150910..da5620f8057 100644 --- a/include/grpcpp/security/tls_credentials_options.h +++ b/include/grpcpp/security/tls_credentials_options.h @@ -28,7 +28,6 @@ #include #include #include -#include #include namespace grpc { @@ -105,8 +104,6 @@ class TlsCredentialsOptions { // version > 1.1. void set_crl_directory(const std::string& path); - void set_crl_provider(std::shared_ptr crl_provider); - // ----- Getters for member fields ---- // Get the internal c options. This function shall be used only internally. grpc_tls_credentials_options* c_credentials_options() const { diff --git a/include/grpcpp/security/tls_crl_provider.h b/include/grpcpp/security/tls_crl_provider.h deleted file mode 100644 index 1408327457b..00000000000 --- a/include/grpcpp/security/tls_crl_provider.h +++ /dev/null @@ -1,39 +0,0 @@ -// -// -// Copyright 2023 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. -// -// - -#ifndef GRPCPP_SECURITY_TLS_CRL_PROVIDER_H -#define GRPCPP_SECURITY_TLS_CRL_PROVIDER_H - -#include -#include -#include - -namespace grpc { -namespace experimental { - -using grpc_core::experimental:: - CertificateInfo; // NOLINT(misc-unused-using-decls) -using grpc_core::experimental:: - CreateStaticCrlProvider; // NOLINT(misc-unused-using-decls) -using grpc_core::experimental::Crl; // NOLINT(misc-unused-using-decls) -using grpc_core::experimental::CrlProvider; // NOLINT(misc-unused-using-decls) - -} // namespace experimental -} // namespace grpc - -#endif // GRPCPP_SECURITY_TLS_CRL_PROVIDER_H diff --git a/package.xml b/package.xml index 5169be567da..b5942950107 100644 --- a/package.xml +++ b/package.xml @@ -45,7 +45,6 @@ - @@ -1579,8 +1578,6 @@ - - diff --git a/src/core/BUILD b/src/core/BUILD index b04251a2e16..ed9a9325d94 100644 --- a/src/core/BUILD +++ b/src/core/BUILD @@ -3072,29 +3072,6 @@ grpc_cc_library( ], ) -grpc_cc_library( - name = "grpc_crl_provider", - srcs = [ - "lib/security/credentials/tls/grpc_tls_crl_provider.cc", - ], - hdrs = [ - "lib/security/credentials/tls/grpc_tls_crl_provider.h", - ], - external_deps = [ - "absl/container:flat_hash_map", - "absl/status", - "absl/status:statusor", - "absl/strings", - "absl/types:span", - "libcrypto", - "libssl", - ], - deps = [ - "//:gpr", - "//:grpc_base", - ], -) - grpc_cc_library( name = "grpc_fake_credentials", srcs = [ 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 5e314700428..57f753b974c 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 @@ -22,7 +22,6 @@ #include -#include #include #include "src/core/lib/debug/trace.h" @@ -131,10 +130,3 @@ void grpc_tls_credentials_options_set_send_client_ca_list( } options->set_send_client_ca_list(send_client_ca_list); } - -void grpc_tls_credentials_options_set_crl_provider( - grpc_tls_credentials_options* options, - std::shared_ptr provider) { - GPR_ASSERT(options != nullptr); - options->set_crl_provider(std::move(provider)); -} 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 0994fb2ff91..d0f451ed6e3 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 @@ -61,8 +61,6 @@ struct grpc_tls_credentials_options const std::string& identity_cert_name() const { return identity_cert_name_; } const std::string& tls_session_key_log_file_path() const { return tls_session_key_log_file_path_; } const std::string& crl_directory() const { return crl_directory_; } - // Returns the CRL Provider - std::shared_ptr crl_provider() const { return crl_provider_; } bool send_client_ca_list() const { return send_client_ca_list_; } // Setters for member fields. @@ -84,7 +82,6 @@ struct grpc_tls_credentials_options void set_tls_session_key_log_file_path(std::string tls_session_key_log_file_path) { tls_session_key_log_file_path_ = std::move(tls_session_key_log_file_path); } // 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 crl_directory) { crl_directory_ = std::move(crl_directory); } - void set_crl_provider(std::shared_ptr crl_provider) { crl_provider_ = std::move(crl_provider); } void set_send_client_ca_list(bool send_client_ca_list) { send_client_ca_list_ = send_client_ca_list; } bool operator==(const grpc_tls_credentials_options& other) const { @@ -101,7 +98,6 @@ struct grpc_tls_credentials_options identity_cert_name_ == other.identity_cert_name_ && tls_session_key_log_file_path_ == other.tls_session_key_log_file_path_ && crl_directory_ == other.crl_directory_ && - (crl_provider_ == other.crl_provider_) && send_client_ca_list_ == other.send_client_ca_list_; } @@ -119,7 +115,6 @@ struct grpc_tls_credentials_options std::string identity_cert_name_; std::string tls_session_key_log_file_path_; std::string crl_directory_; - std::shared_ptr crl_provider_; bool send_client_ca_list_ = false; }; diff --git a/src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc b/src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc deleted file mode 100644 index ff25d73c28c..00000000000 --- a/src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc +++ /dev/null @@ -1,118 +0,0 @@ -// -// -// Copyright 2023 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 "src/core/lib/security/credentials/tls/grpc_tls_crl_provider.h" - -#include - -#include -#include - -// IWYU pragma: no_include -#include -#include // IWYU pragma: keep -#include -#include - -#include "absl/container/flat_hash_map.h" -#include "absl/status/status.h" -#include "absl/status/statusor.h" -#include "absl/strings/str_cat.h" -#include "absl/types/span.h" - -#include - -namespace grpc_core { -namespace experimental { - -namespace { -std::string IssuerFromCrl(X509_CRL* crl) { - char* buf = X509_NAME_oneline(X509_CRL_get_issuer(crl), nullptr, 0); - std::string ret; - if (buf != nullptr) { - ret = buf; - } - OPENSSL_free(buf); - return ret; -} - -} // namespace - -absl::StatusOr> Crl::Parse(absl::string_view crl_string) { - if (crl_string.size() >= INT_MAX) { - return absl::InvalidArgumentError("crl_string cannot be of size INT_MAX"); - } - BIO* crl_bio = - BIO_new_mem_buf(crl_string.data(), static_cast(crl_string.size())); - // Errors on BIO - if (crl_bio == nullptr) { - return absl::InvalidArgumentError( - "Conversion from crl string to BIO failed."); - } - X509_CRL* crl = PEM_read_bio_X509_CRL(crl_bio, nullptr, nullptr, nullptr); - BIO_free(crl_bio); - if (crl == nullptr) { - return absl::InvalidArgumentError( - "Conversion from PEM string to X509 CRL failed."); - } - return CrlImpl::Create(crl); -} - -absl::StatusOr> CrlImpl::Create(X509_CRL* crl) { - std::string issuer = IssuerFromCrl(crl); - if (issuer.empty()) { - return absl::InvalidArgumentError("Issuer of crl cannot be empty"); - } - return std::make_unique(crl, issuer); -} - -CrlImpl::~CrlImpl() { X509_CRL_free(crl_); } - -absl::StatusOr> CreateStaticCrlProvider( - absl::Span crls) { - absl::flat_hash_map> crl_map; - for (const auto& raw_crl : crls) { - absl::StatusOr> crl = Crl::Parse(raw_crl); - if (!crl.ok()) { - return absl::InvalidArgumentError(absl::StrCat( - "Parsing crl string failed with result ", crl.status().ToString())); - } - bool inserted = crl_map.emplace((*crl)->Issuer(), std::move(*crl)).second; - if (!inserted) { - gpr_log(GPR_ERROR, - "StaticCrlProvider received multiple CRLs with the same issuer. " - "The first one in the span will be used."); - } - } - StaticCrlProvider provider = StaticCrlProvider(std::move(crl_map)); - return std::make_shared(std::move(provider)); -} - -std::shared_ptr StaticCrlProvider::GetCrl( - const CertificateInfo& certificate_info) { - auto it = crls_.find(certificate_info.Issuer()); - if (it == crls_.end()) { - return nullptr; - } - return it->second; -} - -} // namespace experimental -} // namespace grpc_core diff --git a/src/core/lib/security/credentials/tls/grpc_tls_crl_provider.h b/src/core/lib/security/credentials/tls/grpc_tls_crl_provider.h deleted file mode 100644 index 75039d9a790..00000000000 --- a/src/core/lib/security/credentials/tls/grpc_tls_crl_provider.h +++ /dev/null @@ -1,83 +0,0 @@ -// -// -// Copyright 2023 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. -// -// - -#ifndef GRPC_SRC_CORE_LIB_SECURITY_CREDENTIALS_TLS_GRPC_TLS_CRL_PROVIDER_H -#define GRPC_SRC_CORE_LIB_SECURITY_CREDENTIALS_TLS_GRPC_TLS_CRL_PROVIDER_H - -#include - -#include -#include -#include - -#include - -#include "absl/container/flat_hash_map.h" -#include "absl/status/statusor.h" -#include "absl/strings/string_view.h" - -#include - -namespace grpc_core { -namespace experimental { - -class StaticCrlProvider : public CrlProvider { - public: - // Each element of the input vector is expected to be the raw contents of a - // CRL file. - explicit StaticCrlProvider( - absl::flat_hash_map> crls) - : crls_(std::move(crls)) {} - std::shared_ptr GetCrl(const CertificateInfo& certificate_info) override; - - private: - const absl::flat_hash_map> crls_; -}; - -class CrlImpl : public Crl { - public: - static absl::StatusOr> Create(X509_CRL* crl); - // Takes ownership of the X509_CRL pointer. - CrlImpl(X509_CRL* crl, absl::string_view issuer) - : crl_(crl), issuer_(issuer) {} - ~CrlImpl() override; - // Returns a string view representation of the issuer pulled from the CRL. - absl::string_view Issuer() override { return issuer_; } - // The caller should not take ownership of the returned pointer. - X509_CRL* crl() const { return crl_; } - - private: - X509_CRL* crl_; - const std::string issuer_; -}; - -class CertificateInfoImpl : public CertificateInfo { - public: - explicit CertificateInfoImpl(absl::string_view issuer) : issuer_(issuer) {} - // Returns a string representation of the issuer pulled from the - // certificate. - absl::string_view Issuer() const override { return issuer_; } - - private: - const std::string issuer_; -}; - -} // namespace experimental -} // namespace grpc_core - -#endif // GRPC_SRC_CORE_LIB_SECURITY_CREDENTIALS_TLS_GRPC_TLS_CRL_PROVIDER_H \ No newline at end of file diff --git a/src/core/lib/security/credentials/tls/tls_credentials.cc b/src/core/lib/security/credentials/tls/tls_credentials.cc index c1a27343fb7..c13cb8609f0 100644 --- a/src/core/lib/security/credentials/tls/tls_credentials.cc +++ b/src/core/lib/security/credentials/tls/tls_credentials.cc @@ -20,7 +20,6 @@ #include "src/core/lib/security/credentials/tls/tls_credentials.h" -#include #include #include @@ -46,14 +45,6 @@ bool CredentialOptionSanityCheck(grpc_tls_credentials_options* options, gpr_log(GPR_ERROR, "TLS credentials options is nullptr."); return false; } - if (!options->crl_directory().empty() && options->crl_provider() != nullptr) { - gpr_log(GPR_ERROR, - "Setting crl_directory and crl_provider not supported. Using the " - "crl_provider."); - // TODO(gtcooke94) - Maybe return false here. Right now object lifetime of - // this options struct is leaky if false is returned and represents a more - // complex fix to handle in another PR. - } // In the following conditions, there won't be any issues, but it might // indicate callers are doing something wrong with the API. if (is_client && options->cert_request_type() != diff --git a/src/core/lib/security/security_connector/ssl_utils.cc b/src/core/lib/security/security_connector/ssl_utils.cc index 47acbf068da..230be953dcc 100644 --- a/src/core/lib/security/security_connector/ssl_utils.cc +++ b/src/core/lib/security/security_connector/ssl_utils.cc @@ -23,7 +23,6 @@ #include #include -#include #include #include "absl/strings/match.h" @@ -411,7 +410,6 @@ grpc_security_status grpc_ssl_tsi_client_handshaker_factory_init( tsi_tls_version max_tls_version, tsi_ssl_session_cache* ssl_session_cache, tsi::TlsSessionKeyLoggerCache::TlsSessionKeyLogger* tls_session_key_logger, const char* crl_directory, - std::shared_ptr crl_provider, tsi_ssl_client_handshaker_factory** handshaker_factory) { const char* root_certs; const tsi_ssl_root_certs_store* root_store; @@ -450,7 +448,6 @@ grpc_security_status grpc_ssl_tsi_client_handshaker_factory_init( options.min_tls_version = min_tls_version; options.max_tls_version = max_tls_version; options.crl_directory = crl_directory; - options.crl_provider = std::move(crl_provider); const tsi_result result = tsi_create_ssl_client_handshaker_factory_with_options(&options, handshaker_factory); @@ -470,7 +467,6 @@ grpc_security_status grpc_ssl_tsi_server_handshaker_factory_init( tsi_tls_version min_tls_version, tsi_tls_version max_tls_version, tsi::TlsSessionKeyLoggerCache::TlsSessionKeyLogger* tls_session_key_logger, const char* crl_directory, bool send_client_ca_list, - std::shared_ptr crl_provider, tsi_ssl_server_handshaker_factory** handshaker_factory) { size_t num_alpn_protocols = 0; const char** alpn_protocol_strings = @@ -488,7 +484,6 @@ grpc_security_status grpc_ssl_tsi_server_handshaker_factory_init( options.max_tls_version = max_tls_version; options.key_logger = tls_session_key_logger; options.crl_directory = crl_directory; - options.crl_provider = std::move(crl_provider); options.send_client_ca_list = send_client_ca_list; const tsi_result result = tsi_create_ssl_server_handshaker_factory_with_options(&options, diff --git a/src/core/lib/security/security_connector/ssl_utils.h b/src/core/lib/security/security_connector/ssl_utils.h index 4053f3abed3..d264a1f8ed7 100644 --- a/src/core/lib/security/security_connector/ssl_utils.h +++ b/src/core/lib/security/security_connector/ssl_utils.h @@ -23,7 +23,6 @@ #include -#include #include #include #include @@ -31,7 +30,6 @@ #include "absl/status/status.h" #include "absl/strings/string_view.h" -#include #include #include #include @@ -87,7 +85,6 @@ grpc_security_status grpc_ssl_tsi_client_handshaker_factory_init( tsi_tls_version max_tls_version, tsi_ssl_session_cache* ssl_session_cache, tsi::TlsSessionKeyLoggerCache::TlsSessionKeyLogger* tls_session_key_logger, const char* crl_directory, - std::shared_ptr crl_provider, tsi_ssl_client_handshaker_factory** handshaker_factory); grpc_security_status grpc_ssl_tsi_server_handshaker_factory_init( @@ -97,7 +94,6 @@ grpc_security_status grpc_ssl_tsi_server_handshaker_factory_init( tsi_tls_version min_tls_version, tsi_tls_version max_tls_version, tsi::TlsSessionKeyLoggerCache::TlsSessionKeyLogger* tls_session_key_logger, const char* crl_directory, bool send_client_ca_list, - std::shared_ptr crl_provider, tsi_ssl_server_handshaker_factory** handshaker_factory); // Free the memory occupied by key cert pairs. 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 633171175bf..7dfd24dfb9a 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 @@ -550,7 +550,7 @@ TlsChannelSecurityConnector::UpdateHandshakerFactoryLocked() { grpc_get_tsi_tls_version(options_->min_tls_version()), grpc_get_tsi_tls_version(options_->max_tls_version()), ssl_session_cache_, tls_session_key_logger_.get(), options_->crl_directory().c_str(), - options_->crl_provider(), &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); @@ -817,8 +817,7 @@ TlsServerSecurityConnector::UpdateHandshakerFactoryLocked() { grpc_get_tsi_tls_version(options_->min_tls_version()), grpc_get_tsi_tls_version(options_->max_tls_version()), tls_session_key_logger_.get(), options_->crl_directory().c_str(), - options_->send_client_ca_list(), options_->crl_provider(), - &server_handshaker_factory_); + options_->send_client_ca_list(), &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 9f702d90c32..91519650b6d 100644 --- a/src/core/tsi/ssl_transport_security.cc +++ b/src/core/tsi/ssl_transport_security.cc @@ -57,7 +57,6 @@ #include "src/core/lib/gpr/useful.h" #include "src/core/lib/gprpp/crash.h" -#include "src/core/lib/security/credentials/tls/grpc_tls_crl_provider.h" #include "src/core/tsi/ssl/key_logging/ssl_key_logging.h" #include "src/core/tsi/ssl/session_cache/ssl_session_cache.h" #include "src/core/tsi/ssl_transport_security_utils.h" @@ -145,7 +144,6 @@ struct tsi_ssl_frame_protector { static gpr_once g_init_openssl_once = GPR_ONCE_INIT; static int g_ssl_ctx_ex_factory_index = -1; -static int g_ssl_ctx_ex_crl_provider_index = -1; static const unsigned char kSslSessionIdContext[] = {'g', 'r', 'p', 'c'}; static int g_ssl_ex_verified_root_cert_index = -1; #if !defined(OPENSSL_IS_BORINGSSL) && !defined(OPENSSL_NO_ENGINE) @@ -201,10 +199,6 @@ static void init_openssl(void) { SSL_CTX_get_ex_new_index(0, nullptr, nullptr, nullptr, nullptr); GPR_ASSERT(g_ssl_ctx_ex_factory_index != -1); - g_ssl_ctx_ex_crl_provider_index = - SSL_CTX_get_ex_new_index(0, nullptr, nullptr, nullptr, nullptr); - GPR_ASSERT(g_ssl_ctx_ex_crl_provider_index != -1); - g_ssl_ex_verified_root_cert_index = SSL_get_ex_new_index(0, nullptr, nullptr, nullptr, nullptr); GPR_ASSERT(g_ssl_ex_verified_root_cert_index != -1); @@ -882,9 +876,9 @@ static tsi_result build_alpn_protocol_name_list( static int verify_cb(int ok, X509_STORE_CTX* ctx) { int cert_error = X509_STORE_CTX_get_error(ctx); if (cert_error == X509_V_ERR_UNABLE_TO_GET_CRL) { - gpr_log(GPR_INFO, - "Certificate verification failed to find relevant CRL file. " - "Ignoring error."); + gpr_log( + GPR_INFO, + "Certificate verification failed to get CRL files. Ignoring error."); return 1; } if (cert_error != 0) { @@ -946,16 +940,8 @@ static int RootCertExtractCallback(int preverify_ok, X509_STORE_CTX* ctx) { return preverify_ok; } - ERR_clear_error(); - int ssl_index = SSL_get_ex_data_X509_STORE_CTX_idx(); - if (ssl_index < 0) { - char err_str[256]; - ERR_error_string_n(ERR_get_error(), err_str, sizeof(err_str)); - gpr_log(GPR_ERROR, - "error getting the SSL index from the X509_STORE_CTX: %s", err_str); - return preverify_ok; - } - SSL* ssl = static_cast(X509_STORE_CTX_get_ex_data(ctx, ssl_index)); + SSL* ssl = static_cast( + X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx())); if (ssl == nullptr) { return preverify_ok; } @@ -967,69 +953,6 @@ static int RootCertExtractCallback(int preverify_ok, X509_STORE_CTX* ctx) { return preverify_ok; } -// X509_STORE_set_get_crl() sets the function to get the crl for a given -// certificate x. When found, the crl must be assigned to *crl. This function -// must return 0 on failure and 1 on success. If no function to get the issuer -// is provided, the internal default function will be used instead. -static int GetCrlFromProvider(X509_STORE_CTX* ctx, X509_CRL** crl_out, - X509* cert) { - ERR_clear_error(); - int ssl_index = SSL_get_ex_data_X509_STORE_CTX_idx(); - if (ssl_index < 0) { - char err_str[256]; - ERR_error_string_n(ERR_get_error(), err_str, sizeof(err_str)); - gpr_log(GPR_ERROR, - "error getting the SSL index from the X509_STORE_CTX while looking " - "up Crl: %s", - err_str); - return 0; - } - SSL* ssl = static_cast(X509_STORE_CTX_get_ex_data(ctx, ssl_index)); - if (ssl == nullptr) { - gpr_log(GPR_ERROR, - "error while fetching from CrlProvider. SSL object is null"); - return 0; - } - SSL_CTX* ssl_ctx = SSL_get_SSL_CTX(ssl); - auto* provider = static_cast( - SSL_CTX_get_ex_data(ssl_ctx, g_ssl_ctx_ex_crl_provider_index)); - - char* buf = X509_NAME_oneline(X509_get_issuer_name(cert), nullptr, 0); - if (buf == nullptr) { - gpr_log(GPR_ERROR, "Certificate has null issuer, cannot do CRL lookup"); - return 0; - } - grpc_core::experimental::CertificateInfoImpl cert_impl(buf); - std::shared_ptr internal_crl = - provider->GetCrl(cert_impl); - OPENSSL_free(buf); - // There wasn't a CRL found in the provider. Returning 0 will end up causing - // OpenSSL to return X509_V_ERR_UNABLE_TO_GET_CRL. We then catch that error - // and behave how we want for a missing CRL. - // It is important to treat missing CRLs and empty CRLs differently. - if (internal_crl == nullptr) { - return 0; - } - X509_CRL* crl = - std::static_pointer_cast(internal_crl) - ->crl(); - - X509_CRL* copy = X509_CRL_dup(crl); - *crl_out = copy; - return 1; -} - -// When using CRL Providers, this function used to override the default -// `check_crl` function in OpenSSL using `X509_STORE_set_check_crl`. -// CrlProviders put the onus on the users to provide the CRLs that they want to -// provide, and because we override default CRL fetching behavior, we can expect -// some of these verification checks to fails for custom CRL providers as well. -// Thus, we need a passthrough to indicate to OpenSSL that we've provided a CRL -// and we are good with it. -static int CheckCrlPassthrough(X509_STORE_CTX* /*ctx*/, X509_CRL* /*crl*/) { - return 1; -} - // Sets the min and max TLS version of |ssl_context| to |min_tls_version| and // |max_tls_version|, respectively. Calling this method is a no-op when using // OpenSSL versions < 1.1. @@ -2158,18 +2081,10 @@ tsi_result tsi_create_ssl_client_handshaker_factory_with_options( } #if OPENSSL_VERSION_NUMBER >= 0x10100000 - if (options->crl_provider != nullptr) { - SSL_CTX_set_ex_data(impl->ssl_context, g_ssl_ctx_ex_crl_provider_index, - options->crl_provider.get()); - X509_STORE* cert_store = SSL_CTX_get_cert_store(impl->ssl_context); - X509_STORE_set_get_crl(cert_store, GetCrlFromProvider); - X509_STORE_set_check_crl(cert_store, CheckCrlPassthrough); - X509_STORE_set_verify_cb(cert_store, verify_cb); - X509_VERIFY_PARAM* param = X509_STORE_get0_param(cert_store); - X509_VERIFY_PARAM_set_flags( - param, X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL); - } else if (options->crl_directory != nullptr && - strcmp(options->crl_directory, "") != 0) { + 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, @@ -2179,6 +2094,7 @@ tsi_result tsi_create_ssl_client_handshaker_factory_with_options( X509_VERIFY_PARAM* param = X509_STORE_get0_param(cert_store); X509_VERIFY_PARAM_set_flags( param, X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL); + gpr_log(GPR_INFO, "enabled client side CRL checking."); } } #endif @@ -2353,19 +2269,10 @@ tsi_result tsi_create_ssl_server_handshaker_factory_with_options( } #if OPENSSL_VERSION_NUMBER >= 0x10100000 - if (options->crl_provider != nullptr) { - SSL_CTX_set_ex_data(impl->ssl_contexts[i], - g_ssl_ctx_ex_crl_provider_index, - options->crl_provider.get()); - X509_STORE* cert_store = SSL_CTX_get_cert_store(impl->ssl_contexts[i]); - X509_STORE_set_get_crl(cert_store, GetCrlFromProvider); - X509_STORE_set_check_crl(cert_store, CheckCrlPassthrough); - X509_STORE_set_verify_cb(cert_store, verify_cb); - X509_VERIFY_PARAM* param = X509_STORE_get0_param(cert_store); - X509_VERIFY_PARAM_set_flags( - param, X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL); - } else if (options->crl_directory != nullptr && - strcmp(options->crl_directory, "") != 0) { + 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, @@ -2375,6 +2282,7 @@ tsi_result tsi_create_ssl_server_handshaker_factory_with_options( X509_VERIFY_PARAM* param = X509_STORE_get0_param(cert_store); X509_VERIFY_PARAM_set_flags( param, X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL); + gpr_log(GPR_INFO, "enabled server CRL checking."); } } #endif diff --git a/src/core/tsi/ssl_transport_security.h b/src/core/tsi/ssl_transport_security.h index b26eec2755b..75e549fc3cc 100644 --- a/src/core/tsi/ssl_transport_security.h +++ b/src/core/tsi/ssl_transport_security.h @@ -21,13 +21,10 @@ #include -#include - #include #include "absl/strings/string_view.h" -#include #include #include "src/core/tsi/ssl/key_logging/ssl_key_logging.h" @@ -182,17 +179,9 @@ struct tsi_ssl_client_handshaker_options { // 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. Cannot be used in conjunction with - // `crl_provider`. + // > 1.1 is supported for CRL checking const char* crl_directory; - // A provider of CRLs. If set, when doing handshakes the `CrlProvider`'s - // `GetCrl` function will be called to find CRLs when checking certificates - // for revocation. Cannot be used in conjunction with `crl_directory`. - // This provider is created and owned by the user and passed in through - // options as a shared_ptr. - std::shared_ptr crl_provider; - tsi_ssl_client_handshaker_options() : pem_key_cert_pair(nullptr), pem_root_certs(nullptr), @@ -336,13 +325,6 @@ struct tsi_ssl_server_handshaker_options { // crl checking. Only OpenSSL version > 1.1 is supported for CRL checking const char* crl_directory; - // A provider of CRLs. If set, when doing handshakes the `CrlProvider`'s - // `GetCrl` function will be called to find CRLs when checking certificates - // for revocation. Cannot be used in conjunction with `crl_directory`. - // This provider is created and owned by the user and passed in through - // options as a shared_ptr. - std::shared_ptr crl_provider; - // If true, the SSL server sends a list of CA names to the client in the // ServerHello. This list of CA names is extracted from the server's trust // bundle, and the client may use this lint as a hint to decide which diff --git a/src/cpp/common/tls_credentials_options.cc b/src/cpp/common/tls_credentials_options.cc index 8336b69ffb8..6a515055c13 100644 --- a/src/cpp/common/tls_credentials_options.cc +++ b/src/cpp/common/tls_credentials_options.cc @@ -20,14 +20,12 @@ #include #include -#include #include #include #include #include #include #include -#include namespace grpc { namespace experimental { @@ -45,12 +43,6 @@ void TlsCredentialsOptions::set_certificate_provider( } } -void TlsCredentialsOptions::set_crl_provider( - std::shared_ptr crl_provider) { - grpc_tls_credentials_options_set_crl_provider(c_credentials_options_, - std::move(crl_provider)); -} - void TlsCredentialsOptions::watch_root_certs() { grpc_tls_credentials_options_watch_root_certs(c_credentials_options_); } diff --git a/src/cpp/server/secure_server_credentials.cc b/src/cpp/server/secure_server_credentials.cc index 9ec5d24031f..d88b5950ef9 100644 --- a/src/cpp/server/secure_server_credentials.cc +++ b/src/cpp/server/secure_server_credentials.cc @@ -151,13 +151,8 @@ std::shared_ptr LocalServerCredentials( std::shared_ptr TlsServerCredentials( const grpc::experimental::TlsServerCredentialsOptions& options) { - grpc_server_credentials* c_creds = - grpc_tls_server_credentials_create(options.c_credentials_options()); - if (c_creds == nullptr) { - return nullptr; - } - return std::shared_ptr( - new SecureServerCredentials(c_creds)); + return std::shared_ptr(new SecureServerCredentials( + grpc_tls_server_credentials_create(options.c_credentials_options()))); } } // namespace experimental diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index 2ab962aaa53..19e57ea8949 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -745,7 +745,6 @@ CORE_SOURCE_FILES = [ 'src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.cc', 'src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.cc', 'src/core/lib/security/credentials/tls/grpc_tls_credentials_options.cc', - 'src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc', 'src/core/lib/security/credentials/tls/tls_credentials.cc', 'src/core/lib/security/credentials/tls/tls_utils.cc', 'src/core/lib/security/credentials/xds/xds_credentials.cc', diff --git a/test/core/security/BUILD b/test/core/security/BUILD index b53edcccac1..1c2207bd1e7 100644 --- a/test/core/security/BUILD +++ b/test/core/security/BUILD @@ -560,19 +560,3 @@ grpc_cc_test( "//test/core/util:grpc_test_util", ], ) - -grpc_cc_test( - name = "grpc_tls_crl_provider_test", - srcs = ["grpc_tls_crl_provider_test.cc"], - data = [ - "//test/core/tsi/test_creds/crl_data/crls:ab06acdd.r0", - ], - external_deps = ["gtest"], - language = "C++", - deps = [ - "//:gpr", - "//:grpc", - "//src/core:grpc_crl_provider", - "//test/core/util:grpc_test_util", - ], -) diff --git a/test/core/security/grpc_tls_credentials_options_comparator_test.cc b/test/core/security/grpc_tls_credentials_options_comparator_test.cc index 3aeff611c86..aad509ebbe8 100644 --- a/test/core/security/grpc_tls_credentials_options_comparator_test.cc +++ b/test/core/security/grpc_tls_credentials_options_comparator_test.cc @@ -161,16 +161,6 @@ TEST(TlsCredentialsOptionsComparatorTest, DifferentCrlDirectory) { delete options_1; delete options_2; } -TEST(TlsCredentialsOptionsComparatorTest, DifferentCrlProvider) { - auto* options_1 = grpc_tls_credentials_options_create(); - auto* options_2 = grpc_tls_credentials_options_create(); - options_1->set_crl_provider(*experimental::CreateStaticCrlProvider({})); - options_2->set_crl_provider(*experimental::CreateStaticCrlProvider({})); - EXPECT_FALSE(*options_1 == *options_2); - EXPECT_FALSE(*options_2 == *options_1); - delete options_1; - delete options_2; -} TEST(TlsCredentialsOptionsComparatorTest, DifferentSendClientCaListValues) { auto* options_1 = grpc_tls_credentials_options_create(); auto* options_2 = grpc_tls_credentials_options_create(); diff --git a/test/core/security/grpc_tls_credentials_options_test.cc b/test/core/security/grpc_tls_credentials_options_test.cc index 5fe2c821209..3ab8ca671ee 100644 --- a/test/core/security/grpc_tls_credentials_options_test.cc +++ b/test/core/security/grpc_tls_credentials_options_test.cc @@ -562,44 +562,6 @@ TEST_F(GrpcTlsCredentialsOptionsTest, EXPECT_EQ(tls_connector->ServerHandshakerFactoryForTesting(), nullptr); } -TEST_F(GrpcTlsCredentialsOptionsTest, CrlProvider) { - auto options = MakeRefCounted(); - auto provider = experimental::CreateStaticCrlProvider({}); - ASSERT_TRUE(provider.ok()); - options->set_crl_provider(std::move(*provider)); - auto credentials = MakeRefCounted(options); - ASSERT_NE(credentials, nullptr); - ChannelArgs new_args; - auto connector = credentials->create_security_connector( - nullptr, "random targets", &new_args); - ASSERT_NE(connector, nullptr); - TlsChannelSecurityConnector* tls_connector = - static_cast(connector.get()); - EXPECT_NE(tls_connector->ClientHandshakerFactoryForTesting(), nullptr); -} - -TEST_F(GrpcTlsCredentialsOptionsTest, CrlProviderWithServerCredentials) { - auto options = MakeRefCounted(); - auto provider = MakeRefCounted( - root_cert_, MakeCertKeyPairs(private_key_.c_str(), cert_chain_.c_str())); - options->set_certificate_provider(std::move(provider)); - options->set_watch_root_cert(true); - options->set_watch_identity_pair(true); - options->set_cert_request_type( - GRPC_SSL_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_AND_VERIFY); - auto crl_provider = experimental::CreateStaticCrlProvider({}); - ASSERT_TRUE(crl_provider.ok()); - options->set_crl_provider(std::move(*crl_provider)); - auto credentials = MakeRefCounted(options); - ASSERT_NE(credentials, nullptr); - ChannelArgs new_args; - auto connector = credentials->create_security_connector(new_args); - ASSERT_NE(connector, nullptr); - TlsServerSecurityConnector* tls_connector = - static_cast(connector.get()); - EXPECT_NE(tls_connector->ServerHandshakerFactoryForTesting(), nullptr); -} - } // namespace testing } // namespace grpc_core diff --git a/test/core/security/grpc_tls_crl_provider_test.cc b/test/core/security/grpc_tls_crl_provider_test.cc deleted file mode 100644 index 793a0de2415..00000000000 --- a/test/core/security/grpc_tls_crl_provider_test.cc +++ /dev/null @@ -1,95 +0,0 @@ -// -// -// Copyright 2023 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 "src/core/lib/security/credentials/tls/grpc_tls_crl_provider.h" - -#include -#include -#include - -#include - -#include "absl/status/statusor.h" - -#include -#include - -#include "test/core/util/test_config.h" -#include "test/core/util/tls_utils.h" - -const char* kCrlPath = "test/core/tsi/test_creds/crl_data/crls/ab06acdd.r0"; -const absl::string_view kCrlIssuer = - "/C=AU/ST=Some-State/O=Internet Widgits Pty Ltd/CN=testca"; - -namespace grpc_core { -namespace testing { - -using ::grpc_core::experimental::CertificateInfoImpl; -using ::grpc_core::experimental::Crl; -using ::grpc_core::experimental::CrlProvider; - -TEST(CrlProviderTest, CanParseCrl) { - std::string crl_string = GetFileContents(kCrlPath); - absl::StatusOr> crl = Crl::Parse(crl_string); - ASSERT_TRUE(crl.ok()); - ASSERT_NE(*crl, nullptr); - EXPECT_EQ((*crl)->Issuer(), kCrlIssuer); -} - -TEST(CrlProviderTest, InvalidFile) { - std::string crl_string = "INVALID CRL FILE"; - absl::StatusOr> crl = Crl::Parse(crl_string); - EXPECT_EQ(crl.status(), - absl::InvalidArgumentError( - "Conversion from PEM string to X509 CRL failed.")); -} - -TEST(CrlProviderTest, StaticCrlProviderLookup) { - std::vector crl_strings = {GetFileContents(kCrlPath)}; - absl::StatusOr> provider = - experimental::CreateStaticCrlProvider(crl_strings); - ASSERT_TRUE(provider.ok()) << provider.status(); - CertificateInfoImpl cert = CertificateInfoImpl(kCrlIssuer); - auto crl = (*provider)->GetCrl(cert); - ASSERT_NE(crl, nullptr); - EXPECT_EQ(crl->Issuer(), kCrlIssuer); -} - -TEST(CrlProviderTest, StaticCrlProviderLookupBad) { - std::vector crl_strings = {GetFileContents(kCrlPath)}; - absl::StatusOr> provider = - experimental::CreateStaticCrlProvider(crl_strings); - ASSERT_TRUE(provider.ok()) << provider.status(); - CertificateInfoImpl bad_cert = CertificateInfoImpl("BAD CERT"); - auto crl = (*provider)->GetCrl(bad_cert); - EXPECT_EQ(crl, nullptr); -} - -} // namespace testing -} // namespace grpc_core - -int main(int argc, char** argv) { - grpc::testing::TestEnvironment env(&argc, argv); - ::testing::InitGoogleTest(&argc, argv); - grpc_init(); - int ret = RUN_ALL_TESTS(); - grpc_shutdown(); - return ret; -} diff --git a/test/core/tsi/BUILD b/test/core/tsi/BUILD index 372ba9a3747..2c2a31ab90c 100644 --- a/test/core/tsi/BUILD +++ b/test/core/tsi/BUILD @@ -125,8 +125,6 @@ grpc_cc_test( "//test/core/tsi/test_creds/crl_data:valid.pem", "//test/core/tsi/test_creds/crl_data/crls:ab06acdd.r0", "//test/core/tsi/test_creds/crl_data/crls:b9322cac.r0", - "//test/core/tsi/test_creds/crl_data/crls:current.crl", - "//test/core/tsi/test_creds/crl_data/crls:intermediate.crl", "//test/core/tsi/test_creds/crl_data/crls_missing_intermediate:ab06acdd.r0", "//test/core/tsi/test_creds/crl_data/crls_missing_root:b9322cac.r0", ], diff --git a/test/core/tsi/crl_ssl_transport_security_test.cc b/test/core/tsi/crl_ssl_transport_security_test.cc index af26ba85301..b13f6a0f1a7 100644 --- a/test/core/tsi/crl_ssl_transport_security_test.cc +++ b/test/core/tsi/crl_ssl_transport_security_test.cc @@ -16,8 +16,6 @@ #include #include -#include - #include #include @@ -36,7 +34,6 @@ #include "src/core/tsi/transport_security_interface.h" #include "test/core/tsi/transport_security_test_lib.h" #include "test/core/util/test_config.h" -#include "test/core/util/tls_utils.h" extern "C" { #include @@ -45,6 +42,9 @@ extern "C" { namespace { +const int kSslTsiTestRevokedKeyCertPairsNum = 1; +const int kSslTsiTestValidKeyCertPairsNum = 1; +const int kSslTsiTestRevokedIntermedidateKeyCertPairsNum = 1; const char* kSslTsiTestCrlSupportedCredentialsDir = "test/core/tsi/test_creds/crl_data/"; const char* kSslTsiTestCrlSupportedCrlDir = @@ -54,18 +54,6 @@ const char* kSslTsiTestCrlSupportedCrlDirMissingIntermediate = const char* kSslTsiTestCrlSupportedCrlDirMissingRoot = "test/core/tsi/test_creds/crl_data/crls_missing_root/"; const char* kSslTsiTestFaultyCrlsDir = "bad_path/"; -const char* kRevokedKeyPath = "test/core/tsi/test_creds/crl_data/revoked.key"; -const char* kRevokedCertPath = "test/core/tsi/test_creds/crl_data/revoked.pem"; -const char* kValidKeyPath = "test/core/tsi/test_creds/crl_data/valid.key"; -const char* kValidCertPath = "test/core/tsi/test_creds/crl_data/valid.pem"; - -const char* kRevokedIntermediateKeyPath = - "test/core/tsi/test_creds/crl_data/leaf_signed_by_intermediate.key"; -const char* kRevokedIntermediateCertPath = - "test/core/tsi/test_creds/crl_data/leaf_and_intermediate_chain.pem"; -const char* kRootCrlPath = "test/core/tsi/test_creds/crl_data/crls/current.crl"; -const char* kIntermediateCrlPath = - "test/core/tsi/test_creds/crl_data/crls/intermediate.crl"; class CrlSslTransportSecurityTest : public testing::TestWithParam { @@ -73,40 +61,22 @@ class CrlSslTransportSecurityTest // A tsi_test_fixture implementation. class SslTsiTestFixture { public: - SslTsiTestFixture( - absl::string_view server_key_path, absl::string_view server_cert_path, - absl::string_view client_key_path, absl::string_view client_cert_path, - const char* crl_directory, - std::shared_ptr crl_provider, - bool expect_server_success, bool expect_client_success_1_2, - bool expect_client_success_1_3) { - tsi_test_fixture_init(&base_); - base_.test_unused_bytes = true; - base_.vtable = &kVtable; - server_key_ = grpc_core::testing::GetFileContents(server_key_path.data()); - server_cert_ = - grpc_core::testing::GetFileContents(server_cert_path.data()); - client_key_ = grpc_core::testing::GetFileContents(client_key_path.data()); - client_cert_ = - grpc_core::testing::GetFileContents(client_cert_path.data()); - root_cert_ = grpc_core::testing::GetFileContents( - absl::StrCat(kSslTsiTestCrlSupportedCredentialsDir, "ca.pem").data()); - root_store_ = tsi_ssl_root_certs_store_create(root_cert_.c_str()); - crl_directory_ = crl_directory; - crl_provider_ = std::move(crl_provider); - expect_server_success_ = expect_server_success; - expect_client_success_1_2_ = expect_client_success_1_2; - expect_client_success_1_3_ = expect_client_success_1_3; - - server_pem_key_cert_pairs_ = static_cast( - gpr_malloc(sizeof(tsi_ssl_pem_key_cert_pair))); - server_pem_key_cert_pairs_[0].private_key = server_key_.c_str(); - server_pem_key_cert_pairs_[0].cert_chain = server_cert_.c_str(); - client_pem_key_cert_pairs_ = static_cast( - gpr_malloc(sizeof(tsi_ssl_pem_key_cert_pair))); - client_pem_key_cert_pairs_[0].private_key = client_key_.c_str(); - client_pem_key_cert_pairs_[0].cert_chain = client_cert_.c_str(); - GPR_ASSERT(root_store_ != nullptr); + // When use_faulty_crl_directory is set, the crl_directory of the + // client is set to a non-existant path. + static SslTsiTestFixture* Create(bool use_revoked_server_cert, + bool use_revoked_client_cert, + bool use_faulty_crl_directory) { + return new SslTsiTestFixture( + use_revoked_server_cert, use_revoked_client_cert, + use_faulty_crl_directory, false, false, false); + } + + static SslTsiTestFixture* CreateWithIntermediate( + bool use_revoked_intermediate, bool use_missing_intermediate_crl, + bool use_missing_root_crl) { + return new SslTsiTestFixture( + false, false, false, use_revoked_intermediate, + use_missing_intermediate_crl, use_missing_root_crl); } void Run() { @@ -114,16 +84,73 @@ class CrlSslTransportSecurityTest tsi_test_fixture_destroy(&base_); } - ~SslTsiTestFixture() { - gpr_free(server_pem_key_cert_pairs_); - gpr_free(client_pem_key_cert_pairs_); + private: + SslTsiTestFixture(bool use_revoked_server_cert, + bool use_revoked_client_cert, + bool use_faulty_crl_directory, + bool use_revoked_intermediate, + bool use_missing_intermediate_crl, + bool use_missing_root_crl) + : use_revoked_server_cert_(use_revoked_server_cert), + use_revoked_client_cert_(use_revoked_client_cert), + use_faulty_crl_directory_(use_faulty_crl_directory), + use_revoked_intermediate_(use_revoked_intermediate), + use_missing_intermediate_crl_(use_missing_intermediate_crl), + use_missing_root_crl_(use_missing_root_crl) { + 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")); + revoked_intermediate_pem_key_cert_pairs_ = + static_cast( + gpr_malloc(sizeof(tsi_ssl_pem_key_cert_pair) * + kSslTsiTestRevokedIntermedidateKeyCertPairsNum)); + revoked_intermediate_pem_key_cert_pairs_[0].private_key = + LoadFile(absl::StrCat(kSslTsiTestCrlSupportedCredentialsDir, + "leaf_signed_by_intermediate.key")); + revoked_intermediate_pem_key_cert_pairs_[0].cert_chain = + LoadFile(absl::StrCat(kSslTsiTestCrlSupportedCredentialsDir, + "leaf_and_intermediate_chain.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_); + for (size_t i = 0; i < kSslTsiTestRevokedIntermedidateKeyCertPairsNum; + i++) { + PemKeyCertPairDestroy(revoked_intermediate_pem_key_cert_pairs_[i]); + } + gpr_free(revoked_intermediate_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_); } - private: static void SetupHandshakers(tsi_test_fixture* fixture) { GPR_ASSERT(fixture != nullptr); auto* self = reinterpret_cast(fixture); @@ -133,10 +160,22 @@ class CrlSslTransportSecurityTest void SetupHandshakers() { // Create client handshaker factory. tsi_ssl_client_handshaker_options client_options; - client_options.pem_root_certs = root_cert_.c_str(); - client_options.pem_key_cert_pair = client_pem_key_cert_pairs_; - client_options.crl_directory = crl_directory_; - client_options.crl_provider = crl_provider_; + 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_; + } + if (use_faulty_crl_directory_) { + client_options.crl_directory = kSslTsiTestFaultyCrlsDir; + } else if (use_missing_intermediate_crl_) { + client_options.crl_directory = + kSslTsiTestCrlSupportedCrlDirMissingIntermediate; + } else if (use_missing_root_crl_) { + client_options.crl_directory = kSslTsiTestCrlSupportedCrlDirMissingRoot; + } else { + client_options.crl_directory = kSslTsiTestCrlSupportedCrlDir; + } client_options.root_store = root_store_; client_options.min_tls_version = GetParam(); client_options.max_tls_version = GetParam(); @@ -145,11 +184,27 @@ class CrlSslTransportSecurityTest TSI_OK); // Create server handshaker factory. tsi_ssl_server_handshaker_options server_options; - server_options.pem_key_cert_pairs = server_pem_key_cert_pairs_; - server_options.num_key_cert_pairs = 1; - server_options.pem_client_root_certs = root_cert_.c_str(); - server_options.crl_directory = crl_directory_; - server_options.crl_provider = crl_provider_; + if (use_revoked_server_cert_) { + server_options.pem_key_cert_pairs = revoked_pem_key_cert_pairs_; + server_options.num_key_cert_pairs = kSslTsiTestRevokedKeyCertPairsNum; + } else if (!use_revoked_intermediate_) { + server_options.pem_key_cert_pairs = valid_pem_key_cert_pairs_; + server_options.num_key_cert_pairs = kSslTsiTestValidKeyCertPairsNum; + } else { + server_options.pem_key_cert_pairs = + revoked_intermediate_pem_key_cert_pairs_; + server_options.num_key_cert_pairs = + kSslTsiTestRevokedIntermedidateKeyCertPairsNum; + } + server_options.pem_client_root_certs = root_cert_; + if (use_missing_intermediate_crl_) { + server_options.crl_directory = + kSslTsiTestCrlSupportedCrlDirMissingIntermediate; + } else if (use_missing_root_crl_) { + server_options.crl_directory = kSslTsiTestCrlSupportedCrlDirMissingRoot; + } else { + server_options.crl_directory = kSslTsiTestCrlSupportedCrlDir; + } server_options.client_certificate_request = TSI_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_AND_VERIFY; server_options.session_ticket_key = nullptr; @@ -197,17 +252,20 @@ class CrlSslTransportSecurityTest // 3. CRL Directory without CA's CRL with but Intermediate CA's CRL -> // Handshake succeeds because the CRL that revokes the cert is not // present. - bool expect_server_success = expect_server_success_; + bool expect_server_success = + !(use_revoked_server_cert_ || use_revoked_client_cert_ || + (use_revoked_intermediate_ & !use_missing_root_crl_)); #if OPENSSL_VERSION_NUMBER >= 0x10100000 - bool expect_client_success = GetParam() == tsi_tls_version::TSI_TLS1_2 - ? expect_client_success_1_2_ - : expect_client_success_1_3_; + bool expect_client_success = + GetParam() == tsi_tls_version::TSI_TLS1_2 + ? expect_server_success + : !(use_revoked_server_cert_ || + (use_revoked_intermediate_ & !use_missing_root_crl_)); #else - // If using OpenSSL version < 1.1, the CRL revocation won't - // be enabled anyways, so we always expect the connection to - // be successful. + // If using OpenSSL version < 1.1, the CRL revocation won't be enabled + // anyways, so we always expect the connection to be successful. expect_server_success = true; - expect_client_success = expect_server_success; + bool expect_client_success = expect_server_success; #endif tsi_peer peer; if (expect_client_success) { @@ -228,30 +286,41 @@ class CrlSslTransportSecurityTest } } + 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) == + absl::OkStatus()); + 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_; - std::string root_cert_; + bool use_revoked_server_cert_; + bool use_revoked_client_cert_; + bool use_faulty_crl_directory_; + bool use_revoked_intermediate_; + bool use_missing_intermediate_crl_; + bool use_missing_root_crl_; + 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_pem_key_cert_pair* revoked_intermediate_pem_key_cert_pairs_; tsi_ssl_server_handshaker_factory* server_handshaker_factory_; tsi_ssl_client_handshaker_factory* client_handshaker_factory_; - - std::string server_key_; - std::string server_cert_; - std::string client_key_; - std::string client_cert_; - const char* crl_directory_; - bool expect_server_success_; - bool expect_client_success_1_2_; - bool expect_client_success_1_3_; - tsi_ssl_pem_key_cert_pair* client_pem_key_cert_pairs_; - tsi_ssl_pem_key_cert_pair* server_pem_key_cert_pairs_; - std::shared_ptr crl_provider_; }; }; @@ -262,149 +331,55 @@ struct tsi_test_fixture_vtable &CrlSslTransportSecurityTest::SslTsiTestFixture::Destruct}; TEST_P(CrlSslTransportSecurityTest, RevokedServerCert) { - auto* fixture = new SslTsiTestFixture( - kRevokedKeyPath, kRevokedCertPath, kValidKeyPath, kValidCertPath, - kSslTsiTestCrlSupportedCrlDir, nullptr, false, false, false); + auto* fixture = SslTsiTestFixture::Create(/*use_revoked_server_cert=*/true, + /*use_revoked_client_cert=*/false, + /*use_faulty_crl_directory=*/false); fixture->Run(); } TEST_P(CrlSslTransportSecurityTest, RevokedClientCert) { - auto* fixture = new SslTsiTestFixture( - kValidKeyPath, kValidCertPath, kRevokedKeyPath, kRevokedCertPath, - kSslTsiTestCrlSupportedCrlDir, nullptr, false, false, true); + auto* fixture = SslTsiTestFixture::Create(/*use_revoked_server_cert=*/false, + /*use_revoked_client_cert=*/true, + /*use_faulty_crl_directory=*/false); fixture->Run(); } TEST_P(CrlSslTransportSecurityTest, ValidCerts) { - auto* fixture = new SslTsiTestFixture( - kValidKeyPath, kValidCertPath, kValidKeyPath, kValidCertPath, - kSslTsiTestCrlSupportedCrlDir, nullptr, true, true, true); + auto* fixture = SslTsiTestFixture::Create(/*use_revoked_server_cert=*/false, + /*use_revoked_client_cert=*/false, + /*use_faulty_crl_directory=*/false); fixture->Run(); } TEST_P(CrlSslTransportSecurityTest, UseFaultyCrlDirectory) { - auto* fixture = new SslTsiTestFixture( - kRevokedKeyPath, kRevokedCertPath, kValidKeyPath, kValidCertPath, - kSslTsiTestFaultyCrlsDir, nullptr, true, true, true); + auto* fixture = SslTsiTestFixture::Create(/*use_revoked_server_cert=*/false, + /*use_revoked_client_cert=*/false, + /*use_faulty_crl_directory=*/true); fixture->Run(); } -TEST_P(CrlSslTransportSecurityTest, UseRevokedIntermediateValidCrl) { - auto* fixture = new SslTsiTestFixture( - kRevokedIntermediateKeyPath, kRevokedIntermediateCertPath, kValidKeyPath, - kValidCertPath, kSslTsiTestCrlSupportedCrlDir, nullptr, false, false, - false); +TEST_P(CrlSslTransportSecurityTest, UseRevokedIntermediate) { + auto* fixture = SslTsiTestFixture::CreateWithIntermediate( + /*use_revoked_intermediate=*/true, + /*use_missing_intermediate_crl=*/false, + /*use_missing_root_crl=*/false); fixture->Run(); } TEST_P(CrlSslTransportSecurityTest, UseRevokedIntermediateWithMissingIntermediateCrl) { - auto* fixture = new SslTsiTestFixture( - kRevokedIntermediateKeyPath, kRevokedIntermediateCertPath, kValidKeyPath, - kValidCertPath, kSslTsiTestCrlSupportedCrlDirMissingIntermediate, nullptr, - false, false, false); + auto* fixture = SslTsiTestFixture::CreateWithIntermediate( + /*use_revoked_intermediate=*/true, + /*use_missing_intermediate_crl=*/true, + /*use_missing_root_crl=*/false); fixture->Run(); } TEST_P(CrlSslTransportSecurityTest, UseRevokedIntermediateWithMissingRootCrl) { - auto* fixture = new SslTsiTestFixture( - kRevokedIntermediateKeyPath, kRevokedIntermediateCertPath, kValidKeyPath, - kValidCertPath, kSslTsiTestCrlSupportedCrlDirMissingRoot, nullptr, true, - true, true); - fixture->Run(); -} - -TEST_P(CrlSslTransportSecurityTest, CrlProviderValidCerts) { - std::string root_crl = grpc_core::testing::GetFileContents(kRootCrlPath); - std::string intermediate_crl = - grpc_core::testing::GetFileContents(kIntermediateCrlPath); - - absl::StatusOr> - provider = grpc_core::experimental::CreateStaticCrlProvider( - {root_crl, intermediate_crl}); - ASSERT_TRUE(provider.ok()); - - auto* fixture = new SslTsiTestFixture(kValidKeyPath, kValidCertPath, - kValidKeyPath, kValidCertPath, nullptr, - *provider, true, true, true); - fixture->Run(); -} - -TEST_P(CrlSslTransportSecurityTest, CrlProviderRevokedServer) { - std::string root_crl = grpc_core::testing::GetFileContents(kRootCrlPath); - std::string intermediate_crl = - grpc_core::testing::GetFileContents(kIntermediateCrlPath); - - absl::StatusOr> - provider = grpc_core::experimental::CreateStaticCrlProvider( - {root_crl, intermediate_crl}); - ASSERT_TRUE(provider.ok()); - - auto* fixture = new SslTsiTestFixture(kRevokedKeyPath, kRevokedCertPath, - kValidKeyPath, kValidCertPath, nullptr, - *provider, false, false, false); - fixture->Run(); -} - -TEST_P(CrlSslTransportSecurityTest, CrlProviderRevokedClient) { - std::string root_crl = grpc_core::testing::GetFileContents(kRootCrlPath); - std::string intermediate_crl = - grpc_core::testing::GetFileContents(kIntermediateCrlPath); - - absl::StatusOr> - provider = grpc_core::experimental::CreateStaticCrlProvider( - {root_crl, intermediate_crl}); - ASSERT_TRUE(provider.ok()); - - auto* fixture = new SslTsiTestFixture(kValidKeyPath, kValidCertPath, - kRevokedKeyPath, kRevokedCertPath, - nullptr, *provider, false, false, true); - fixture->Run(); -} - -TEST_P(CrlSslTransportSecurityTest, CrlProviderRevokedIntermediateValidCrl) { - std::string root_crl = grpc_core::testing::GetFileContents(kRootCrlPath); - std::string intermediate_crl = - grpc_core::testing::GetFileContents(kIntermediateCrlPath); - - absl::StatusOr> - provider = grpc_core::experimental::CreateStaticCrlProvider( - {root_crl, intermediate_crl}); - ASSERT_TRUE(provider.ok()); - - auto* fixture = new SslTsiTestFixture( - kRevokedIntermediateKeyPath, kRevokedIntermediateCertPath, kValidKeyPath, - kValidCertPath, nullptr, *provider, false, false, false); - fixture->Run(); -} - -TEST_P(CrlSslTransportSecurityTest, - CrlProviderRevokedIntermediateMissingIntermediateCrl) { - std::string root_crl = grpc_core::testing::GetFileContents(kRootCrlPath); - - absl::StatusOr> - provider = grpc_core::experimental::CreateStaticCrlProvider({root_crl}); - ASSERT_TRUE(provider.ok()); - - auto* fixture = new SslTsiTestFixture( - kRevokedIntermediateKeyPath, kRevokedIntermediateCertPath, kValidKeyPath, - kValidCertPath, nullptr, *provider, false, false, false); - fixture->Run(); -} - -TEST_P(CrlSslTransportSecurityTest, - CrlProviderRevokedIntermediateMissingRootCrl) { - std::string intermediate_crl = - grpc_core::testing::GetFileContents(kIntermediateCrlPath); - - absl::StatusOr> - provider = - grpc_core::experimental::CreateStaticCrlProvider({intermediate_crl}); - ASSERT_TRUE(provider.ok()); - - auto* fixture = new SslTsiTestFixture( - kRevokedIntermediateKeyPath, kRevokedIntermediateCertPath, kValidKeyPath, - kValidCertPath, nullptr, *provider, true, true, true); + auto* fixture = SslTsiTestFixture::CreateWithIntermediate( + /*use_revoked_intermediate=*/true, + /*use_missing_intermediate_crl=*/false, + /*use_missing_root_crl=*/true); fixture->Run(); } @@ -415,11 +390,6 @@ std::string TestNameSuffix( return "TLS_1_3"; } -// TODO(gtcooke94) Add nullptr issuer test cases - this is not simple to test -// the way the code is currently designed - we plan to refactor ways the OpenSSL -// callback functions are written to have more easily testable chunks in -// ssl_transport_security_utils and will add nullptr issuer tests at that time. - INSTANTIATE_TEST_SUITE_P(TLSVersionsTest, CrlSslTransportSecurityTest, testing::Values(tsi_tls_version::TSI_TLS1_2, tsi_tls_version::TSI_TLS1_3), diff --git a/test/core/tsi/test_creds/crl_data/crls/BUILD b/test/core/tsi/test_creds/crl_data/crls/BUILD index 922d279aa6a..911cfaa1e13 100644 --- a/test/core/tsi/test_creds/crl_data/crls/BUILD +++ b/test/core/tsi/test_creds/crl_data/crls/BUILD @@ -17,6 +17,4 @@ licenses(["notice"]) exports_files([ "ab06acdd.r0", "b9322cac.r0", - "current.crl", - "intermediate.crl", ]) diff --git a/test/core/util/tls_utils.cc b/test/core/util/tls_utils.cc index 26eb647ed8d..27f90c4d272 100644 --- a/test/core/util/tls_utils.cc +++ b/test/core/util/tls_utils.cc @@ -13,7 +13,6 @@ // See the License for the specific language governing permissions and // limitations under the License. // -#include #include "test/core/util/tls_utils.h" @@ -139,8 +138,8 @@ void DestroyExternalVerifier(void* arg) { void AsyncExternalVerifier::Destruct(void* user_data) { auto* self = static_cast(user_data); - // Spawn a detached thread to destroy the verifier, to make sure that we - // don't try to join the worker thread from within the worker thread. + // Spawn a detached thread to destroy the verifier, to make sure that we don't + // try to join the worker thread from within the worker thread. Thread destroy_thread("DestroyExternalVerifier", DestroyExternalVerifier, self, nullptr, Thread::Options().set_joinable(false)); destroy_thread.Start(); diff --git a/test/cpp/client/BUILD b/test/cpp/client/BUILD index 95fc4aede8e..5b2dc69a850 100644 --- a/test/cpp/client/BUILD +++ b/test/cpp/client/BUILD @@ -33,7 +33,6 @@ grpc_cc_test( "//:gpr", "//:grpc", "//:grpc++", - "//src/core:grpc_crl_provider", "//test/core/util:grpc_test_util", "//test/cpp/util:tls_test_utils", ], diff --git a/test/cpp/client/credentials_test.cc b/test/cpp/client/credentials_test.cc index 1c833921fd3..bd4137c9e2a 100644 --- a/test/cpp/client/credentials_test.cc +++ b/test/cpp/client/credentials_test.cc @@ -22,7 +22,6 @@ #include #include -#include #include #include #include @@ -398,30 +397,6 @@ TEST(CredentialsTest, TlsChannelCredentialsWithCrlDirectory) { GPR_ASSERT(channel_credentials.get() != nullptr); } -TEST(CredentialsTest, TlsChannelCredentialsWithCrlProvider) { - auto provider = experimental::CreateStaticCrlProvider({}); - ASSERT_TRUE(provider.ok()); - grpc::experimental::TlsChannelCredentialsOptions options; - options.set_crl_provider(*provider); - auto channel_credentials = grpc::experimental::TlsCredentials(options); - GPR_ASSERT(channel_credentials.get() != nullptr); -} - -TEST(CredentialsTest, TlsChannelCredentialsWithCrlProviderAndDirectory) { - auto provider = experimental::CreateStaticCrlProvider({}); - ASSERT_TRUE(provider.ok()); - grpc::experimental::TlsChannelCredentialsOptions options; - options.set_crl_directory(CRL_DIR_PATH); - options.set_crl_provider(*provider); - auto channel_credentials = grpc::experimental::TlsCredentials(options); - // TODO(gtcooke94) - behavior might change to make this return nullptr in the - // future - GPR_ASSERT(channel_credentials.get() != nullptr); -} - -// TODO(gtcooke94) - Add test to make sure Tls*CredentialsOptions does not leak -// when not moved into TlsCredentials - } // namespace } // namespace testing } // namespace grpc diff --git a/test/cpp/end2end/BUILD b/test/cpp/end2end/BUILD index 06bdf745c0f..24bfaf0fbf6 100644 --- a/test/cpp/end2end/BUILD +++ b/test/cpp/end2end/BUILD @@ -1037,30 +1037,3 @@ grpc_cc_test( "//test/cpp/util:test_util", ], ) - -grpc_cc_test( - name = "crl_provider_test", - srcs = ["crl_provider_test.cc"], - data = [ - "//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", - "//test/core/tsi/test_creds/crl_data/crls:current.crl", - ], - external_deps = [ - "gtest", - ], - tags = ["crl_provider_test"], - deps = [ - ":test_service_impl", - "//:gpr", - "//:grpc", - "//:grpc++", - "//src/proto/grpc/testing:echo_messages_proto", - "//src/proto/grpc/testing:echo_proto", - "//test/core/util:grpc_test_util", - "//test/cpp/util:test_util", - ], -) diff --git a/test/cpp/end2end/crl_provider_test.cc b/test/cpp/end2end/crl_provider_test.cc deleted file mode 100644 index ddb0baedf04..00000000000 --- a/test/cpp/end2end/crl_provider_test.cc +++ /dev/null @@ -1,221 +0,0 @@ -// -// -// Copyright 2023 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 "absl/synchronization/notification.h" - -#include -#include -#include -#include -#include -#include -#include -#include - -#include "src/core/lib/iomgr/load_file.h" -#include "src/cpp/client/secure_credentials.h" -#include "test/core/util/port.h" -#include "test/core/util/test_config.h" -#include "test/core/util/tls_utils.h" -#include "test/cpp/end2end/test_service_impl.h" - -namespace grpc { -namespace testing { -namespace { - -const char* kRootPath = "test/core/tsi/test_creds/crl_data/ca.pem"; -const char* kRevokedKeyPath = "test/core/tsi/test_creds/crl_data/revoked.key"; -const char* kRevokedCertPath = "test/core/tsi/test_creds/crl_data/revoked.pem"; -const char* kValidKeyPath = "test/core/tsi/test_creds/crl_data/valid.key"; -const char* kValidCertPath = "test/core/tsi/test_creds/crl_data/valid.pem"; -const char* kRootCrlPath = "test/core/tsi/test_creds/crl_data/crls/current.crl"; -constexpr char kMessage[] = "Hello"; - -class CrlProviderTest : public ::testing::Test { - protected: - void RunServer(absl::Notification* notification, absl::string_view server_key, - absl::string_view server_cert) { - experimental::IdentityKeyCertPair key_cert_pair; - std::string root = grpc_core::testing::GetFileContents(kRootPath); - key_cert_pair.private_key = server_key.data(); - key_cert_pair.certificate_chain = server_cert.data(); - std::vector identity_key_cert_pairs; - identity_key_cert_pairs.emplace_back(key_cert_pair); - auto certificate_provider = - std::make_shared( - root, identity_key_cert_pairs); - grpc::experimental::TlsServerCredentialsOptions options( - certificate_provider); - options.watch_root_certs(); - options.set_root_cert_name("root"); - options.watch_identity_key_cert_pairs(); - options.set_identity_cert_name("identity"); - options.set_cert_request_type( - GRPC_SSL_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_AND_VERIFY); - auto server_credentials = grpc::experimental::TlsServerCredentials(options); - GPR_ASSERT(server_credentials.get() != nullptr); - - grpc::ServerBuilder builder; - TestServiceImpl service_; - - builder.AddListeningPort(server_addr_, server_credentials); - builder.RegisterService("foo.test.google.fr", &service_); - server_ = builder.BuildAndStart(); - notification->Notify(); - server_->Wait(); - } - - void TearDown() override { - if (server_ != nullptr) { - server_->Shutdown(); - server_thread_->join(); - delete server_thread_; - } - } - - TestServiceImpl service_; - std::unique_ptr server_ = nullptr; - std::thread* server_thread_ = nullptr; - std::string server_addr_; -}; - -void DoRpc(const std::string& server_addr, - const experimental::TlsChannelCredentialsOptions& tls_options, - bool expect_success) { - ChannelArguments channel_args; - channel_args.SetSslTargetNameOverride("foo.test.google.fr"); - std::shared_ptr channel = grpc::CreateCustomChannel( - server_addr, grpc::experimental::TlsCredentials(tls_options), - channel_args); - - auto stub = grpc::testing::EchoTestService::NewStub(channel); - grpc::testing::EchoRequest request; - grpc::testing::EchoResponse response; - request.set_message(kMessage); - ClientContext context; - context.set_deadline(grpc_timeout_seconds_to_deadline(/*time_s=*/10)); - grpc::Status result = stub->Echo(&context, request, &response); - if (expect_success) { - EXPECT_TRUE(result.ok()); - if (!result.ok()) { - gpr_log(GPR_ERROR, "%s, %s", result.error_message().c_str(), - result.error_details().c_str()); - } - EXPECT_EQ(response.message(), kMessage); - } else { - EXPECT_FALSE(result.ok()); - } -} - -TEST_F(CrlProviderTest, CrlProviderValid) { - server_addr_ = absl::StrCat("localhost:", - std::to_string(grpc_pick_unused_port_or_die())); - absl::Notification notification; - std::string server_key = grpc_core::testing::GetFileContents(kValidKeyPath); - std::string server_cert = grpc_core::testing::GetFileContents(kValidCertPath); - server_thread_ = new std::thread( - [&]() { RunServer(¬ification, server_key, server_cert); }); - notification.WaitForNotification(); - - std::string root_cert = grpc_core::testing::GetFileContents(kRootPath); - std::string client_key = grpc_core::testing::GetFileContents(kValidKeyPath); - std::string client_cert = grpc_core::testing::GetFileContents(kValidCertPath); - experimental::IdentityKeyCertPair key_cert_pair; - key_cert_pair.private_key = client_key; - key_cert_pair.certificate_chain = client_cert; - std::vector identity_key_cert_pairs; - identity_key_cert_pairs.emplace_back(key_cert_pair); - auto certificate_provider = - std::make_shared( - root_cert, identity_key_cert_pairs); - grpc::experimental::TlsChannelCredentialsOptions options; - options.set_certificate_provider(certificate_provider); - options.watch_root_certs(); - options.set_root_cert_name("root"); - options.watch_identity_key_cert_pairs(); - options.set_identity_cert_name("identity"); - std::string root_crl = grpc_core::testing::GetFileContents(kRootCrlPath); - - absl::StatusOr> - provider = grpc_core::experimental::CreateStaticCrlProvider({root_crl}); - ASSERT_TRUE(provider.ok()); - - options.set_crl_provider(*provider); - options.set_check_call_host(false); - auto verifier = std::make_shared(); - options.set_certificate_verifier(verifier); - - DoRpc(server_addr_, options, true); -} - -TEST_F(CrlProviderTest, CrlProviderRevokedServer) { - server_addr_ = absl::StrCat("localhost:", - std::to_string(grpc_pick_unused_port_or_die())); - absl::Notification notification; - std::string server_key = grpc_core::testing::GetFileContents(kRevokedKeyPath); - std::string server_cert = - grpc_core::testing::GetFileContents(kRevokedCertPath); - server_thread_ = new std::thread( - [&]() { RunServer(¬ification, server_key, server_cert); }); - notification.WaitForNotification(); - - std::string root_cert = grpc_core::testing::GetFileContents(kRootPath); - std::string client_key = grpc_core::testing::GetFileContents(kValidKeyPath); - std::string client_cert = grpc_core::testing::GetFileContents(kValidCertPath); - experimental::IdentityKeyCertPair key_cert_pair; - key_cert_pair.private_key = client_key; - key_cert_pair.certificate_chain = client_cert; - std::vector identity_key_cert_pairs; - identity_key_cert_pairs.emplace_back(key_cert_pair); - auto certificate_provider = - std::make_shared( - root_cert, identity_key_cert_pairs); - grpc::experimental::TlsChannelCredentialsOptions options; - options.set_certificate_provider(certificate_provider); - options.watch_root_certs(); - options.set_root_cert_name("root"); - options.watch_identity_key_cert_pairs(); - options.set_identity_cert_name("identity"); - std::string root_crl = grpc_core::testing::GetFileContents(kRootCrlPath); - - absl::StatusOr> - provider = grpc_core::experimental::CreateStaticCrlProvider({root_crl}); - ASSERT_TRUE(provider.ok()); - - options.set_crl_provider(*provider); - options.set_check_call_host(false); - auto verifier = std::make_shared(); - options.set_certificate_verifier(verifier); - - DoRpc(server_addr_, options, false); -} - -} // namespace -} // namespace testing -} // namespace grpc - -int main(int argc, char** argv) { - grpc::testing::TestEnvironment env(&argc, argv); - ::testing::InitGoogleTest(&argc, argv); - int ret = RUN_ALL_TESTS(); - return ret; -} diff --git a/test/cpp/server/BUILD b/test/cpp/server/BUILD index 0927de6c7a0..a4d7511f608 100644 --- a/test/cpp/server/BUILD +++ b/test/cpp/server/BUILD @@ -78,7 +78,6 @@ grpc_cc_test( "//:gpr", "//:grpc", "//:grpc++", - "//:grpc++_base", "//test/core/util:grpc_test_util", "//test/cpp/util:tls_test_utils", ], diff --git a/test/cpp/server/credentials_test.cc b/test/cpp/server/credentials_test.cc index 0b6fdf331f0..97db8d5be10 100644 --- a/test/cpp/server/credentials_test.cc +++ b/test/cpp/server/credentials_test.cc @@ -20,11 +20,9 @@ #include #include -#include #include #include #include -#include #include "src/cpp/client/secure_credentials.h" #include "test/core/util/port.h" @@ -178,34 +176,6 @@ TEST(CredentialsTest, TlsServerCredentialsWithAsyncExternalVerifier) { GPR_ASSERT(server_credentials.get() != nullptr); } -TEST(CredentialsTest, TlsServerCredentialsWithCrlProvider) { - auto provider = experimental::CreateStaticCrlProvider({}); - ASSERT_TRUE(provider.ok()); - auto certificate_provider = std::make_shared( - SERVER_KEY_PATH, SERVER_CERT_PATH, CA_CERT_PATH, 1); - grpc::experimental::TlsServerCredentialsOptions options(certificate_provider); - options.set_crl_provider(*provider); - auto channel_credentials = grpc::experimental::TlsServerCredentials(options); - GPR_ASSERT(channel_credentials.get() != nullptr); -} - -TEST(CredentialsTest, TlsServerCredentialsWithCrlProviderAndDirectory) { - auto provider = experimental::CreateStaticCrlProvider({}); - ASSERT_TRUE(provider.ok()); - auto certificate_provider = std::make_shared( - SERVER_KEY_PATH, SERVER_CERT_PATH, CA_CERT_PATH, 1); - grpc::experimental::TlsServerCredentialsOptions options(certificate_provider); - options.set_crl_directory(CRL_DIR_PATH); - options.set_crl_provider(*provider); - auto server_credentials = grpc::experimental::TlsServerCredentials(options); - // TODO(gtcooke94) - behavior might change to make this return nullptr in - // the future - GPR_ASSERT(server_credentials != nullptr); -} - -// TODO(gtcooke94) - Add test to make sure Tls*CredentialsOptions does not leak -// when not moved into TlsCredentials - } // namespace } // namespace testing } // namespace grpc diff --git a/tools/codegen/core/gen_grpc_tls_credentials_options.py b/tools/codegen/core/gen_grpc_tls_credentials_options.py index 7180fa7aa1e..d3bcb92ae0c 100755 --- a/tools/codegen/core/gen_grpc_tls_credentials_options.py +++ b/tools/codegen/core/gen_grpc_tls_credentials_options.py @@ -220,16 +220,6 @@ _DATA_MEMBERS = [ test_value_1='"crl_directory_1"', test_value_2='"crl_directory_2"', ), - DataMember( - name="crl_provider", - type="std::shared_ptr", - getter_comment=("Returns the CRL Provider"), - setter_move_semantics=True, - special_comparator=("(crl_provider_ == other.crl_provider_)"), - test_name="DifferentCrlProvider", - test_value_1=("*experimental::CreateStaticCrlProvider({})"), - test_value_2=("*experimental::CreateStaticCrlProvider({})"), - ), DataMember( name="send_client_ca_list", type="bool", diff --git a/tools/distrib/fix_build_deps.py b/tools/distrib/fix_build_deps.py index 4c7783ca1a3..59e6be828c5 100755 --- a/tools/distrib/fix_build_deps.py +++ b/tools/distrib/fix_build_deps.py @@ -150,7 +150,6 @@ EXTERNAL_DEPS = { "openssl/err.h": "libcrypto", "openssl/evp.h": "libcrypto", "openssl/hmac.h": "libcrypto", - "openssl/mem.h": "libcrypto", "openssl/param_build.h": "libcrypto", "openssl/pem.h": "libcrypto", "openssl/rsa.h": "libcrypto", diff --git a/tools/doxygen/Doxyfile.c++ b/tools/doxygen/Doxyfile.c++ index 63dbd861883..51dea99f9cf 100644 --- a/tools/doxygen/Doxyfile.c++ +++ b/tools/doxygen/Doxyfile.c++ @@ -892,7 +892,6 @@ include/grpc/event_engine/slice_buffer.h \ include/grpc/fork.h \ include/grpc/grpc.h \ include/grpc/grpc_audit_logging.h \ -include/grpc/grpc_crl_provider.h \ include/grpc/grpc_posix.h \ include/grpc/grpc_security.h \ include/grpc/grpc_security_constants.h \ @@ -1048,7 +1047,6 @@ include/grpcpp/security/server_credentials.h \ include/grpcpp/security/tls_certificate_provider.h \ include/grpcpp/security/tls_certificate_verifier.h \ include/grpcpp/security/tls_credentials_options.h \ -include/grpcpp/security/tls_crl_provider.h \ include/grpcpp/server.h \ include/grpcpp/server_builder.h \ include/grpcpp/server_context.h \ diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index cfd3658b2fe..f159937a115 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -892,7 +892,6 @@ include/grpc/event_engine/slice_buffer.h \ include/grpc/fork.h \ include/grpc/grpc.h \ include/grpc/grpc_audit_logging.h \ -include/grpc/grpc_crl_provider.h \ include/grpc/grpc_posix.h \ include/grpc/grpc_security.h \ include/grpc/grpc_security_constants.h \ @@ -1048,7 +1047,6 @@ include/grpcpp/security/server_credentials.h \ include/grpcpp/security/tls_certificate_provider.h \ include/grpcpp/security/tls_certificate_verifier.h \ include/grpcpp/security/tls_credentials_options.h \ -include/grpcpp/security/tls_crl_provider.h \ include/grpcpp/server.h \ include/grpcpp/server_builder.h \ include/grpcpp/server_context.h \ @@ -2596,8 +2594,6 @@ src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.cc \ src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.h \ src/core/lib/security/credentials/tls/grpc_tls_credentials_options.cc \ src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h \ -src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc \ -src/core/lib/security/credentials/tls/grpc_tls_crl_provider.h \ src/core/lib/security/credentials/tls/tls_credentials.cc \ src/core/lib/security/credentials/tls/tls_credentials.h \ src/core/lib/security/credentials/tls/tls_utils.cc \ diff --git a/tools/doxygen/Doxyfile.core b/tools/doxygen/Doxyfile.core index 91c4e7c623f..96dec88021d 100644 --- a/tools/doxygen/Doxyfile.core +++ b/tools/doxygen/Doxyfile.core @@ -825,7 +825,6 @@ include/grpc/event_engine/slice_buffer.h \ include/grpc/fork.h \ include/grpc/grpc.h \ include/grpc/grpc_audit_logging.h \ -include/grpc/grpc_crl_provider.h \ include/grpc/grpc_posix.h \ include/grpc/grpc_security.h \ include/grpc/grpc_security_constants.h \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 4cfd44b272f..f59baf1f014 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -825,7 +825,6 @@ include/grpc/event_engine/slice_buffer.h \ include/grpc/fork.h \ include/grpc/grpc.h \ include/grpc/grpc_audit_logging.h \ -include/grpc/grpc_crl_provider.h \ include/grpc/grpc_posix.h \ include/grpc/grpc_security.h \ include/grpc/grpc_security_constants.h \ @@ -2375,8 +2374,6 @@ src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.cc \ src/core/lib/security/credentials/tls/grpc_tls_certificate_verifier.h \ src/core/lib/security/credentials/tls/grpc_tls_credentials_options.cc \ src/core/lib/security/credentials/tls/grpc_tls_credentials_options.h \ -src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc \ -src/core/lib/security/credentials/tls/grpc_tls_crl_provider.h \ src/core/lib/security/credentials/tls/tls_credentials.cc \ src/core/lib/security/credentials/tls/tls_credentials.h \ src/core/lib/security/credentials/tls/tls_utils.cc \ diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index c35ba3e2033..e3b79761c75 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -2615,30 +2615,6 @@ ], "uses_polling": false }, - { - "args": [], - "benchmark": false, - "ci_platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "gtest": true, - "language": "c++", - "name": "crl_provider_test", - "platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "uses_polling": true - }, { "args": [], "benchmark": false, @@ -4231,30 +4207,6 @@ ], "uses_polling": true }, - { - "args": [], - "benchmark": false, - "ci_platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "gtest": true, - "language": "c++", - "name": "grpc_tls_crl_provider_test", - "platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "uses_polling": true - }, { "args": [], "benchmark": false,