fix(keycloak): pin the Vault leg to the configured CA and roll on rotation

SSL_CERT_FILE alone does not narrow trust. crypto/x509 replaces only its file
list and then scans SSL_CERT_DIR unconditionally, and the pinned proxy image
ships /etc/ssl/certs/ca-certificates.crt, so the connection guarding the KEK
trusted the private CA plus every public root — a mis-issued certificate for the
Vault hostname was still accepted. Set SSL_CERT_DIR to the mount as well, which
collapses the pool to the CA the operator configured.

An unvalidated caBundle failed open on top of that: a non-PEM value is dropped
silently by AppendCertsFromPEM, leaving the proxy on the image's roots with no
error at all. Reject it at render time, the way backend and auth already are.

Rotating an inline caBundle had no effect either. The pod template did not
change, so no new pods, and crypto/x509 memoizes the pool per process — the
upgrade reported success while the old CA stayed in use until some later
restart. Derive a checksum/vault-ca annotation from the bundle so the rollout
happens with the change.

Also add vault.caSecretKey: the projected key was hardcoded to ca.crt, and a
bundle stored under another name (a cert-manager Certificate from an ACME issuer
emits no ca.crt at all) wedged the pod at mount time with no way to correct it
from values.

Drop the claim that the database leg verifies against its own CA pool: the chart
never sets KKP_BACKEND_CA_FILE, so that leg is plaintext today, as the note 30
lines above already says. The conclusion holds — it never consults the system
trust store — but for the other reason.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Kirill Ilin <stitch14@yandex.ru>

packages/system/keycloak/templates/proxy.yaml

@@ -11,29 +11,44 @@
 {{- if and (ne $backend "static") (ne $backend "vault-transit") }}
 {{- fail (printf "encryption.kms.backend must be 'static' or 'vault-transit', got %q" $backend) }}
 {{- end }}
+{{- $vault := ($kms.vault | default dict) }}
 {{- /* The proxy needs its ServiceAccount token only for Vault Kubernetes auth. */}}
-{{- $vaultAuth := (($kms.vault | default dict).auth | default "token") }}
+{{- $vaultAuth := ($vault.auth | default "token") }}
 {{- $k8sAuth := and (eq $backend "vault-transit") (eq $vaultAuth "kubernetes") }}
 {{- /*
   CA the proxy trusts for the Vault connection: either seeded by the operator
-  (caSecretName) or rendered here from an inline PEM (caBundle). It is exposed
-  as SSL_CERT_FILE, which only redirects the system trust store — the database
-  connection verifies against its own CA pool and is unaffected.
+  (caSecretName) or rendered here from an inline PEM (caBundle). It reaches the
+  proxy as SSL_CERT_FILE plus SSL_CERT_DIR — see the env block for why both are
+  needed. Neither affects the database leg, which never consults the system
+  trust store: that connection is plaintext today, and once KKP_BACKEND_CA_FILE
+  is wired it will verify against an explicit RootCAs pool.
 
   Like every other vault.* key, both are inert unless the backend is
   vault-transit: the volume and mount below sit at pod level, so keying them off
   an ungated caSecretName would make a static-backend Deployment mount a Secret
   the chart never creates and wedge the proxy in ContainerCreating.
 */}}
-{{- $vaultTLS := ($kms.vault | default dict) }}
 {{- $vaultCASecret := "" }}
+{{- $vaultCASecretKey := "" }}
 {{- if eq $backend "vault-transit" }}
-{{- if and $vaultTLS.caBundle $vaultTLS.caSecretName }}
+{{- if and $vault.caBundle $vault.caSecretName }}
 {{- fail "encryption.kms.vault.caBundle and caSecretName are mutually exclusive — set one" }}
 {{- end }}
-{{- $vaultCASecret = $vaultTLS.caSecretName | default "" }}
-{{- if $vaultTLS.caBundle }}
+{{- if $vault.caBundle }}
+{{- /*
+  A bundle that is not PEM would be written to the Secret, mounted, and then
+  silently dropped by AppendCertsFromPEM, leaving the proxy on whatever roots
+  the image ships — a failure that only shows up as an unrelated TLS error, or
+  not at all against a publicly trusted Vault. Reject it at render time.
+*/}}
+{{- if not (regexMatch "-----BEGIN CERTIFICATE-----" $vault.caBundle) }}
+{{- fail "encryption.kms.vault.caBundle must be a PEM bundle containing at least one -----BEGIN CERTIFICATE----- block" }}
+{{- end }}
 {{- $vaultCASecret = "keycloak-kms-proxy-vault-ca" }}
+{{- $vaultCASecretKey = "ca.crt" }}
+{{- else if $vault.caSecretName }}
+{{- $vaultCASecret = $vault.caSecretName }}
+{{- $vaultCASecretKey = ($vault.caSecretKey | default "ca.crt") }}
 {{- end }}
 {{- end }}
 {{- $replicas := .Values.encryption.replicas | default 1 | int }}
@@ -61,16 +76,15 @@ data:
   kek: {{ $kek | quote }}
 ---
 {{- end }}
-{{- if and (eq $backend "vault-transit") $vaultTLS.caBundle }}
+{{- if and (eq $backend "vault-transit") $vault.caBundle }}
 apiVersion: v1
 kind: Secret
 metadata:
   name: keycloak-kms-proxy-vault-ca
   labels:
     app: keycloak-kms-proxy
-type: Opaque
-data:
-  ca.crt: {{ $vaultTLS.caBundle | b64enc }}
+stringData:
+  ca.crt: {{ $vault.caBundle | quote }}
 ---
 {{- end }}
 apiVersion: v1
@@ -100,6 +114,14 @@ spec:
       app: keycloak-kms-proxy
   template:
     metadata:
+      {{- if and (eq $backend "vault-transit") $vault.caBundle }}
+      annotations:
+        # The chart owns this Secret, so it owns the rollout. crypto/x509
+        # memoizes the system pool per process (sync.Once), so a rotated bundle
+        # reaches the proxy only in a fresh pod — without this the upgrade
+        # succeeds and the old CA stays in use until some unrelated restart.
+        checksum/vault-ca: {{ $vault.caBundle | sha256sum }}
+      {{- end }}
       labels:
         app: keycloak-kms-proxy
     spec:
@@ -166,10 +188,8 @@ spec:
             - name: KKP_LENIENT
               value: {{ .Values.encryption.lenient | quote }}
             {{- if eq $backend "vault-transit" }}
-            {{- $vault := $kms.vault | default dict }}
-            {{- $auth := $vault.auth | default "token" }}
-            {{- if and (ne $auth "token") (ne $auth "approle") (ne $auth "kubernetes") }}
-            {{- fail (printf "encryption.kms.vault.auth must be 'token', 'approle', or 'kubernetes', got %q" $auth) }}
+            {{- if and (ne $vaultAuth "token") (ne $vaultAuth "approle") (ne $vaultAuth "kubernetes") }}
+            {{- fail (printf "encryption.kms.vault.auth must be 'token', 'approle', or 'kubernetes', got %q" $vaultAuth) }}
             {{- end }}
             - name: KKP_VAULT_ADDR
               value: {{ required "encryption.kms.vault.address is required when kms.backend=vault-transit" $vault.address | quote }}
@@ -178,12 +198,18 @@ spec:
             - name: KKP_VAULT_MOUNT
               value: {{ $vault.mount | default "transit" | quote }}
             {{- with $vaultCASecret }}
-            # Go reads SSL_CERT_FILE in place of the system CA bundle. Only the
-            # Vault client consults it; the database TLS uses an explicit pool.
+            # SSL_CERT_FILE alone would ADD this CA to the roots the image ships
+            # (crypto/x509 replaces only its file list, then scans SSL_CERT_DIR
+            # unconditionally, and this image carries /etc/ssl/certs). Setting
+            # both collapses the pool to the mount, so the leg that guards the
+            # KEK trusts this CA and nothing else. The database leg is untouched:
+            # it never reads the system pool.
             - name: SSL_CERT_FILE
               value: /etc/kkp/vault-ca/ca.crt
+            - name: SSL_CERT_DIR
+              value: /etc/kkp/vault-ca
             {{- end }}
-            {{- if eq $auth "approle" }}
+            {{- if eq $vaultAuth "approle" }}
             {{- $appRole := $vault.appRole | default dict }}
             {{- $secretName := required "encryption.kms.vault.appRole.secretName is required when vault.auth=approle" $appRole.secretName }}
             - name: KKP_VAULT_APPROLE_MOUNT
@@ -198,7 +224,7 @@ spec:
                 secretKeyRef:
                   name: {{ $secretName }}
                   key: {{ $appRole.secretIdKey | default "secret-id" }}
-            {{- else if eq $auth "kubernetes" }}
+            {{- else if eq $vaultAuth "kubernetes" }}
             {{- $k8s := $vault.kubernetes | default dict }}
             # The proxy logs in with its own ServiceAccount token; bind the
             # Vault role to system:serviceaccount:<namespace>:keycloak-kms-proxy.
@@ -266,12 +292,14 @@ spec:
         - name: vault-ca
           secret:
             secretName: {{ . }}
-            # Project ca.crt alone. A pre-seeded Secret is often a cert-manager
-            # TLS Secret, whose tls.key has no business inside the proxy; and a
-            # bundle stored under another key fails the mount here instead of
-            # surfacing later as an opaque "unknown authority" TLS error.
+            # One key, projected under a fixed name so SSL_CERT_FILE is constant.
+            # A pre-seeded Secret is often a cert-manager TLS Secret, whose
+            # tls.key has no business inside the proxy. The trade-off is
+            # deliberate: a key that does not exist wedges the pod at mount time
+            # (vault.caSecretKey is there to point at the right one) rather than
+            # letting an empty trust store surface later as an opaque TLS error.
             items:
-              - key: ca.crt
+              - key: {{ $vaultCASecretKey }}
                 path: ca.crt
         {{- end }}
       {{- end }}

packages/system/keycloak/tests/encryption_test.yaml

@@ -489,6 +489,14 @@ tests:
             name: dekset
             secret:
               secretName: keycloak-kms-proxy-dekset
+      # Exactly one of each: the dekset and Vault CA blocks share these keys, so
+      # a CA volume rendered without its conditional shows up as a second entry.
+      - lengthEqual:
+          path: spec.template.spec.volumes
+          count: 1
+      - lengthEqual:
+          path: spec.template.spec.containers[0].volumeMounts
+          count: 1
 
   - it: rejects HA (replicas > 1) without a dekset
     template: templates/proxy.yaml
@@ -555,11 +563,17 @@ tests:
         cluster-domain: cozy.local
     template: templates/proxy.yaml
     asserts:
-      - exists:
-          path: data["ca.crt"]
+      - equal:
+          path: stringData["ca.crt"]
+          value: |
+            -----BEGIN CERTIFICATE-----
+            dGVzdA==
+            -----END CERTIFICATE-----
         documentSelector:
           path: metadata.name
           value: keycloak-kms-proxy-vault-ca
+      # SSL_CERT_FILE alone leaves the image's public roots in the pool; both
+      # variables together pin the Vault leg to this CA.
       - contains:
           path: spec.template.spec.containers[0].env
           content:
@@ -568,6 +582,22 @@ tests:
         documentSelector:
           path: kind
           value: Deployment
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: SSL_CERT_DIR
+            value: /etc/kkp/vault-ca
+        documentSelector:
+          path: kind
+          value: Deployment
+      # Derived from the bundle, so rotating it rolls the pods — the process
+      # memoizes its root pool and would otherwise keep the old CA.
+      - equal:
+          path: spec.template.metadata.annotations["checksum/vault-ca"]
+          value: de63913efff35e2e9be70b97bd7a0e12a2205f3a0ca00a114606d778abf65fe0
+        documentSelector:
+          path: kind
+          value: Deployment
       - contains:
           path: spec.template.spec.containers[0].volumeMounts
           content:
