mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 22:13:18 +09:00
fix(backups): never skip TLS verification in place of a vanished CA
The Redis probe reads the CA Secret when it runs, not when the backup Job did. If the pod mounted a CA Secret that has since been deleted and the strategy also opts into S3_INSECURE_SKIP_VERIFY, the probe skipped verification against an endpoint the Job may have verified. Refuse to probe in that case. Without skip-verify a missing CA Secret still falls back to the system roots, as the script does. The endpoint comment claimed a bare host; it can carry a path, which the shared client now refuses. Assisted-by: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com>
This commit is contained in:
1 parent
1d80a543dc
commit
db545c6e8e
2 files changed
+24
-6
No files matched your search
@@ -52,8 +52,9 @@ func (r *BackupJobReconciler) redisArtifact(ctx context.Context, namespace, obje
|
||||
}
|
||||
|
||||
// projectedS3Connection reads the backup credentials Secret the controller
|
||||
// projects into the application namespace. The projector always stores the
|
||||
// endpoint as a bare host, which the strategy scripts reach over https.
|
||||
// projects into the application namespace. The projector stores the endpoint
|
||||
// without a scheme, and the strategy scripts reach it over https. It can
|
||||
// still carry a path, which minio-go refuses.
|
||||
func (r *BackupJobReconciler) projectedS3Connection(ctx context.Context, namespace string) (string, s3Connection, error) {
|
||||
var conn s3Connection
|
||||
name := r.CredentialsConfig.TargetSecretName
|
||||
@@ -85,12 +86,15 @@ const (
|
||||
// endpoint: it trusts ca.crt, then tls.crt, from the Secret mounted as the
|
||||
// s3-ca volume, and only when neither exists does it honour
|
||||
// S3_INSECURE_SKIP_VERIFY=true. Both are read from the pod spec the backup
|
||||
// Job ran with. A missing CA Secret is not an error, because the script
|
||||
// mounts it optionally.
|
||||
// Job ran with, but the Secret is read now, so a CA Secret the pod mounts
|
||||
// and that is gone by now never lets the probe skip verification: the Job
|
||||
// may have verified against it. Without skip-verify a missing CA Secret
|
||||
// leaves the system roots, as in the script, which mounts it optionally.
|
||||
func (r *BackupJobReconciler) redisS3Trust(ctx context.Context, namespace string, podSpec *corev1.PodSpec) ([]byte, bool, error) {
|
||||
if podSpec == nil {
|
||||
return nil, false, nil
|
||||
}
|
||||
caMissing := false
|
||||
for _, v := range podSpec.Volumes {
|
||||
if v.Name != redisS3CAVolume || v.Secret == nil {
|
||||
continue
|
||||
@@ -98,6 +102,7 @@ func (r *BackupJobReconciler) redisS3Trust(ctx context.Context, namespace string
|
||||
secret := &corev1.Secret{}
|
||||
err := r.Get(ctx, types.NamespacedName{Namespace: namespace, Name: v.Secret.SecretName}, secret)
|
||||
if apierrors.IsNotFound(err) {
|
||||
caMissing = true
|
||||
break
|
||||
}
|
||||
if err != nil {
|
||||
@@ -112,6 +117,9 @@ func (r *BackupJobReconciler) redisS3Trust(ctx context.Context, namespace string
|
||||
for _, c := range podSpec.Containers {
|
||||
for _, e := range c.Env {
|
||||
if e.Name == redisS3InsecureSkipEnv && e.Value == "true" {
|
||||
if caMissing {
|
||||
return nil, false, fmt.Errorf("CA Secret of the %s volume is gone; not skipping TLS verification in its place", redisS3CAVolume)
|
||||
}
|
||||
return nil, true, nil
|
||||
}
|
||||
}
|
||||
|
||||
@@ -188,8 +188,11 @@ func TestRedisS3Trust(t *testing.T) {
|
||||
objs: []client.Object{caSecret(map[string]string{"bundle.pem": "MAPPED", "ca.crt": "UNMAPPED"})},
|
||||
wantCA: "MAPPED",
|
||||
},
|
||||
"missing CA Secret falls through to skip-verify": {
|
||||
spec: redisPodSpec("s3-endpoint-ca", "true"),
|
||||
"missing CA Secret falls back to the system roots": {
|
||||
spec: redisPodSpec("s3-endpoint-ca", ""),
|
||||
},
|
||||
"skip-verify without a CA volume": {
|
||||
spec: redisPodSpec("", "true"),
|
||||
wantSkip: true,
|
||||
},
|
||||
"skip-verify only when it is exactly true": {
|
||||
@@ -197,6 +200,13 @@ func TestRedisS3Trust(t *testing.T) {
|
||||
},
|
||||
"no pod spec": {},
|
||||
}
|
||||
t.Run("a mounted CA Secret that is gone never turns into skip-verify", func(t *testing.T) {
|
||||
r := &BackupJobReconciler{Client: newMariaDBStrategyTestClient(t)}
|
||||
ca, skip, err := r.redisS3Trust(context.Background(), "tenant", redisPodSpec("s3-endpoint-ca", "true"))
|
||||
if err == nil || skip || ca != nil {
|
||||
t.Errorf("trust = (%q, %v, %v), want an error and no skip-verify", ca, skip, err)
|
||||
}
|
||||
})
|
||||
for name, tc := range cases {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
c := newMariaDBStrategyTestClient(t, tc.objs...)
|
||||
|
||||
Reference in new issue
Block a user