Skip to content

[fuzz] exclude unknown security extensions from fuzz tests#16074

Closed
asraa wants to merge 3 commits intoenvoyproxy:mainfrom
asraa:conslidate-config-fuzz-test
Closed

[fuzz] exclude unknown security extensions from fuzz tests#16074
asraa wants to merge 3 commits intoenvoyproxy:mainfrom
asraa:conslidate-config-fuzz-test

Conversation

@asraa
Copy link
Copy Markdown
Contributor

@asraa asraa commented Apr 19, 2021

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

Commit Message: exclude extensions with unknown extensions from fuzz tests
Additional Description: this hopefully will help gcc builds that are breaking with too many args.

asraa added 2 commits April 19, 2021 19:50
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
@asraa asraa assigned rgs1 and htuch Apr 19, 2021
@asraa
Copy link
Copy Markdown
Contributor Author

asraa commented Apr 19, 2021

cc @chaoqin-li1123

Copy link
Copy Markdown
Member

@htuch htuch 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, it will also save us on chasing down bugs in unsupported filters. Question on method..

# Extensions with unknown security posture. This will be kept in sync by CI.
# This is used to exclude while fuzzing.
UNKNOWN_SECURITY_EXTENSIONS = [
"envoy.access_loggers.wasm",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we do this differently? In the fuzz rule, can we make use of the security posture information somehow and nop it? Just worried about maintainability and single SoT.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah, right it's checked validated in CI against security posture info.

I get that, I wasn't sure at all how to reflect back on the status information from the extensions, but mostly want to block CI asap. If you have a Bazel docs link (or example?) please link!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How about having

def envoy_cc_extension(
add to the binary some static registration of security posture?

I think we want this anyway, since when we add the "secure profile", we want to refuse to initialize if any insecure extensions are linked. LMK if this seems too much time given the desire to land something soon, but it seems the most robust for future needs.

Signed-off-by: Asra Ali <asraa@google.com>
jmarantz pushed a commit that referenced this pull request Apr 23, 2021
Signed-off-by: Asra Ali <asraa@google.com>

Commit Message: Temporary CI fix before #16074
Risk Level: Low
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
gokulnair pushed a commit to gokulnair/envoy that referenced this pull request May 6, 2021
Signed-off-by: Asra Ali <asraa@google.com>

Commit Message: Temporary CI fix before envoyproxy#16074
Risk Level: Low
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:

Signed-off-by: Gokul Nair <gnair@twitter.com>
@github-actions
Copy link
Copy Markdown

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 20, 2021
@github-actions
Copy link
Copy Markdown

This pull request has been automatically closed because it has not had activity in the last 37 days. 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 closed this May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale stalebot believes this issue/PR has not been touched recently

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants