Skip to content

Incomplete external TLS Secret pairs are accepted for secure clusters #1160

Description

@autokctrl-team

Environment

  • Kubernetes: local kind cluster
  • Operator image: cockroachdb/cockroach-operator:v2.18.3
  • CockroachDB image: cockroachdb/cockroach:v25.2.12
  • Test topology: 3-node secure CrdbCluster

What Happened

The operator lets users provide external TLS Secrets for secure CockroachDB clusters, but it treats the node TLS Secret and the client/root TLS Secret as independent optional fields. This is not safe, because either the user provides both or the operator generates both.

If the user sets tlsEnabled=true and provides only nodeTLSSecret, the operator accepts the CR, skips generated certificate creation because NodeTLSSecret is non-empty, and still writes a secure StatefulSet that mounts both a node Secret and a client/root Secret. The node Secret exists, but the client/root Secret falls back to the generated name and does not exist:

node secret        = external-node-tls
client/root secret = <cluster>-root
client/root exists = false

The pod then fails before cockroach start can run:

MountVolume.SetUp failed for volume "certs" : secret "<cluster>-root" not found

On the other hand, if the user sets tlsEnabled=true and provides only clientTLSSecret, the operator accepts the CR, generates node TLS material because NodeTLSSecret is empty, and then writes a secure StatefulSet that combines generated node certs with the external client/root Secret:

node secret source        = generated by operator
client/root secret source = external user-provided Secret

That is also an incomplete external TLS configuration. The operator should reject it before writing the secure workload, rather than silently mixing generated and external TLS.

Expected Behavior

For tlsEnabled=true, external TLS Secrets should be all-or-nothing:

nodeTLSSecret empty, clientTLSSecret empty       -> OK, operator generates both
nodeTLSSecret set,   clientTLSSecret set         -> OK, user provides both
nodeTLSSecret set,   clientTLSSecret empty       -> reject
nodeTLSSecret empty, clientTLSSecret set         -> reject

The best fix location is the admission webhook, because the CR itself is invalid before reconcile reaches certificate generation or StatefulSet construction. The check should run in both ValidateCreate and ValidateUpdate, because both create and update can produce the bad configuration.

Where The Source Code Goes Wrong

The validating webhook already checks ingress, CockroachDB version, and volume mode, but it does not validate the TLS Secret pair. ValidateCreate starts at apis/v1alpha1/webhook.go and returns success without checking whether NodeTLSSecret and ClientTLSSecret are set together: apis/v1alpha1/webhook.go. ValidateUpdate has the same issue: apis/v1alpha1/webhook.go.

After admission accepts the incomplete CR, the certificate actor and StatefulSet builder make the two bad variants possible. First, generated certificate creation is skipped whenever NodeTLSSecret is set:

if !cluster.Spec().TLSEnabled || cluster.Spec().NodeTLSSecret != "" {
	log.V(DEBUGLEVEL).Info("Skipping TLS cert generation", "enabled", cluster.Spec().TLSEnabled, "secret", cluster.Spec().NodeTLSSecret)
	return nil
}

Source: pkg/actor/generate_cert.go.

As a result, with only external nodeTLSSecret, generated cert creation is skipped, so the fallback generated client/root Secret is never created.

Second, the secure StatefulSet always projects both a node Secret and a client/root Secret into the certs volume:

Secret: &corev1.SecretProjection{
	LocalObjectReference: corev1.LocalObjectReference{
		Name: b.nodeTLSSecretName(),
	},
	...
}
...
Secret: &corev1.SecretProjection{
	LocalObjectReference: corev1.LocalObjectReference{
		Name: b.clientTLSSecretName(),
	},
	...
}

Source: pkg/resource/statefulset.go.

The builder chooses the two Secret names independently:

func (b StatefulSetBuilder) nodeTLSSecretName() string {
	if b.Spec().NodeTLSSecret == "" {
		return b.Cluster.NodeTLSSecretName()
	}
	return b.Spec().NodeTLSSecret
}

func (b StatefulSetBuilder) clientTLSSecretName() string {
	if b.Spec().ClientTLSSecret == "" {
		return b.Cluster.ClientTLSSecretName()
	}
	return b.Spec().ClientTLSSecret
}

Source: pkg/resource/statefulset.go.

The node side and client/root side should be selected as one compatible bundle, not chosen separately. The mixed generated/external TLS material can create a secure StatefulSet whose certificates do not trust each other, so CockroachDB or operator-to-CockroachDB communication can fail later with TLS errors.

Fix

The admission webhook should reject the invalid CR before the operator writes a broken secure StatefulSet. A minimal helper would be:

func (r *CrdbCluster) ValidateTLSSecrets() error {
	if !r.Spec.TLSEnabled {
		return nil
	}

	nodeSet := r.Spec.NodeTLSSecret != ""
	clientSet := r.Spec.ClientTLSSecret != ""
	if nodeSet != clientSet {
		return fmt.Errorf("nodeTLSSecret and clientTLSSecret must be set together when tlsEnabled is true")
	}
	return nil
}

Then ValidateCreate and ValidateUpdate should both append this error before returning success.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions