Skip to content

Commit

Permalink
fix: only allow istio gateways to set x509 client certificate header (#…
Browse files Browse the repository at this point in the history
…572)

## 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-certificate` header.
Keycloak then uses this as the x509 client identity.

https://github.com/defenseunicorns/uds-core/blob/e505dc936d16c763be0f7a487f727f91cf9d00b3/src/keycloak/chart/templates/uds-package.yaml#L87-L92

When Istio sets the header, we also delete any existing
`istio-mtls-client-certificate` header. This ensures that all users
through the ingress gateway cannot forge a
`istio-mtls-client-certificate` when 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-certificate` header not originating from the admin or
tenant gateway.

Credit to @rjferguson21 for asking great questions and catching this. 

## Type of change

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Other (security config, docs update, etc)

## Checklist before merging

- [ ] Test, docs, adr added or updated as needed
- [ ] [Contributor
Guide](https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md)
followed

Co-authored-by: Micah Nagel <micah.nagel@defenseunicorns.com>
  • Loading branch information
bburky and mjnagel committed Jul 15, 2024
1 parent d4afe00 commit 5c62279
Showing 1 changed file with 8 additions and 0 deletions.
8 changes: 8 additions & 0 deletions src/keycloak/chart/templates/istio-admin.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,12 @@ spec:
from:
- source:
notNamespaces: ["pepr-system"]
- when:
- key: request.headers[istio-mtls-client-certificate]
values: ["*"]
from:
- source:
notNamespaces:
- istio-tenant-gateway
- istio-admin-gateway
{{- end }}

0 comments on commit 5c62279

Please sign in to comment.