Skip to content

Commit 868df53

Browse files
lexfreilexfrei
committed
feat(redis)!: refuse external access without TLS at admission
A Redis release with external: true and TLS off publishes a plaintext endpoint outside the cluster, and nothing stopped a tenant from asking for one. The chart never infers TLS from external, because enabling it moves Redis to a TLS-only listener, so external: true without an explicit tls.enabled: true is exactly that case. A ValidatingAdmissionPolicy on the Redis kind now refuses it with a message naming the release and a remedy that works. TLS is fixed when a Redis instance is created: the operator's RedisFailover schema refuses a change, but the Redis API accepts it. A spec's tls.enabled is therefore a request, not the instance's state, and cannot prove on UPDATE that an instance has TLS; trusting it would let a tenant flip it, expose the instance, and remove it again. So external access can be enabled only at creation, where tls.enabled does decide the instance, and any UPDATE turning it on is refused. The RedisFailover schema does not exist before the first reconcile, so a release admitted as external with TLS could have TLS dropped before the instance is created. tls.enabled is therefore frozen while external access stays on. Withdrawing external access in the same edit lifts the freeze, which leaves a tenant a way to unblock an instance whose TLS was flipped earlier, and the callers trusted by the tenant host policy are exempt so an administrator can repair one in place. Other edits to an instance that is already external pass, so releases exposed in plaintext before the policy keep working. Postgres is left out on purpose. CNPG pins ssl=on, and its default pg_hba accepts plaintext from any client that asks, whatever the chart's tls.enabled says; that field only decides whether the external hostname goes into the certificate. A policy keyed on it would close nothing. The decisions are tested by compiling the shipped template with the apiserver's own CEL compiler, since a structural chart test cannot show what the policy admits. BREAKING CHANGE: creating a Redis with external: true and without tls.enabled: true, which applied before, is now refused, including a GitOps re-apply of such a manifest into a fresh cluster. Turning external on for an existing Redis is refused as well, so exposing an instance means creating a new one with TLS, and tls.enabled can no longer be changed on an instance with external access. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>
1 parent bf03c84 commit 868df53

10 files changed

Lines changed: 446 additions & 22 deletions

File tree

‎api/apps/v1alpha1/redis/types.go‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎docs/security/threat-model.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ If a future `ApplicationDefinition`, RBAC change, or API change ever let a tenan
102102
Cozystack has two admission enforcement points, and the distinction matters when reasoning about which policies protect which resources:
103103

104104
- The **core kube-apiserver** evaluates ValidatingAdmissionPolicy for resources it serves directly — Namespaces, Gateways, HTTPRoutes/TLSRoutes/GRPCRoutes, Ingresses, and the no-delete guardrail.
105-
- **`cozystack-api` itself** evaluates the admission chain (validating webhooks and ValidatingAdmissionPolicy) for aggregated `apps.cozystack.io` resources, because those are served by the aggregated API server and never reach the core apiserver's admission chain. The `cozystack-tenant-host-policy` on `apps.cozystack.io/tenants` is therefore enforced inside `cozystack-api`.
105+
- **`cozystack-api` itself** evaluates the admission chain (validating webhooks and ValidatingAdmissionPolicy) for aggregated `apps.cozystack.io` resources, because those are served by the aggregated API server and never reach the core apiserver's admission chain. The `cozystack-tenant-host-policy` on `apps.cozystack.io/tenants` is therefore enforced inside `cozystack-api`. So is `cozystack-database-external-tls-policy`, which refuses a `redises` object that asks for a plaintext endpoint outside the cluster: `external: true` without `tls.enabled: true` on create, any update that turns external access on, since an existing instance's spec does not prove its TLS state, and any change of `tls.enabled` by a tenant while external access stays on (the callers trusted by `cozystack-tenant-host-policy` are exempt, to repair such an instance), since before the first reconcile nothing else stops TLS being dropped. A release already external and plaintext before the policy existed stays editable.
106106

107107
Two hostname-policy details are worth stating precisely. The **HTTPRoute/TLSRoute/GRPCRoute** policies (`route-hostname-policy.yaml`) and the **legacy-Ingress** policy (`ingress-hostname-policy.yaml`) are fail-closed: a route or Ingress in a tenant namespace missing `namespace.cozystack.io/host` is denied, so an absent label cannot open those layers (the route policies also deny a route that declares no hostnames, since such a route inherits the hostnames of the listener it attaches to rather than matching nothing; the Ingress policy likewise denies hostless rules and `spec.defaultBackend`, and permits external custom domains outside the platform root apex). The **namespace host-label** policy is an immutability guard — it stops an untrusted caller from scrubbing or changing an already-set label, but it does not itself reject a namespace that never carried one. The **Gateway-listener** policy (`gateway-hostname-policy.yaml`) applies the same subdomain rule but its CEL short-circuits to allow when the host label is empty, so it is permissive (fail-open) for a tenant namespace that lacks the label. In normal operation the platform sets that label at tenant creation and the namespace host-label policy keeps it immutable, closing the gap in practice — but the Gateway-side guarantee is conditional on the label being present, not unconditional.
108108

