Skip to content

Commit b62f1ca

Browse files
kvapskvaps
committed
fix(keycloak): replace only the old static KEK form, refuse the rest
Any stored KEK that did not decode to exactly 44 characters was overwritten. The proxy skips CR and LF when it decodes KKP_KEK, so a key restored from a file ending in a newline works, and the chart replaced it silently. A 44-character value that is not base64 of 32 bytes was kept, and the proxy could not start with it. Only the 32-character alphanumeric form earlier chart versions wrote is replaced now. A key the proxy accepts is kept, and anything else fails the render, so key material is never overwritten. Assisted-by: LLM Signed-off-by: Andrei Kvapil <[email protected]>
1 parent 9754a05 commit b62f1ca

3 files changed

Lines changed: 79 additions & 8 deletions

File tree

‎packages/system/keycloak/templates/proxy.yaml‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,17 +73,26 @@
7373
credentials in sts.yaml). For production prefer kms.backend=vault-transit.
7474
*/}}
7575
{{- /*
76-
The proxy reads KKP_KEK as base64 of 32 random bytes (44 characters).
77-
Earlier chart versions stored 32 alphanumeric characters instead, which
78-
decode to 24 bytes: the proxy refused to start with them, so nothing was
79-
ever encrypted under such a KEK and it is safe to replace it.
76+
The proxy reads KKP_KEK as base64 of 32 random bytes. Earlier chart
77+
versions stored 32 alphanumeric characters instead, which decode to 24
78+
bytes: the proxy refused to start with them, so nothing was ever encrypted
79+
under such a KEK and it is safe to replace it. Any other stored value may be
80+
a key the operator restored by hand, so it is kept when the proxy accepts
81+
it and refused otherwise, never overwritten. b64dec ignores CR and LF like
82+
the proxy does, so a restored key with a trailing newline still counts.
8083
*/}}
8184
{{- $existing := lookup "v1" "Secret" .Release.Namespace "keycloak-kms-proxy-kek" }}
8285
{{- $kek := randBytes 32 | b64enc }}
86+
{{- $current := "" }}
8387
{{- if and $existing $existing.data }}
84-
{{- $current := index $existing.data "kek" | default "" }}
85-
{{- if eq (len (b64dec $current)) 44 }}
88+
{{- $current = index $existing.data "kek" | default "" }}
89+
{{- end }}
90+
{{- if $current }}
91+
{{- $stored := b64dec $current }}
92+
{{- if eq (len (b64dec $stored)) 32 }}
8693
{{- $kek = $current }}
94+
{{- else if not (regexMatch "^[A-Za-z0-9]{32}$" $stored) }}
95+
{{- fail "the kek key of Secret keycloak-kms-proxy-kek is not base64 of 32 bytes, so the KMS proxy cannot use it. The chart does not overwrite key material: restore the KEK the data was encrypted with, or delete the Secret if nothing was encrypted under it yet" }}
8796
{{- end }}
8897
{{- end }}
8998
apiVersion: v1

‎packages/system/keycloak/tests/encryption_test.yaml‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,67 @@ tests:
194194
path: metadata.annotations["helm.sh/resource-policy"]
195195
value: keep
196196

197+
- it: keeps a restored static KEK with a trailing newline
198+
# The proxy decodes KKP_KEK with base64.StdEncoding, which skips CR and
199+
# LF, so a key restored from a file ending in a newline still works.
200+
template: templates/proxy.yaml
201+
documentSelector:
202+
path: kind
203+
value: Secret
204+
kubernetesProvider:
205+
scheme:
206+
"v1/Secret":
207+
gvr:
208+
version: "v1"
209+
resource: "secrets"
210+
namespaced: true
211+
objects:
212+
- kind: Secret
213+
apiVersion: v1
214+
metadata:
215+
name: keycloak-kms-proxy-kek
216+
namespace: cozy-keycloak
217+
data:
218+
kek: QUFFQ0F3UUZCZ2NJQ1FvTERBME9EeEFSRWhNVUZSWVhHQmthR3h3ZEhoOD0K
219+
set:
220+
encryption.enabled: true
221+
encryption.fields: disabled
222+
_cluster:
223+
root-host: example.org
224+
cluster-domain: cozy.local
225+
asserts:
226+
- equal:
227+
path: data.kek
228+
value: QUFFQ0F3UUZCZ2NJQ1FvTERBME9EeEFSRWhNVUZSWVhHQmthR3h3ZEhoOD0K
229+
230+
- it: refuses a stored static KEK the proxy cannot decode instead of replacing it
231+
# 44 characters of valid base64 that decode to 33 bytes.
232+
template: templates/proxy.yaml
233+
kubernetesProvider:
234+
scheme:
235+
"v1/Secret":
236+
gvr:
237+
version: "v1"
238+
resource: "secrets"
239+
namespaced: true
240+
objects:
241+
- kind: Secret
242+
apiVersion: v1
243+
metadata:
244+
name: keycloak-kms-proxy-kek
245+
namespace: cozy-keycloak
246+
data:
247+
kek: QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUE=
248+
set:
249+
encryption.enabled: true
250+
encryption.fields: disabled
251+
_cluster:
252+
root-host: example.org
253+
cluster-domain: cozy.local
254+
asserts:
255+
- failedTemplate:
256+
errorPattern: "is not base64 of 32 bytes"
257+
197258
- it: exposes the proxy on a postgres Service
198259
template: templates/proxy.yaml
199260
documentSelector:

‎packages/system/keycloak/values.yaml‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,8 +137,9 @@ extraEnv: []
137137
# every previously-encrypted value becomes permanently undecryptable, so it
138138
# MUST be part of the backup set. Helm keeps the Secret when encryption is
139139
# disabled or the package is removed, and the next install reuses it; delete
140-
# it by hand only once nothing encrypted under it is left. For production
141-
# prefer kms.backend=vault-transit.
140+
# it by hand only once nothing encrypted under it is left. A stored value
141+
# the proxy cannot use fails the render instead of being replaced. For
142+
# production prefer kms.backend=vault-transit.
142143
encryption:
143144
enabled: false
144145
image: ghcr.io/cozystack/keycloak-kms-proxy:0.2.3@sha256:6ad52ca209b31afe0fb118ca5c2ada418ec22c16a20c5153003c954d9bb54e8e

0 commit comments

Comments
 (0)