@@ -612,13 +642,22 @@ tests:
           content:
             name: SSL_CERT_FILE
             value: /etc/kkp/vault-ca/ca.crt
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: SSL_CERT_DIR
+            value: /etc/kkp/vault-ca
       - contains:
           path: spec.template.spec.containers[0].volumeMounts
           content:
             name: vault-ca
             mountPath: /etc/kkp/vault-ca
             readOnly: true
-      # Only ca.crt is projected: a cert-manager TLS Secret would otherwise hand
+      # No checksum annotation on this path: the chart does not own the Secret
+      # and cannot see its contents, so rotation needs a manual rollout restart.
+      - notExists:
+          path: spec.template.metadata.annotations
+      # Only one key is projected: a cert-manager TLS Secret would otherwise hand
       # tls.key to the proxy, and a bundle under another key would fail the mount
       # instead of looking like an unconfigured CA at runtime.
       - contains:
@@ -630,6 +669,14 @@ tests:
               items:
                 - key: ca.crt
                   path: ca.crt
+      # No dekset here, so a dekset volume rendered without its conditional
+      # (with an empty secretName) shows up as a second entry.
+      - lengthEqual:
+          path: spec.template.spec.volumes
+          count: 1
+      - lengthEqual:
+          path: spec.template.spec.containers[0].volumeMounts
+          count: 1
 
   - it: fails when both caBundle and caSecretName are set
     set:
@@ -735,3 +782,133 @@ tests:
       - notFailedTemplate: {}
       - hasDocuments:
           count: 4
