Skip to content

Commit 2e4e6c1

Browse files
dc-tecdc-tec
authored
fix(security): harden transit unseal credential handling (#473)
Signed-off-by: Roel de Cort <roel.decort@adfinis.com>
1 parent 445090a commit 2e4e6c1

17 files changed

Lines changed: 478 additions & 27 deletions

File tree

‎api/v1alpha1/openbaocluster_types.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1217,7 +1217,7 @@ type GatewayReference struct {
12171217
// TransitSealConfig configures the Transit seal type.
12181218
// See: https://openbao.org/docs/configuration/seal/transit/
12191219
type TransitSealConfig struct {
1220-
// Address is the full address to the OpenBao cluster.
1220+
// Address is the full HTTPS address to the OpenBao cluster providing the Transit seal.
12211221
// +kubebuilder:validation:MinLength=1
12221222
Address string `json:"address"`
12231223

‎charts/openbao-operator/crds/openbao.org_openbaoclusters.yaml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4314,7 +4314,8 @@ spec:
43144314
Required when Type is "transit".
43154315
properties:
43164316
address:
4317-
description: Address is the full address to the OpenBao cluster.
4317+
description: Address is the full HTTPS address to the OpenBao
4318+
cluster providing the Transit seal.
43184319
minLength: 1
43194320
type: string
43204321
disableRenewal:

‎charts/openbao-operator/templates/admission/validate-openbaocluster.yaml‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -157,13 +157,27 @@ spec:
157157
size(object.spec.unseal.transit.token) == 0
158158
message: Hardened profile does not allow spec.unseal.transit.token; use spec.unseal.credentialsSecretRef instead.
159159
- expression: >-
160-
!has(object.spec.profile) ||
161-
object.spec.profile != "Hardened" ||
162160
!has(object.spec.unseal) ||
163161
!has(object.spec.unseal.transit) ||
164162
!has(object.spec.unseal.transit.address) ||
165163
object.spec.unseal.transit.address.startsWith("https://")
166-
message: Hardened profile requires spec.unseal.transit.address to use HTTPS.
164+
message: Transit unseal address must use HTTPS.
165+
- expression: >-
166+
!has(object.spec.unseal) ||
167+
!has(object.spec.unseal.transit) ||
168+
!has(object.spec.unseal.transit.address) ||
169+
!(
170+
object.spec.unseal.transit.address.contains("@") ||
171+
object.spec.unseal.transit.address.contains("?") ||
172+
object.spec.unseal.transit.address.contains("#") ||
173+
object.spec.unseal.transit.address.matches("^https://([^/?#@]*\\.)?localhost([:/?#].*)?$") ||
174+
object.spec.unseal.transit.address.matches("^https://127\\.0\\.0\\.1([:/?#].*)?$") ||
175+
object.spec.unseal.transit.address.contains("[::1]") ||
176+
object.spec.unseal.transit.address.matches("^https://169\\.254\\..*") ||
177+
object.spec.unseal.transit.address.contains("[fe80:") ||
178+
object.spec.unseal.transit.address.matches("^https://[0-9]+([:/?#].*)?$")
179+
)
180+
message: Transit unseal address must not include userinfo, query, fragment, localhost, loopback, link-local, or numeric host forms.
167181
- expression: >-
168182
!has(object.spec.unseal) ||
169183
!has(object.spec.unseal.transit) ||
@@ -333,13 +347,21 @@ spec:
333347
object.spec.profile != "Hardened" ||
334348
object.spec.replicas >= 3
335349
message: "Hardened profile requires at least 3 replicas for Raft quorum HA. Use Profile=Development for non-HA deployments."
336-
# Block references to system secrets in backup/upgrade
350+
# Confused-deputy protection: users configuring unseal Secret credentials must be able to read that Secret.
351+
- expression: >-
352+
!has(object.spec.unseal) ||
353+
!has(object.spec.unseal.credentialsSecretRef) ||
354+
authorizer.group("").resource("secrets").namespace(request.namespace).name(object.spec.unseal.credentialsSecretRef.name).check("get").allowed()
355+
message: "Users configuring unseal credentials must be authorized to get spec.unseal.credentialsSecretRef."
356+
# Block references to system secrets in unseal/backup/upgrade
337357
- expression: >-
358+
(!has(object.spec.unseal) || !has(object.spec.unseal.credentialsSecretRef) ||
359+
!object.spec.unseal.credentialsSecretRef.name.matches("^.*-(unseal-key|root-token|tls-ca|tls-server)$")) &&
338360
(!has(object.spec.backup) || !has(object.spec.backup.target.credentialsSecretRef) ||
339361
!object.spec.backup.target.credentialsSecretRef.name.matches("^.*-(unseal-key|root-token|tls-ca|tls-server)$")) &&
340362
(!has(object.spec.backup) || !has(object.spec.backup.tokenSecretRef) ||
341363
!object.spec.backup.tokenSecretRef.name.matches("^.*-(unseal-key|root-token|tls-ca|tls-server)$"))
342-
message: "References to system secrets (unseal-key, root-token, tls-ca, tls-server) are prohibited in backup configurations to prevent confused deputy attacks."
364+
message: "References to system secrets (unseal-key, root-token, tls-ca, tls-server) are prohibited in unseal and backup configurations to prevent confused deputy attacks."
343365
# Storage Immutability: Prevent storage class changes (immutable in StatefulSet)
344366
- expression: >-
345367
oldObject == null ||

‎config/crd/bases/openbao.org_openbaoclusters.yaml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4313,7 +4313,8 @@ spec:
43134313
Required when Type is "transit".
43144314
properties:
43154315
address:
4316-
description: Address is the full address to the OpenBao cluster.
4316+
description: Address is the full HTTPS address to the OpenBao
4317+
cluster providing the Transit seal.
43174318
minLength: 1
43184319
type: string
43194320
disableRenewal:

‎config/policy/openbao-validate-openbaocluster.yaml‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -154,13 +154,27 @@ spec:
154154
size(object.spec.unseal.transit.token) == 0
155155
message: Hardened profile does not allow spec.unseal.transit.token; use spec.unseal.credentialsSecretRef instead.
156156
- expression: >-
157-
!has(object.spec.profile) ||
158-
object.spec.profile != "Hardened" ||
159157
!has(object.spec.unseal) ||
160158
!has(object.spec.unseal.transit) ||
161159
!has(object.spec.unseal.transit.address) ||
162160
object.spec.unseal.transit.address.startsWith("https://")
163-
message: Hardened profile requires spec.unseal.transit.address to use HTTPS.
161+
message: Transit unseal address must use HTTPS.
162+
- expression: >-
163+
!has(object.spec.unseal) ||
164+
!has(object.spec.unseal.transit) ||
165+
!has(object.spec.unseal.transit.address) ||
166+
!(
167+
object.spec.unseal.transit.address.contains("@") ||
168+
object.spec.unseal.transit.address.contains("?") ||
169+
object.spec.unseal.transit.address.contains("#") ||
170+
object.spec.unseal.transit.address.matches("^https://([^/?#@]*\\.)?localhost([:/?#].*)?$") ||
171+
object.spec.unseal.transit.address.matches("^https://127\\.0\\.0\\.1([:/?#].*)?$") ||
172+
object.spec.unseal.transit.address.contains("[::1]") ||
173+
object.spec.unseal.transit.address.matches("^https://169\\.254\\..*") ||
174+
object.spec.unseal.transit.address.contains("[fe80:") ||
175+
object.spec.unseal.transit.address.matches("^https://[0-9]+([:/?#].*)?$")
176+
)
177+
message: Transit unseal address must not include userinfo, query, fragment, localhost, loopback, link-local, or numeric host forms.
164178
- expression: >-
165179
!has(object.spec.unseal) ||
166180
!has(object.spec.unseal.transit) ||
@@ -330,13 +344,21 @@ spec:
330344
object.spec.profile != "Hardened" ||
331345
object.spec.replicas >= 3
332346
message: "Hardened profile requires at least 3 replicas for Raft quorum HA. Use Profile=Development for non-HA deployments."
333-
# Block references to system secrets in backup/upgrade
347+
# Confused-deputy protection: users configuring unseal Secret credentials must be able to read that Secret.
348+
- expression: >-
349+
!has(object.spec.unseal) ||
350+
!has(object.spec.unseal.credentialsSecretRef) ||
351+
authorizer.group("").resource("secrets").namespace(request.namespace).name(object.spec.unseal.credentialsSecretRef.name).check("get").allowed()
352+
message: "Users configuring unseal credentials must be authorized to get spec.unseal.credentialsSecretRef."
353+
# Block references to system secrets in unseal/backup/upgrade
334354
- expression: >-
355+
(!has(object.spec.unseal) || !has(object.spec.unseal.credentialsSecretRef) ||
356+
!object.spec.unseal.credentialsSecretRef.name.matches("^.*-(unseal-key|root-token|tls-ca|tls-server)$")) &&
335357
(!has(object.spec.backup) || !has(object.spec.backup.target.credentialsSecretRef) ||
336358
!object.spec.backup.target.credentialsSecretRef.name.matches("^.*-(unseal-key|root-token|tls-ca|tls-server)$")) &&
337359
(!has(object.spec.backup) || !has(object.spec.backup.tokenSecretRef) ||
338360
!object.spec.backup.tokenSecretRef.name.matches("^.*-(unseal-key|root-token|tls-ca|tls-server)$"))
339-
message: "References to system secrets (unseal-key, root-token, tls-ca, tls-server) are prohibited in backup configurations to prevent confused deputy attacks."
361+
message: "References to system secrets (unseal-key, root-token, tls-ca, tls-server) are prohibited in unseal and backup configurations to prevent confused deputy attacks."
340362
# Storage Immutability: Prevent storage class changes (immutable in StatefulSet)
341363
- expression: >-
342364
oldObject == null ||

‎config/samples/production/openbao_v1alpha1_openbaocluster_transit.yaml‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,9 @@ spec:
8181
# namespace: openbaocluster-transit
8282
# type: Opaque
8383
# stringData:
84-
# VAULT_TOKEN: "s.Qf1s5zigZ4OX6akYjQXJC1jY"
84+
# token: "s.Qf1s5zigZ4OX6akYjQXJC1jY"
8585
# # Optional: CA certificate for TLS verification
8686
# # ca.crt: |
8787
# # -----BEGIN CERTIFICATE-----
8888
# # ...
8989
# # -----END CERTIFICATE-----
90-

‎docs/reference/api.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1830,7 +1830,7 @@ _Appears in:_
18301830

18311831
| Field | Description | Default | Validation |
18321832
| --- | --- | --- | --- |
1833-
| `address` _string_ | Address is the full address to the OpenBao cluster. | | MinLength: 1 <br /> |
1833+
| `address` _string_ | Address is the full HTTPS address to the OpenBao cluster providing the Transit seal. | | MinLength: 1 <br /> |
18341834
| `token` _string_ | Token is the OpenBao token to use for authentication.<br />Note: It is strongly recommended to use CredentialsSecretRef instead of setting this directly. | | Optional: \{\} <br /> |
18351835
| `keyName` _string_ | KeyName is the transit key to use for encryption and decryption. | | MinLength: 1 <br /> |
18361836
| `mountPath` _string_ | MountPath is the mount path to the transit secret engine. | | MinLength: 1 <br /> |

‎docs/user-guide/openbaocluster/configuration/unseal.md‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ For production-oriented clusters, use an external trust source such as cloud KMS
6363
<Callout type="note" title="What the operator validates before Pods can use Secret-backed credentials">
6464

6565
- `spec.unseal.credentialsSecretRef` must reference a Secret in the same namespace as the `OpenBaoCluster`.
66+
- Transit unseal addresses must use HTTPS and must not include userinfo, query, fragment, localhost, loopback, link-local, or numeric host forms.
67+
- Users configuring transit `credentialsSecretRef` must also be authorized to `get` the referenced Secret.
68+
- Transit unseal credentials cannot reference operator-managed system Secrets such as `<cluster>-root-token`, `<cluster>-unseal-key`, `<cluster>-tls-ca`, or `<cluster>-tls-server`.
6669
- Any unseal field that points to a mounted credential file must use a path under `/etc/bao/seal-creds`.
6770
- The Secret key name must match the filename used in the mounted path.
6871
- When you use private ACME trust rooted under `/etc/bao/seal-creds`, include `pki-ca.crt` in the same Secret so probes and day-2 operations can trust the ACME issuer too.
@@ -111,7 +114,7 @@ For production-oriented clusters, use an external trust source such as cloud KMS
111114
"Transit",
112115
"Needed whenever you do not rely only on an inline token and Secret-backed files are referenced.",
113116
"`token`, plus any mounted files referenced by `tlsCACert`, `tlsClientCert`, and `tlsClientKey`.",
114-
"If client cert auth is used, the certificate and key must both be present and form a valid key pair.",
117+
"Use an orphan or periodic token with only update permissions on the transit encrypt/decrypt paths. If client cert auth is used, the certificate and key must both be present and form a valid key pair.",
115118
],
116119
emphasis: "recommended",
117120
},

‎internal/service/bootstrap/unseal_validation.go‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,24 @@ func (m *Manager) validateUnsealPrerequisites(ctx context.Context, cluster *open
1515
if cluster == nil || cluster.Spec.Unseal == nil {
1616
return nil
1717
}
18+
if err := validateUnsealCredentialsSecretRef(cluster); err != nil {
19+
return providerPrerequisitesError(err)
20+
}
1821

1922
switch cluster.Spec.Unseal.Type {
20-
case "", "static":
23+
case "", portopenbao.SealTypeStatic:
2124
return nil
2225
case unsealTypeTransit:
2326
return m.validateTransitUnsealPrerequisites(ctx, cluster)
24-
case "awskms":
27+
case portopenbao.SealTypeAWSKMS:
2528
return m.validateAWSKMSUnsealPrerequisites(ctx, cluster)
26-
case "azurekeyvault":
29+
case portopenbao.SealTypeAzureKeyVault:
2730
return m.validateAzureKeyVaultUnsealPrerequisites(ctx, cluster)
28-
case "gcpckms":
31+
case portopenbao.SealTypeGCPCKMS:
2932
return m.validateGCPCKMSUnsealPrerequisites(ctx, cluster)
30-
case "kmip":
33+
case portopenbao.SealTypeKMIP:
3134
return m.validateKMIPUnsealPrerequisites(ctx, cluster)
32-
case "ocikms":
35+
case portopenbao.SealTypeOCIKMS:
3336
return m.validateOCIKMSUnsealPrerequisites(ctx, cluster)
3437
case portopenbao.SealTypePKCS11:
3538
return m.validatePKCS11UnsealPrerequisites(ctx, cluster)

‎internal/service/bootstrap/unseal_validation_test.go‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,104 @@ func TestValidateTransitUnsealPrerequisites(t *testing.T) {
4141
}
4242
})
4343

44+
t.Run("transit address rejects insecure and ambiguous destinations", func(t *testing.T) {
45+
tests := []struct {
46+
name string
47+
address string
48+
want string
49+
}{
50+
{
51+
name: "plain http",
52+
address: "http://infra-bao.example",
53+
want: "must use HTTPS",
54+
},
55+
{
56+
name: "userinfo",
57+
address: "https://token@infra-bao.example",
58+
want: "must not include userinfo",
59+
},
60+
{
61+
name: "localhost",
62+
address: "https://localhost:8200",
63+
want: "must not point to localhost",
64+
},
65+
{
66+
name: "loopback ip",
67+
address: "https://127.0.0.1:8200",
68+
want: "must not point to loopback",
69+
},
70+
{
71+
name: "link local ip",
72+
address: "https://169.254.169.254/latest/meta-data",
73+
want: "must not point to loopback",
74+
},
75+
{
76+
name: "query",
77+
address: "https://infra-bao.example?token=leak",
78+
want: "must not include query or fragment",
79+
},
80+
{
81+
name: "numeric host",
82+
address: "https://2130706433:8200",
83+
want: "must not use numeric host forms",
84+
},
85+
}
86+
87+
for _, tt := range tests {
88+
t.Run(tt.name, func(t *testing.T) {
89+
cluster := newMinimalCluster("transit-address-"+tt.name, "default")
90+
cluster.Spec.Unseal = &openbaov1alpha1.UnsealConfig{
91+
Type: "transit",
92+
Transit: &openbaov1alpha1.TransitSealConfig{
93+
Address: tt.address,
94+
KeyName: "autounseal",
95+
MountPath: "transit/",
96+
Token: "inline-token",
97+
},
98+
}
99+
100+
mgr := NewManager(newTestClient(t), testScheme, "operator-system")
101+
err := mgr.validateUnsealPrerequisites(context.Background(), cluster)
102+
if err == nil {
103+
t.Fatal("validateUnsealPrerequisites() error = nil, want error")
104+
}
105+
if !errors.Is(err, operatorerrors.ErrPermanentPrerequisitesMissing) {
106+
t.Fatalf("expected permanent prerequisites missing error, got %v", err)
107+
}
108+
if !strings.Contains(err.Error(), tt.want) {
109+
t.Fatalf("error = %q, want substring %q", err.Error(), tt.want)
110+
}
111+
})
112+
}
113+
})
114+
115+
t.Run("transit credentials Secret cannot reference operator managed system Secrets", func(t *testing.T) {
116+
cluster := newMinimalCluster("transit-system-secret", "default")
117+
cluster.Spec.Unseal = &openbaov1alpha1.UnsealConfig{
118+
Type: "transit",
119+
CredentialsSecretRef: &corev1.LocalObjectReference{
120+
Name: "transit-system-secret-root-token",
121+
},
122+
Transit: &openbaov1alpha1.TransitSealConfig{
123+
Address: "https://infra-bao.example",
124+
KeyName: "autounseal",
125+
MountPath: "transit/",
126+
},
127+
}
128+
129+
mgr := NewManager(newTestClient(t), testScheme, "operator-system")
130+
err := mgr.validateUnsealPrerequisites(context.Background(), cluster)
131+
if err == nil {
132+
t.Fatal("validateUnsealPrerequisites() error = nil, want error")
133+
}
134+
if !errors.Is(err, operatorerrors.ErrPermanentPrerequisitesMissing) {
135+
t.Fatalf("expected permanent prerequisites missing error, got %v", err)
136+
}
137+
if !strings.Contains(err.Error(), "operator-managed system Secret") {
138+
t.Fatalf("error = %q, want operator-managed system Secret", err.Error())
139+
}
140+
})
141+
44142
t.Run("secret-backed transit files require credentials Secret", func(t *testing.T) {
45143
cluster := newMinimalCluster("transit-missing-secret-ref", "default")
46144
cluster.Spec.Unseal = &openbaov1alpha1.UnsealConfig{
@@ -352,6 +450,30 @@ func TestValidateAWSKMSUnsealPrerequisites(t *testing.T) {
352450
}
353451
})
354452

453+
t.Run("credentials Secret cannot reference operator managed system Secrets", func(t *testing.T) {
454+
cluster := newMinimalCluster("awskms-system-secret", "default")
455+
cluster.Spec.Unseal = &openbaov1alpha1.UnsealConfig{
456+
Type: "awskms",
457+
CredentialsSecretRef: &corev1.LocalObjectReference{Name: "awskms-system-secret-root-token"},
458+
AWSKMS: &openbaov1alpha1.AWSKMSSealConfig{
459+
Region: "eu-central-1",
460+
KMSKeyID: "alias/openbao",
461+
},
462+
}
463+
464+
mgr := NewManager(newTestClient(t), testScheme, "operator-system")
465+
err := mgr.validateUnsealPrerequisites(context.Background(), cluster)
466+
if err == nil {
467+
t.Fatal("validateUnsealPrerequisites() error = nil, want error")
468+
}
469+
if !errors.Is(err, operatorerrors.ErrPermanentPrerequisitesMissing) {
470+
t.Fatalf("expected permanent prerequisites missing error, got %v", err)
471+
}
472+
if !strings.Contains(err.Error(), "operator-managed system Secret") {
473+
t.Fatalf("error = %q, want operator-managed system Secret", err.Error())
474+
}
475+
})
476+
355477
t.Run("secret-backed aws kms requires access key and secret key", func(t *testing.T) {
356478
cluster := newMinimalCluster("awskms-secret", "default")
357479
cluster.Spec.Unseal = &openbaov1alpha1.UnsealConfig{

0 commit comments

Comments
 (0)