From d106e87a753dfa7024f12f2f0324851a6d03b343 Mon Sep 17 00:00:00 2001 From: Rayhan Hossain Date: Wed, 22 Apr 2026 16:25:10 -0700 Subject: [PATCH] fix(security): remove Disabled TLS gateway mode to prevent plaintext traffic Remove `spec.tls.gateway.mode: Disabled` option which served plaintext Mongo wire protocol, contradicting docs that stated TLS was always active. - Remove Disabled from GatewayTLS.Mode enum validation - Default empty/unset mode to SelfSigned for automatic TLS - Update certificate controller to treat empty mode as SelfSigned - Add unit tests for empty mode defaulting and Disabled rejection - Update documentation to remove Disabled mode references - Regenerate CRDs with updated enum BREAKING CHANGE: Users with `mode: Disabled` must remove this setting or set `mode: SelfSigned`. The gateway will use cert-manager generated self-signed certificates automatically. Fixes #356 Signed-off-by: Rayhan Hossain --- CHANGELOG.md | 1 + .../preview/api-reference.md | 2 +- .../preview/configuration/tls.md | 39 +------------- .../connecting-to-documentdb.md | 1 - .../crds/documentdb.io_dbs.yaml | 6 ++- operator/src/api/preview/documentdb_types.go | 6 ++- .../src/api/preview/zz_generated.deepcopy.go | 24 ++++----- .../config/crd/bases/documentdb.io_dbs.yaml | 6 ++- operator/src/internal/cnpg/cnpg_sync.go | 4 +- .../controller/certificate_controller.go | 17 +++--- .../controller/certificate_controller_test.go | 53 +++++++++++++++++++ .../controller/documentdb_controller.go | 1 - operator/src/internal/utils/constants.go | 4 +- operator/src/internal/utils/pv_recovery.go | 6 +-- 14 files changed, 94 insertions(+), 76 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b52b8f9f..35f726da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ ### Breaking Changes - **Validating webhook added**: A new `ValidatingWebhookConfiguration` enforces that `spec.schemaVersion` never exceeds the binary version and blocks `spec.documentDBVersion` rollbacks below the committed schema version. This requires [cert-manager](https://cert-manager.io/) to be installed in the cluster (it is already a prerequisite for the sidecar injector). Existing clusters upgrading to this release will have the webhook activated automatically via `helm upgrade`. +- **Removed `Disabled` TLS gateway mode**: The `spec.tls.gateway.mode: Disabled` option has been removed to eliminate the security risk of plaintext Mongo wire protocol traffic. Previously, `Disabled` mode served connections in plaintext, contradicting documentation that stated TLS was always active. Empty or unset mode now defaults to `SelfSigned`. Users with `mode: Disabled` should remove this setting or explicitly set `mode: SelfSigned` — the gateway will automatically use a cert-manager generated self-signed certificate. See [issue #356](https://github.com/documentdb/documentdb-kubernetes-operator/issues/356) for details. ## [0.2.0] - 2026-03-25 diff --git a/docs/operator-public-documentation/preview/api-reference.md b/docs/operator-public-documentation/preview/api-reference.md index 73f5144e..d91cf3f0 100644 --- a/docs/operator-public-documentation/preview/api-reference.md +++ b/docs/operator-public-documentation/preview/api-reference.md @@ -201,7 +201,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `mode` _string_ | Mode selects the TLS management strategy. | | Enum: [Disabled SelfSigned CertManager Provided]
| +| `mode` _string_ | Mode selects the TLS management strategy. Defaults to SelfSigned if not specified. | SelfSigned | Enum: [SelfSigned CertManager Provided]
| | `certManager` _[CertManagerTLS](#certmanagertls)_ | CertManager config when Mode=CertManager. | | | | `provided` _[ProvidedTLS](#providedtls)_ | Provided secret reference when Mode=Provided. | | | diff --git a/docs/operator-public-documentation/preview/configuration/tls.md b/docs/operator-public-documentation/preview/configuration/tls.md index 258df049..5320d4c2 100644 --- a/docs/operator-public-documentation/preview/configuration/tls.md +++ b/docs/operator-public-documentation/preview/configuration/tls.md @@ -1,6 +1,6 @@ --- title: TLS Configuration -description: Configure TLS certificate management for DocumentDB gateway connections — Disabled, SelfSigned, CertManager, and Provided modes, private CA guidance, certificate rotation, and troubleshooting. +description: Configure TLS certificate management for DocumentDB gateway connections — SelfSigned, CertManager, and Provided modes, private CA guidance, certificate rotation, and troubleshooting. tags: - configuration - tls @@ -23,7 +23,7 @@ metadata: spec: tls: gateway: - mode: SelfSigned # Disabled | SelfSigned | CertManager | Provided + mode: SelfSigned # SelfSigned | CertManager | Provided ``` For the full field reference, see [TLSConfiguration](../api-reference.md#tlsconfiguration) in the API Reference. @@ -32,41 +32,6 @@ For the full field reference, see [TLSConfiguration](../api-reference.md#tlsconf Select your TLS mode below. Each tab shows prerequisites, the complete YAML configuration, and connection instructions. -=== "Disabled" - - **Best for:** Development and testing only - - !!! danger "Not recommended for production" - - **Prerequisites:** None - - Disabled mode means the operator does not manage TLS certificates. However, the gateway still encrypts all connections using an internally generated self-signed certificate. Clients must connect with `tls=true&tlsAllowInvalidCertificates=true`. - - ```yaml title="documentdb-tls-disabled.yaml" - apiVersion: documentdb.io/preview - kind: DocumentDB - metadata: - name: my-documentdb - namespace: default - spec: - nodeCount: 1 - instancesPerNode: 1 - resource: - storage: - pvcSize: 10Gi - exposeViaService: - serviceType: ClusterIP - tls: - gateway: - mode: Disabled - ``` - - **Connect with mongosh:** - - ```bash - mongosh "mongodb://:@:10260/?directConnection=true&authMechanism=SCRAM-SHA-256&tls=true&tlsAllowInvalidCertificates=true" - ``` - === "SelfSigned" **Best for:** Development, testing, and environments without external PKI (Public Key Infrastructure) diff --git a/docs/operator-public-documentation/preview/getting-started/connecting-to-documentdb.md b/docs/operator-public-documentation/preview/getting-started/connecting-to-documentdb.md index c6ccdc62..dc8a45fc 100644 --- a/docs/operator-public-documentation/preview/getting-started/connecting-to-documentdb.md +++ b/docs/operator-public-documentation/preview/getting-started/connecting-to-documentdb.md @@ -427,7 +427,6 @@ DocumentDB Gateway serves TLS by default. The TLS mode is configurable via the ` | `SelfSigned` (default) | Operator generates a self-signed certificate | Use `tlsAllowInvalidCertificates=true` | | `CertManager` | cert-manager issues a trusted certificate | Validate against your CA bundle | | `Provided` | You supply your own certificate Secret | Validate against your CA | -| `Disabled` | TLS disabled on the gateway | Not recommended | ### Connecting with a trusted certificate diff --git a/operator/documentdb-helm-chart/crds/documentdb.io_dbs.yaml b/operator/documentdb-helm-chart/crds/documentdb.io_dbs.yaml index bc7001b5..407336a8 100644 --- a/operator/documentdb-helm-chart/crds/documentdb.io_dbs.yaml +++ b/operator/documentdb-helm-chart/crds/documentdb.io_dbs.yaml @@ -1389,9 +1389,11 @@ spec: - issuerRef type: object mode: - description: Mode selects the TLS management strategy. + default: SelfSigned + description: |- + Mode selects the TLS management strategy. + Defaults to SelfSigned if not specified. enum: - - Disabled - SelfSigned - CertManager - Provided diff --git a/operator/src/api/preview/documentdb_types.go b/operator/src/api/preview/documentdb_types.go index 64bbabe8..4b9a5afd 100644 --- a/operator/src/api/preview/documentdb_types.go +++ b/operator/src/api/preview/documentdb_types.go @@ -148,7 +148,7 @@ type BootstrapConfiguration struct { } // RecoveryConfiguration defines recovery settings for bootstrapping a DocumentDB cluster. -// +kubebuilder:validation:XValidation:rule="!(has(self.backup) && self.backup.name != '' && has(self.persistentVolume) && self.persistentVolume.name != '')",message="cannot specify both backup and persistentVolume recovery at the same time" +// +kubebuilder:validation:XValidation:rule="!(has(self.backup) && self.backup.name != ” && has(self.persistentVolume) && self.persistentVolume.name != ”)",message="cannot specify both backup and persistentVolume recovery at the same time" type RecoveryConfiguration struct { // Backup specifies the source backup to restore from. // +optional @@ -268,7 +268,9 @@ type TLSConfiguration struct { // GatewayTLS defines TLS configuration for the gateway sidecar (Phase 1: certificate provisioning only) type GatewayTLS struct { // Mode selects the TLS management strategy. - // +kubebuilder:validation:Enum=Disabled;SelfSigned;CertManager;Provided + // Defaults to SelfSigned if not specified. + // +kubebuilder:validation:Enum=SelfSigned;CertManager;Provided + // +kubebuilder:default=SelfSigned Mode string `json:"mode,omitempty"` // CertManager config when Mode=CertManager. diff --git a/operator/src/api/preview/zz_generated.deepcopy.go b/operator/src/api/preview/zz_generated.deepcopy.go index e53e0d62..7d654398 100644 --- a/operator/src/api/preview/zz_generated.deepcopy.go +++ b/operator/src/api/preview/zz_generated.deepcopy.go @@ -470,46 +470,46 @@ func (in *OTLPExporterSpec) DeepCopy() *OTLPExporterSpec { } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *PrometheusExporterSpec) DeepCopyInto(out *PrometheusExporterSpec) { +func (in *PVRecoveryConfiguration) DeepCopyInto(out *PVRecoveryConfiguration) { *out = *in } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PrometheusExporterSpec. -func (in *PrometheusExporterSpec) DeepCopy() *PrometheusExporterSpec { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PVRecoveryConfiguration. +func (in *PVRecoveryConfiguration) DeepCopy() *PVRecoveryConfiguration { if in == nil { return nil } - out := new(PrometheusExporterSpec) + out := new(PVRecoveryConfiguration) in.DeepCopyInto(out) return out } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *PVRecoveryConfiguration) DeepCopyInto(out *PVRecoveryConfiguration) { +func (in *PostgresTLS) DeepCopyInto(out *PostgresTLS) { *out = *in } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PVRecoveryConfiguration. -func (in *PVRecoveryConfiguration) DeepCopy() *PVRecoveryConfiguration { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PostgresTLS. +func (in *PostgresTLS) DeepCopy() *PostgresTLS { if in == nil { return nil } - out := new(PVRecoveryConfiguration) + out := new(PostgresTLS) in.DeepCopyInto(out) return out } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *PostgresTLS) DeepCopyInto(out *PostgresTLS) { +func (in *PrometheusExporterSpec) DeepCopyInto(out *PrometheusExporterSpec) { *out = *in } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PostgresTLS. -func (in *PostgresTLS) DeepCopy() *PostgresTLS { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PrometheusExporterSpec. +func (in *PrometheusExporterSpec) DeepCopy() *PrometheusExporterSpec { if in == nil { return nil } - out := new(PostgresTLS) + out := new(PrometheusExporterSpec) in.DeepCopyInto(out) return out } diff --git a/operator/src/config/crd/bases/documentdb.io_dbs.yaml b/operator/src/config/crd/bases/documentdb.io_dbs.yaml index bc7001b5..407336a8 100644 --- a/operator/src/config/crd/bases/documentdb.io_dbs.yaml +++ b/operator/src/config/crd/bases/documentdb.io_dbs.yaml @@ -1389,9 +1389,11 @@ spec: - issuerRef type: object mode: - description: Mode selects the TLS management strategy. + default: SelfSigned + description: |- + Mode selects the TLS management strategy. + Defaults to SelfSigned if not specified. enum: - - Disabled - SelfSigned - CertManager - Provided diff --git a/operator/src/internal/cnpg/cnpg_sync.go b/operator/src/internal/cnpg/cnpg_sync.go index 5e227805..da0bf6e6 100644 --- a/operator/src/internal/cnpg/cnpg_sync.go +++ b/operator/src/internal/cnpg/cnpg_sync.go @@ -202,8 +202,8 @@ func SyncCnpgCluster( // JSON Patch "add" requires the parent path to exist. if current.Annotations == nil { patchOps = append(patchOps, JSONPatch{ - Op: PatchOpAdd, - Path: "/metadata/annotations", + Op: PatchOpAdd, + Path: "/metadata/annotations", Value: map[string]string{ "kubectl.kubernetes.io/restartedAt": time.Now().Format(time.RFC3339Nano), }, diff --git a/operator/src/internal/controller/certificate_controller.go b/operator/src/internal/controller/certificate_controller.go index 3b9013c2..d8d96b18 100644 --- a/operator/src/internal/controller/certificate_controller.go +++ b/operator/src/internal/controller/certificate_controller.go @@ -61,16 +61,11 @@ func (r *CertificateReconciler) reconcileCertificates(ctx context.Context, ddb * } gatewayCfg := ddb.Spec.TLS.Gateway - if gatewayCfg.Mode == "" || gatewayCfg.Mode == "Disabled" { - if ddb.Status.TLS != nil && ddb.Status.TLS.Ready { - if err := r.updateTLSStatus(ctx, ddb, func(status *dbpreview.TLSStatus) { - status.Ready = false - status.Message = "Gateway TLS disabled" - }); err != nil { - return ctrl.Result{}, err - } - } - return ctrl.Result{}, nil + + // Empty mode defaults to SelfSigned for security - TLS is always enabled + mode := gatewayCfg.Mode + if mode == "" { + mode = "SelfSigned" } if ddb.Status.TLS == nil { @@ -81,7 +76,7 @@ func (r *CertificateReconciler) reconcileCertificates(ctx context.Context, ddb * } } - switch gatewayCfg.Mode { + switch mode { case "SelfSigned": return r.ensureSelfSignedCert(ctx, ddb) case "Provided": diff --git a/operator/src/internal/controller/certificate_controller_test.go b/operator/src/internal/controller/certificate_controller_test.go index c5986c38..9bcc3cc6 100644 --- a/operator/src/internal/controller/certificate_controller_test.go +++ b/operator/src/internal/controller/certificate_controller_test.go @@ -144,3 +144,56 @@ func TestEnsureSelfSignedCert(t *testing.T) { require.True(t, ddb.Status.TLS.Ready) require.NotEmpty(t, ddb.Status.TLS.SecretName) } + +// TestEmptyModeDefaultsToSelfSigned verifies that when mode is empty, +// the controller treats it as SelfSigned to ensure TLS is always enabled. +// This is a security fix - see https://github.com/documentdb/documentdb-kubernetes-operator/issues/356 +func TestEmptyModeDefaultsToSelfSigned(t *testing.T) { + ctx := context.Background() + ddb := baseDocumentDB("ddb-empty-mode", "default") + // Empty mode should default to SelfSigned behavior + ddb.Spec.TLS = &dbpreview.TLSConfiguration{Gateway: &dbpreview.GatewayTLS{Mode: ""}} + ddb.Status.TLS = &dbpreview.TLSStatus{} + r := buildCertificateReconciler(t, ddb) + + // First call should create issuer and certificate (SelfSigned behavior) + res, err := r.reconcileCertificates(ctx, ddb) + require.NoError(t, err) + require.Equal(t, RequeueAfterShort, res.RequeueAfter) + + // Certificate should exist, proving SelfSigned was used as default + cert := &cmapi.Certificate{} + require.NoError(t, r.Client.Get(ctx, types.NamespacedName{Name: "ddb-empty-mode-gateway-cert", Namespace: "default"}, cert)) + + // Simulate ready condition and verify TLS becomes ready + cert.Status.Conditions = append(cert.Status.Conditions, cmapi.CertificateCondition{Type: cmapi.CertificateConditionReady, Status: cmmeta.ConditionTrue, LastTransitionTime: &metav1.Time{Time: time.Now()}}) + require.NoError(t, r.Client.Update(ctx, cert)) + res, err = r.reconcileCertificates(ctx, ddb) + require.NoError(t, err) + require.Zero(t, res.RequeueAfter) + require.True(t, ddb.Status.TLS.Ready, "TLS should be ready with empty mode defaulting to SelfSigned") + require.NotEmpty(t, ddb.Status.TLS.SecretName) +} + +// TestDisabledModeNotSupported verifies that "Disabled" is no longer a valid mode. +// This test documents the breaking change per issue #356. +func TestDisabledModeNotSupported(t *testing.T) { + ctx := context.Background() + ddb := baseDocumentDB("ddb-disabled", "default") + // "Disabled" mode should fall through to the default case and do nothing + // In production, the webhook validation prevents this invalid value + ddb.Spec.TLS = &dbpreview.TLSConfiguration{Gateway: &dbpreview.GatewayTLS{Mode: "Disabled"}} + ddb.Status.TLS = &dbpreview.TLSStatus{} + r := buildCertificateReconciler(t, ddb) + + // With "Disabled" mode (which is now invalid), the controller should + // fall through the switch default case and return without action + res, err := r.reconcileCertificates(ctx, ddb) + require.NoError(t, err) + require.Zero(t, res.RequeueAfter) + + // No certificate should be created since "Disabled" hits the default case + cert := &cmapi.Certificate{} + err = r.Client.Get(ctx, types.NamespacedName{Name: "ddb-disabled-gateway-cert", Namespace: "default"}, cert) + require.True(t, err != nil, "No certificate should be created for invalid 'Disabled' mode") +} diff --git a/operator/src/internal/controller/documentdb_controller.go b/operator/src/internal/controller/documentdb_controller.go index 5c89fa8d..4a8b7dd4 100644 --- a/operator/src/internal/controller/documentdb_controller.go +++ b/operator/src/internal/controller/documentdb_controller.go @@ -1019,7 +1019,6 @@ func (r *DocumentDBReconciler) determineSchemaTarget( } } - // updateImageStatus reads the current extension and gateway images from the CNPG cluster // and persists them into the DocumentDB status fields. This is a no-op if both fields // are already up to date. diff --git a/operator/src/internal/utils/constants.go b/operator/src/internal/utils/constants.go index 6d560e79..5eafb7cd 100644 --- a/operator/src/internal/utils/constants.go +++ b/operator/src/internal/utils/constants.go @@ -26,11 +26,11 @@ const ( MinK8sMinorVersion = 35 // DEFAULT_DOCUMENTDB_IMAGE is the extension image used in ImageVolume mode. - DEFAULT_DOCUMENTDB_IMAGE = DOCUMENTDB_EXTENSION_IMAGE_REPO + ":0.109.0" + DEFAULT_DOCUMENTDB_IMAGE = DOCUMENTDB_EXTENSION_IMAGE_REPO + ":0.109.0" // NOTE: Keep in sync with operator/cnpg-plugins/sidecar-injector/internal/config/config.go:applyDefaults() DEFAULT_GATEWAY_IMAGE = GATEWAY_IMAGE_REPO + ":0.109.0" DEFAULT_DOCUMENTDB_CREDENTIALS_SECRET = "documentdb-credentials" - DEFAULT_OTEL_COLLECTOR_IMAGE = "otel/opentelemetry-collector-contrib:0.149.0" + DEFAULT_OTEL_COLLECTOR_IMAGE = "otel/opentelemetry-collector-contrib:0.149.0" // TODO: remove these constants once change stream support is included in the official images. CHANGESTREAM_DOCUMENTDB_IMAGE_REPOSITORY = "ghcr.io/wentingwu666666/documentdb-kubernetes-operator" diff --git a/operator/src/internal/utils/pv_recovery.go b/operator/src/internal/utils/pv_recovery.go index 6c09796b..16cd387e 100644 --- a/operator/src/internal/utils/pv_recovery.go +++ b/operator/src/internal/utils/pv_recovery.go @@ -13,10 +13,10 @@ import ( const ( // Label for identifying temporary PVCs created for PV recovery LabelRecoveryTemp = "documentdb.io/recovery-temp" - + // Label for identifying the DocumentDB cluster a PV/PVC belongs to - LabelCluster = "documentdb.io/cluster" - LabelNamespace = "documentdb.io/namespace" + LabelCluster = "documentdb.io/cluster" + LabelNamespace = "documentdb.io/namespace" ) // TempPVCNameForPVRecovery generates the name for a temporary PVC used during PV recovery.