Skip to content

Commit 032c9ba

Browse files
DocEmmetBrownDocEmmetBrown
authored
Merge commit from fork
fix(ClickhouseUser/ServiceUser)!: remove cross-namespace secret exfiltration
2 parents 3af3eb9 + 4e41e38 commit 032c9ba

16 files changed

Lines changed: 47 additions & 393 deletions

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
- Add `ServiceUser` field `accessControl`, type `object`: AccessControl configures service-specific access control rules for the user.
66
When this block is present, the operator manages the full access-control scope it contains
77
- Add `OpenSearchACLConfig` to manage OpenSearch ACL
8+
- **BREAKING**: Removed `ClickhouseUser`/`ServiceUser` field `connInfoSecretSource.namespace`, type `string`: cross-namespace
9+
secret references are no longer supported. The source secret must be in the same namespace as the resource.
10+
This fixes a potential confused deputy vulnerability where the operator could be exploited to exfiltrate secrets from other namespaces.
811

912
## v0.36.0 - 2026-03-05
1013

‎api/v1alpha1/common.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,9 @@ type ConnInfoSecretTarget struct {
4646
type ConnInfoSecretSource struct {
4747
// +kubebuilder:validation:Required
4848
// +kubebuilder:validation:MinLength=1
49-
// Name of the secret resource to read connection parameters from
49+
// Name of the secret resource to read connection parameters from.
50+
// The secret must be in the same namespace as the resource.
5051
Name string `json:"name"`
51-
// Namespace of the source secret. If not specified, defaults to the same namespace as the resource
52-
Namespace string `json:"namespace,omitempty"`
5352
// +kubebuilder:validation:Required
5453
// +kubebuilder:validation:MinLength=1
5554
// Key in the secret containing the password to use for authentication

‎charts/aiven-operator-crds/templates/aiven.io_clickhouseusers.yaml‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -77,16 +77,11 @@ spec:
7777
when the secret data is updated.
7878
properties:
7979
name:
80-
description:
81-
Name of the secret resource to read connection parameters
82-
from
80+
description: |-
81+
Name of the secret resource to read connection parameters from.
82+
The secret must be in the same namespace as the resource.
8383
minLength: 1
8484
type: string
85-
namespace:
86-
description:
87-
Namespace of the source secret. If not specified,
88-
defaults to the same namespace as the resource
89-
type: string
9085
passwordKey:
9186
description:
9287
Key in the secret containing the password to use

‎charts/aiven-operator-crds/templates/aiven.io_serviceusers.yaml‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -110,16 +110,11 @@ spec:
110110
when the secret data is updated.
111111
properties:
112112
name:
113-
description:
114-
Name of the secret resource to read connection parameters
115-
from
113+
description: |-
114+
Name of the secret resource to read connection parameters from.
115+
The secret must be in the same namespace as the resource.
116116
minLength: 1
117117
type: string
118-
namespace:
119-
description:
120-
Namespace of the source secret. If not specified,
121-
defaults to the same namespace as the resource
122-
type: string
123118
passwordKey:
124119
description:
125120
Key in the secret containing the password to use

‎config/crd/bases/aiven.io_clickhouseusers.yaml‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -77,16 +77,11 @@ spec:
7777
when the secret data is updated.
7878
properties:
7979
name:
80-
description:
81-
Name of the secret resource to read connection parameters
82-
from
80+
description: |-
81+
Name of the secret resource to read connection parameters from.
82+
The secret must be in the same namespace as the resource.
8383
minLength: 1
8484
type: string
85-
namespace:
86-
description:
87-
Namespace of the source secret. If not specified,
88-
defaults to the same namespace as the resource
89-
type: string
9085
passwordKey:
9186
description:
9287
Key in the secret containing the password to use

‎config/crd/bases/aiven.io_serviceusers.yaml‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -110,16 +110,11 @@ spec:
110110
when the secret data is updated.
111111
properties:
112112
name:
113-
description:
114-
Name of the secret resource to read connection parameters
115-
from
113+
description: |-
114+
Name of the secret resource to read connection parameters from.
115+
The secret must be in the same namespace as the resource.
116116
minLength: 1
117117
type: string
118-
namespace:
119-
description:
120-
Namespace of the source secret. If not specified,
121-
defaults to the same namespace as the resource
122-
type: string
123118
passwordKey:
124119
description:
125120
Key in the secret containing the password to use

‎controllers/secret_password_manager.go‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,31 +25,28 @@ func GetPasswordFromSecret(ctx context.Context, k8sClient client.Client, resourc
2525
return "", nil
2626
}
2727

28-
sourceNamespace := secretSource.Namespace
29-
if sourceNamespace == "" {
30-
sourceNamespace = resource.GetNamespace()
31-
}
28+
ns := resource.GetNamespace()
3229

3330
sourceSecret := &corev1.Secret{}
3431
err := k8sClient.Get(ctx, types.NamespacedName{
3532
Name: secretSource.Name,
36-
Namespace: sourceNamespace,
33+
Namespace: ns,
3734
}, sourceSecret)
3835
if err != nil {
39-
return "", fmt.Errorf("failed to read connInfoSecretSource %s/%s: %w", sourceNamespace, secretSource.Name, err)
36+
return "", fmt.Errorf("failed to read connInfoSecretSource %s/%s: %w", ns, secretSource.Name, err)
4037
}
4138

4239
passwordBytes, exists := sourceSecret.Data[secretSource.PasswordKey]
4340
if !exists {
44-
return "", fmt.Errorf("password not found in source secret %s/%s (expected %s key)", sourceNamespace, secretSource.Name, secretSource.PasswordKey)
41+
return "", fmt.Errorf("password not found in source secret %s/%s (expected %s key)", ns, secretSource.Name, secretSource.PasswordKey)
4542
}
4643

4744
newPassword := string(passwordBytes)
4845

4946
// validate password length according to API requirements
5047
if len(newPassword) < 8 || len(newPassword) > 256 {
5148
return "", fmt.Errorf("password length must be between 8 and 256 characters, got %d characters from source secret %s/%s (key: %s)",
52-
len(newPassword), sourceNamespace, secretSource.Name, secretSource.PasswordKey)
49+
len(newPassword), ns, secretSource.Name, secretSource.PasswordKey)
5350
}
5451

5552
return newPassword, nil

‎controllers/secret_password_manager_test.go‎

Lines changed: 21 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -53,26 +53,6 @@ func TestPasswordManager_GetPasswordFromSecret(t *testing.T) {
5353
expectedResult: "ValidPassword123!",
5454
expectError: false,
5555
},
56-
{
57-
name: "Valid password from secret in different namespace",
58-
secretSource: &v1alpha1.ConnInfoSecretSource{
59-
Name: "test-secret",
60-
Namespace: "other-ns",
61-
PasswordKey: "PASSWORD",
62-
},
63-
resourceNS: "default",
64-
secret: &corev1.Secret{
65-
ObjectMeta: metav1.ObjectMeta{
66-
Name: "test-secret",
67-
Namespace: "other-ns",
68-
},
69-
Data: map[string][]byte{
70-
"PASSWORD": []byte("CrossNSPassword123!"),
71-
},
72-
},
73-
expectedResult: "CrossNSPassword123!",
74-
expectError: false,
75-
},
7656
{
7757
name: "Secret not found",
7858
secretSource: &v1alpha1.ConnInfoSecretSource{
@@ -256,72 +236,33 @@ func TestPasswordManager_NamespaceResolution(t *testing.T) {
256236
require.NoError(t, corev1.AddToScheme(scheme))
257237
require.NoError(t, v1alpha1.AddToScheme(scheme))
258238

259-
tests := []struct {
260-
name string
261-
resourceNS string
262-
sourceNS string
263-
expectedNS string
264-
secretExists bool
265-
}{
266-
{
267-
name: "Uses resource namespace when source namespace is empty",
268-
resourceNS: "resource-ns",
269-
sourceNS: "",
270-
expectedNS: "resource-ns",
271-
secretExists: true,
239+
// Secret must always be in the same namespace as the resource
240+
secret := &corev1.Secret{
241+
ObjectMeta: metav1.ObjectMeta{
242+
Name: "test-secret",
243+
Namespace: "resource-ns",
272244
},
273-
{
274-
name: "Uses source namespace when specified",
275-
resourceNS: "resource-ns",
276-
sourceNS: "source-ns",
277-
expectedNS: "source-ns",
278-
secretExists: true,
245+
Data: map[string][]byte{
246+
"PASSWORD": []byte("ValidPassword123!"),
279247
},
280248
}
281249

282-
for _, tt := range tests {
283-
t.Run(tt.name, func(t *testing.T) {
284-
var objects []client.Object
285-
if tt.secretExists {
286-
secret := &corev1.Secret{
287-
ObjectMeta: metav1.ObjectMeta{
288-
Name: "test-secret",
289-
Namespace: tt.expectedNS,
290-
},
291-
Data: map[string][]byte{
292-
"PASSWORD": []byte("ValidPassword123!"),
293-
},
294-
}
295-
objects = append(objects, secret)
296-
}
297-
298-
secretSource := &v1alpha1.ConnInfoSecretSource{
250+
user := &v1alpha1.ClickhouseUser{
251+
ObjectMeta: metav1.ObjectMeta{
252+
Name: "test-user",
253+
Namespace: "resource-ns",
254+
},
255+
Spec: v1alpha1.ClickhouseUserSpec{
256+
ConnInfoSecretSource: &v1alpha1.ConnInfoSecretSource{
299257
Name: "test-secret",
300258
PasswordKey: "PASSWORD",
301-
}
302-
if tt.sourceNS != "" {
303-
secretSource.Namespace = tt.sourceNS
304-
}
305-
306-
user := &v1alpha1.ClickhouseUser{
307-
ObjectMeta: metav1.ObjectMeta{
308-
Name: "test-user",
309-
Namespace: tt.resourceNS,
310-
},
311-
Spec: v1alpha1.ClickhouseUserSpec{
312-
ConnInfoSecretSource: secretSource,
313-
},
314-
}
259+
},
260+
},
261+
}
315262

316-
k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).Build()
317-
result, err := GetPasswordFromSecret(context.Background(), k8sClient, user)
263+
k8sClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(secret).Build()
264+
result, err := GetPasswordFromSecret(context.Background(), k8sClient, user)
318265

319-
if tt.secretExists {
320-
require.NoError(t, err)
321-
assert.Equal(t, "ValidPassword123!", result)
322-
} else {
323-
require.Error(t, err)
324-
}
325-
})
326-
}
266+
require.NoError(t, err)
267+
assert.Equal(t, "ValidPassword123!", result)
327268
}

‎controllers/secret_watch_controller.go‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,7 @@ func (c *SecretWatchController) getResourcesWithSecretSource() []SecretSourceRes
112112
func connInfoSecretRefIndexFunc(o client.Object) []string {
113113
if resource, ok := o.(SecretSourceResource); ok {
114114
if secretSource := resource.GetConnInfoSecretSource(); secretSource != nil {
115-
sourceNamespace := secretSource.Namespace
116-
if sourceNamespace == "" {
117-
sourceNamespace = resource.GetNamespace()
118-
}
119-
120-
return []string{fmt.Sprintf("%s/%s", sourceNamespace, secretSource.Name)}
115+
return []string{fmt.Sprintf("%s/%s", resource.GetNamespace(), secretSource.Name)}
121116
}
122117
}
123118

‎controllers/secret_watch_controller_test.go‎

Lines changed: 1 addition & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -132,23 +132,6 @@ func TestConnInfoSecretRefIndexFunc(t *testing.T) {
132132
},
133133
expected: []string{"default/my-secret"},
134134
},
135-
{
136-
name: "ServiceUser with secretSource in different namespace",
137-
resource: &v1alpha1.ServiceUser{
138-
ObjectMeta: metav1.ObjectMeta{
139-
Name: "test-user",
140-
Namespace: "default",
141-
},
142-
Spec: v1alpha1.ServiceUserSpec{
143-
ConnInfoSecretSource: &v1alpha1.ConnInfoSecretSource{
144-
Name: "my-secret",
145-
Namespace: "other-namespace",
146-
PasswordKey: "password",
147-
},
148-
},
149-
},
150-
expected: []string{"other-namespace/my-secret"},
151-
},
152135
{
153136
name: "ServiceUser without secretSource",
154137
resource: &v1alpha1.ServiceUser{
@@ -178,23 +161,6 @@ func TestConnInfoSecretRefIndexFunc(t *testing.T) {
178161
},
179162
expected: []string{"default/ch-secret"},
180163
},
181-
{
182-
name: "ClickhouseUser with cross-namespace secretSource",
183-
resource: &v1alpha1.ClickhouseUser{
184-
ObjectMeta: metav1.ObjectMeta{
185-
Name: "test-ch-user",
186-
Namespace: "app-namespace",
187-
},
188-
Spec: v1alpha1.ClickhouseUserSpec{
189-
ConnInfoSecretSource: &v1alpha1.ConnInfoSecretSource{
190-
Name: "shared-secret",
191-
Namespace: "secrets-namespace",
192-
PasswordKey: "password",
193-
},
194-
},
195-
},
196-
expected: []string{"secrets-namespace/shared-secret"},
197-
},
198164
{
199165
name: "Non-SecretSourceResource",
200166
resource: &corev1.Secret{
@@ -246,29 +212,6 @@ func TestSecretWatchController_resourceMatchesSecret(t *testing.T) {
246212
},
247213
expected: true,
248214
},
249-
{
250-
name: "ServiceUser matches secret in different namespace",
251-
resource: &v1alpha1.ServiceUser{
252-
ObjectMeta: metav1.ObjectMeta{
253-
Name: "test-user",
254-
Namespace: "app-ns",
255-
},
256-
Spec: v1alpha1.ServiceUserSpec{
257-
ConnInfoSecretSource: &v1alpha1.ConnInfoSecretSource{
258-
Name: "my-secret",
259-
Namespace: "secret-ns",
260-
PasswordKey: "password",
261-
},
262-
},
263-
},
264-
secret: &corev1.Secret{
265-
ObjectMeta: metav1.ObjectMeta{
266-
Name: "my-secret",
267-
Namespace: "secret-ns",
268-
},
269-
},
270-
expected: true,
271-
},
272215
{
273216
name: "ServiceUser does not match different secret name",
274217
resource: &v1alpha1.ServiceUser{
@@ -342,12 +285,7 @@ func TestSecretWatchController_resourceMatchesSecret(t *testing.T) {
342285
return
343286
}
344287

345-
sourceNamespace := secretSource.Namespace
346-
if sourceNamespace == "" {
347-
sourceNamespace = tt.resource.GetNamespace()
348-
}
349-
350-
matches := secretSource.Name == tt.secret.Name && sourceNamespace == tt.secret.Namespace
288+
matches := secretSource.Name == tt.secret.Name && tt.resource.GetNamespace() == tt.secret.Namespace
351289
assert.Equal(t, tt.expected, matches)
352290
})
353291
}

0 commit comments

Comments
 (0)