Skip to content

Commit 01ae81d

Browse files
dc-tecdc-tec
authored
fix(security): harden backup and restore helper inputs (#474)
1 parent 2e4e6c1 commit 01ae81d

18 files changed

Lines changed: 914 additions & 13 deletions

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,12 +347,32 @@ spec:
347347
object.spec.profile != "Hardened" ||
348348
object.spec.replicas >= 3
349349
message: "Hardened profile requires at least 3 replicas for Raft quorum HA. Use Profile=Development for non-HA deployments."
350+
# Confused-deputy protection: custom backup helper images run with backup credentials.
351+
- expression: >-
352+
!has(object.spec.backup) ||
353+
!has(object.spec.backup.image) ||
354+
object.spec.backup.image == "" ||
355+
(request.operation == "UPDATE" &&
356+
oldObject != null &&
357+
has(oldObject.spec.backup) &&
358+
has(oldObject.spec.backup.image) &&
359+
oldObject.spec.backup.image == object.spec.backup.image) ||
360+
authorizer.group("openbao.org").resource("openbaoclusters").namespace(request.namespace).name(object.metadata.name).check("usehelperimages").allowed()
361+
message: "Users configuring custom backup helper images must be authorized to use helper image overrides on this OpenBaoCluster."
350362
# Confused-deputy protection: users configuring unseal Secret credentials must be able to read that Secret.
351363
- expression: >-
352364
!has(object.spec.unseal) ||
353365
!has(object.spec.unseal.credentialsSecretRef) ||
354366
authorizer.group("").resource("secrets").namespace(request.namespace).name(object.spec.unseal.credentialsSecretRef.name).check("get").allowed()
355367
message: "Users configuring unseal credentials must be authorized to get spec.unseal.credentialsSecretRef."
368+
# Confused-deputy protection: users configuring backup Secret credentials must be able to read those Secrets.
369+
- expression: >-
370+
!has(object.spec.backup) ||
371+
((!has(object.spec.backup.target.credentialsSecretRef) ||
372+
authorizer.group("").resource("secrets").namespace(request.namespace).name(object.spec.backup.target.credentialsSecretRef.name).check("get").allowed()) &&
373+
(!has(object.spec.backup.tokenSecretRef) ||
374+
authorizer.group("").resource("secrets").namespace(request.namespace).name(object.spec.backup.tokenSecretRef.name).check("get").allowed()))
375+
message: "Users configuring backup credentials must be authorized to get referenced backup Secrets."
356376
# Block references to system secrets in unseal/backup/upgrade
357377
- expression: >-
358378
(!has(object.spec.unseal) || !has(object.spec.unseal.credentialsSecretRef) ||

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,23 @@ spec:
5050
object.spec.source.target.endpoint.startsWith("s3://") ||
5151
object.spec.source.target.endpoint.matches("^http://[^/]+\\.svc(\\.|:|/|$).*")
5252
message: "Restore endpoint must use HTTPS or S3 scheme, unless it targets an in-cluster Service (*.svc)."
53+
# Confused-deputy protection: custom restore helper images run with restore credentials.
54+
- expression: >-
55+
!has(object.spec.image) ||
56+
object.spec.image == "" ||
57+
(request.operation == "UPDATE" &&
58+
oldObject != null &&
59+
has(oldObject.spec.image) &&
60+
oldObject.spec.image == object.spec.image) ||
61+
authorizer.group("openbao.org").resource("openbaoclusters").namespace(request.namespace).name(object.spec.cluster).check("usehelperimages").allowed()
62+
message: "Users configuring custom restore helper images must be authorized to use helper image overrides on the target OpenBaoCluster."
63+
# Confused-deputy protection: users configuring restore Secret credentials must be able to read those Secrets.
64+
- expression: >-
65+
(!has(object.spec.source.target.credentialsSecretRef) ||
66+
authorizer.group("").resource("secrets").namespace(request.namespace).name(object.spec.source.target.credentialsSecretRef.name).check("get").allowed()) &&
67+
(!has(object.spec.tokenSecretRef) ||
68+
authorizer.group("").resource("secrets").namespace(request.namespace).name(object.spec.tokenSecretRef.name).check("get").allowed())
69+
message: "Users configuring restore credentials must be authorized to get referenced restore Secrets."
5370
# Confused-deputy protection: Block references to system secrets (restore auth + object store credentials)
5471
- expression: >-
5572
(!has(object.spec.source.target.credentialsSecretRef) ||

‎charts/openbao-operator/templates/rbac/aggregated-clusterroles.yaml‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,22 @@ rules:
4747
- get
4848
---
4949

50+
apiVersion: rbac.authorization.k8s.io/v1
51+
kind: ClusterRole
52+
metadata:
53+
labels:
54+
{{- include "openbao-operator.labels" . | nindent 4 }}
55+
name: {{ include "openbao-operator.fullname" . }}-openbaocluster-helper-image
56+
rules:
57+
- apiGroups:
58+
- openbao.org
59+
resources:
60+
- openbaoclusters
61+
verbs:
62+
- get
63+
- usehelperimages
64+
---
65+
5066
apiVersion: rbac.authorization.k8s.io/v1
5167
kind: ClusterRole
5268
metadata:

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,12 +344,32 @@ spec:
344344
object.spec.profile != "Hardened" ||
345345
object.spec.replicas >= 3
346346
message: "Hardened profile requires at least 3 replicas for Raft quorum HA. Use Profile=Development for non-HA deployments."
347+
# Confused-deputy protection: custom backup helper images run with backup credentials.
348+
- expression: >-
349+
!has(object.spec.backup) ||
350+
!has(object.spec.backup.image) ||
351+
object.spec.backup.image == "" ||
352+
(request.operation == "UPDATE" &&
353+
oldObject != null &&
354+
has(oldObject.spec.backup) &&
355+
has(oldObject.spec.backup.image) &&
356+
oldObject.spec.backup.image == object.spec.backup.image) ||
357+
authorizer.group("openbao.org").resource("openbaoclusters").namespace(request.namespace).name(object.metadata.name).check("usehelperimages").allowed()
358+
message: "Users configuring custom backup helper images must be authorized to use helper image overrides on this OpenBaoCluster."
347359
# Confused-deputy protection: users configuring unseal Secret credentials must be able to read that Secret.
348360
- expression: >-
349361
!has(object.spec.unseal) ||
350362
!has(object.spec.unseal.credentialsSecretRef) ||
351363
authorizer.group("").resource("secrets").namespace(request.namespace).name(object.spec.unseal.credentialsSecretRef.name).check("get").allowed()
352364
message: "Users configuring unseal credentials must be authorized to get spec.unseal.credentialsSecretRef."
365+
# Confused-deputy protection: users configuring backup Secret credentials must be able to read those Secrets.
366+
- expression: >-
367+
!has(object.spec.backup) ||
368+
((!has(object.spec.backup.target.credentialsSecretRef) ||
369+
authorizer.group("").resource("secrets").namespace(request.namespace).name(object.spec.backup.target.credentialsSecretRef.name).check("get").allowed()) &&
370+
(!has(object.spec.backup.tokenSecretRef) ||
371+
authorizer.group("").resource("secrets").namespace(request.namespace).name(object.spec.backup.tokenSecretRef.name).check("get").allowed()))
372+
message: "Users configuring backup credentials must be authorized to get referenced backup Secrets."
353373
# Block references to system secrets in unseal/backup/upgrade
354374
- expression: >-
355375
(!has(object.spec.unseal) || !has(object.spec.unseal.credentialsSecretRef) ||

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,23 @@ spec:
4747
object.spec.source.target.endpoint.startsWith("s3://") ||
4848
object.spec.source.target.endpoint.matches("^http://[^/]+\\.svc(\\.|:|/|$).*")
4949
message: "Restore endpoint must use HTTPS or S3 scheme, unless it targets an in-cluster Service (*.svc)."
50+
# Confused-deputy protection: custom restore helper images run with restore credentials.
51+
- expression: >-
52+
!has(object.spec.image) ||
53+
object.spec.image == "" ||
54+
(request.operation == "UPDATE" &&
55+
oldObject != null &&
56+
has(oldObject.spec.image) &&
57+
oldObject.spec.image == object.spec.image) ||
58+
authorizer.group("openbao.org").resource("openbaoclusters").namespace(request.namespace).name(object.spec.cluster).check("usehelperimages").allowed()
59+
message: "Users configuring custom restore helper images must be authorized to use helper image overrides on the target OpenBaoCluster."
60+
# Confused-deputy protection: users configuring restore Secret credentials must be able to read those Secrets.
61+
- expression: >-
62+
(!has(object.spec.source.target.credentialsSecretRef) ||
63+
authorizer.group("").resource("secrets").namespace(request.namespace).name(object.spec.source.target.credentialsSecretRef.name).check("get").allowed()) &&
64+
(!has(object.spec.tokenSecretRef) ||
65+
authorizer.group("").resource("secrets").namespace(request.namespace).name(object.spec.tokenSecretRef.name).check("get").allowed())
66+
message: "Users configuring restore credentials must be authorized to get referenced restore Secrets."
5067
# Confused-deputy protection: Block references to system secrets (restore auth + object store credentials)
5168
- expression: >-
5269
(!has(object.spec.source.target.credentialsSecretRef) ||

‎config/rbac/kustomization.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,6 @@ resources:
3535
# if you do not want those helpers be installed with your Project.
3636
- openbaocluster_admin_role.yaml
3737
- openbaocluster_editor_role.yaml
38+
- openbaocluster_helper_image_role.yaml
3839
- openbaocluster_maintenance_role.yaml
3940
- openbaocluster_viewer_role.yaml

‎config/rbac/namespace_scoped_example.yaml‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,47 @@ subjects:
110110
name: team-a-break-glass # Change to actual group name
111111
apiGroup: rbac.authorization.k8s.io
112112
---
113+
# Example: Grant custom backup/restore helper image selection on one cluster
114+
#
115+
# This namespaced Role grants the custom `usehelperimages` verb only for a
116+
# single OpenBaoCluster object. Grant it only to operators trusted to choose
117+
# executor images, because those images run with backup/restore credentials.
118+
apiVersion: rbac.authorization.k8s.io/v1
119+
kind: Role
120+
metadata:
121+
name: openbaocluster-helper-image-team-a-example
122+
namespace: team-a-prod # Change to target namespace
123+
labels:
124+
app.kubernetes.io/name: openbao-operator
125+
app.kubernetes.io/managed-by: kustomize
126+
rules:
127+
- apiGroups:
128+
- openbao.org
129+
resources:
130+
- openbaoclusters
131+
resourceNames:
132+
- example-cluster # Change to target OpenBaoCluster name
133+
verbs:
134+
- get
135+
- usehelperimages
136+
---
137+
apiVersion: rbac.authorization.k8s.io/v1
138+
kind: RoleBinding
139+
metadata:
140+
name: openbaocluster-helper-image-team-a-example
141+
namespace: team-a-prod # Change to target namespace
142+
labels:
143+
app.kubernetes.io/name: openbao-operator
144+
app.kubernetes.io/managed-by: kustomize
145+
roleRef:
146+
apiGroup: rbac.authorization.k8s.io
147+
kind: Role
148+
name: openbaocluster-helper-image-team-a-example
149+
subjects:
150+
- kind: Group
151+
name: team-a-platform-operators # Change to actual group name
152+
apiGroup: rbac.authorization.k8s.io
153+
---
113154
# Example: Grant admin permissions to platform team (cluster-wide)
114155
#
115156
# This ClusterRoleBinding grants platform administrators full access

‎config/rbac/openbaocluster_helper_image_role.yaml‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
# This rule is not used by the project openbao-operator itself.
2+
# It is provided to allow cluster administrators to delegate custom
3+
# backup/restore helper image selection without granting full
4+
# OpenBaoCluster admin permissions.
5+
#
6+
# Multi-tenancy: This ClusterRole can be bound via RoleBinding for
7+
# namespace-scoped helper image permissions, or copied into a namespaced
8+
# Role with resourceNames for one explicitly trusted OpenBaoCluster.
9+
10+
apiVersion: rbac.authorization.k8s.io/v1
11+
kind: ClusterRole
12+
metadata:
13+
labels:
14+
app.kubernetes.io/name: openbao-operator
15+
app.kubernetes.io/managed-by: kustomize
16+
name: openbaocluster-helper-image-role
17+
rules:
18+
- apiGroups:
19+
- openbao.org
20+
resources:
21+
- openbaoclusters
22+
verbs:
23+
- get
24+
- usehelperimages

‎hack/helmchart/main.go‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -664,16 +664,24 @@ subjects:
664664

665665
// syncAggregatedRBAC syncs aggregated ClusterRoles.
666666
func syncAggregatedRBAC(opts options) error {
667-
parts := make([]string, 0, 4) // 3 cluster roles + 1 tenant role
668-
669-
// OpenBaoCluster admin/editor/viewer roles
670-
for _, suffix := range []string{"admin", "editor", "viewer"} {
671-
filename := fmt.Sprintf("openbaocluster_%s_role.yaml", suffix)
667+
parts := make([]string, 0, 5) // 4 cluster roles + 1 tenant role
668+
669+
// OpenBaoCluster admin/editor/viewer and helper-image delegation roles.
670+
for _, role := range []struct {
671+
filename string
672+
nameSuffix string
673+
}{
674+
{filename: "openbaocluster_admin_role.yaml", nameSuffix: "openbaocluster-admin"},
675+
{filename: "openbaocluster_editor_role.yaml", nameSuffix: "openbaocluster-editor"},
676+
{filename: "openbaocluster_helper_image_role.yaml", nameSuffix: "openbaocluster-helper-image"},
677+
{filename: "openbaocluster_viewer_role.yaml", nameSuffix: "openbaocluster-viewer"},
678+
} {
679+
filename := role.filename
672680
content, err := readFile(filepath.Join(opts.rbacInputDir, filename))
673681
if err != nil {
674682
return fmt.Errorf("read %s: %w", filename, err)
675683
}
676-
content = transformRBACToHelm(content, fmt.Sprintf("openbaocluster-%s", suffix), false)
684+
content = transformRBACToHelm(content, role.nameSuffix, false)
677685
parts = append(parts, content)
678686
}
679687

‎hack/helmchart/main_test.go‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package main
22

33
import (
44
"bytes"
5+
"os"
56
"os/exec"
67
"path/filepath"
78
"runtime"
@@ -66,6 +67,58 @@ rules:
6667
}
6768
}
6869

70+
func TestSyncAggregatedRBAC_IncludesHelperImageDelegationRole(t *testing.T) {
71+
inputDir := t.TempDir()
72+
outputDir := t.TempDir()
73+
74+
writeRole := func(filename, name string, verbs ...string) {
75+
t.Helper()
76+
77+
var builder strings.Builder
78+
builder.WriteString("apiVersion: rbac.authorization.k8s.io/v1\n")
79+
builder.WriteString("kind: ClusterRole\n")
80+
builder.WriteString("metadata:\n")
81+
builder.WriteString(" name: " + name + "\n")
82+
builder.WriteString("rules:\n")
83+
builder.WriteString(" - apiGroups:\n")
84+
builder.WriteString(" - openbao.org\n")
85+
builder.WriteString(" resources:\n")
86+
builder.WriteString(" - openbaoclusters\n")
87+
builder.WriteString(" verbs:\n")
88+
for _, verb := range verbs {
89+
builder.WriteString(" - " + verb + "\n")
90+
}
91+
92+
if err := os.WriteFile(filepath.Join(inputDir, filename), []byte(builder.String()), 0o600); err != nil {
93+
t.Fatalf("write %s: %v", filename, err)
94+
}
95+
}
96+
97+
writeRole("openbaocluster_admin_role.yaml", "openbaocluster-admin-role", "*")
98+
writeRole("openbaocluster_editor_role.yaml", "openbaocluster-editor-role", "create", "update")
99+
writeRole("openbaocluster_helper_image_role.yaml", "openbaocluster-helper-image-role", "get", "usehelperimages")
100+
writeRole("openbaocluster_viewer_role.yaml", "openbaocluster-viewer-role", "get", "list")
101+
writeRole("openbaotenant_editor_role.yaml", "openbaotenant-editor-role", "create", "update")
102+
103+
if err := syncAggregatedRBAC(options{rbacInputDir: inputDir, rbacOutputDir: outputDir}); err != nil {
104+
t.Fatalf("syncAggregatedRBAC() failed: %v", err)
105+
}
106+
107+
got, err := os.ReadFile(filepath.Join(outputDir, "aggregated-clusterroles.yaml"))
108+
if err != nil {
109+
t.Fatalf("read generated aggregated roles: %v", err)
110+
}
111+
output := string(got)
112+
for _, want := range []string{
113+
`{{ include "openbao-operator.fullname" . }}-openbaocluster-helper-image`,
114+
"usehelperimages",
115+
} {
116+
if !strings.Contains(output, want) {
117+
t.Fatalf("generated aggregated RBAC missing %q:\n%s", want, output)
118+
}
119+
}
120+
}
121+
69122
func TestAddNamespacePodSecurityLabelRBACMode_ConditionsNamespaceMutationVerbs(t *testing.T) {
70123
input := `apiVersion: rbac.authorization.k8s.io/v1
71124
kind: ClusterRole

0 commit comments

Comments
 (0)