Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[fuzz] Create ext authz http fuzzer with dynamic metadata #15520

Merged
merged 10 commits into from
Dec 7, 2021
Merged

[fuzz] Create ext authz http fuzzer with dynamic metadata #15520

merged 10 commits into from
Dec 7, 2021

Conversation

asraa
Copy link
Contributor

@asraa asraa commented Mar 16, 2021 •
edited
Loading

Signed-off-by: Asra Ali asraa@google.com

Commit Message: Added dedicated fuzzer for ext_authz filter.
Additional Description:

  • Speed: ~400-500 exec/sec locally over the course of 5ish minutes.
  • Coverage: Covers 70.5% of http/ext_authz/ directory code. Gaps are:
    • Configuration code (merge context extensions)
    • Encode pathway
    • response features like headers to add (only status is fuzzed)

Risk Level: Low
Testing: Added corpus entries mirroring unit test cases, including metadata.

@asraa
Create ext authz http fuzzer with dynamic metadata
11e9d63 
Signed-off-by: Asra Ali <asraa@google.com>
@asraa asraa changed the title [DRAFT] [fuzz] Create ext authz http fuzzer with dynamic metadata [fuzz] Create ext authz http fuzzer with dynamic metadata Mar 31, 2021
@asraa asraa marked this pull request as ready for review March 31, 2021 13:43
@asraa asraa requested a review from dio as a code owner March 31, 2021 13:43
@asraa
Copy link
Contributor Author

asraa commented Mar 31, 2021

This is ready for review!

@fcfort

@asraa
add corpus entries
bda386c 
Signed-off-by: Asra Ali <asraa@google.com>
@esmet
Copy link
Contributor

esmet commented Apr 5, 2021

This looks reasonable though I admit that I need to learn more about how fuzz tests work in Envoy.

@asraa
Copy link
Contributor Author

asraa commented Apr 5, 2021 •
edited
Loading

No worries! I've defined a protobuf that is fuzzed by the engine that defines a sequence of actions and executes them with the ext_authz filter.

@fcfort would you be able to review (or find a review)?
(@chaoqin-li1123 I'm adding a new fuzzer here)

fcfort
fcfort reviewed Apr 9, 2021
View reviewed changes
Copy link
Contributor

@fcfort fcfort left a comment

Choose a reason for hiding this comment

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

Can you fix the envoy-presubmit errors? Looks like format_pre failed.

test/extensions/filters/http/ext_authz/ext_authz_corpus/custom_status
@@ -0,0 +1,23 @@
config {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the purpose of these files in ext_authz_corpus?

test/extensions/filters/http/ext_authz/ext_authz_fuzz_test.cc Outdated
Filters::Common::ExtAuthz::ResponsePtr response =
std::make_unique<Filters::Common::ExtAuthz::Response>();
response->status = status;
// TODO: add headers to remove, append, set, body, status_code.
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this TODO mean?

test/extensions/filters/http/ext_authz/ext_authz_fuzz.proto Outdated Show resolved Hide resolved
test/extensions/filters/http/ext_authz/ext_authz_fuzz.proto Outdated Show resolved Hide resolved
test/extensions/filters/http/ext_authz/ext_authz_fuzz.proto Outdated Show resolved Hide resolved
test/extensions/filters/http/ext_authz/ext_authz_fuzz.proto Outdated Show resolved Hide resolved
test/extensions/filters/http/ext_authz/ext_authz_fuzz.proto Outdated Show resolved Hide resolved
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label May 27, 2021
@junr03
Copy link
Member

junr03 commented Jun 2, 2021

@asraa I am closing so we keep active assignment on PRs. Please re-open if you want to continue this workstream.

@junr03 junr03 closed this Jun 2, 2021
@asraa asraa reopened this Nov 16, 2021
@github-actions github-actions bot removed the stale stalebot believes this issue/PR has not been touched recently label Nov 16, 2021
@rojkov
Copy link
Member

rojkov commented Nov 17, 2021

/wait

@asraa
Copy link
Contributor Author

asraa commented Dec 1, 2021

cc @adisuissa

@asraa asraa requested a review from adisuissa December 1, 2021 18:50
@adisuissa adisuissa self-assigned this Dec 1, 2021
adisuissa
adisuissa requested changes Dec 2, 2021
View reviewed changes
Copy link
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

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

Thanks for doing this work.
Left a few small nits, and the only thing that needs to be changed is surrounding the FilterConfig instantiation inside a try-catch clause.

test/extensions/filters/http/ext_authz/ext_authz_fuzz_test.cc Show resolved Hide resolved
test/extensions/filters/http/ext_authz/ext_authz_fuzz_test.cc Outdated Show resolved Hide resolved
test/extensions/filters/http/ext_authz/ext_authz_fuzz_test.cc Outdated Show resolved Hide resolved
test/extensions/filters/http/ext_authz/ext_authz_fuzz_test.cc Outdated Show resolved Hide resolved
test/extensions/filters/http/ext_authz/ext_authz_fuzz_test.cc Outdated Show resolved Hide resolved
test/extensions/filters/http/ext_authz/ext_authz_corpus/metadata_context Outdated Show resolved Hide resolved
test/extensions/filters/http/ext_authz/ext_authz_corpus/example Outdated Show resolved Hide resolved
test/extensions/filters/http/ext_authz/ext_authz_corpus/error_fail_close Outdated Show resolved Hide resolved
test/extensions/filters/http/ext_authz/ext_authz_corpus/custom_status Outdated Show resolved Hide resolved
.bazelrc Show resolved Hide resolved
@asraa
update adi comments
69124bf 
Signed-off-by: Asra Ali <asraa@google.com>
@asraa
update
5ed09bf 
Signed-off-by: Asra Ali <asraa@google.com>
@asraa
fix format
b0a1d10 
Signed-off-by: Asra Ali <asraa@google.com>
adisuissa
adisuissa approved these changes Dec 7, 2021
View reviewed changes
Copy link
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

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

Thanks for working on this!

@yanavlasov yanavlasov self-assigned this Dec 7, 2021
@yanavlasov yanavlasov merged commit aa4058f into envoyproxy:main Dec 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fcfort fcfort fcfort left review comments

@phlax phlax phlax left review comments

@adisuissa adisuissa adisuissa approved these changes

@yanavlasov yanavlasov yanavlasov approved these changes

@dio dio Awaiting requested review from dio

Assignees

@esmet esmet

@adisuissa adisuissa

@yanavlasov yanavlasov

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@asraa @esmet @junr03 @rojkov @fcfort @phlax @adisuissa @yanavlasov