-
Notifications
You must be signed in to change notification settings - Fork 111
Optional Gorouter mTLS Client Certificate Metadata Verification #355
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
Merged
ameowlia
merged 6 commits into
cloudfoundry:develop
from
sap-contributions:mtls-metadata-verification
Oct 11, 2023
Merged
Optional Gorouter mTLS Client Certificate Metadata Verification #355
ameowlia
merged 6 commits into
cloudfoundry:develop
from
sap-contributions:mtls-metadata-verification
Oct 11, 2023
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
f822101 to
4ff9bf4
Compare
5 tasks
4ff9bf4 to
98267c9
Compare
domdom82
suggested changes
Oct 6, 2023
domdom82
reviewed
Oct 9, 2023
domdom82
reviewed
Oct 11, 2023
domdom82
reviewed
Oct 11, 2023
domdom82
reviewed
Oct 11, 2023
spec/gorouter_templates_spec.rb
Outdated
| } | ||
| ] | ||
| end | ||
| it 'fails and explains the validpopulates the properties after a successful check' do |
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this sentence seems odd 🧐
domdom82
previously approved these changes
Oct 11, 2023
Contributor
domdom82
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only some minor gripes, approving.
peanball
commented
Oct 11, 2023
Co-authored-by: Dominik Froehlich <Dominik.Frolic@gmail.com>
domdom82
approved these changes
Oct 11, 2023
ameowlia
approved these changes
Oct 11, 2023
9 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is this change about?
We want to add optional mTLS client certificate metadata verificiation in addition to basic validity checks.
Given a particular signing CA (identified by its subject) we want to limit the allowed client certificate subjects.
While the underlying issue is better solved with changes to the PKI and trust relationship between CAs, the PKI or its use by other sub-entities is not always under the control of the operator.
This PR pulls in the Gorouter change that implements the feature and provides the configuration spec and template to configure the optional property.`
What type of change is this?
[Breaking Change]: the change removes a feature or introduces a behavior change to core functionality (request routing, request logging)[Minor Feature/Improvement]: the change introduces a new feature or improvement that doesn't alter core behavior[Bug Fix]: the change addresses a defectBackwards Compatibility
This is a non-breaking change that is completely optional. When the property is not set, no callback is registered at all.
How should this be tested?
See Gorouter PR. There are also automated tests as part of the Gorouter PR.
Additional Context
Gorouter PR: cloudfoundry/gorouter#367
PR Checklist
developbranch.scripts/run-unit-tests-in-dockerscripts/test-in-docker-locally.bash.