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.

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

Add mTLS Failure Response filter #35006

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

arulthileeban
Copy link
Contributor

@arulthileeban arulthileeban commented Jul 2, 2024

Commit Message: Addition of a network filter to respond with a non-SSL failure response for client cert verification failure (close connection abruptly or tarpit)

Risk Level: Low
Testing: Unit/Integration/Manual
Docs Changes: Done
Release Notes: TBD
Fixes #34617

@ggreenway Thanks for the initial direction. I had a couple of initial questions:

  • Does this look okay at a high level? Any preliminary changes before we get into the actual code review?
  • As per extension policy, adding a filter requires a sponsor for this to be part of the repository (non-contrib). Would you be willing to help with that?
  • I was thinking of adding another option to respond with a HTTP 403/401 like how NGINX helps handle mTLS, but that wouldn't be ideal since this is a network filter. Is there any recommendation you have on adding an option like "SET_METADATA_AND_CONTINUE" for eg. where we set some metadata for a filter higher in the stack like the CustomResponseFilter to match against it and issue a 403 or redirect to a standard error page? Or do we want to maybe take care of that in the core code by adding the validated/presented values in the matchers

Signed-off-by: Arul Thileeban Sagayam <[email protected]>
Signed-off-by: Arul Thileeban Sagayam <[email protected]>
Signed-off-by: Arul Thileeban Sagayam <[email protected]>
Signed-off-by: Arul Thileeban Sagayam <[email protected]>
Signed-off-by: Arul Thileeban Sagayam <[email protected]>
Signed-off-by: Arul Thileeban Sagayam <[email protected]>
Signed-off-by: Arul Thileeban Sagayam <[email protected]>
Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #35006 was opened by arulthileeban.

see: more, trace.

Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @abeyad
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #35006 was opened by arulthileeban.

see: more, trace.

@abeyad
Copy link
Contributor

abeyad commented Jul 2, 2024

/wait

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tls: Expand options to handle client certificate verification failure
2 participants