+
+  - it: rolls the pods when a chart-owned caBundle is rotated
+    set:
+      encryption.enabled: true
+      encryption.kms.backend: vault-transit
+      encryption.kms.vault.address: https://vault.example
+      encryption.kms.vault.keyName: keycloak
+      encryption.kms.vault.auth: approle
+      encryption.kms.vault.appRole.secretName: keycloak-vault-approle
+      encryption.kms.vault.caBundle: |
+        -----BEGIN CERTIFICATE-----
+        cm90YXRlZA==
+        -----END CERTIFICATE-----
+      _cluster:
+        root-host: example.org
+        cluster-domain: cozy.local
+    template: templates/proxy.yaml
+    documentSelector:
+      path: kind
+      value: Deployment
+    asserts:
+      # Differs from the checksum asserted for the bundle above.
+      - equal:
+          path: spec.template.metadata.annotations["checksum/vault-ca"]
+          value: 76da7d3279a7bc1e42a021a7c3fdc0e79b506bc152171128003ae15f56512741
+
+  - it: rejects a caBundle that is not a PEM certificate bundle
+    set:
+      encryption.enabled: true
+      encryption.kms.backend: vault-transit
+      encryption.kms.vault.address: https://vault.example
+      encryption.kms.vault.keyName: keycloak
+      encryption.kms.vault.auth: approle
+      encryption.kms.vault.appRole.secretName: keycloak-vault-approle
+      encryption.kms.vault.caBundle: not-a-certificate
+      _cluster:
+        root-host: example.org
+        cluster-domain: cozy.local
+    template: templates/proxy.yaml
+    asserts:
+      - failedTemplate:
+          errorPattern: "caBundle must be a PEM bundle"
+
+  - it: reads a pre-seeded bundle from a custom caSecretKey
+    set:
+      encryption.enabled: true
+      encryption.kms.backend: vault-transit
+      encryption.kms.vault.address: https://vault.example
+      encryption.kms.vault.keyName: keycloak
+      encryption.kms.vault.auth: approle
+      encryption.kms.vault.appRole.secretName: keycloak-vault-approle
+      encryption.kms.vault.caSecretName: vault-serving-cert
+      encryption.kms.vault.caSecretKey: ca-bundle.crt
+      _cluster:
+        root-host: example.org
+        cluster-domain: cozy.local
+    template: templates/proxy.yaml
+    documentSelector:
+      path: kind
+      value: Deployment
+    asserts:
+      # The key varies, the projected path does not, so SSL_CERT_FILE is fixed.
+      - contains:
+          path: spec.template.spec.volumes
+          content:
+            name: vault-ca
+            secret:
+              secretName: vault-serving-cert
+              items:
+                - key: ca-bundle.crt
+                  path: ca.crt
+      - contains:
+          path: spec.template.spec.containers[0].env
+          content:
+            name: SSL_CERT_FILE
+            value: /etc/kkp/vault-ca/ca.crt
+
+  # Both optional mounts at once: the dekset and CA blocks share the volumes and
+  # volumeMounts keys, so dropping either inner conditional must fail here.
+  - it: mounts the dekset and the Vault CA side by side
+    set:
+      encryption.enabled: true
+      encryption.kms.backend: vault-transit
+      encryption.kms.vault.address: https://vault.example
+      encryption.kms.vault.keyName: keycloak
+      encryption.kms.vault.auth: approle
+      encryption.kms.vault.appRole.secretName: keycloak-vault-approle
+      encryption.kms.vault.caSecretName: vault-ca
+      encryption.deksetSecretName: keycloak-kms-proxy-dekset
+      _cluster:
+        root-host: example.org
+        cluster-domain: cozy.local
+    template: templates/proxy.yaml
+    documentSelector:
+      path: kind
+      value: Deployment
+    asserts:
+      - lengthEqual:
+          path: spec.template.spec.volumes
+          count: 2
+      - lengthEqual:
+          path: spec.template.spec.containers[0].volumeMounts
+          count: 2
+      - contains:
+          path: spec.template.spec.volumes
+          content:
+            name: dekset
+            secret:
+              secretName: keycloak-kms-proxy-dekset
+      - contains:
+          path: spec.template.spec.volumes
+          content:
+            name: vault-ca
+            secret:
+              secretName: vault-ca
+              items:
+                - key: ca.crt
+                  path: ca.crt
+      - contains:
+          path: spec.template.spec.containers[0].volumeMounts
+          content:
+            name: dekset
+            mountPath: /etc/kkp/dekset
+            readOnly: true
+      - contains:
+          path: spec.template.spec.containers[0].volumeMounts
+          content:
+            name: vault-ca
+            mountPath: /etc/kkp/vault-ca
+            readOnly: true

packages/system/keycloak/values.yaml

@@ -140,13 +140,21 @@ encryption:
         roleIdKey: role-id      # key in secretName holding the role id
         secretIdKey: secret-id  # key in secretName holding the secret id
       # CA to trust when Vault is served over HTTPS by a private CA — as an
-      # inline PEM (the chart renders the Secret) or as a Secret seeded
-      # beforehand holding it under key ca.crt (only that key is mounted). The
-      # two are mutually exclusive, and both are ignored unless
-      # kms.backend=vault-transit. Only the Vault client is affected: the proxy
-      # verifies the database with its own CA pool, not the system trust store.
+      # inline PEM (the chart renders the Secret and rolls the pods when it
+      # changes) or as a Secret seeded beforehand. Exactly one key of that
+      # Secret is mounted; caSecretKey names it. Rotating a pre-seeded Secret
+      # needs a manual `kubectl rollout restart deploy/keycloak-kms-proxy`,
+      # because the chart cannot see its contents.
+      #
+      # The Vault leg then trusts this CA and nothing else — the public roots in
+      # the proxy image are excluded. The database leg is unaffected either way:
+      # it never consults the system trust store.
+      #
+      # caBundle and caSecretName are mutually exclusive, and both are ignored
+      # unless kms.backend=vault-transit.
       caBundle: ""
       caSecretName: ""
+      caSecretKey: ca.crt      # key of caSecretName holding the PEM bundle
   resources:
     requests:
       memory: 128Mi