From 59d886cb5cc2463078a5d30c94b8442967554f96 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 25 Sep 2023 10:17:58 -0700 Subject: [PATCH] [fuzzing] Expose random number generator to some fuzzers (#34415) Expand our fuzzing capabilities by allowing fuzzers to choose the bits that go into random number distribution generators. --------- Co-authored-by: ctiller --- BUILD | 3 + CMakeLists.txt | 17 ++++-- build_autogenerated.yaml | 8 +++ gRPC-C++.podspec | 2 + gRPC-Core.podspec | 2 + grpc.gemspec | 1 + grpc.gyp | 4 ++ package.xml | 1 + src/core/BUILD | 6 +- src/core/ext/transport/chaotic_good/frame.cc | 21 ++++--- src/core/ext/transport/chaotic_good/frame.h | 6 ++ .../chttp2/transport/hpack_parser.cc | 29 ++++++--- .../transport/chttp2/transport/hpack_parser.h | 2 + .../ext/transport/chttp2/transport/internal.h | 2 + .../ext/transport/chttp2/transport/parsing.cc | 4 +- .../lib/backoff/random_early_detection.cc | 6 +- src/core/lib/backoff/random_early_detection.h | 6 +- test/core/backoff/BUILD | 5 +- .../backoff/random_early_detection_test.cc | 4 +- test/core/transport/chaotic_good/BUILD | 6 +- .../transport/chaotic_good/frame_fuzzer.cc | 15 ++++- .../core/transport/chaotic_good/frame_test.cc | 5 +- test/core/transport/chttp2/BUILD | 3 + ...h-aa717f415a8284b815642cd61c8e84a9c48b22ac | 11 ++++ .../chttp2/hpack_parser_fuzzer.proto | 1 + .../chttp2/hpack_parser_fuzzer_test.cc | 4 ++ .../chttp2/hpack_parser_input_size_fuzzer.cc | 11 +++- .../transport/chttp2/hpack_parser_test.cc | 5 +- .../transport/chttp2/hpack_sync_fuzzer.cc | 10 ++- .../transport/chttp2/hpack_sync_fuzzer.proto | 21 ++++--- test/core/util/BUILD | 7 +++ test/core/util/proto_bit_gen.h | 61 +++++++++++++++++++ test/cpp/microbenchmarks/bm_chttp2_hpack.cc | 8 ++- tools/distrib/fix_build_deps.py | 2 + 34 files changed, 247 insertions(+), 52 deletions(-) create mode 100644 test/core/transport/chttp2/hpack_parser_corpus/crash-aa717f415a8284b815642cd61c8e84a9c48b22ac create mode 100644 test/core/util/proto_bit_gen.h diff --git a/BUILD b/BUILD index bbf5f1e92ee..160c1aec16f 100644 --- a/BUILD +++ b/BUILD @@ -3858,6 +3858,7 @@ grpc_cc_library( ], external_deps = [ "absl/base:core_headers", + "absl/random:bit_gen_ref", "absl/status", "absl/strings", "absl/types:optional", @@ -3980,6 +3981,8 @@ grpc_cc_library( "absl/container:flat_hash_map", "absl/hash", "absl/meta:type_traits", + "absl/random", + "absl/random:bit_gen_ref", "absl/status", "absl/strings", "absl/strings:cord", diff --git a/CMakeLists.txt b/CMakeLists.txt index 5e7832b8f9a..508670546e8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -166,6 +166,7 @@ set(gRPC_ABSL_USED_TARGETS absl_numeric_representation absl_optional absl_prefetch + absl_random_bit_gen_ref absl_random_distributions absl_random_internal_distribution_caller absl_random_internal_fast_uniform_bits @@ -2553,6 +2554,8 @@ target_link_libraries(grpc absl::function_ref absl::hash absl::type_traits + absl::random_bit_gen_ref + absl::random_distributions absl::statusor absl::span absl::utility @@ -3212,6 +3215,8 @@ target_link_libraries(grpc_unsecure absl::function_ref absl::hash absl::type_traits + absl::random_bit_gen_ref + absl::random_distributions absl::statusor absl::span absl::utility @@ -18370,6 +18375,8 @@ target_include_directories(random_early_detection_test target_link_libraries(random_early_detection_test ${_gRPC_ALLTARGETS_LIBRARIES} gtest + absl::random_bit_gen_ref + absl::random_distributions absl::random_random ) @@ -24131,6 +24138,8 @@ target_link_libraries(test_core_transport_chaotic_good_frame_test absl::function_ref absl::hash absl::type_traits + absl::random_bit_gen_ref + absl::random_distributions absl::statusor absl::span absl::utility @@ -29820,7 +29829,7 @@ generate_pkgconfig( "gRPC" "high performance general RPC framework" "${gRPC_CORE_VERSION}" - "absl_algorithm_container absl_any_invocable absl_base absl_bind_front absl_cleanup absl_cord absl_core_headers absl_flags absl_flags_marshalling absl_flat_hash_map absl_flat_hash_set absl_function_ref absl_hash absl_inlined_vector absl_memory absl_optional absl_random_random absl_span absl_status absl_statusor absl_str_format absl_strings absl_synchronization absl_time absl_type_traits absl_utility absl_variant gpr" + "absl_algorithm_container absl_any_invocable absl_base absl_bind_front absl_cleanup absl_cord absl_core_headers absl_flags absl_flags_marshalling absl_flat_hash_map absl_flat_hash_set absl_function_ref absl_hash absl_inlined_vector absl_memory absl_optional absl_random_bit_gen_ref absl_random_distributions absl_random_random absl_span absl_status absl_statusor absl_str_format absl_strings absl_synchronization absl_time absl_type_traits absl_utility absl_variant gpr" "libcares openssl re2 zlib" "-lgrpc" "-laddress_sorting -lupb_textformat_lib -lupb_json_lib -lupb -lutf8_range_lib -lupb_collections_lib" @@ -29831,7 +29840,7 @@ generate_pkgconfig( "gRPC unsecure" "high performance general RPC framework without SSL" "${gRPC_CORE_VERSION}" - "absl_algorithm_container absl_any_invocable absl_base absl_bind_front absl_cleanup absl_cord absl_core_headers absl_flags absl_flags_marshalling absl_flat_hash_map absl_flat_hash_set absl_function_ref absl_hash absl_inlined_vector absl_memory absl_optional absl_random_random absl_span absl_status absl_statusor absl_str_format absl_strings absl_synchronization absl_time absl_type_traits absl_utility absl_variant gpr" + "absl_algorithm_container absl_any_invocable absl_base absl_bind_front absl_cleanup absl_cord absl_core_headers absl_flags absl_flags_marshalling absl_flat_hash_map absl_flat_hash_set absl_function_ref absl_hash absl_inlined_vector absl_memory absl_optional absl_random_bit_gen_ref absl_random_distributions absl_random_random absl_span absl_status absl_statusor absl_str_format absl_strings absl_synchronization absl_time absl_type_traits absl_utility absl_variant gpr" "libcares zlib" "-lgrpc_unsecure" "-laddress_sorting -lupb -lutf8_range_lib -lupb_collections_lib" @@ -29842,7 +29851,7 @@ generate_pkgconfig( "gRPC++" "C++ wrapper for gRPC" "${gRPC_CPP_VERSION}" - "absl_algorithm_container absl_any_invocable absl_base absl_bind_front absl_cleanup absl_cord absl_core_headers absl_flags absl_flags_marshalling absl_flat_hash_map absl_flat_hash_set absl_function_ref absl_hash absl_inlined_vector absl_memory absl_optional absl_random_random absl_span absl_status absl_statusor absl_str_format absl_strings absl_synchronization absl_time absl_type_traits absl_utility absl_variant gpr grpc" + "absl_algorithm_container absl_any_invocable absl_base absl_bind_front absl_cleanup absl_cord absl_core_headers absl_flags absl_flags_marshalling absl_flat_hash_map absl_flat_hash_set absl_function_ref absl_hash absl_inlined_vector absl_memory absl_optional absl_random_bit_gen_ref absl_random_distributions absl_random_random absl_span absl_status absl_statusor absl_str_format absl_strings absl_synchronization absl_time absl_type_traits absl_utility absl_variant gpr grpc" "libcares openssl re2 zlib" "-lgrpc++" "-laddress_sorting -lupb_textformat_lib -lupb_json_lib -lupb -lutf8_range_lib -lupb_collections_lib" @@ -29853,7 +29862,7 @@ generate_pkgconfig( "gRPC++ unsecure" "C++ wrapper for gRPC without SSL" "${gRPC_CPP_VERSION}" - "absl_algorithm_container absl_any_invocable absl_base absl_bind_front absl_cleanup absl_cord absl_core_headers absl_flags absl_flags_marshalling absl_flat_hash_map absl_flat_hash_set absl_function_ref absl_hash absl_inlined_vector absl_memory absl_optional absl_random_random absl_span absl_status absl_statusor absl_str_format absl_strings absl_synchronization absl_time absl_type_traits absl_utility absl_variant gpr grpc_unsecure" + "absl_algorithm_container absl_any_invocable absl_base absl_bind_front absl_cleanup absl_cord absl_core_headers absl_flags absl_flags_marshalling absl_flat_hash_map absl_flat_hash_set absl_function_ref absl_hash absl_inlined_vector absl_memory absl_optional absl_random_bit_gen_ref absl_random_distributions absl_random_random absl_span absl_status absl_statusor absl_str_format absl_strings absl_synchronization absl_time absl_type_traits absl_utility absl_variant gpr grpc_unsecure" "libcares zlib" "-lgrpc++_unsecure" "-laddress_sorting -lupb -lutf8_range_lib -lupb_collections_lib" diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 543ab4b9148..934056cb3a8 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -1819,6 +1819,8 @@ libs: - absl/functional:function_ref - absl/hash:hash - absl/meta:type_traits + - absl/random:bit_gen_ref + - absl/random:distributions - absl/status:statusor - absl/types:span - absl/utility:utility @@ -2791,6 +2793,8 @@ libs: - absl/functional:function_ref - absl/hash:hash - absl/meta:type_traits + - absl/random:bit_gen_ref + - absl/random:distributions - absl/status:statusor - absl/types:span - absl/utility:utility @@ -12244,6 +12248,8 @@ targets: - test/core/backoff/random_early_detection_test.cc deps: - gtest + - absl/random:bit_gen_ref + - absl/random:distributions - absl/random:random uses_polling: false - name: raw_end2end_test @@ -16181,6 +16187,8 @@ targets: - absl/functional:function_ref - absl/hash:hash - absl/meta:type_traits + - absl/random:bit_gen_ref + - absl/random:distributions - absl/status:statusor - absl/types:span - absl/utility:utility diff --git a/gRPC-C++.podspec b/gRPC-C++.podspec index 316a1e5d74e..4b8c32003c3 100644 --- a/gRPC-C++.podspec +++ b/gRPC-C++.podspec @@ -231,6 +231,8 @@ Pod::Spec.new do |s| ss.dependency 'abseil/hash/hash', abseil_version ss.dependency 'abseil/memory/memory', abseil_version ss.dependency 'abseil/meta/type_traits', abseil_version + ss.dependency 'abseil/random/bit_gen_ref', abseil_version + ss.dependency 'abseil/random/distributions', abseil_version ss.dependency 'abseil/random/random', abseil_version ss.dependency 'abseil/status/status', abseil_version ss.dependency 'abseil/status/statusor', abseil_version diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 167607b950a..5d324027b7e 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -199,6 +199,8 @@ Pod::Spec.new do |s| ss.dependency 'abseil/hash/hash', abseil_version ss.dependency 'abseil/memory/memory', abseil_version ss.dependency 'abseil/meta/type_traits', abseil_version + ss.dependency 'abseil/random/bit_gen_ref', abseil_version + ss.dependency 'abseil/random/distributions', abseil_version ss.dependency 'abseil/random/random', abseil_version ss.dependency 'abseil/status/status', abseil_version ss.dependency 'abseil/status/statusor', abseil_version diff --git a/grpc.gemspec b/grpc.gemspec index 643d80bd006..185591a3cd9 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -1955,6 +1955,7 @@ Gem::Specification.new do |s| s.files += %w( third_party/abseil-cpp/absl/profiling/internal/sample_recorder.h ) s.files += %w( third_party/abseil-cpp/absl/random/bernoulli_distribution.h ) s.files += %w( third_party/abseil-cpp/absl/random/beta_distribution.h ) + s.files += %w( third_party/abseil-cpp/absl/random/bit_gen_ref.h ) s.files += %w( third_party/abseil-cpp/absl/random/discrete_distribution.cc ) s.files += %w( third_party/abseil-cpp/absl/random/discrete_distribution.h ) s.files += %w( third_party/abseil-cpp/absl/random/distributions.h ) diff --git a/grpc.gyp b/grpc.gyp index 48849be61d9..1a51da59d51 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -262,6 +262,8 @@ 'absl/functional:function_ref', 'absl/hash:hash', 'absl/meta:type_traits', + 'absl/random:bit_gen_ref', + 'absl/random:distributions', 'absl/status:statusor', 'absl/types:span', 'absl/utility:utility', @@ -1106,6 +1108,8 @@ 'absl/functional:function_ref', 'absl/hash:hash', 'absl/meta:type_traits', + 'absl/random:bit_gen_ref', + 'absl/random:distributions', 'absl/status:statusor', 'absl/types:span', 'absl/utility:utility', diff --git a/package.xml b/package.xml index 2bb4386c27a..93e76ff2a3c 100644 --- a/package.xml +++ b/package.xml @@ -1959,6 +1959,7 @@ + diff --git a/src/core/BUILD b/src/core/BUILD index 46cb5dd68ee..73889256d22 100644 --- a/src/core/BUILD +++ b/src/core/BUILD @@ -4091,7 +4091,10 @@ grpc_cc_library( hdrs = [ "lib/backoff/random_early_detection.h", ], - external_deps = ["absl/random"], + external_deps = [ + "absl/random:bit_gen_ref", + "absl/random:distributions", + ], deps = ["//:gpr_platform"], ) @@ -5893,6 +5896,7 @@ grpc_cc_library( "ext/transport/chaotic_good/frame.h", ], external_deps = [ + "absl/random:bit_gen_ref", "absl/status", "absl/status:statusor", "absl/types:variant", diff --git a/src/core/ext/transport/chaotic_good/frame.cc b/src/core/ext/transport/chaotic_good/frame.cc index 34b8a493e8c..ac353d6bd7c 100644 --- a/src/core/ext/transport/chaotic_good/frame.cc +++ b/src/core/ext/transport/chaotic_good/frame.cc @@ -112,7 +112,8 @@ class FrameDeserializer { template absl::StatusOr> ReadMetadata( HPackParser* parser, absl::StatusOr maybe_slices, - uint32_t stream_id, bool is_header, bool is_client) { + uint32_t stream_id, bool is_header, bool is_client, + absl::BitGenRef bitsrc) { if (!maybe_slices.ok()) return maybe_slices.status(); auto& slices = *maybe_slices; Arena::PoolPtr metadata; @@ -128,7 +129,7 @@ absl::StatusOr> ReadMetadata( is_client}); for (size_t i = 0; i < slices.Count(); i++) { GRPC_RETURN_IF_ERROR(parser->Parse(slices.c_slice_at(i), - i == slices.Count() - 1, + i == slices.Count() - 1, bitsrc, /*call_tracer=*/nullptr)); } parser->FinishFrame(); @@ -137,6 +138,7 @@ absl::StatusOr> ReadMetadata( } // namespace absl::Status SettingsFrame::Deserialize(HPackParser*, const FrameHeader& header, + absl::BitGenRef, SliceBuffer& slice_buffer) { if (header.type != FrameType::kSettings) { return absl::InvalidArgumentError("Expected settings frame"); @@ -155,6 +157,7 @@ SliceBuffer SettingsFrame::Serialize(HPackCompressor*) const { absl::Status ClientFragmentFrame::Deserialize(HPackParser* parser, const FrameHeader& header, + absl::BitGenRef bitsrc, SliceBuffer& slice_buffer) { if (header.stream_id == 0) { return absl::InvalidArgumentError("Expected non-zero stream id"); @@ -166,7 +169,7 @@ absl::Status ClientFragmentFrame::Deserialize(HPackParser* parser, FrameDeserializer deserializer(header, slice_buffer); if (header.flags.is_set(0)) { auto r = ReadMetadata(parser, deserializer.ReceiveHeaders(), - header.stream_id, true, true); + header.stream_id, true, true, bitsrc); if (!r.ok()) return r.status(); } if (header.flags.is_set(1)) { @@ -194,6 +197,7 @@ SliceBuffer ClientFragmentFrame::Serialize(HPackCompressor* encoder) const { absl::Status ServerFragmentFrame::Deserialize(HPackParser* parser, const FrameHeader& header, + absl::BitGenRef bitsrc, SliceBuffer& slice_buffer) { if (header.stream_id == 0) { return absl::InvalidArgumentError("Expected non-zero stream id"); @@ -204,13 +208,15 @@ absl::Status ServerFragmentFrame::Deserialize(HPackParser* parser, } FrameDeserializer deserializer(header, slice_buffer); if (header.flags.is_set(0)) { - auto r = ReadMetadata(parser, deserializer.ReceiveHeaders(), - header.stream_id, true, false); + auto r = + ReadMetadata(parser, deserializer.ReceiveHeaders(), + header.stream_id, true, false, bitsrc); if (!r.ok()) return r.status(); } if (header.flags.is_set(1)) { - auto r = ReadMetadata( - parser, deserializer.ReceiveTrailers(), header.stream_id, false, false); + auto r = + ReadMetadata(parser, deserializer.ReceiveTrailers(), + header.stream_id, false, false, bitsrc); } return deserializer.Finish(); } @@ -228,6 +234,7 @@ SliceBuffer ServerFragmentFrame::Serialize(HPackCompressor* encoder) const { } absl::Status CancelFrame::Deserialize(HPackParser*, const FrameHeader& header, + absl::BitGenRef, SliceBuffer& slice_buffer) { if (header.type != FrameType::kCancel) { return absl::InvalidArgumentError("Expected cancel frame"); diff --git a/src/core/ext/transport/chaotic_good/frame.h b/src/core/ext/transport/chaotic_good/frame.h index 03b53d195f1..9f3538af71f 100644 --- a/src/core/ext/transport/chaotic_good/frame.h +++ b/src/core/ext/transport/chaotic_good/frame.h @@ -21,6 +21,7 @@ #include #include +#include "absl/random/bit_gen_ref.h" #include "absl/status/status.h" #include "absl/types/variant.h" @@ -39,6 +40,7 @@ class FrameInterface { public: virtual absl::Status Deserialize(HPackParser* parser, const FrameHeader& header, + absl::BitGenRef bitsrc, SliceBuffer& slice_buffer) = 0; virtual SliceBuffer Serialize(HPackCompressor* encoder) const = 0; @@ -62,6 +64,7 @@ class FrameInterface { struct SettingsFrame final : public FrameInterface { absl::Status Deserialize(HPackParser* parser, const FrameHeader& header, + absl::BitGenRef bitsrc, SliceBuffer& slice_buffer) override; SliceBuffer Serialize(HPackCompressor* encoder) const override; @@ -70,6 +73,7 @@ struct SettingsFrame final : public FrameInterface { struct ClientFragmentFrame final : public FrameInterface { absl::Status Deserialize(HPackParser* parser, const FrameHeader& header, + absl::BitGenRef bitsrc, SliceBuffer& slice_buffer) override; SliceBuffer Serialize(HPackCompressor* encoder) const override; @@ -86,6 +90,7 @@ struct ClientFragmentFrame final : public FrameInterface { struct ServerFragmentFrame final : public FrameInterface { absl::Status Deserialize(HPackParser* parser, const FrameHeader& header, + absl::BitGenRef bitsrc, SliceBuffer& slice_buffer) override; SliceBuffer Serialize(HPackCompressor* encoder) const override; @@ -101,6 +106,7 @@ struct ServerFragmentFrame final : public FrameInterface { struct CancelFrame final : public FrameInterface { absl::Status Deserialize(HPackParser* parser, const FrameHeader& header, + absl::BitGenRef bitsrc, SliceBuffer& slice_buffer) override; SliceBuffer Serialize(HPackCompressor* encoder) const override; diff --git a/src/core/ext/transport/chttp2/transport/hpack_parser.cc b/src/core/ext/transport/chttp2/transport/hpack_parser.cc index 3ca3e5c8121..31bf46456f1 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_parser.cc +++ b/src/core/ext/transport/chttp2/transport/hpack_parser.cc @@ -91,12 +91,13 @@ constexpr Base64InverseTable kBase64InverseTable; class HPackParser::Input { public: Input(grpc_slice_refcount* current_slice_refcount, const uint8_t* begin, - const uint8_t* end, HpackParseResult& error) + const uint8_t* end, absl::BitGenRef bitsrc, HpackParseResult& error) : current_slice_refcount_(current_slice_refcount), begin_(begin), end_(end), frontier_(begin), - error_(error) {} + error_(error), + bitsrc_(bitsrc) {} // If input is backed by a slice, retrieve its refcount. If not, return // nullptr. @@ -279,6 +280,9 @@ class HPackParser::Input { // Get the frontier - for buffering should we fail due to eof const uint8_t* frontier() const { return frontier_; } + // Access the rng + absl::BitGenRef bitsrc() { return bitsrc_; } + private: // Helper to set the error to out of range for ParseVarint absl::optional ParseVarintOutOfRange(uint32_t value, @@ -324,6 +328,8 @@ class HPackParser::Input { // (We've failed parsing a request for whatever reason, but we're still // continuing the connection so we need to see future opcodes after this bit). size_t skip_bytes_ = 0; + // Random number generator + absl::BitGenRef bitsrc_; }; absl::string_view HPackParser::String::string_view() const { @@ -1144,7 +1150,7 @@ void HPackParser::BeginFrame(grpc_metadata_batch* metadata_buffer, } grpc_error_handle HPackParser::Parse( - const grpc_slice& slice, bool is_last, + const grpc_slice& slice, bool is_last, absl::BitGenRef bitsrc, CallTracerAnnotationInterface* call_tracer) { if (GPR_UNLIKELY(!unparsed_bytes_.empty())) { unparsed_bytes_.insert(unparsed_bytes_.end(), GRPC_SLICE_START_PTR(slice), @@ -1155,20 +1161,23 @@ grpc_error_handle HPackParser::Parse( return absl::OkStatus(); } std::vector buffer = std::move(unparsed_bytes_); - return ParseInput(Input(nullptr, buffer.data(), - buffer.data() + buffer.size(), state_.frame_error), - is_last, call_tracer); + return ParseInput( + Input(nullptr, buffer.data(), buffer.data() + buffer.size(), bitsrc, + state_.frame_error), + is_last, call_tracer); } - return ParseInput(Input(slice.refcount, GRPC_SLICE_START_PTR(slice), - GRPC_SLICE_END_PTR(slice), state_.frame_error), - is_last, call_tracer); + return ParseInput( + Input(slice.refcount, GRPC_SLICE_START_PTR(slice), + GRPC_SLICE_END_PTR(slice), bitsrc, state_.frame_error), + is_last, call_tracer); } grpc_error_handle HPackParser::ParseInput( Input input, bool is_last, CallTracerAnnotationInterface* call_tracer) { ParseInputInner(&input); if (is_last && is_boundary()) { - if (state_.metadata_early_detection.Reject(state_.frame_length)) { + if (state_.metadata_early_detection.Reject(state_.frame_length, + input.bitsrc())) { HandleMetadataSoftSizeLimitExceeded(&input); } global_stats().IncrementHttp2MetadataSize(state_.frame_length); diff --git a/src/core/ext/transport/chttp2/transport/hpack_parser.h b/src/core/ext/transport/chttp2/transport/hpack_parser.h index 631f9d5a62d..6b28daad61d 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_parser.h +++ b/src/core/ext/transport/chttp2/transport/hpack_parser.h @@ -28,6 +28,7 @@ #include #include +#include "absl/random/bit_gen_ref.h" #include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" @@ -101,6 +102,7 @@ class HPackParser { void StopBufferingFrame() { metadata_buffer_ = nullptr; } // Parse one slice worth of data grpc_error_handle Parse(const grpc_slice& slice, bool is_last, + absl::BitGenRef bitsrc, CallTracerAnnotationInterface* call_tracer); // Reset state ready for the next BeginFrame void FinishFrame(); diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h index 8bdc5156d5b..fade9c9e348 100644 --- a/src/core/ext/transport/chttp2/transport/internal.h +++ b/src/core/ext/transport/chttp2/transport/internal.h @@ -28,6 +28,7 @@ #include "absl/container/flat_hash_map.h" #include "absl/meta/type_traits.h" +#include "absl/random/random.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" @@ -269,6 +270,7 @@ struct grpc_chttp2_transport : public grpc_core::KeepsGrpcInitialized { std::shared_ptr event_engine; grpc_core::Combiner* combiner; + absl::BitGen bitgen; grpc_closure* notify_on_receive_settings = nullptr; grpc_closure* notify_on_close = nullptr; diff --git a/src/core/ext/transport/chttp2/transport/parsing.cc b/src/core/ext/transport/chttp2/transport/parsing.cc index 6df3353987d..9bc56a286c2 100644 --- a/src/core/ext/transport/chttp2/transport/parsing.cc +++ b/src/core/ext/transport/chttp2/transport/parsing.cc @@ -26,6 +26,7 @@ #include "absl/base/attributes.h" #include "absl/container/flat_hash_map.h" +#include "absl/random/bit_gen_ref.h" #include "absl/status/status.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" @@ -869,7 +870,8 @@ grpc_error_handle grpc_chttp2_header_parser_parse(void* hpack_parser, .value); } } - grpc_error_handle error = parser->Parse(slice, is_last != 0, call_tracer); + grpc_error_handle error = parser->Parse( + slice, is_last != 0, absl::BitGenRef(t->bitgen), call_tracer); if (!error.ok()) { return error; } diff --git a/src/core/lib/backoff/random_early_detection.cc b/src/core/lib/backoff/random_early_detection.cc index eea11d02a8b..d9cd1a31700 100644 --- a/src/core/lib/backoff/random_early_detection.cc +++ b/src/core/lib/backoff/random_early_detection.cc @@ -16,12 +16,14 @@ #include "src/core/lib/backoff/random_early_detection.h" +#include "absl/random/distributions.h" + namespace grpc_core { -bool RandomEarlyDetection::Reject(uint64_t size) { +bool RandomEarlyDetection::Reject(uint64_t size, absl::BitGenRef bitsrc) const { if (size <= soft_limit_) return false; if (size < hard_limit_) { - return absl::Bernoulli(bitgen_, + return absl::Bernoulli(bitsrc, static_cast(size - soft_limit_) / static_cast(hard_limit_ - soft_limit_)); } diff --git a/src/core/lib/backoff/random_early_detection.h b/src/core/lib/backoff/random_early_detection.h index 2cce4469ed8..e233c136149 100644 --- a/src/core/lib/backoff/random_early_detection.h +++ b/src/core/lib/backoff/random_early_detection.h @@ -21,7 +21,7 @@ #include -#include "absl/random/random.h" +#include "absl/random/bit_gen_ref.h" namespace grpc_core { @@ -38,7 +38,7 @@ class RandomEarlyDetection { bool MustReject(uint64_t size) { return size >= hard_limit_; } // Returns true if the item should be rejected. - bool Reject(uint64_t size); + bool Reject(uint64_t size, absl::BitGenRef bitsrc) const; uint64_t soft_limit() const { return soft_limit_; } uint64_t hard_limit() const { return hard_limit_; } @@ -55,8 +55,6 @@ class RandomEarlyDetection { uint64_t soft_limit_; // The hard limit is the size at which we reject all items. uint64_t hard_limit_; - // The bit generator used to generate random numbers. - absl::InsecureBitGen bitgen_; }; } // namespace grpc_core diff --git a/test/core/backoff/BUILD b/test/core/backoff/BUILD index e8d29d24a24..b8712f0b81d 100644 --- a/test/core/backoff/BUILD +++ b/test/core/backoff/BUILD @@ -42,7 +42,10 @@ grpc_cc_test( grpc_cc_test( name = "random_early_detection_test", srcs = ["random_early_detection_test.cc"], - external_deps = ["gtest"], + external_deps = [ + "absl/random", + "gtest", + ], language = "C++", uses_event_engine = False, uses_polling = False, diff --git a/test/core/backoff/random_early_detection_test.cc b/test/core/backoff/random_early_detection_test.cc index c8698cc1e06..38b3de1e86a 100644 --- a/test/core/backoff/random_early_detection_test.cc +++ b/test/core/backoff/random_early_detection_test.cc @@ -14,6 +14,7 @@ #include "src/core/lib/backoff/random_early_detection.h" +#include "absl/random/random.h" #include "gtest/gtest.h" namespace grpc_core { @@ -26,11 +27,12 @@ TEST(RandomEarlyDetectionTest, NoOp) { } TEST(RandomEarlyDetectionTest, Distribution) { + absl::BitGen bitgen; RandomEarlyDetection red(100, 200); int64_t counts[300] = {}; for (int round = 0; round < 10000; round++) { for (int64_t i = 0; i < 300; i++) { - if (red.Reject(i)) counts[i]++; + if (red.Reject(i, absl::BitGenRef(bitgen))) counts[i]++; } } for (int64_t i = 0; i < 100; i++) { diff --git a/test/core/transport/chaotic_good/BUILD b/test/core/transport/chaotic_good/BUILD index 6d693256c2d..9c7cd464cdb 100644 --- a/test/core/transport/chaotic_good/BUILD +++ b/test/core/transport/chaotic_good/BUILD @@ -46,6 +46,7 @@ grpc_cc_test( name = "frame_test", srcs = ["frame_test.cc"], external_deps = [ + "absl/random", "absl/status", "absl/status:statusor", "gtest", @@ -57,7 +58,10 @@ grpc_fuzzer( name = "frame_fuzzer", srcs = ["frame_fuzzer.cc"], corpus = "frame_fuzzer_corpus", - external_deps = ["absl/status:statusor"], + external_deps = [ + "absl/random:bit_gen_ref", + "absl/status:statusor", + ], language = "C++", tags = ["no_windows"], deps = [ diff --git a/test/core/transport/chaotic_good/frame_fuzzer.cc b/test/core/transport/chaotic_good/frame_fuzzer.cc index 91dbb7623c5..cf3ef86ac23 100644 --- a/test/core/transport/chaotic_good/frame_fuzzer.cc +++ b/test/core/transport/chaotic_good/frame_fuzzer.cc @@ -15,8 +15,10 @@ #include #include +#include #include +#include "absl/random/bit_gen_ref.h" #include "absl/status/statusor.h" #include @@ -40,6 +42,11 @@ bool squelch = false; namespace grpc_core { namespace chaotic_good { +struct DeterministicBitGen : public std::numeric_limits { + using result_type = uint64_t; + uint64_t operator()() { return 42; } +}; + template void AssertRoundTrips(const T& input, FrameType expected_frame_type) { HPackCompressor hpack_compressor; @@ -53,7 +60,9 @@ void AssertRoundTrips(const T& input, FrameType expected_frame_type) { GPR_ASSERT(header->type == expected_frame_type); T output; HPackParser hpack_parser; - auto deser = output.Deserialize(&hpack_parser, header.value(), serialized); + DeterministicBitGen bitgen; + auto deser = output.Deserialize(&hpack_parser, header.value(), + absl::BitGenRef(bitgen), serialized); GPR_ASSERT(deser.ok()); GPR_ASSERT(output == input); } @@ -66,7 +75,9 @@ void FinishParseAndChecks(const FrameHeader& header, const uint8_t* data, HPackParser hpack_parser; SliceBuffer serialized; serialized.Append(Slice::FromCopiedBuffer(data, size)); - auto deser = parsed.Deserialize(&hpack_parser, header, serialized); + DeterministicBitGen bitgen; + auto deser = parsed.Deserialize(&hpack_parser, header, + absl::BitGenRef(bitgen), serialized); if (!deser.ok()) return; AssertRoundTrips(parsed, header.type); } diff --git a/test/core/transport/chaotic_good/frame_test.cc b/test/core/transport/chaotic_good/frame_test.cc index 4961aa4b276..00908f75a6e 100644 --- a/test/core/transport/chaotic_good/frame_test.cc +++ b/test/core/transport/chaotic_good/frame_test.cc @@ -16,6 +16,7 @@ #include +#include "absl/random/random.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "gtest/gtest.h" @@ -27,6 +28,7 @@ namespace { template void AssertRoundTrips(const T input, FrameType expected_frame_type) { HPackCompressor hpack_compressor; + absl::BitGen bitgen; auto serialized = input.Serialize(&hpack_compressor); EXPECT_GE(serialized.Length(), 24); uint8_t header_bytes[24]; @@ -36,7 +38,8 @@ void AssertRoundTrips(const T input, FrameType expected_frame_type) { EXPECT_EQ(header->type, expected_frame_type); T output; HPackParser hpack_parser; - auto deser = output.Deserialize(&hpack_parser, header.value(), serialized); + auto deser = output.Deserialize(&hpack_parser, header.value(), + absl::BitGenRef(bitgen), serialized); EXPECT_TRUE(deser.ok()) << deser; EXPECT_EQ(output, input); } diff --git a/test/core/transport/chttp2/BUILD b/test/core/transport/chttp2/BUILD index 3d8c7bfb947..00366873a1b 100644 --- a/test/core/transport/chttp2/BUILD +++ b/test/core/transport/chttp2/BUILD @@ -32,6 +32,7 @@ grpc_proto_fuzzer( "//:grpc", "//test/core/util:fuzz_config_vars", "//test/core/util:grpc_test_util", + "//test/core/util:proto_bit_gen", ], ) @@ -47,6 +48,7 @@ grpc_proto_fuzzer( "//:grpc", "//test/core/util:fuzz_config_vars", "//test/core/util:grpc_test_util", + "//test/core/util:proto_bit_gen", ], ) @@ -75,6 +77,7 @@ grpc_fuzzer( "absl/cleanup", "absl/status:statusor", "absl/status", + "absl/random:mocking_bit_gen", ], tags = ["no_windows"], uses_polling = False, diff --git a/test/core/transport/chttp2/hpack_parser_corpus/crash-aa717f415a8284b815642cd61c8e84a9c48b22ac b/test/core/transport/chttp2/hpack_parser_corpus/crash-aa717f415a8284b815642cd61c8e84a9c48b22ac new file mode 100644 index 00000000000..39df64542bc --- /dev/null +++ b/test/core/transport/chttp2/hpack_parser_corpus/crash-aa717f415a8284b815642cd61c8e84a9c48b22ac @@ -0,0 +1,11 @@ +frames { + end_of_stream: true + max_metadata_length: 4096 + parse: "\244p\203\333\360\244\037!\203\333\360c" + absolute_max_metadata_length: 1 +} +frames { + end_of_stream: true + max_metadata_length: 4096 + parse: "\020\244\007\244\360\244\017-b\000\020content-location\000in\213c[)(?*\240\244\256@\010\020\244\037\007:" +} diff --git a/test/core/transport/chttp2/hpack_parser_fuzzer.proto b/test/core/transport/chttp2/hpack_parser_fuzzer.proto index f3824f906e3..a48909be4d8 100644 --- a/test/core/transport/chttp2/hpack_parser_fuzzer.proto +++ b/test/core/transport/chttp2/hpack_parser_fuzzer.proto @@ -33,4 +33,5 @@ message Frame { message Msg { repeated Frame frames = 2; grpc.testing.FuzzConfigVars config_vars = 3; + repeated uint64 random_numbers = 4; } diff --git a/test/core/transport/chttp2/hpack_parser_fuzzer_test.cc b/test/core/transport/chttp2/hpack_parser_fuzzer_test.cc index f87d80794a8..d7dc1553378 100644 --- a/test/core/transport/chttp2/hpack_parser_fuzzer_test.cc +++ b/test/core/transport/chttp2/hpack_parser_fuzzer_test.cc @@ -24,6 +24,7 @@ #include #include "absl/cleanup/cleanup.h" +#include "absl/random/bit_gen_ref.h" #include #include @@ -42,6 +43,7 @@ #include "src/libfuzzer/libfuzzer_macro.h" #include "test/core/transport/chttp2/hpack_parser_fuzzer.pb.h" #include "test/core/util/fuzz_config_vars.h" +#include "test/core/util/proto_bit_gen.h" // IWYU pragma: no_include @@ -52,6 +54,7 @@ static void dont_log(gpr_log_func_args* /*args*/) {} DEFINE_PROTO_FUZZER(const hpack_parser_fuzzer::Msg& msg) { if (squelch) gpr_set_log_function(dont_log); + grpc_core::ProtoBitGen proto_bit_src(msg.random_numbers()); grpc_core::ApplyFuzzConfigVars(msg.config_vars()); grpc_core::TestOnlyReloadExperimentsFromConfigVariables(); grpc_init(); @@ -117,6 +120,7 @@ DEFINE_PROTO_FUZZER(const hpack_parser_fuzzer::Msg& msg) { grpc_slice buffer = grpc_slice_from_copied_buffer(parse.data(), parse.size()); auto err = parser->Parse(buffer, idx == frame.parse_size() - 1, + absl::BitGenRef(proto_bit_src), /*call_tracer=*/nullptr); grpc_slice_unref(buffer); stop_buffering_ctr--; diff --git a/test/core/transport/chttp2/hpack_parser_input_size_fuzzer.cc b/test/core/transport/chttp2/hpack_parser_input_size_fuzzer.cc index d83e5d9ff28..070fd7a9f16 100644 --- a/test/core/transport/chttp2/hpack_parser_input_size_fuzzer.cc +++ b/test/core/transport/chttp2/hpack_parser_input_size_fuzzer.cc @@ -19,10 +19,12 @@ #include #include +#include #include #include #include "absl/cleanup/cleanup.h" +#include "absl/random/bit_gen_ref.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" @@ -50,6 +52,11 @@ bool leak_check = true; namespace grpc_core { namespace { +struct DeterministicBitGen : public std::numeric_limits { + using result_type = uint64_t; + uint64_t operator()() { return 42; } +}; + class TestEncoder { public: std::string result() { return out_; } @@ -102,8 +109,10 @@ absl::StatusOr TestVector(grpc_slice_split_mode mode, absl::Status found_err; for (i = 0; i < nslices; i++) { ExecCtx exec_ctx; + DeterministicBitGen bitgen; auto err = - parser.Parse(slices[i], i == nslices - 1, /*call_tracer=*/nullptr); + parser.Parse(slices[i], i == nslices - 1, absl::BitGenRef(bitgen), + /*call_tracer=*/nullptr); if (!err.ok()) { if (!IsStreamError(err)) return err; if (found_err.ok()) found_err = err; diff --git a/test/core/transport/chttp2/hpack_parser_test.cc b/test/core/transport/chttp2/hpack_parser_test.cc index d7a72d01e3c..3772d909b9b 100644 --- a/test/core/transport/chttp2/hpack_parser_test.cc +++ b/test/core/transport/chttp2/hpack_parser_test.cc @@ -22,6 +22,7 @@ #include #include "absl/cleanup/cleanup.h" +#include "absl/random/random.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" @@ -115,6 +116,7 @@ class ParseTest : public ::testing::TestWithParam { grpc_slice* slices; size_t nslices; size_t i; + absl::BitGen bitgen; grpc_metadata_batch b(arena.get()); @@ -141,7 +143,8 @@ class ParseTest : public ::testing::TestWithParam { for (i = 0; i < nslices; i++) { ExecCtx exec_ctx; auto err = - parser_->Parse(slices[i], i == nslices - 1, /*call_tracer=*/nullptr); + parser_->Parse(slices[i], i == nslices - 1, absl::BitGenRef(bitgen), + /*call_tracer=*/nullptr); if (!err.ok() && (flags & kFailureIsConnectionError) == 0) { EXPECT_TRUE(IsStreamError(err)) << err; } diff --git a/test/core/transport/chttp2/hpack_sync_fuzzer.cc b/test/core/transport/chttp2/hpack_sync_fuzzer.cc index 291f6775142..81809afcf71 100644 --- a/test/core/transport/chttp2/hpack_sync_fuzzer.cc +++ b/test/core/transport/chttp2/hpack_sync_fuzzer.cc @@ -23,6 +23,7 @@ #include #include +#include "absl/random/bit_gen_ref.h" #include "absl/status/status.h" #include "absl/strings/escaping.h" #include "absl/strings/match.h" @@ -47,6 +48,7 @@ #include "src/libfuzzer/libfuzzer_macro.h" #include "test/core/transport/chttp2/hpack_sync_fuzzer.pb.h" #include "test/core/util/fuzz_config_vars.h" +#include "test/core/util/proto_bit_gen.h" bool squelch = true; bool leak_check = true; @@ -62,6 +64,8 @@ bool IsStreamError(const absl::Status& status) { } void FuzzOneInput(const hpack_sync_fuzzer::Msg& msg) { + ProtoBitGen proto_bit_src(msg.random_numbers()); + // STAGE 1: Encode the fuzzing input into a buffer (encode_output) HPackCompressor compressor; SliceBuffer encode_output; @@ -126,9 +130,9 @@ void FuzzOneInput(const hpack_sync_fuzzer::Msg& msg) { HPackParser::LogInfo{1, HPackParser::LogInfo::kHeaders, false}); std::vector> seen_errors; for (size_t i = 0; i < encode_output.Count(); i++) { - auto err = - parser.Parse(encode_output.c_slice_at(i), - i == (encode_output.Count() - 1), /*call_tracer=*/nullptr); + auto err = parser.Parse( + encode_output.c_slice_at(i), i == (encode_output.Count() - 1), + absl::BitGenRef(proto_bit_src), /*call_tracer=*/nullptr); if (!err.ok()) { seen_errors.push_back(std::make_pair(i, err)); // If we get a connection error (i.e. not a stream error), stop parsing, diff --git a/test/core/transport/chttp2/hpack_sync_fuzzer.proto b/test/core/transport/chttp2/hpack_sync_fuzzer.proto index b4485c0aab4..72792b60d64 100644 --- a/test/core/transport/chttp2/hpack_sync_fuzzer.proto +++ b/test/core/transport/chttp2/hpack_sync_fuzzer.proto @@ -21,26 +21,27 @@ import "test/core/util/fuzz_config_vars.proto"; message Empty {} message StringKeyValue { - string key = 1; - string value = 2; + string key = 1; + string value = 2; } message IndexedKeyValue { - uint32 index = 1; - string value = 2; + uint32 index = 1; + string value = 2; } message Header { - oneof ty { - uint32 indexed = 1; - StringKeyValue literal_inc_idx = 2; - StringKeyValue literal_not_idx = 3; - IndexedKeyValue literal_not_idx_from_idx = 4; - } + oneof ty { + uint32 indexed = 1; + StringKeyValue literal_inc_idx = 2; + StringKeyValue literal_not_idx = 3; + IndexedKeyValue literal_not_idx_from_idx = 4; + } } message Msg { bool use_true_binary_metadata = 1; repeated Header headers = 2; grpc.testing.FuzzConfigVars config_vars = 3; + repeated uint64 random_numbers = 4; } diff --git a/test/core/util/BUILD b/test/core/util/BUILD index e520da58a06..e2e83ae81cc 100644 --- a/test/core/util/BUILD +++ b/test/core/util/BUILD @@ -47,6 +47,13 @@ TEST_UTILS_THAT_USE_GRPC_H_HEADERS = [ "test_tcp_server.h", ] +grpc_cc_library( + name = "proto_bit_gen", + language = "c++", + public_hdrs = ["proto_bit_gen.h"], + deps = ["//:gpr_platform"], +) + grpc_cc_library( name = "grpc_test_util_base", srcs = [ diff --git a/test/core/util/proto_bit_gen.h b/test/core/util/proto_bit_gen.h new file mode 100644 index 00000000000..fe299d65f46 --- /dev/null +++ b/test/core/util/proto_bit_gen.h @@ -0,0 +1,61 @@ +// 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_TEST_CORE_UTIL_PROTO_BIT_GEN_H +#define GRPC_TEST_CORE_UTIL_PROTO_BIT_GEN_H + +#include + +#include + +#include +#include +#include + +namespace grpc_core { + +// Set of random numbers from a proto file (or other container) forming a bit +// source. Satisfies the requirements for a URNG. +class ProtoBitGen : public std::numeric_limits { + public: + template + explicit ProtoBitGen(const SourceContainer& c) { + for (auto r : c) { + results_.push_back(r); + } + } + + using result_type = uint64_t; + + uint64_t operator()() { + if (results_.empty()) { + ++current_; + return current_; + } + // We loop through but increment by one each round, to guarantee to see all + // values eventually. + uint64_t out = + results_[current_ % results_.size()] + (current_ / results_.size()); + ++current_; + return out; + } + + private: + std::vector results_; + size_t current_ = 0; +}; + +} // namespace grpc_core + +#endif // GRPC_TEST_CORE_UTIL_PROTO_BIT_GEN_H diff --git a/test/cpp/microbenchmarks/bm_chttp2_hpack.cc b/test/cpp/microbenchmarks/bm_chttp2_hpack.cc index b512386655a..22da025e5cb 100644 --- a/test/cpp/microbenchmarks/bm_chttp2_hpack.cc +++ b/test/cpp/microbenchmarks/bm_chttp2_hpack.cc @@ -25,6 +25,8 @@ #include +#include "absl/random/random.h" + #include #include #include @@ -354,10 +356,12 @@ static void BM_HpackParserParseHeader(benchmark::State& state) { grpc_core::HPackParser::Priority::None, grpc_core::HPackParser::LogInfo{ 1, grpc_core::HPackParser::LogInfo::kHeaders, false}); - auto parse_vec = [&p](const std::vector& slices) { + auto parse_vec = [&p, bitgen = absl::BitGen()]( + const std::vector& slices) mutable { for (size_t i = 0; i < slices.size(); ++i) { auto error = - p.Parse(slices[i], i == slices.size() - 1, /*call_tracer=*/nullptr); + p.Parse(slices[i], i == slices.size() - 1, absl::BitGenRef(bitgen), + /*call_tracer=*/nullptr); GPR_ASSERT(error.ok()); } }; diff --git a/tools/distrib/fix_build_deps.py b/tools/distrib/fix_build_deps.py index a271be05d12..9a9e8bf15a9 100755 --- a/tools/distrib/fix_build_deps.py +++ b/tools/distrib/fix_build_deps.py @@ -67,6 +67,8 @@ EXTERNAL_DEPS = { "absl/meta/type_traits.h": "absl/meta:type_traits", "absl/numeric/int128.h": "absl/numeric:int128", "absl/random/random.h": "absl/random", + "absl/random/bit_gen_ref.h": "absl/random:bit_gen_ref", + "absl/random/mocking_bit_gen.h": "absl/random:mocking_bit_gen", "absl/random/distributions.h": "absl/random:distributions", "absl/random/uniform_int_distribution.h": "absl/random:distributions", "absl/status/status.h": "absl/status",