Skip to content

Commit

Permalink
[fuzz] fix fuzz crashes related to not implemented protos (envoyproxy…
Browse files Browse the repository at this point in the history
…#11385)

Fixes crashes (panic: not reached) due to not implemented protos being fuzzed.

- load balancing policy LOAD_BALANCING_POLICY_CONFIG not implemented, causing server_fuzz_test to crash
- connect matcher not supported in Jwt authentication filter, but matcher available @qiwzhang

Risk Level: Low
Testing: Regression testcases added

Fixes OSS-fuzz issues
- https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=21876
- https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=17762

Signed-off-by: Asra Ali <[email protected]>
  • Loading branch information
asraa authored and songhu committed Jun 25, 2020
1 parent 1f75dca commit 74300a3
Show file tree
Hide file tree
Showing 9 changed files with 39 additions and 5 deletions.
3 changes: 2 additions & 1 deletion api/envoy/config/cluster/v3/cluster.proto
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,8 @@ message Cluster {

// The :ref:`load balancer type <arch_overview_load_balancing_types>` to use
// when picking a host in the cluster.
LbPolicy lb_policy = 6 [(validate.rules).enum = {defined_only: true}];
// [#comment:TODO: Remove enum constraint :ref:`LOAD_BALANCING_POLICY_CONFIG<envoy_api_enum_value_config.cluster.v3.Cluster.LbPolicy.LOAD_BALANCING_POLICY_CONFIG>` when implemented.]
LbPolicy lb_policy = 6 [(validate.rules).enum = {defined_only: true not_in: 7}];

// Setting this is required for specifying members of
// :ref:`STATIC<envoy_api_enum_value_config.cluster.v3.Cluster.DiscoveryType.STATIC>`,
Expand Down
3 changes: 2 additions & 1 deletion api/envoy/config/cluster/v4alpha/cluster.proto
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,8 @@ message Cluster {

// The :ref:`load balancer type <arch_overview_load_balancing_types>` to use
// when picking a host in the cluster.
LbPolicy lb_policy = 6 [(validate.rules).enum = {defined_only: true}];
// [#comment:TODO: Remove enum constraint :ref:`LOAD_BALANCING_POLICY_CONFIG<envoy_api_enum_value_config.cluster.v4alpha.Cluster.LbPolicy.LOAD_BALANCING_POLICY_CONFIG>` when implemented.]
LbPolicy lb_policy = 6 [(validate.rules).enum = {defined_only: true not_in: 7}];

// Setting this is required for specifying members of
// :ref:`STATIC<envoy_api_enum_value_config.cluster.v4alpha.Cluster.DiscoveryType.STATIC>`,
Expand Down
3 changes: 2 additions & 1 deletion generated_api_shadow/envoy/config/cluster/v3/cluster.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions source/extensions/filters/http/jwt_authn/matcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,11 @@ MatcherConstPtr Matcher::create(const RequirementRule& rule) {
case RouteMatch::PathSpecifierCase::kHiddenEnvoyDeprecatedRegex:
case RouteMatch::PathSpecifierCase::kSafeRegex:
return std::make_unique<RegexMatcherImpl>(rule);
case RouteMatch::PathSpecifierCase::kConnectMatcher:
// TODO: When CONNECT match support is implemented, remove the manual clean-up of CONNECT
// matching in the filter fuzzer implementation:
// //test/extensions/filters/http/common/fuzz/uber_per_filter.cc
NOT_IMPLEMENTED_GCOVR_EXCL_LINE;
// path specifier is required.
case RouteMatch::PathSpecifierCase::PATH_SPECIFIER_NOT_SET:
default:
Expand Down
1 change: 1 addition & 0 deletions test/extensions/filters/http/common/fuzz/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ envoy_cc_test_library(
"//test/mocks/server:server_mocks",
"//test/proto:bookstore_proto_cc_proto",
"@envoy_api//envoy/extensions/filters/http/grpc_json_transcoder/v3:pkg_cc_proto",
"@envoy_api//envoy/extensions/filters/http/jwt_authn/v3:pkg_cc_proto",
"@envoy_api//envoy/extensions/filters/http/squash/v3:pkg_cc_proto",
"@envoy_api//envoy/extensions/filters/http/tap/v3:pkg_cc_proto",
],
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 17 additions & 1 deletion test/extensions/filters/http/common/fuzz/uber_per_filter.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "envoy/extensions/filters/http/grpc_json_transcoder/v3/transcoder.pb.h"
#include "envoy/extensions/filters/http/jwt_authn/v3/config.pb.h"
#include "envoy/extensions/filters/http/squash/v3/squash.pb.h"
#include "envoy/extensions/filters/http/tap/v3/tap.pb.h"

Expand Down Expand Up @@ -73,6 +74,16 @@ void UberFilterFuzzer::guideAnyProtoType(test::fuzz::HttpData* mutable_data, uin
mutable_any->set_type_url(type_url);
}

void removeConnectMatcher(Protobuf::Message* message) {
envoy::extensions::filters::http::jwt_authn::v3::JwtAuthentication& config =
dynamic_cast<envoy::extensions::filters::http::jwt_authn::v3::JwtAuthentication&>(*message);
for (auto& rules : *config.mutable_rules()) {
if (rules.match().has_connect_matcher()) {
rules.mutable_match()->set_path("/");
}
}
}

void cleanAttachmentTemplate(Protobuf::Message* message) {
envoy::extensions::filters::http::squash::v3::Squash& config =
dynamic_cast<envoy::extensions::filters::http::squash::v3::Squash&>(*message);
Expand All @@ -99,14 +110,19 @@ void UberFilterFuzzer::cleanFuzzedConfig(absl::string_view filter_name,
const std::string name = Extensions::HttpFilters::Common::FilterNameUtil::canonicalFilterName(
std::string(filter_name));
// Map filter name to clean-up function.
if (name == HttpFilterNames::get().GrpcJsonTranscoder) {
if (filter_name == HttpFilterNames::get().GrpcJsonTranscoder) {
// Add a valid service proto descriptor.
addBookstoreProtoDescriptor(message);
} else if (name == HttpFilterNames::get().Squash) {
cleanAttachmentTemplate(message);
} else if (name == HttpFilterNames::get().Tap) {
// TapDS oneof field not implemented.
cleanTapConfig(message);
}
if (filter_name == HttpFilterNames::get().JwtAuthn) {
// Remove when connect matcher is implemented for Jwt Authentication filter.
removeConnectMatcher(message);
}
}

void UberFilterFuzzer::perFilterSetup() {
Expand Down
1 change: 1 addition & 0 deletions test/server/server_corpus/not_reached

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 74300a3

Please sign in to comment.