From 0d4e1ef5df0751393003d1745cd4de184ab494ed Mon Sep 17 00:00:00 2001 From: Gregory Cooke Date: Fri, 3 Nov 2023 16:48:05 -0400 Subject: [PATCH] [Security - Revocation] Crl Directory Watcher Implementation (#34749) This adds the directory reloader implementation of the CrlProvider. This will periodically reload CRL files in a directory per [gRFC A69](https://github.com/grpc/proposal/pull/382) Included in this is the following: * A public API to create the `DirectoryReloaderCrlProvider` * A basic directory interface in gprpp and platform specific impls for getting the list of files in a directory (unfortunately prior C++17, there is no std::filesystem, so we have to have platform specific impls) * The implementation of `DirectoryReloaderCrlProvider` takes an event_engine and a directory interface. This allows us to test using the fuzzing event engine for time mocking, and to implement a test directory interface so we avoid having to make temporary directories and files in the tests. This is notably not in `include`, and the `CreateDirectoryReloaderCrlProvider` is the only way to construct one from the public API, so we don't expose the event engine and directory details to the user. --------- Co-authored-by: gtcooke94 --- CMakeLists.txt | 50 +++++ Makefile | 4 + Package.swift | 3 + build_autogenerated.yaml | 23 +- config.m4 | 2 + config.w32 | 2 + gRPC-C++.podspec | 2 + gRPC-Core.podspec | 4 + grpc.gemspec | 3 + grpc.gyp | 2 + include/grpc/grpc_crl_provider.h | 13 +- package.xml | 3 + src/core/BUILD | 29 +++ src/core/lib/gprpp/directory_reader.h | 48 +++++ src/core/lib/gprpp/posix/directory_reader.cc | 82 +++++++ .../lib/gprpp/windows/directory_reader.cc | 80 +++++++ .../credentials/tls/grpc_tls_crl_provider.cc | 120 +++++++++++ .../credentials/tls/grpc_tls_crl_provider.h | 50 +++++ src/python/grpcio/grpc_core_dependencies.py | 2 + test/core/gprpp/BUILD | 21 ++ test/core/gprpp/directory_reader_test.cc | 64 ++++++ test/core/security/BUILD | 10 + .../security/grpc_tls_crl_provider_test.cc | 202 ++++++++++++++++-- tools/doxygen/Doxyfile.c++.internal | 3 + tools/doxygen/Doxyfile.core.internal | 3 + tools/run_tests/generated/tests.json | 24 +++ 26 files changed, 832 insertions(+), 17 deletions(-) create mode 100644 src/core/lib/gprpp/directory_reader.h create mode 100644 src/core/lib/gprpp/posix/directory_reader.cc create mode 100644 src/core/lib/gprpp/windows/directory_reader.cc create mode 100644 test/core/gprpp/directory_reader_test.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index 4bc00175554..ac4f4aa3854 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -822,6 +822,9 @@ protobuf_generate_grpc_cpp_with_import_path_correction( protobuf_generate_grpc_cpp_with_import_path_correction( test/core/tsi/alts/fake_handshaker/transport_security_common.proto test/core/tsi/alts/fake_handshaker/transport_security_common.proto ) +protobuf_generate_grpc_cpp_with_import_path_correction( + test/core/util/fuzz_config_vars.proto test/core/util/fuzz_config_vars.proto +) if(gRPC_BUILD_TESTS) add_custom_target(buildtests_c) @@ -980,6 +983,7 @@ if(gRPC_BUILD_TESTS) add_dependencies(buildtests_cxx default_host_test) add_dependencies(buildtests_cxx delegating_channel_test) add_dependencies(buildtests_cxx destroy_grpclb_channel_with_active_connect_stress_test) + add_dependencies(buildtests_cxx directory_reader_test) add_dependencies(buildtests_cxx disappearing_server_test) add_dependencies(buildtests_cxx dns_resolver_cooldown_test) add_dependencies(buildtests_cxx dns_resolver_test) @@ -2267,11 +2271,13 @@ add_library(grpc src/core/lib/experiments/experiments.cc src/core/lib/gprpp/load_file.cc src/core/lib/gprpp/per_cpu.cc + src/core/lib/gprpp/posix/directory_reader.cc src/core/lib/gprpp/ref_counted_string.cc src/core/lib/gprpp/status_helper.cc src/core/lib/gprpp/time.cc src/core/lib/gprpp/time_averaged_stats.cc src/core/lib/gprpp/validation_errors.cc + src/core/lib/gprpp/windows/directory_reader.cc src/core/lib/gprpp/work_serializer.cc src/core/lib/handshaker/proxy_mapper_registry.cc src/core/lib/http/format_request.cc @@ -10297,6 +10303,39 @@ target_link_libraries(destroy_grpclb_channel_with_active_connect_stress_test ) +endif() +if(gRPC_BUILD_TESTS) + +add_executable(directory_reader_test + test/core/gprpp/directory_reader_test.cc +) +target_compile_features(directory_reader_test PUBLIC cxx_std_14) +target_include_directories(directory_reader_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(directory_reader_test + ${_gRPC_ALLTARGETS_LIBRARIES} + gtest + grpc_test_util +) + + endif() if(gRPC_BUILD_TESTS) @@ -13426,6 +13465,16 @@ endif() if(gRPC_BUILD_TESTS) add_executable(grpc_tls_crl_provider_test + ${_gRPC_PROTO_GENS_DIR}/test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.pb.cc + ${_gRPC_PROTO_GENS_DIR}/test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.grpc.pb.cc + ${_gRPC_PROTO_GENS_DIR}/test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.pb.h + ${_gRPC_PROTO_GENS_DIR}/test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.grpc.pb.h + ${_gRPC_PROTO_GENS_DIR}/test/core/util/fuzz_config_vars.pb.cc + ${_gRPC_PROTO_GENS_DIR}/test/core/util/fuzz_config_vars.grpc.pb.cc + ${_gRPC_PROTO_GENS_DIR}/test/core/util/fuzz_config_vars.pb.h + ${_gRPC_PROTO_GENS_DIR}/test/core/util/fuzz_config_vars.grpc.pb.h + test/core/event_engine/event_engine_test_utils.cc + test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.cc test/core/security/grpc_tls_crl_provider_test.cc ) target_compile_features(grpc_tls_crl_provider_test PUBLIC cxx_std_14) @@ -13451,6 +13500,7 @@ target_include_directories(grpc_tls_crl_provider_test target_link_libraries(grpc_tls_crl_provider_test ${_gRPC_ALLTARGETS_LIBRARIES} gtest + ${_gRPC_PROTOBUF_LIBRARIES} grpc_test_util ) diff --git a/Makefile b/Makefile index cab8c8dbe46..384c374fad8 100644 --- a/Makefile +++ b/Makefile @@ -1484,11 +1484,13 @@ LIBGRPC_SRC = \ src/core/lib/experiments/experiments.cc \ src/core/lib/gprpp/load_file.cc \ src/core/lib/gprpp/per_cpu.cc \ + src/core/lib/gprpp/posix/directory_reader.cc \ src/core/lib/gprpp/ref_counted_string.cc \ src/core/lib/gprpp/status_helper.cc \ src/core/lib/gprpp/time.cc \ src/core/lib/gprpp/time_averaged_stats.cc \ src/core/lib/gprpp/validation_errors.cc \ + src/core/lib/gprpp/windows/directory_reader.cc \ src/core/lib/gprpp/work_serializer.cc \ src/core/lib/handshaker/proxy_mapper_registry.cc \ src/core/lib/http/format_request.cc \ @@ -3663,6 +3665,8 @@ src/core/ext/xds/xds_route_config.cc: $(OPENSSL_DEP) src/core/ext/xds/xds_routing.cc: $(OPENSSL_DEP) src/core/ext/xds/xds_server_config_fetcher.cc: $(OPENSSL_DEP) src/core/ext/xds/xds_transport_grpc.cc: $(OPENSSL_DEP) +src/core/lib/gprpp/posix/directory_reader.cc: $(OPENSSL_DEP) +src/core/lib/gprpp/windows/directory_reader.cc: $(OPENSSL_DEP) src/core/lib/http/httpcli_security_connector.cc: $(OPENSSL_DEP) src/core/lib/json/json_util.cc: $(OPENSSL_DEP) src/core/lib/matchers/matchers.cc: $(OPENSSL_DEP) diff --git a/Package.swift b/Package.swift index d80643d5683..0296a882cb5 100644 --- a/Package.swift +++ b/Package.swift @@ -1223,6 +1223,7 @@ let package = Package( "src/core/lib/gprpp/crash.cc", "src/core/lib/gprpp/crash.h", "src/core/lib/gprpp/debug_location.h", + "src/core/lib/gprpp/directory_reader.h", "src/core/lib/gprpp/dual_ref_counted.h", "src/core/lib/gprpp/env.h", "src/core/lib/gprpp/examine_stack.cc", @@ -1247,6 +1248,7 @@ let package = Package( "src/core/lib/gprpp/packed_table.h", "src/core/lib/gprpp/per_cpu.cc", "src/core/lib/gprpp/per_cpu.h", + "src/core/lib/gprpp/posix/directory_reader.cc", "src/core/lib/gprpp/posix/env.cc", "src/core/lib/gprpp/posix/stat.cc", "src/core/lib/gprpp/posix/thd.cc", @@ -1276,6 +1278,7 @@ let package = Package( "src/core/lib/gprpp/unique_type_name.h", "src/core/lib/gprpp/validation_errors.cc", "src/core/lib/gprpp/validation_errors.h", + "src/core/lib/gprpp/windows/directory_reader.cc", "src/core/lib/gprpp/windows/env.cc", "src/core/lib/gprpp/windows/stat.cc", "src/core/lib/gprpp/windows/thd.cc", diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 92822acda00..d81f2a77913 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -752,6 +752,7 @@ libs: - src/core/lib/gprpp/bitset.h - src/core/lib/gprpp/chunked_vector.h - src/core/lib/gprpp/cpp_impl_of.h + - src/core/lib/gprpp/directory_reader.h - src/core/lib/gprpp/dual_ref_counted.h - src/core/lib/gprpp/if_list.h - src/core/lib/gprpp/load_file.h @@ -1569,11 +1570,13 @@ libs: - src/core/lib/experiments/experiments.cc - src/core/lib/gprpp/load_file.cc - src/core/lib/gprpp/per_cpu.cc + - src/core/lib/gprpp/posix/directory_reader.cc - src/core/lib/gprpp/ref_counted_string.cc - src/core/lib/gprpp/status_helper.cc - src/core/lib/gprpp/time.cc - src/core/lib/gprpp/time_averaged_stats.cc - src/core/lib/gprpp/validation_errors.cc + - src/core/lib/gprpp/windows/directory_reader.cc - src/core/lib/gprpp/work_serializer.cc - src/core/lib/handshaker/proxy_mapper_registry.cc - src/core/lib/http/format_request.cc @@ -7529,6 +7532,17 @@ targets: deps: - gtest - grpc++_test_util +- name: directory_reader_test + gtest: true + build: test + language: c++ + headers: [] + src: + - test/core/gprpp/directory_reader_test.cc + deps: + - gtest + - grpc_test_util + uses_polling: false - name: disappearing_server_test gtest: true build: test @@ -9469,11 +9483,18 @@ targets: gtest: true build: test language: c++ - headers: [] + headers: + - test/core/event_engine/event_engine_test_utils.h + - test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.h src: + - test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.proto + - test/core/util/fuzz_config_vars.proto + - test/core/event_engine/event_engine_test_utils.cc + - test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.cc - test/core/security/grpc_tls_crl_provider_test.cc deps: - gtest + - protobuf - grpc_test_util - name: grpc_tool_test gtest: true diff --git a/config.m4 b/config.m4 index dbe3f16207e..2b0b5814e1e 100644 --- a/config.m4 +++ b/config.m4 @@ -608,6 +608,7 @@ if test "$PHP_GRPC" != "no"; then src/core/lib/gprpp/load_file.cc \ src/core/lib/gprpp/mpscq.cc \ src/core/lib/gprpp/per_cpu.cc \ + src/core/lib/gprpp/posix/directory_reader.cc \ src/core/lib/gprpp/posix/env.cc \ src/core/lib/gprpp/posix/stat.cc \ src/core/lib/gprpp/posix/thd.cc \ @@ -619,6 +620,7 @@ if test "$PHP_GRPC" != "no"; then src/core/lib/gprpp/time_averaged_stats.cc \ src/core/lib/gprpp/time_util.cc \ src/core/lib/gprpp/validation_errors.cc \ + src/core/lib/gprpp/windows/directory_reader.cc \ src/core/lib/gprpp/windows/env.cc \ src/core/lib/gprpp/windows/stat.cc \ src/core/lib/gprpp/windows/thd.cc \ diff --git a/config.w32 b/config.w32 index 9ff76d86af3..87fca3a97d7 100644 --- a/config.w32 +++ b/config.w32 @@ -573,6 +573,7 @@ if (PHP_GRPC != "no") { "src\\core\\lib\\gprpp\\load_file.cc " + "src\\core\\lib\\gprpp\\mpscq.cc " + "src\\core\\lib\\gprpp\\per_cpu.cc " + + "src\\core\\lib\\gprpp\\posix\\directory_reader.cc " + "src\\core\\lib\\gprpp\\posix\\env.cc " + "src\\core\\lib\\gprpp\\posix\\stat.cc " + "src\\core\\lib\\gprpp\\posix\\thd.cc " + @@ -584,6 +585,7 @@ if (PHP_GRPC != "no") { "src\\core\\lib\\gprpp\\time_averaged_stats.cc " + "src\\core\\lib\\gprpp\\time_util.cc " + "src\\core\\lib\\gprpp\\validation_errors.cc " + + "src\\core\\lib\\gprpp\\windows\\directory_reader.cc " + "src\\core\\lib\\gprpp\\windows\\env.cc " + "src\\core\\lib\\gprpp\\windows\\stat.cc " + "src\\core\\lib\\gprpp\\windows\\thd.cc " + diff --git a/gRPC-C++.podspec b/gRPC-C++.podspec index 567042b81c4..38b01e92c86 100644 --- a/gRPC-C++.podspec +++ b/gRPC-C++.podspec @@ -836,6 +836,7 @@ Pod::Spec.new do |s| 'src/core/lib/gprpp/cpp_impl_of.h', 'src/core/lib/gprpp/crash.h', 'src/core/lib/gprpp/debug_location.h', + 'src/core/lib/gprpp/directory_reader.h', 'src/core/lib/gprpp/dual_ref_counted.h', 'src/core/lib/gprpp/env.h', 'src/core/lib/gprpp/examine_stack.h', @@ -1912,6 +1913,7 @@ Pod::Spec.new do |s| 'src/core/lib/gprpp/cpp_impl_of.h', 'src/core/lib/gprpp/crash.h', 'src/core/lib/gprpp/debug_location.h', + 'src/core/lib/gprpp/directory_reader.h', 'src/core/lib/gprpp/dual_ref_counted.h', 'src/core/lib/gprpp/env.h', 'src/core/lib/gprpp/examine_stack.h', diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 9b966afbc7c..0291f72a5ee 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -1326,6 +1326,7 @@ Pod::Spec.new do |s| 'src/core/lib/gprpp/crash.cc', 'src/core/lib/gprpp/crash.h', 'src/core/lib/gprpp/debug_location.h', + 'src/core/lib/gprpp/directory_reader.h', 'src/core/lib/gprpp/dual_ref_counted.h', 'src/core/lib/gprpp/env.h', 'src/core/lib/gprpp/examine_stack.cc', @@ -1350,6 +1351,7 @@ Pod::Spec.new do |s| 'src/core/lib/gprpp/packed_table.h', 'src/core/lib/gprpp/per_cpu.cc', 'src/core/lib/gprpp/per_cpu.h', + 'src/core/lib/gprpp/posix/directory_reader.cc', 'src/core/lib/gprpp/posix/env.cc', 'src/core/lib/gprpp/posix/stat.cc', 'src/core/lib/gprpp/posix/thd.cc', @@ -1379,6 +1381,7 @@ Pod::Spec.new do |s| 'src/core/lib/gprpp/unique_type_name.h', 'src/core/lib/gprpp/validation_errors.cc', 'src/core/lib/gprpp/validation_errors.h', + 'src/core/lib/gprpp/windows/directory_reader.cc', 'src/core/lib/gprpp/windows/env.cc', 'src/core/lib/gprpp/windows/stat.cc', 'src/core/lib/gprpp/windows/thd.cc', @@ -2670,6 +2673,7 @@ Pod::Spec.new do |s| 'src/core/lib/gprpp/cpp_impl_of.h', 'src/core/lib/gprpp/crash.h', 'src/core/lib/gprpp/debug_location.h', + 'src/core/lib/gprpp/directory_reader.h', 'src/core/lib/gprpp/dual_ref_counted.h', 'src/core/lib/gprpp/env.h', 'src/core/lib/gprpp/examine_stack.h', diff --git a/grpc.gemspec b/grpc.gemspec index e3f14b7b7e3..ae972119229 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -1229,6 +1229,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/gprpp/crash.cc ) s.files += %w( src/core/lib/gprpp/crash.h ) s.files += %w( src/core/lib/gprpp/debug_location.h ) + s.files += %w( src/core/lib/gprpp/directory_reader.h ) s.files += %w( src/core/lib/gprpp/dual_ref_counted.h ) s.files += %w( src/core/lib/gprpp/env.h ) s.files += %w( src/core/lib/gprpp/examine_stack.cc ) @@ -1253,6 +1254,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/gprpp/packed_table.h ) s.files += %w( src/core/lib/gprpp/per_cpu.cc ) s.files += %w( src/core/lib/gprpp/per_cpu.h ) + s.files += %w( src/core/lib/gprpp/posix/directory_reader.cc ) s.files += %w( src/core/lib/gprpp/posix/env.cc ) s.files += %w( src/core/lib/gprpp/posix/stat.cc ) s.files += %w( src/core/lib/gprpp/posix/thd.cc ) @@ -1282,6 +1284,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/gprpp/unique_type_name.h ) s.files += %w( src/core/lib/gprpp/validation_errors.cc ) s.files += %w( src/core/lib/gprpp/validation_errors.h ) + s.files += %w( src/core/lib/gprpp/windows/directory_reader.cc ) s.files += %w( src/core/lib/gprpp/windows/env.cc ) s.files += %w( src/core/lib/gprpp/windows/stat.cc ) s.files += %w( src/core/lib/gprpp/windows/thd.cc ) diff --git a/grpc.gyp b/grpc.gyp index 9d76638ef4b..5a0c623053e 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -802,11 +802,13 @@ 'src/core/lib/experiments/experiments.cc', 'src/core/lib/gprpp/load_file.cc', 'src/core/lib/gprpp/per_cpu.cc', + 'src/core/lib/gprpp/posix/directory_reader.cc', 'src/core/lib/gprpp/ref_counted_string.cc', 'src/core/lib/gprpp/status_helper.cc', 'src/core/lib/gprpp/time.cc', 'src/core/lib/gprpp/time_averaged_stats.cc', 'src/core/lib/gprpp/validation_errors.cc', + 'src/core/lib/gprpp/windows/directory_reader.cc', 'src/core/lib/gprpp/work_serializer.cc', 'src/core/lib/handshaker/proxy_mapper_registry.cc', 'src/core/lib/http/format_request.cc', diff --git a/include/grpc/grpc_crl_provider.h b/include/grpc/grpc_crl_provider.h index a66e15e62ab..43f0310b2aa 100644 --- a/include/grpc/grpc_crl_provider.h +++ b/include/grpc/grpc_crl_provider.h @@ -28,7 +28,6 @@ #include "absl/strings/string_view.h" #include -#include namespace grpc_core { namespace experimental { @@ -68,6 +67,17 @@ class CrlProvider { absl::StatusOr> CreateStaticCrlProvider( absl::Span crls); +// Creates a CRL Provider that periodically and asynchronously reloads a +// directory. The refresh_duration minimum is 60 seconds. The +// reload_error_callback provides a way for the user to specifically log or +// otherwise notify of errors during reloading. Since reloading is asynchronous +// and not on the main codepath, the grpc process will continue to run through +// reloading errors, so this mechanism is an important way to provide signals to +// your monitoring and alerting setup. +absl::StatusOr> CreateDirectoryReloaderCrlProvider( + absl::string_view directory, std::chrono::seconds refresh_duration, + std::function reload_error_callback); + } // namespace experimental } // namespace grpc_core @@ -81,5 +91,4 @@ absl::StatusOr> CreateStaticCrlProvider( 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/package.xml b/package.xml index 2f2810960ed..fbf8f025506 100644 --- a/package.xml +++ b/package.xml @@ -1211,6 +1211,7 @@ + @@ -1235,6 +1236,7 @@ + @@ -1264,6 +1266,7 @@ + diff --git a/src/core/BUILD b/src/core/BUILD index c1dd1e080ba..d5a6b668a65 100644 --- a/src/core/BUILD +++ b/src/core/BUILD @@ -239,6 +239,27 @@ grpc_cc_library( ], ) +grpc_cc_library( + name = "directory_reader", + srcs = [ + "lib/gprpp/posix/directory_reader.cc", + "lib/gprpp/windows/directory_reader.cc", + ], + hdrs = [ + "lib/gprpp/directory_reader.h", + ], + external_deps = [ + "absl/functional:function_ref", + "absl/status", + "absl/status:statusor", + "absl/strings", + ], + deps = [ + "//:gpr", + "//:gpr_platform", + ], +) + grpc_cc_library( name = "chunked_vector", hdrs = ["lib/gprpp/chunked_vector.h"], @@ -3082,15 +3103,23 @@ grpc_cc_library( "lib/security/credentials/tls/grpc_tls_crl_provider.h", ], external_deps = [ + "absl/base:core_headers", "absl/container:flat_hash_map", "absl/status", "absl/status:statusor", "absl/strings", + "absl/types:optional", "absl/types:span", "libcrypto", "libssl", ], deps = [ + "default_event_engine", + "directory_reader", + "load_file", + "slice", + "time", + "//:exec_ctx", "//:gpr", "//:grpc_base", ], diff --git a/src/core/lib/gprpp/directory_reader.h b/src/core/lib/gprpp/directory_reader.h new file mode 100644 index 00000000000..0f0bbcf0b0c --- /dev/null +++ b/src/core/lib/gprpp/directory_reader.h @@ -0,0 +1,48 @@ +// +// +// 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_GPRPP_DIRECTORY_READER_H +#define GRPC_SRC_CORE_LIB_GPRPP_DIRECTORY_READER_H + +#include + +#include + +#include "absl/functional/function_ref.h" +#include "absl/status/status.h" +#include "absl/strings/string_view.h" + +namespace grpc_core { + +class DirectoryReader { + public: + virtual ~DirectoryReader() = default; + // Returns the name of the directory being read. + virtual absl::string_view Name() const = 0; + // Calls callback for each name in the directory except for "." and "..". + // Returns non-OK if there was an error reading the directory. + virtual absl::Status ForEach( + absl::FunctionRef callback) = 0; +}; + +std::unique_ptr MakeDirectoryReader( + absl::string_view filename); + +} // namespace grpc_core + +#endif // GRPC_SRC_CORE_LIB_GPRPP_DIRECTORY_READER_H \ No newline at end of file diff --git a/src/core/lib/gprpp/posix/directory_reader.cc b/src/core/lib/gprpp/posix/directory_reader.cc new file mode 100644 index 00000000000..ecdf3c68c66 --- /dev/null +++ b/src/core/lib/gprpp/posix/directory_reader.cc @@ -0,0 +1,82 @@ +// +// +// 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 "absl/functional/function_ref.h" +#include "absl/status/status.h" +#include "absl/strings/string_view.h" + +#if defined(GPR_LINUX) || defined(GPR_ANDROID) || defined(GPR_FREEBSD) || \ + defined(GPR_APPLE) + +#include + +#include + +#include "src/core/lib/gprpp/directory_reader.h" + +namespace grpc_core { + +namespace { +const char kSkipEntriesSelf[] = "."; +const char kSkipEntriesParent[] = ".."; +} // namespace + +class DirectoryReaderImpl : public DirectoryReader { + public: + explicit DirectoryReaderImpl(absl::string_view directory_path) + : directory_path_(directory_path) {} + absl::string_view Name() const override { return directory_path_; } + absl::Status ForEach(absl::FunctionRef) override; + + private: + const std::string directory_path_; +}; + +std::unique_ptr MakeDirectoryReader( + absl::string_view filename) { + return std::make_unique(filename); +} + +absl::Status DirectoryReaderImpl::ForEach( + absl::FunctionRef callback) { + // Open the dir for reading + DIR* directory = opendir(directory_path_.c_str()); + if (directory == nullptr) { + return absl::InternalError("Could not read crl directory."); + } + struct dirent* directory_entry; + // Iterate over everything in the directory + while ((directory_entry = readdir(directory)) != nullptr) { + const absl::string_view file_name = directory_entry->d_name; + // Skip "." and ".." + if (file_name == kSkipEntriesParent || file_name == kSkipEntriesSelf) { + continue; + } + // Call the callback with this filename + callback(file_name); + } + closedir(directory); + return absl::OkStatus(); +} +} // namespace grpc_core + +#endif // GPR_LINUX || GPR_ANDROID || GPR_FREEBSD || GPR_APPLE diff --git a/src/core/lib/gprpp/windows/directory_reader.cc b/src/core/lib/gprpp/windows/directory_reader.cc new file mode 100644 index 00000000000..790e213e789 --- /dev/null +++ b/src/core/lib/gprpp/windows/directory_reader.cc @@ -0,0 +1,80 @@ +// +// +// 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 + +#if defined(GPR_WINDOWS) + +#include +#include + +#include +#include + +#include "absl/status/statusor.h" +#include "absl/strings/string_view.h" + +#include + +#include "src/core/lib/gprpp/directory_reader.h" +namespace grpc_core { + +namespace { +const char kSkipEntriesSelf[] = "."; +const char kSkipEntriesParent[] = ".."; +} // namespace + +class DirectoryReaderImpl : public DirectoryReader { + public: + explicit DirectoryReaderImpl(absl::string_view directory_path) + : directory_path_(directory_path) {} + absl::string_view Name() const override { return directory_path_; } + absl::Status ForEach(absl::FunctionRef) override; + + private: + const std::string directory_path_; +}; + +std::unique_ptr MakeDirectoryReader( + absl::string_view filename) { + return std::make_unique(filename); +} + +// Reference for reading directory in Windows: +// https://stackoverflow.com/questions/612097/how-can-i-get-the-list-of-files-in-a-directory-using-c-or-c +// https://learn.microsoft.com/en-us/windows/win32/fileio/listing-the-files-in-a-directory +absl::Status DirectoryReaderImpl::ForEach( + absl::FunctionRef callback) { + std::string search_path = absl::StrCat(directory_path_, "/*"); + WIN32_FIND_DATAA find_data; + HANDLE hFind = ::FindFirstFileA(search_path.c_str(), &find_data); + if (hFind == INVALID_HANDLE_VALUE) { + return absl::InternalError("Could not read crl directory."); + } + do { + if (!(find_data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)) { + callback(find_data.cFileName); + } + } while (::FindNextFileA(hFind, &find_data)); + ::FindClose(hFind); + return absl::OkStatus(); +} + +} // namespace grpc_core + +#endif // GPR_WINDOWS 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 index ff25d73c28c..c3f52547197 100644 --- a/src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc +++ b/src/core/lib/security/credentials/tls/grpc_tls_crl_provider.cc @@ -22,8 +22,11 @@ #include +// IWYU pragma: no_include #include +#include #include +#include // IWYU pragma: no_include #include @@ -35,15 +38,25 @@ #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" +#include "absl/strings/str_join.h" #include "absl/types/span.h" #include +#include "src/core/lib/event_engine/default_event_engine.h" +#include "src/core/lib/gprpp/directory_reader.h" +#include "src/core/lib/gprpp/load_file.h" +#include "src/core/lib/iomgr/exec_ctx.h" +#include "src/core/lib/slice/slice.h" + namespace grpc_core { namespace experimental { namespace { std::string IssuerFromCrl(X509_CRL* crl) { + if (crl == nullptr) { + return ""; + } char* buf = X509_NAME_oneline(X509_CRL_get_issuer(crl), nullptr, 0); std::string ret; if (buf != nullptr) { @@ -53,6 +66,20 @@ std::string IssuerFromCrl(X509_CRL* crl) { return ret; } +absl::StatusOr> ReadCrlFromFile( + const std::string& crl_path) { + absl::StatusOr crl_slice = LoadFile(crl_path, false); + if (!crl_slice.ok()) { + return crl_slice.status(); + } + absl::StatusOr> crl = + Crl::Parse(crl_slice->as_string_view()); + if (!crl.ok()) { + return crl.status(); + } + return crl; +} + } // namespace absl::StatusOr> Crl::Parse(absl::string_view crl_string) { @@ -114,5 +141,98 @@ std::shared_ptr StaticCrlProvider::GetCrl( return it->second; } +absl::StatusOr> CreateDirectoryReloaderCrlProvider( + absl::string_view directory, std::chrono::seconds refresh_duration, + std::function reload_error_callback) { + if (refresh_duration < std::chrono::seconds(60)) { + return absl::InvalidArgumentError("Refresh duration minimum is 60 seconds"); + } + auto provider = std::make_shared( + refresh_duration, reload_error_callback, + grpc_event_engine::experimental::GetDefaultEventEngine(), + MakeDirectoryReader(directory)); + // This could be slow to do at startup, but we want to + // make sure it's done before the provider is used. + provider->UpdateAndStartTimer(); + return provider; +} + +DirectoryReloaderCrlProvider::~DirectoryReloaderCrlProvider() { + if (refresh_handle_.has_value()) { + event_engine_->Cancel(refresh_handle_.value()); + } +} + +void DirectoryReloaderCrlProvider::UpdateAndStartTimer() { + absl::Status status = Update(); + if (!status.ok() && reload_error_callback_ != nullptr) { + reload_error_callback_(status); + } + std::weak_ptr self = shared_from_this(); + refresh_handle_ = + event_engine_->RunAfter(refresh_duration_, [self = std::move(self)]() { + ApplicationCallbackExecCtx callback_exec_ctx; + ExecCtx exec_ctx; + if (std::shared_ptr valid_ptr = + self.lock()) { + valid_ptr->UpdateAndStartTimer(); + } + }); +} + +absl::Status DirectoryReloaderCrlProvider::Update() { + absl::flat_hash_map> new_crls; + std::vector files_with_errors; + absl::Status status = crl_directory_->ForEach([&](absl::string_view file) { + std::string file_path = absl::StrCat(crl_directory_->Name(), "/", file); + // Build a map of new_crls to update to. If all files successful, do a + // full swap of the map. Otherwise update in place. + absl::StatusOr> crl = ReadCrlFromFile(file_path); + if (!crl.ok()) { + files_with_errors.push_back( + absl::StrCat(file_path, ": ", crl.status().ToString())); + return; + } + // Now we have a good CRL to update in our map. + // It's not safe to say crl->Issuer() on the LHS and std::move(crl) on the + // RHS, because C++ does not guarantee which of those will be executed + // first. + std::string issuer((*crl)->Issuer()); + new_crls[std::move(issuer)] = std::move(*crl); + }); + if (!status.ok()) { + return status; + } + MutexLock lock(&mu_); + if (!files_with_errors.empty()) { + // Need to make sure CRLs we read successfully into new_crls are still + // in-place updated in crls_. + for (auto& kv : new_crls) { + std::shared_ptr& crl = kv.second; + // It's not safe to say crl->Issuer() on the LHS and std::move(crl) on the + // RHS, because C++ does not guarantee which of those will be executed + // first. + std::string issuer(crl->Issuer()); + crls_[std::move(issuer)] = std::move(crl); + } + return absl::UnknownError(absl::StrCat( + "Errors reading the following files in the CRL directory: [", + absl::StrJoin(files_with_errors, "; "), "]")); + } else { + crls_ = std::move(new_crls); + } + return absl::OkStatus(); +} + +std::shared_ptr DirectoryReloaderCrlProvider::GetCrl( + const CertificateInfo& certificate_info) { + MutexLock lock(&mu_); + 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 index 75039d9a790..d1ea0c09661 100644 --- a/src/core/lib/security/credentials/tls/grpc_tls_crl_provider.h +++ b/src/core/lib/security/credentials/tls/grpc_tls_crl_provider.h @@ -21,18 +21,29 @@ #include +#include +#include #include +#include #include #include #include +#include "absl/base/thread_annotations.h" #include "absl/container/flat_hash_map.h" +#include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/string_view.h" +#include "absl/types/optional.h" +#include #include +#include "src/core/lib/gprpp/directory_reader.h" +#include "src/core/lib/gprpp/sync.h" +#include "src/core/lib/gprpp/time.h" + namespace grpc_core { namespace experimental { @@ -77,6 +88,45 @@ class CertificateInfoImpl : public CertificateInfo { const std::string issuer_; }; +// Defining this here lets us hide implementation details (and includes) from +// the header in include +class DirectoryReloaderCrlProvider + : public CrlProvider, + public std::enable_shared_from_this { + public: + DirectoryReloaderCrlProvider( + std::chrono::seconds duration, std::function callback, + std::shared_ptr + event_engine, + std::shared_ptr directory_impl) + : refresh_duration_(Duration::FromSecondsAsDouble(duration.count())), + reload_error_callback_(std::move(callback)), + event_engine_(std::move(event_engine)), + crl_directory_(std::move(directory_impl)) {} + + ~DirectoryReloaderCrlProvider() override; + std::shared_ptr GetCrl(const CertificateInfo& certificate_info) override; + // Reads the configured directory and updates the internal crls_ map, called + // asynchronously by event engine then schedules the timer for the next + // update. + void UpdateAndStartTimer(); + + private: + // Reads the configured directory and updates the internal crls_ map, called + // asynchronously by event engine. + absl::Status Update(); + Duration refresh_duration_; + std::function reload_error_callback_; + std::shared_ptr event_engine_; + std::shared_ptr crl_directory_; + // guards the crls_ map + Mutex mu_; + absl::flat_hash_map<::std::string, ::std::shared_ptr> crls_ + ABSL_GUARDED_BY(mu_); + absl::optional + refresh_handle_; +}; + } // namespace experimental } // namespace grpc_core diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index 2ab962aaa53..14ad4b18b2d 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -582,6 +582,7 @@ CORE_SOURCE_FILES = [ 'src/core/lib/gprpp/load_file.cc', 'src/core/lib/gprpp/mpscq.cc', 'src/core/lib/gprpp/per_cpu.cc', + 'src/core/lib/gprpp/posix/directory_reader.cc', 'src/core/lib/gprpp/posix/env.cc', 'src/core/lib/gprpp/posix/stat.cc', 'src/core/lib/gprpp/posix/thd.cc', @@ -593,6 +594,7 @@ CORE_SOURCE_FILES = [ 'src/core/lib/gprpp/time_averaged_stats.cc', 'src/core/lib/gprpp/time_util.cc', 'src/core/lib/gprpp/validation_errors.cc', + 'src/core/lib/gprpp/windows/directory_reader.cc', 'src/core/lib/gprpp/windows/env.cc', 'src/core/lib/gprpp/windows/stat.cc', 'src/core/lib/gprpp/windows/thd.cc', diff --git a/test/core/gprpp/BUILD b/test/core/gprpp/BUILD index 283b9a44b3d..620004d7696 100644 --- a/test/core/gprpp/BUILD +++ b/test/core/gprpp/BUILD @@ -20,6 +20,27 @@ licenses(["notice"]) grpc_package(name = "test/core/gprpp") +grpc_cc_test( + name = "directory_reader_test", + srcs = ["directory_reader_test.cc"], + data = [ + "//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", + ], + external_deps = [ + "gtest", + ], + language = "C++", + uses_event_engine = False, + uses_polling = False, + deps = [ + "//src/core:directory_reader", + "//test/core/util:grpc_test_util", + ], +) + grpc_cc_test( name = "examine_stack_test", srcs = ["examine_stack_test.cc"], diff --git a/test/core/gprpp/directory_reader_test.cc b/test/core/gprpp/directory_reader_test.cc new file mode 100644 index 00000000000..3aa4c9d5cc5 --- /dev/null +++ b/test/core/gprpp/directory_reader_test.cc @@ -0,0 +1,64 @@ +// +// 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 "src/core/lib/gprpp/directory_reader.h" + +#include +#include + +#include "absl/strings/string_view.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +#include "test/core/util/test_config.h" + +static constexpr absl::string_view kCrlDirectory = + "test/core/tsi/test_creds/crl_data/crls/"; + +namespace grpc_core { +namespace testing { +namespace { + +TEST(DirectoryReader, CanListFiles) { + auto reader = MakeDirectoryReader(kCrlDirectory); + std::vector contents; + absl::Status status = reader->ForEach([&](absl::string_view filename) { + contents.push_back(std::string(filename)); + }); + ASSERT_TRUE(status.ok()) << status; + // IsSupersetOf() is needed instead of UnorderedElementsAre() because some + // builds/OS combinations will include the BUILD file in this directory when + // the tests are run + EXPECT_THAT(contents, + ::testing::IsSupersetOf({"ab06acdd.r0", "b9322cac.r0", + "current.crl", "intermediate.crl"})); +} + +TEST(DirectoryReader, NonexistentDirectory) { + auto reader = MakeDirectoryReader("DOES_NOT_EXIST"); + absl::Status status = reader->ForEach([](absl::string_view) {}); + ASSERT_FALSE(status.ok()) << status; +} + +} // namespace +} // namespace testing +} // namespace grpc_core + +int main(int argc, char** argv) { + grpc::testing::TestEnvironment env(&argc, argv); + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/test/core/security/BUILD b/test/core/security/BUILD index c37ac640d57..7c955f22afc 100644 --- a/test/core/security/BUILD +++ b/test/core/security/BUILD @@ -565,14 +565,24 @@ grpc_cc_test( name = "grpc_tls_crl_provider_test", srcs = ["grpc_tls_crl_provider_test.cc"], data = [ + "//test/core/tsi/test_creds/crl_data:ca.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", ], external_deps = ["gtest"], language = "C++", deps = [ "//:gpr", "//:grpc", + "//:iomgr_timer", + "//src/core:default_event_engine", "//src/core:grpc_crl_provider", + "//test/core/event_engine:event_engine_test_utils", + "//test/core/event_engine/fuzzing_event_engine", + "//test/core/event_engine/fuzzing_event_engine:fuzzing_event_engine_proto", + "//test/core/util:fuzz_config_vars_proto", "//test/core/util:grpc_test_util", ], ) diff --git a/test/core/security/grpc_tls_crl_provider_test.cc b/test/core/security/grpc_tls_crl_provider_test.cc index e2729c5980f..f47028b160b 100644 --- a/test/core/security/grpc_tls_crl_provider_test.cc +++ b/test/core/security/grpc_tls_crl_provider_test.cc @@ -18,6 +18,8 @@ #include "src/core/lib/security/credentials/tls/grpc_tls_crl_provider.h" +#include +#include #include #include #include @@ -26,30 +28,122 @@ #include "absl/status/status.h" #include "absl/status/statusor.h" +#include "absl/strings/str_split.h" #include "absl/strings/string_view.h" #include #include #include +#include "src/core/lib/event_engine/default_event_engine.h" +#include "src/core/lib/iomgr/timer_manager.h" +#include "test/core/event_engine/event_engine_test_utils.h" +#include "test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.h" +#include "test/core/event_engine/fuzzing_event_engine/fuzzing_event_engine.pb.h" #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/current.crl"; -const absl::string_view kCrlIssuer = +static constexpr absl::string_view kCrlPath = + "test/core/tsi/test_creds/crl_data/crls/current.crl"; +static constexpr absl::string_view kCrlName = "current.crl"; +static constexpr absl::string_view kCrlIssuer = "/C=AU/ST=Some-State/O=Internet Widgits Pty Ltd/CN=testca"; - -namespace grpc_core { -namespace testing { +static constexpr absl::string_view kCrlIntermediateIssuer = + "/CN=intermediatecert.example.com"; +static constexpr absl::string_view kCrlDirectory = + "test/core/tsi/test_creds/crl_data/crls"; +static constexpr absl::string_view kRootCert = + "test/core/tsi/test_creds/crl_data/ca.pem"; using ::grpc_core::experimental::CertificateInfoImpl; using ::grpc_core::experimental::Crl; using ::grpc_core::experimental::CrlProvider; +namespace grpc_core { +namespace testing { + +class FakeDirectoryReader : public DirectoryReader { + public: + ~FakeDirectoryReader() override = default; + absl::Status ForEach( + absl::FunctionRef callback) override { + if (!files_in_directory_.ok()) { + return files_in_directory_.status(); + } + for (const auto& file : *files_in_directory_) { + callback(file); + } + return absl::OkStatus(); + } + absl::string_view Name() const override { return kCrlDirectory; } + + void SetFilesInDirectory(std::vector files) { + files_in_directory_ = std::move(files); + } + + void SetStatus(absl::Status status) { files_in_directory_ = status; } + + private: + absl::StatusOr> files_in_directory_ = + std::vector(); +}; + +class DirectoryReloaderCrlProviderTest : public ::testing::Test { + public: + void SetUp() override { + event_engine_ = + std::make_shared( + grpc_event_engine::experimental::FuzzingEventEngine::Options(), + fuzzing_event_engine::Actions()); + // Without this the test had a failure dealing with grpc timers on TSAN + grpc_timer_manager_set_start_threaded(false); + grpc_init(); + } + void TearDown() override { + ExecCtx exec_ctx; + event_engine_->FuzzingDone(); + exec_ctx.Flush(); + event_engine_->TickUntilIdle(); + grpc_event_engine::experimental::WaitForSingleOwner( + std::move(event_engine_)); + grpc_shutdown_blocking(); + event_engine_.reset(); + } + + protected: + // Tests that want a fake directory reader can call this without setting the + // last parameter. + absl::StatusOr> CreateCrlProvider( + std::chrono::seconds refresh_duration, + std::function reload_error_callback, + std::shared_ptr directory_reader = nullptr) { + if (directory_reader == nullptr) directory_reader = directory_reader_; + auto provider = + std::make_shared( + refresh_duration, std::move(reload_error_callback), event_engine_, + std::move(directory_reader)); + provider->UpdateAndStartTimer(); + return provider; + } + + // Tests that want a real directory can call this instead of the above. + absl::StatusOr> CreateCrlProvider( + absl::string_view directory, std::chrono::seconds refresh_duration, + std::function reload_error_callback) { + return CreateCrlProvider(refresh_duration, std::move(reload_error_callback), + MakeDirectoryReader(directory)); + } + + std::shared_ptr directory_reader_ = + std::make_shared(); + std::shared_ptr + event_engine_; +}; + TEST(CrlProviderTest, CanParseCrl) { - std::string crl_string = GetFileContents(kCrlPath); + std::string crl_string = GetFileContents(kCrlPath.data()); absl::StatusOr> crl = Crl::Parse(crl_string); - ASSERT_TRUE(crl.ok()); + ASSERT_TRUE(crl.ok()) << crl.status(); ASSERT_NE(*crl, nullptr); EXPECT_EQ((*crl)->Issuer(), kCrlIssuer); } @@ -63,34 +157,114 @@ TEST(CrlProviderTest, InvalidFile) { } TEST(CrlProviderTest, StaticCrlProviderLookup) { - std::vector crl_strings = {GetFileContents(kCrlPath)}; + std::vector crl_strings = {GetFileContents(kCrlPath.data())}; absl::StatusOr> provider = experimental::CreateStaticCrlProvider(crl_strings); ASSERT_TRUE(provider.ok()) << provider.status(); - CertificateInfoImpl cert = CertificateInfoImpl(kCrlIssuer); + CertificateInfoImpl cert(kCrlIssuer); auto crl = (*provider)->GetCrl(cert); ASSERT_NE(crl, nullptr); EXPECT_EQ(crl->Issuer(), kCrlIssuer); } -TEST(CrlProviderTest, StaticCrlProviderLookupBad) { - std::vector crl_strings = {GetFileContents(kCrlPath)}; +TEST(CrlProviderTest, StaticCrlProviderLookupIssuerNotFound) { + std::vector crl_strings = {GetFileContents(kCrlPath.data())}; absl::StatusOr> provider = experimental::CreateStaticCrlProvider(crl_strings); ASSERT_TRUE(provider.ok()) << provider.status(); - CertificateInfoImpl bad_cert = CertificateInfoImpl("BAD CERT"); + CertificateInfoImpl bad_cert("BAD CERT"); auto crl = (*provider)->GetCrl(bad_cert); EXPECT_EQ(crl, nullptr); } +TEST_F(DirectoryReloaderCrlProviderTest, CrlLookupGood) { + auto provider = + CreateCrlProvider(kCrlDirectory, std::chrono::seconds(60), nullptr); + ASSERT_TRUE(provider.ok()) << provider.status(); + CertificateInfoImpl cert(kCrlIssuer); + auto crl = (*provider)->GetCrl(cert); + ASSERT_NE(crl, nullptr); + EXPECT_EQ(crl->Issuer(), kCrlIssuer); + CertificateInfoImpl intermediate(kCrlIntermediateIssuer); + auto intermediate_crl = (*provider)->GetCrl(intermediate); + ASSERT_NE(intermediate_crl, nullptr); + EXPECT_EQ(intermediate_crl->Issuer(), kCrlIntermediateIssuer); +} + +TEST_F(DirectoryReloaderCrlProviderTest, CrlLookupMissingIssuer) { + auto provider = + CreateCrlProvider(kCrlDirectory, std::chrono::seconds(60), nullptr); + ASSERT_TRUE(provider.ok()) << provider.status(); + CertificateInfoImpl bad_cert("BAD CERT"); + auto crl = (*provider)->GetCrl(bad_cert); + ASSERT_EQ(crl, nullptr); +} + +TEST_F(DirectoryReloaderCrlProviderTest, ReloadsAndDeletes) { + const std::chrono::seconds kRefreshDuration(60); + auto provider = CreateCrlProvider(kRefreshDuration, nullptr); + ASSERT_TRUE(provider.ok()) << provider.status(); + CertificateInfoImpl cert(kCrlIssuer); + auto should_be_no_crl = (*provider)->GetCrl(cert); + ASSERT_EQ(should_be_no_crl, nullptr); + // Give the provider files to find in the directory + directory_reader_->SetFilesInDirectory({std::string(kCrlName)}); + event_engine_->TickForDuration(kRefreshDuration); + auto crl = (*provider)->GetCrl(cert); + ASSERT_NE(crl, nullptr); + EXPECT_EQ(crl->Issuer(), kCrlIssuer); + // Now we won't see any files in our directory + directory_reader_->SetFilesInDirectory({}); + event_engine_->TickForDuration(kRefreshDuration); + auto crl_should_be_deleted = (*provider)->GetCrl(cert); + ASSERT_EQ(crl_should_be_deleted, nullptr); +} + +TEST_F(DirectoryReloaderCrlProviderTest, WithCorruption) { + directory_reader_->SetFilesInDirectory({std::string(kCrlName)}); + const std::chrono::seconds kRefreshDuration(60); + std::vector reload_errors; + std::function reload_error_callback = + [&](const absl::Status& status) { reload_errors.push_back(status); }; + auto provider = + CreateCrlProvider(kRefreshDuration, std::move(reload_error_callback)); + ASSERT_TRUE(provider.ok()) << provider.status(); + CertificateInfoImpl cert(kCrlIssuer); + auto crl = (*provider)->GetCrl(cert); + ASSERT_NE(crl, nullptr); + EXPECT_EQ(crl->Issuer(), kCrlIssuer); + EXPECT_EQ(reload_errors.size(), 0); + // Point the provider at a non-crl file so loading fails + // Should result in the CRL Reloader keeping the old CRL data + directory_reader_->SetFilesInDirectory({std::string(kRootCert)}); + event_engine_->TickForDuration(kRefreshDuration); + auto crl_post_update = (*provider)->GetCrl(cert); + ASSERT_NE(crl_post_update, nullptr); + EXPECT_EQ(crl_post_update->Issuer(), kCrlIssuer); + EXPECT_EQ(reload_errors.size(), 1); +} + +TEST_F(DirectoryReloaderCrlProviderTest, WithBadInitialDirectoryStatus) { + absl::Status status = absl::UnknownError(""); + directory_reader_->SetStatus(status); + std::vector reload_errors; + std::function reload_error_callback = + [&](const absl::Status& status) { reload_errors.push_back(status); }; + const std::chrono::seconds kRefreshDuration(60); + auto provider = + CreateCrlProvider(kRefreshDuration, reload_error_callback, nullptr); + // We expect the provider to be created successfully, but the reload error + // callback will have been called + ASSERT_TRUE(provider.ok()) << provider.status(); + EXPECT_EQ(reload_errors.size(), 1); +} + } // 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/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index d8fa3fdab89..1cf301f970e 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -2228,6 +2228,7 @@ src/core/lib/gprpp/cpp_impl_of.h \ src/core/lib/gprpp/crash.cc \ src/core/lib/gprpp/crash.h \ src/core/lib/gprpp/debug_location.h \ +src/core/lib/gprpp/directory_reader.h \ src/core/lib/gprpp/dual_ref_counted.h \ src/core/lib/gprpp/env.h \ src/core/lib/gprpp/examine_stack.cc \ @@ -2252,6 +2253,7 @@ src/core/lib/gprpp/overload.h \ src/core/lib/gprpp/packed_table.h \ src/core/lib/gprpp/per_cpu.cc \ src/core/lib/gprpp/per_cpu.h \ +src/core/lib/gprpp/posix/directory_reader.cc \ src/core/lib/gprpp/posix/env.cc \ src/core/lib/gprpp/posix/stat.cc \ src/core/lib/gprpp/posix/thd.cc \ @@ -2281,6 +2283,7 @@ src/core/lib/gprpp/type_list.h \ src/core/lib/gprpp/unique_type_name.h \ src/core/lib/gprpp/validation_errors.cc \ src/core/lib/gprpp/validation_errors.h \ +src/core/lib/gprpp/windows/directory_reader.cc \ src/core/lib/gprpp/windows/env.cc \ src/core/lib/gprpp/windows/stat.cc \ src/core/lib/gprpp/windows/thd.cc \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 8b99cea7ea0..c332115225f 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -2006,6 +2006,7 @@ src/core/lib/gprpp/cpp_impl_of.h \ src/core/lib/gprpp/crash.cc \ src/core/lib/gprpp/crash.h \ src/core/lib/gprpp/debug_location.h \ +src/core/lib/gprpp/directory_reader.h \ src/core/lib/gprpp/dual_ref_counted.h \ src/core/lib/gprpp/env.h \ src/core/lib/gprpp/examine_stack.cc \ @@ -2030,6 +2031,7 @@ src/core/lib/gprpp/overload.h \ src/core/lib/gprpp/packed_table.h \ src/core/lib/gprpp/per_cpu.cc \ src/core/lib/gprpp/per_cpu.h \ +src/core/lib/gprpp/posix/directory_reader.cc \ src/core/lib/gprpp/posix/env.cc \ src/core/lib/gprpp/posix/stat.cc \ src/core/lib/gprpp/posix/thd.cc \ @@ -2059,6 +2061,7 @@ src/core/lib/gprpp/type_list.h \ src/core/lib/gprpp/unique_type_name.h \ src/core/lib/gprpp/validation_errors.cc \ src/core/lib/gprpp/validation_errors.h \ +src/core/lib/gprpp/windows/directory_reader.cc \ src/core/lib/gprpp/windows/env.cc \ src/core/lib/gprpp/windows/stat.cc \ src/core/lib/gprpp/windows/thd.cc \ diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index 9e0262e3224..b24f41f5d18 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -2757,6 +2757,30 @@ ], "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": "directory_reader_test", + "platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "uses_polling": false + }, { "args": [], "benchmark": false,