Gep-91: Address connection coalescing security issue #3942
Conversation
Relates to kubernetes-sigs#3760 (comment) Signed-off-by: Arko Dasgupta <[email protected]>
k8s-ci-robot
added
release-note
k8s-ci-robot
added
do-not-merge/hold
|
Hi @kl52752. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/assign @robscott @shaneutt @youngnick |
kl52752
force-pushed
the
gep-91-update
branch
2 times, most recently
from
b5c8c13 to
494f360
Compare
| // to a subset of listeners by creating only per-port configurations. Listeners | ||
| // with a port that does not match any TLS configuration will not have | ||
| // `frontendValidation` set. | ||
| type GatewayTLSConfigs = []TLSConfig |
There was a problem hiding this comment.
Is this meant to represent the top level field on the Gateway spec? Assuming that's the case, it could be helpful to clarify that.
There was a problem hiding this comment.
Good point.
I added a snippet with extending GatewaySpec with a new field. I hope it's more clear now.
geps/gep-91/index.md
Outdated
| * Add a new `FrontendValidationModeType` enum within `FrontendTLSValidation` indicating how gateway should validate client certificates. As for now we support following values but it might change in the future: | ||
| * `AllowValidOnly` | ||
| * `AllowInvalidOrMissingCert` |
There was a problem hiding this comment.
This is not rendering as intended in the deploy preview - take a look at https://deploy-preview-3942--kubernetes-sigs-gateway-api.netlify.app/geps/gep-91/
There was a problem hiding this comment.
That's weird in my VS code it's rendering as intended. I changed this to numbers.
There was a problem hiding this comment.
Yeah, mkdocs is quite picky. I think it may require 4 spaces or something, lots of other markdown renderers accept how you had the list formatted.
k8s-ci-robot
added
ok-to-test
|
This LGTM with the latest changes. I'll leave the hold in place in case anyone else wants to review though. /lgtm |
k8s-ci-robot
added
the
lgtm
|
Thanks @kl52752! /approve |
|
Will aim for some lazy consensus on here - we've got 3 approvals. Let's plan on removing the hold tomorrow unless there's any additional feedback. |
k8s-ci-robot
removed
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: arkodg, kl52752, robscott, shaneutt The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…#3942) * GEP 91: Update API Relates to kubernetes-sigs#3760 (comment) Signed-off-by: Arko Dasgupta <[email protected]> * GEP-91: Address connection coalescing security issue * remove changes in API --------- Signed-off-by: Arko Dasgupta <[email protected]> Co-authored-by: Arko Dasgupta <[email protected]>
6 participants
What type of PR is this?
/kind gep
What this PR does / why we need it:
Introduce changes to client certificate validation API to address connection coalescing security issue.
Which issue(s) this PR fixes:
Relates to: #3760 (comment)
Fixes #
Does this PR introduce a user-facing change?: