fix: only allow istio gateways to set x509 client certificate header #572
Conversation
This file contains 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
mjnagel
changed the title
mjnagel
added
security
sso
jeff-mccoy
approved these changes
Jul 15, 2024
mjnagel
approved these changes
Jul 15, 2024
mjnagel
pushed a commit
that referenced
this pull request
Jul 22, 2024
🤖 I have created a release *beep* *boop* --- ## [0.24.1](v0.24.0...v0.24.1) (2024-07-22) ### Bug Fixes * **ci:** snapshot release publish, passthrough test on upgrade ([#575](#575)) ([d4afe00](d4afe00)) * **ci:** workflow permissions ([cacf1b5](cacf1b5)) * only allow istio gateways to set x509 client certificate header ([#572](#572)) ([5c62279](5c62279)) * **sso:** delete orphaned SSO secrets ([#578](#578)) ([5a6b9ef](5a6b9ef)) * unicorn flavor proxy image reference ([#590](#590)) ([db081fa](db081fa)) * update monitor mutation to not overwrite explicitly defined scrape class ([#582](#582)) ([7e550d3](7e550d3)) ### Miscellaneous * **deps:** update grafana chart + sidecar image ([#567](#567)) ([85b6de4](85b6de4)) * **deps:** update pepr to v0.32.7 ([#556](#556)) ([e594f13](e594f13)) * **deps:** update uds-identity-config to v0.5.1 ([#591](#591)) ([b9c5bd3](b9c5bd3)) * **deps:** update uds-k3d to v0.8.0 ([#581](#581)) ([fab8919](fab8919)) * **loki:** default query settings, config as secret ([#579](#579)) ([5fa889c](5fa889c)) * **oscal:** begin integration of composed oscal with validations ([#496](#496)) ([047fd30](047fd30)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
5 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.
Description
We terminate TLS at the Istio ingress gateway, and then we In the Keycloak VirtualService, we securely extract the client certificate, urlencode it, and copy it to the
istio-mtls-client-certificateheader. Keycloak then uses this as the x509 client identity.uds-core/src/keycloak/chart/templates/uds-package.yaml
Lines 87 to 92 in e505dc9
When Istio sets the header, we also delete any existing
istio-mtls-client-certificateheader. This ensures that all users through the ingress gateway cannot forge aistio-mtls-client-certificatewhen they do not use TLS client authentication.However... we only do this at the gateway and we allow in-cluster mesh communication to Keycloak to non-admin URLs.
This PR adds a DENY AuthorizationPolicy rule to the Keycloak Istio sidecar, to deny any incoming requests with a
istio-mtls-client-certificateheader not originating from the admin or tenant gateway.Credit to @rjferguson21 for asking great questions and catching this.
Type of change
Checklist before merging