@@ -115,7 +115,7 @@ The claims in this document can be re-verified against these sources:
115115
- Aggregated API server: `cmd/cozystack-api/`, `pkg/apiserver/apiserver.go`, `pkg/registry/apps/application/rest.go`, `pkg/config/config.go`.
116116
- Dynamic Application kinds: `pkg/cmd/server/start.go`, `api/v1alpha1/applicationdefinitions_types.go`, `internal/controller/applicationdefinition_controller.go`.
117117
- Tenant RBAC: `packages/system/cozystack-basics/templates/clusterroles.yaml`.
118-
- Admission policies: `packages/system/cozystack-basics/templates/gateway-hostname-policy.yaml`, `route-hostname-policy.yaml`, `ingress-hostname-policy.yaml`, `packages/core/platform/templates/deletion-protection.yaml`.
118+
- Admission policies: `packages/system/cozystack-basics/templates/gateway-hostname-policy.yaml`, `route-hostname-policy.yaml`, `ingress-hostname-policy.yaml`, `database-external-tls-policy.yaml`, `packages/core/platform/templates/deletion-protection.yaml`.
119119
- Tenant provisioning and isolation: `packages/apps/tenant/templates/` (`tenant.yaml`, `namespace.yaml`, `networkpolicy.yaml`), `pkg/registry/apps/application/quota.go`.
120120
- UI OIDC front door (oauth2-proxy): `packages/system/dashboard/templates/gatekeeper.yaml`, `packages/system/linstor-gui/templates/gatekeeper.yaml`.
121121
- Reporting and supported versions: `SECURITY.md`.

‎packages/apps/redis/README.md‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,17 @@ Service utilizes the freshworks-oss Redis Operator (a maintained fork of the arc
1515

1616
### Common parameters
1717

18-
| Name | Description | Type | Value |
19-
| ------------------ | ------------------------------------------------------------------------------------------------------------------------------- | ---------- | --------- |
20-
| `replicas` | Number of Redis replicas. | `int` | `2` |
21-
| `resources` | Explicit CPU and memory configuration for each Redis replica. When omitted, the preset defined in `resourcesPreset` is applied. | `object` | `{}` |
22-
| `resources.cpu` | CPU available to each replica. | `quantity` | `""` |
23-
| `resources.memory` | Memory (RAM) available to each replica. | `quantity` | `""` |
24-
| `resourcesPreset` | Default sizing preset used when `resources` is omitted. | `string` | `t1.nano` |
25-
| `size` | Persistent Volume Claim size available for application data. | `quantity` | `1Gi` |
26-
| `storageClass` | StorageClass used to store the data. | `string` | `""` |
27-
| `external` | Enable external access from outside the cluster. | `bool` | `false` |
28-
| `version` | Redis major version to deploy | `string` | `v8` |
18+
| Name | Description | Type | Value |
19+
| ------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ---------- | --------- |
20+
| `replicas` | Number of Redis replicas. | `int` | `2` |
21+
| `resources` | Explicit CPU and memory configuration for each Redis replica. When omitted, the preset defined in `resourcesPreset` is applied. | `object` | `{}` |
22+
| `resources.cpu` | CPU available to each replica. | `quantity` | `""` |
23+
| `resources.memory` | Memory (RAM) available to each replica. | `quantity` | `""` |
24+
| `resourcesPreset` | Default sizing preset used when `resources` is omitted. | `string` | `t1.nano` |
25+
| `size` | Persistent Volume Claim size available for application data. | `quantity` | `1Gi` |
26+
| `storageClass` | StorageClass used to store the data. | `string` | `""` |
27+
| `external` | Enable external access from outside the cluster. Can be enabled only when the instance is created, together with `tls.enabled: true`; turning it on for an existing instance, or changing `tls.enabled` while it is on, is refused at admission. | `bool` | `false` |
28+
| `version` | Redis major version to deploy | `string` | `v8` |
2929

3030

3131
### TLS parameters
@@ -66,9 +66,11 @@ See [`docs/operations/resource-presets.md`](../../../docs/operations/resource-pr
6666

6767
### tls
6868

69-
`tls.enabled` turns on TLS for Redis and Sentinel connections. It is off unless set, including when `external` is true.
69+
`tls.enabled` turns on TLS for Redis and Sentinel connections. It is off unless set and is never inferred from `external`.
7070

71-
> **TLS is chosen when the instance is created.** TLS replaces plaintext rather than running beside it: with TLS on, Redis and Sentinel are configured with `port 0` and serve only on the TLS port. It cannot be switched on or off for a running instance, because a replica restarted onto the TLS listener cannot replicate from a master still on plaintext, and Sentinels already on TLS would fail over to a stale replica. The operator's RedisFailover schema rejects the change; on an existing instance that surfaces as a failed HelmRelease upgrade while the instance keeps serving what it served before. To add TLS, create a new instance with `tls.enabled` and move the clients and data to it. A `Redis` that was already switched keeps its rejected value until it is put back: the API accepts the change, the HelmRelease then fails on it, and every later edit to that instance waits behind the failure, so remove `tls.enabled` to unblock the release. This is also why TLS is not inferred from `external`: an instance reachable from outside the cluster is the one most likely to have clients that an upgrade must not disconnect.
71+
External access needs TLS and is chosen at creation. A `Redis` created with `external: true` must also set `tls.enabled: true`, and turning `external` on for an existing instance is refused at admission. TLS cannot be added to a running instance (see below), and an instance's `tls.enabled` does not prove what it runs, since the API accepts a change the operator then refuses. To expose an instance, create a new one with both set. While external access is on, `tls.enabled` cannot be changed either, so TLS cannot be dropped before the instance first comes up. Withdrawing external access works and lifts that freeze in the same edit, but external access cannot be turned back on. An instance that was already external without TLS before this rule keeps working and stays editable.
72+
73+
> **TLS is chosen when the instance is created.** TLS replaces plaintext rather than running beside it: with TLS on, Redis and Sentinel are configured with `port 0` and serve only on the TLS port. It cannot be switched on or off for a running instance, because a replica restarted onto the TLS listener cannot replicate from a master still on plaintext, and Sentinels already on TLS would fail over to a stale replica. The operator's RedisFailover schema rejects the change; on an existing instance that surfaces as a failed HelmRelease upgrade while the instance keeps serving what it served before. To add TLS, create a new instance with `tls.enabled` and move the clients and data to it. A `Redis` that was already switched keeps its rejected value until it is put back: the API accepts the change, the HelmRelease then fails on it, and every later edit to that instance waits behind the failure, so remove `tls.enabled` to unblock the release; on an instance with external access that edit is refused unless it also sets `external: false`, or is made by a `system:masters` user or a platform service account. This is also why TLS is not inferred from `external`: an instance reachable from outside the cluster is the one most likely to have clients that an upgrade must not disconnect.
7274

7375
Enabling TLS makes the chart issue a per-release cert-manager chain: a self-signed bootstrap Issuer, a CA certificate, a CA Issuer, and the server leaf certificate the operator mounts into the Redis and Sentinel pods. The CA belongs to this release alone; it is not a cluster-wide trust root, and nothing outside the release trusts it.
7476

‎packages/apps/redis/values.schema.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@
123123
}
124124
},
125125
"external": {
126-
"description": "Enable external access from outside the cluster.",
126+
"description": "Enable external access from outside the cluster. Can be enabled only when the instance is created, together with `tls.enabled: true`; turning it on for an existing instance, or changing `tls.enabled` while it is on, is refused at admission.",
127127
"type": "boolean",
128128
"default": false
129129
},

‎packages/apps/redis/values.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ size: 1Gi
7272
## @x-cozystack-options {source: storageclass}
7373
storageClass: ""
7474

75-
## @param {bool} external - Enable external access from outside the cluster.
75+
## @param {bool} external - Enable external access from outside the cluster. Can be enabled only when the instance is created, together with `tls.enabled: true`; turning it on for an existing instance, or changing `tls.enabled` while it is on, is refused at admission.
7676
external: false
7777

7878
## @enum {string} Version

‎packages/system/cozystack-basics/templates/database-external-tls-policy.yaml‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
{{- /* Render only where the ValidatingAdmissionPolicy API is served. Same guard as gateway-hostname-policy.yaml. */}}
2+
{{- if .Capabilities.APIVersions.Has "admissionregistration.k8s.io/v1/ValidatingAdmissionPolicy" }}
3+
---
4+
# Refuses a database exposed outside the cluster with TLS off. The kinds are
5+
# served by the aggregated cozystack-api, which runs cluster policies through
6+
# its own admission chain, so the policy binds to them like to any built-in.
7+
#
8+
# Only engines whose tls.enabled decides whether the server takes plaintext
9+
# belong here. Redis does: with TLS on it serves the TLS port only, and the
10+
# chart never infers TLS, so external needs an explicit true. Postgres does
11+
# not: CNPG pins ssl=on and its default pg_hba takes plaintext from any client
12+
# that asks, whatever the chart's tls.enabled, which only adds the external
13+
# name to the certificate.
14+
#
15+
# TLS is fixed when a Redis instance is created; the operator's RedisFailover
16+
# schema refuses a change, but this API accepts it. A spec's tls.enabled is
17+
# therefore a request, not the instance's state, and cannot prove on UPDATE
18+
# that an instance has TLS. So external access can be turned on only at
19+
# creation, where tls.enabled does decide the instance.
20+
#
21+
# The RedisFailover schema does not exist before the first reconcile, so
22+
# nothing else stops TLS from being dropped from a release admitted as
23+
# external with TLS. tls.enabled is therefore frozen while external stays on.
24+
# Withdrawing external access in the same edit lifts the freeze, which keeps a
25+
# way out for an instance whose tls.enabled was flipped before this policy;
26+
# the callers trusted by cozystack-tenant-host-policy are exempt, to repair
27+
# one without withdrawing it. An instance exposed in plaintext before this
28+
# policy stays editable otherwise.
29+
apiVersion: admissionregistration.k8s.io/v1
30+
kind: ValidatingAdmissionPolicy
31+
metadata:
32+
name: cozystack-database-external-tls-policy
33+
labels:
34+
internal.cozystack.io/managed-by-cozystack: ""
35+
spec:
36+
failurePolicy: Fail
37+
matchConstraints:
38+
resourceRules:
39+
- apiGroups: ["apps.cozystack.io"]
40+
apiVersions: ["v1alpha1"]
41+
operations: ["CREATE", "UPDATE"]
42+
resources: ["redises"]
43+
variables:
44+
- name: newExternal
45+
expression: >-
46+
has(object.spec) && has(object.spec.external) && object.spec.external == true
47+
- name: newTLS
48+
expression: >-
49+
has(object.spec) && has(object.spec.tls) && has(object.spec.tls.enabled) &&
50+
object.spec.tls.enabled == true
51+
- name: oldExternal
52+
expression: >-
53+
oldObject != null && has(oldObject.spec) && has(oldObject.spec.external) &&
54+
oldObject.spec.external == true
55+
- name: oldTLS
56+
expression: >-
57+
oldObject != null && has(oldObject.spec) && has(oldObject.spec.tls) &&
58+
has(oldObject.spec.tls.enabled) && oldObject.spec.tls.enabled == true
59+
- name: trustedCaller
60+
expression: >-
61+
has(request.userInfo.groups) && request.userInfo.groups.exists(g,
62+
g == "system:masters" ||
63+
g == "system:serviceaccounts:cozy-system" ||
64+
g == "system:serviceaccounts:cozy-cert-manager" ||
65+
g == "system:serviceaccounts:cozy-fluxcd" ||
66+
g == "system:serviceaccounts:kube-system")
67+
validations:
68+
- expression: >-
69+
!variables.newExternal ||
70+
(oldObject == null ? variables.newTLS : variables.oldExternal)
71+
messageExpression: >-
72+
oldObject == null
73+
? "Redis " + object.metadata.name + ": external: true requires tls.enabled: true, because a plaintext endpoint must not be exposed outside the cluster and Redis does not turn TLS on by itself. Set tls.enabled: true or set external: false."
74+
: "Redis " + object.metadata.name + ": external access can only be enabled when an instance is created, together with tls.enabled: true, because TLS cannot be added to an existing instance and its spec does not prove which one it has. Keep external: false, or create a new instance with tls.enabled: true and external: true and move the clients to it."
75+
reason: Invalid
76+
- expression: >-
77+
oldObject == null || !(variables.oldExternal && variables.newExternal) ||
78+
variables.oldTLS == variables.newTLS || variables.trustedCaller
79+
messageExpression: >-
80+
"Redis " + object.metadata.name + ": tls.enabled cannot change while external access is on, because TLS is fixed when an instance is created. " +
81+
(variables.newTLS ? "" : "Set external: false in the same edit, or ") +
82+
"create a new instance with tls.enabled: true and move the clients to it."
83+
reason: Invalid
84+
---
85+
apiVersion: admissionregistration.k8s.io/v1
86+
kind: ValidatingAdmissionPolicyBinding
87+
metadata:
88+
name: cozystack-database-external-tls-policy
89+
labels:
90+
internal.cozystack.io/managed-by-cozystack: ""
91+
spec:
92+
policyName: cozystack-database-external-tls-policy
93+
validationActions: [Deny]
94+
{{- end }}

0 commit comments

Comments
 (0)