Skip to content

[Fuzz] Network-layer filter generic fuzzer#12086

Merged
asraa merged 44 commits intoenvoyproxy:masterfrom
jianwen612:redis_generic_fuzzer
Jul 27, 2020
Merged

[Fuzz] Network-layer filter generic fuzzer#12086
asraa merged 44 commits intoenvoyproxy:masterfrom
jianwen612:redis_generic_fuzzer

Conversation

@jianwen612
Copy link
Contributor

Additional Description:
This is a fuzzer for testing network-layer(L3/L4) filters.
Now Envoy has 20 network-layer filters which will deal with raw bytes from untrusted networks and thus they are security-critical to some extent. The idea of this is to write a fuzzer which can be applied to different kinds of network filters(potentially cover all the filters), and when new filters are added to Envoy, we won't need to write dedicated fuzzers one by one to give them fuzz coverage.

To make the fuzzer run with client_ssl_auth, I fixed the issue in api/envoy/extensions/filters/network/client_ssl_auth/v3/client_ssl_auth.proto. The old validation allows the auth_api_cluster to be any strings including those not supported by HTTP header. This will crash the configuration process of client_ssl_auth filter. An error will look like this:

[2020-07-08 20:03:49.026][15][critical][assert] [source/common/http/header_map_impl.cc:116] assert failure: validHeaderString(absl::string_view(data, size)).

It is fixed by adding a validation which checks whether this is a valid HTTP header value.

In addition, envoy_all_network_filters is defined in source/extensions/all_extensions.bzl in order to load the network filters(especially their factories) into the fuzzer.

Currently this fuzzer has already covered 8 network-layer filters. I've added 14 simple test cases(as corpus) for these 8 filters.

    filter_names_ = {NetworkFilterNames::get().ExtAuthorization,
                     NetworkFilterNames::get().LocalRateLimit,
                     NetworkFilterNames::get().RedisProxy,
                     NetworkFilterNames::get().ClientSslAuth,
                     NetworkFilterNames::get().Echo,
                     NetworkFilterNames::get().DirectResponse,
                     NetworkFilterNames::get().DubboProxy,
                     NetworkFilterNames::get().SniCluster};

And I will make pull request to extend this to cover more filters in the following weeks.

Risk Level:Low
Testing: Coverage is not available since coverage script has an issue now.

[Optional Fixes #Issue]#12084

/cc @asraa
/cc @samkerner

Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
for ext_auth by enabling the mocked response.
Fixed the validation problem inside client_ssl_auth's protobuf
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
issues

Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/envoy/.
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #12086 was opened by jianwen612.

see: more, trace.

Signed-off-by: jianwen <jianwendong@google.com>
different files. Cleaned up the deps

Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
@asraa
Copy link
Contributor

asraa commented Jul 20, 2020

Merge main and we can pick up a coverage report! :)

Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
@jianwen612
Copy link
Contributor Author

Coverage report for 8 filters(running for 60 seconds):
Directory name-----------------------------------------------------/ Line Coverage / Functions coverage
source/extensions/filters/network/sni_cluster----------/ 100.0 % 31 / 31 / 100.0 % 7 / 7
source/extensions/filters/network/echo--------------------/94.1 % 16 / 17 /85.7 % 6 / 7
source/extensions/filters/network/local_ratelimit-----/94.0 % 79 / 84 /100.0 % 14 / 14
source/extensions/filters/network/direct_response-----/82.1 % 23 / 28/ 87.5 % 7 / 8
source/extensions/filters/network/ext_authz ------------/ 80.5 % 99 / 123 /84.2 % 16 / 19
source/extensions/filters/network/client_ssl_auth-----/52.9 % 64 / 121 /56.0 % 14 / 25
source/extensions/filters/network/redis_proxy----------/ 19.5 % 236 / 1208/ 26.8 % 38 / 142
source/extensions/filters/network/dubbo_proxy----------/15.2 % 279 / 1830/ 23.0 % 65 / 282

Copy link
Contributor

@asraa asraa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for now, @sqkerner what do you think? Will give you a final sign-off.

I think after this it would be good to iterate on speed (I think it could still be faster, some mock perFilterSetup could happen once or replaced with reals), corpus entries, specific fuzzers, and bugs that come up.

Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: jianwen <jianwendong@google.com>
Copy link
Contributor

@sqkerner sqkerner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Copy link
Contributor

@asraa asraa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Jianwen! I'm excited to see this land on OSS-Fuzz :)

@asraa asraa merged commit ce26fe1 into envoyproxy:master Jul 27, 2020
KBaichoo pushed a commit to KBaichoo/envoy that referenced this pull request Jul 30, 2020
* added generic freamework for testing filters.
This is a fuzzer for testing network-layer(L3/L4) filters.
Now Envoy has 20 network-layer filters which will deal with raw bytes from untrusted networks and thus they are security-critical to some extent. The idea of this is to write a fuzzer which can be applied to different kinds of network filters(potentially cover all the filters), and when new filters are added to Envoy, we won't need to write dedicated fuzzers one by one to give them fuzz coverage.

Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: Kevin Baichoo <kbaichoo@google.com>
chaoqin-li1123 pushed a commit to chaoqin-li1123/envoy that referenced this pull request Aug 7, 2020
* added generic freamework for testing filters.
This is a fuzzer for testing network-layer(L3/L4) filters.
Now Envoy has 20 network-layer filters which will deal with raw bytes from untrusted networks and thus they are security-critical to some extent. The idea of this is to write a fuzzer which can be applied to different kinds of network filters(potentially cover all the filters), and when new filters are added to Envoy, we won't need to write dedicated fuzzers one by one to give them fuzz coverage.

Signed-off-by: jianwen <jianwendong@google.com>
chaoqin-li1123 pushed a commit to chaoqin-li1123/envoy that referenced this pull request Aug 7, 2020
* added generic freamework for testing filters.
This is a fuzzer for testing network-layer(L3/L4) filters.
Now Envoy has 20 network-layer filters which will deal with raw bytes from untrusted networks and thus they are security-critical to some extent. The idea of this is to write a fuzzer which can be applied to different kinds of network filters(potentially cover all the filters), and when new filters are added to Envoy, we won't need to write dedicated fuzzers one by one to give them fuzz coverage.

Signed-off-by: jianwen <jianwendong@google.com>
Signed-off-by: chaoqinli <chaoqinli@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants