fix(helm): tighten readonly-access guards and grant /version

README.md

@@ -157,15 +157,15 @@ kubectl logs -l job-name=<job-name> -n nullify
 Nullify can read an EKS cluster's configuration from Nullify's AWS account through the cluster's public API endpoint, using the read-only integration role. Nothing runs in the cluster; you grant list-only RBAC to a group and map the integration role to that group with an EKS access entry.
 
 - RBAC: the [`nullify-k8s-readonly-access`](helm-charts/nullify-k8s-readonly-access/README.md) chart, or the equivalent raw manifest [`manifests/nullify-readonly-rbac.yaml`](manifests/nullify-readonly-rbac.yaml) for kubectl and Flux.
-- Default: ClusterRole `nullify-readonly` with `list` on 26 kinds, bound to group `nullify-readonly`.
+- Default: ClusterRole `nullify-readonly` with `list` on 26 kinds and `get` on `/version`, bound to group `nullify-readonly`.
 - Do not use `AmazonEKSViewPolicy` (the scan fails); `AmazonEKSAdminViewPolicy` works but is far broader. See the chart README before choosing it.
 
 | | In-cluster collector | Managed EKS scan |
 |---|---|---|
 | Where it runs | CronJob in your cluster | Nullify's AWS account |
 | Network | Outbound from the cluster to AWS S3/STS | Inbound to the EKS public endpoint from Nullify egress IPs |
 | Private-only endpoint | Supported | Not supported |
-| In-cluster objects | Namespace, ServiceAccount, ClusterRole, ClusterRoleBinding, CronJob | ClusterRole, ClusterRoleBinding |
+| In-cluster objects | ServiceAccount, ClusterRole, ClusterRoleBinding, CronJob | ClusterRole, ClusterRoleBinding |
 | A denied kind | Skipped | Whole cluster scan fails |
 | Platforms | EKS, GKE | EKS |
 

helm-charts/nullify-k8s-readonly-access/README.md

@@ -9,7 +9,7 @@ The chart installs exactly two cluster-scoped objects:
 
 | Object | Name (default) | Grants |
 |---|---|---|
-| ClusterRole | `nullify-readonly` | `list` on the 26 kinds below. No `get`, `watch`, exec, logs, proxy, custom resources or writes. |
+| ClusterRole | `nullify-readonly` | `list` on the 26 kinds below, and `get` on the `/version` URL. No `get` or `watch` on resources, exec, logs, proxy, custom resources or writes. |
 | ClusterRoleBinding | `nullify-readonly` | Binds the ClusterRole to Group `nullify-readonly` |
 
 | API group | Kinds (verb `list`) |
@@ -21,10 +21,12 @@ The chart installs exactly two cluster-scoped objects:
 | `rbac.authorization.k8s.io` | roles, rolebindings, clusterroles, clusterrolebindings |
 | `admissionregistration.k8s.io` | validatingwebhookconfigurations, mutatingwebhookconfigurations, validatingadmissionpolicies, validatingadmissionpolicybindings |
 
-These are exactly the lists the scanner makes. It also reads `/version`, which
-every authenticated principal can read through the default
-`system:public-info-viewer` binding. Clusters older than 1.30 do not serve
-ValidatingAdmissionPolicies; the rule is harmless there, and the scanner skips that kind.
+These are exactly the lists the scanner makes. It also reads `/version`. The
+default `system:public-info-viewer` binding usually allows that, but hardened
+clusters sometimes remove it, and a denied `/version` fails the whole scan, so
+the ClusterRole grants `get` on `/version` itself. Clusters older than 1.30 do
+not serve ValidatingAdmissionPolicies; the rule is harmless there, and the
+scanner skips that kind.
 
 The scan runs every namespace and fails the whole cluster if any of these lists
 is denied, so the binding has to be cluster-wide.
@@ -149,15 +151,41 @@ Pin a tag, never `main`:
 kubectl apply -f https://raw.githubusercontent.com/Nullify-Platform/nullify-cloud-connector/nullify-k8s-readonly-access-v0.1.0/manifests/nullify-readonly-rbac.yaml
 ```
 
-To move from the manifest to Helm later, delete the two objects first
-(`kubectl delete clusterrolebinding,clusterrole nullify-readonly`); Helm refuses
-to adopt objects it did not create. Scans fail until the chart is installed.
+To move from the manifest to Helm later, let Helm adopt the two objects instead
+of deleting them, so scans keep working. Helm 3.2 and later adopts an existing
+object that carries its release's ownership metadata; without it, the install
+fails with `invalid ownership metadata`:
+
+```bash
+kubectl label clusterrole/nullify-readonly clusterrolebinding/nullify-readonly \
+  app.kubernetes.io/managed-by=Helm --overwrite
+kubectl annotate clusterrole/nullify-readonly clusterrolebinding/nullify-readonly \
+  meta.helm.sh/release-name=nullify-k8s-readonly-access \
+  meta.helm.sh/release-namespace=default --overwrite
+```
+
+Then run the Helm install above with the same release name and namespace. If a
+Flux Kustomization applied the manifest, remove it with `prune: false` first, or
+it deletes the objects Helm just adopted.
 
 ## 3. Allow Nullify on the API endpoint
 
 Add Nullify's egress IP addresses for your Nullify region to the cluster's
-public access CIDRs. `publicAccessCidrs` **replaces** the list, so merge with
-the current value, and the list holds at most 40 CIDRs.
+public access CIDRs. The region is where your Nullify tenant runs, not where the
+cluster runs.
+
+| Nullify region | Egress IPs |
+|---|---|
+| `ap-southeast-2` | `13.55.32.104/32`, `3.105.146.106/32`, `13.211.99.100/32` |
+| `eu-central-1` | `18.198.60.231/32`, `18.157.227.250/32`, `18.185.152.197/32` |
+| `us-east-2` | `52.15.146.50/32`, `16.58.40.80/32`, `3.133.15.210/32` |
+
+Nullify will also serve this list from an API endpoint; until then this table is
+the source of truth.
+
+`publicAccessCidrs` **replaces** the list, so merge with the current value, and
+the list holds at most 40 CIDRs. If the current value is `0.0.0.0/0`, the
+endpoint is already open to Nullify.
 
 ```bash
 aws eks describe-cluster --name "$CLUSTER" --region "$REGION" \
@@ -229,8 +257,8 @@ the four RBAC kinds and the four admission kinds, so the scan fails.
 | `clusterRoleBindingName` | `nullify-readonly` | ClusterRoleBinding name |
 | `grantSecretsRead` | `true` | Include `secrets`. Required by the scan today. |
 | `grantConfigMapsRead` | `true` | Include `configmaps`. Required by the scan today. |
-| `extraSubjects` | `[]` | Extra binding subjects; `system:` groups are rejected |
-| `extraRules` | `[]` | Extra ClusterRole rules; only `get`, `list` and `watch` verbs are accepted |
+| `extraSubjects` | `[]` | Extra binding subjects of kind `User`, `Group` or `ServiceAccount`. Any name starting `system:` (trimmed, any case) is rejected, as are ServiceAccounts in `kube-system`, `kube-public` and `kube-node-lease`. |
+| `extraRules` | `[]` | Extra ClusterRole rules. Only `get`, `list` and `watch`; no wildcard `apiGroups` or `resources`; no `exec`, `attach`, `portforward`, `proxy` or `log` subresources; `nonResourceURLs` only `/version`. |
 | `labels` | `{}` | Labels added to both objects |
 | `annotations` | `{}` | Annotations added to both objects |
 

helm-charts/nullify-k8s-readonly-access/ci/customised.yaml

@@ -8,10 +8,16 @@ extraSubjects:
   - apiGroup: rbac.authorization.k8s.io
     kind: User
     name: nullify-scanner
+  - kind: ServiceAccount
+    name: nullify-scanner
+    namespace: nullify
 extraRules:
   - apiGroups: ["batch"]
     resources: ["jobs", "cronjobs"]
     verbs: ["list"]
+  - apiGroups: ["apps"]
+    resources: ["deployments/scale"]
+    verbs: ["get"]
 labels:
   team: platform
 annotations:

helm-charts/nullify-k8s-readonly-access/templates/NOTES.txt

@@ -1,6 +1,6 @@
 Nullify read-only access installed.
 
-  ClusterRole:        {{ .Values.clusterRoleName }} (verb: list)
+  ClusterRole:        {{ .Values.clusterRoleName }} (list on cluster configuration, get on /version)
   ClusterRoleBinding: {{ .Values.clusterRoleBindingName }} -> Group "{{ .Values.groupName }}"
   Secrets list:       {{ .Values.grantSecretsRead }}
   ConfigMaps list:    {{ .Values.grantConfigMapsRead }}
@@ -15,7 +15,10 @@ Next steps:
        aws eks create-access-entry --cluster-name <cluster> --region <region> \
          --principal-arn arn:aws:iam::<account-id>:role/AWSIntegration-<CustomerName>-NullifyReadOnlyRole \
          --type STANDARD --kubernetes-groups {{ .Values.groupName }}
-  2. Allow Nullify's egress IP addresses on the cluster's public API endpoint.
+  2. Allow Nullify's egress IP addresses for your Nullify region on the cluster's public API endpoint:
+       ap-southeast-2: 13.55.32.104/32, 3.105.146.106/32, 13.211.99.100/32
+       eu-central-1:   18.198.60.231/32, 18.157.227.250/32, 18.185.152.197/32
+       us-east-2:      52.15.146.50/32, 16.58.40.80/32, 3.133.15.210/32
   3. Verify the group's RBAC (a group can only be impersonated together with a user):
        kubectl auth can-i list secrets --all-namespaces --as nullify-verify --as-group {{ .Values.groupName }}
        kubectl auth can-i get pods --all-namespaces --as nullify-verify --as-group {{ .Values.groupName }}   # expect: no

helm-charts/nullify-k8s-readonly-access/templates/_helpers.tpl

@@ -10,25 +10,62 @@ app.kubernetes.io/part-of: nullify
 {{- end }}
 
 {{/*
-Rejects values that would widen the grant beyond read-only access for one group.
-A "system:" group (for example system:authenticated) would hand this access to
-every principal in it.
+Rejects values that would widen the grant beyond read-only configuration access
+for the intended subjects:
+- any subject named "system:..." (for example system:authenticated or
+  system:anonymous), whatever its kind, compared trimmed and case-insensitively;
+- ServiceAccounts in Kubernetes control-plane namespaces;
+- extraRules verbs other than get, list and watch; wildcard apiGroups or
+  resources; the exec, attach, portforward, proxy and log subresources; and
+  nonResourceURLs other than /version.
 */}}
 {{- define "nullify-readonly.validate" -}}
-{{- $group := required "groupName is required" .Values.groupName -}}
-{{- if hasPrefix "system:" $group -}}
+{{- $group := trim (toString (required "groupName is required" .Values.groupName)) -}}
+{{- if not $group -}}
+{{- fail "groupName is required" -}}
+{{- end -}}
+{{- if hasPrefix "system:" (lower $group) -}}
 {{- fail (printf "groupName %q must not start with \"system:\": binding a system group grants this access to every principal in it" $group) -}}
 {{- end -}}
 {{- range .Values.extraSubjects -}}
-{{- if and (eq (toString .kind) "Group") (hasPrefix "system:" (toString .name)) -}}
-{{- fail (printf "extraSubjects group %q must not start with \"system:\": binding a system group grants this access to every principal in it" (toString .name)) -}}
+{{- $kind := trim (toString .kind) -}}
+{{- $name := trim (toString .name) -}}
+{{- if not (has $kind (list "User" "Group" "ServiceAccount")) -}}
+{{- fail (printf "extraSubjects kind %q is not allowed: use User, Group or ServiceAccount" $kind) -}}
+{{- end -}}
+{{- if hasPrefix "system:" (lower $name) -}}
+{{- fail (printf "extraSubjects %s %q must not start with \"system:\": system identities include anonymous callers and every-principal groups" $kind $name) -}}
+{{- end -}}
+{{- if eq $kind "ServiceAccount" -}}
+{{- $namespace := lower (trim (toString .namespace)) -}}
+{{- if or (hasPrefix "system:" $namespace) (has $namespace (list "kube-system" "kube-public" "kube-node-lease")) -}}
+{{- fail (printf "extraSubjects ServiceAccount namespace %q is not allowed: control-plane namespaces run system components" $namespace) -}}
 {{- end -}}
 {{- end -}}
+{{- end -}}
+{{- $blockedSubresources := list "exec" "attach" "portforward" "proxy" "log" -}}
 {{- range .Values.extraRules -}}
 {{- range .verbs -}}
 {{- if not (has (toString .) (list "get" "list" "watch")) -}}
 {{- fail (printf "extraRules verb %q is not allowed: this chart only grants get, list and watch" (toString .)) -}}
 {{- end -}}
 {{- end -}}
+{{- range .apiGroups -}}
+{{- if contains "*" (toString .) -}}
+{{- fail (printf "extraRules apiGroup %q is not allowed: name each API group" (toString .)) -}}
+{{- end -}}
+{{- end -}}
+{{- range .resources -}}
+{{- $resource := lower (trim (toString .)) -}}
+{{- $parts := splitList "/" $resource -}}
+{{- if or (contains "*" $resource) (and (gt (len $parts) 1) (has (last $parts) $blockedSubresources)) -}}
+{{- fail (printf "extraRules resource %q is not allowed: wildcards and the exec, attach, portforward, proxy and log subresources reach beyond read-only configuration" (toString .)) -}}
+{{- end -}}
+{{- end -}}
+{{- range .nonResourceURLs -}}
+{{- if ne (trim (toString .)) "/version" -}}
+{{- fail (printf "extraRules nonResourceURL %q is not allowed: only /version, which the chart already grants" (toString .)) -}}
+{{- end -}}
+{{- end -}}
 {{- end -}}
 {{- end }}

helm-charts/nullify-k8s-readonly-access/templates/clusterrole.yaml

@@ -47,6 +47,8 @@ rules:
       - validatingadmissionpolicies
       - validatingadmissionpolicybindings
     verbs: ["list"]
+  - nonResourceURLs: ["/version"]
+    verbs: ["get"]
   {{- with .Values.extraRules }}
   {{- toYaml . | nindent 2 }}
   {{- end }}

helm-charts/nullify-k8s-readonly-access/values.yaml

@@ -18,15 +18,18 @@ grantSecretsRead: true
 grantConfigMapsRead: true
 
 # Extra subjects appended to the ClusterRoleBinding, for example the username
-# given to an access entry with --username. Groups starting with "system:" are
-# rejected.
+# given to an access entry with --username. Kinds are User, Group or
+# ServiceAccount. Any name starting with "system:" (trimmed, any case) is
+# rejected, as are ServiceAccounts in kube-system, kube-public and kube-node-lease.
 extraSubjects: []
 #  - apiGroup: rbac.authorization.k8s.io
 #    kind: User
 #    name: nullify-scanner
 
 # Extra rules appended verbatim to the ClusterRole. Only the get, list and watch
-# verbs are accepted.
+# verbs are accepted. Wildcard apiGroups or resources, the exec, attach,
+# portforward, proxy and log subresources, and nonResourceURLs other than
+# /version are rejected.
 extraRules: []
 #  - apiGroups: ["batch"]
 #    resources: ["jobs", "cronjobs"]

manifests/nullify-readonly-rbac.yaml

@@ -1,4 +1,4 @@
-# Nullify managed EKS scan: read-only Kubernetes access (list only).
+# Nullify managed EKS scan: read-only Kubernetes access (list on 26 kinds, get on /version).
 #
 # This is the default render of helm-charts/nullify-k8s-readonly-access without
 # Helm's bookkeeping labels; CI (scripts/check-rbac-manifest.sh) fails if the two
@@ -48,6 +48,8 @@ rules:
       - validatingadmissionpolicies
       - validatingadmissionpolicybindings
     verbs: ["list"]
+  - nonResourceURLs: ["/version"]
+    verbs: ["get"]
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRoleBinding

scripts/check-rbac-manifest.sh

@@ -33,6 +33,10 @@ for side in manifest chart; do
     echo "::error::$side normalised to $count objects, expected the ClusterRole and ClusterRoleBinding"
     exit 1
   fi
+  if ! jq -e '.[] | select(.kind == "ClusterRole") | .rules[] | select(.nonResourceURLs == ["/version"] and .verbs == ["get"])' "$work/$side.json" >/dev/null; then
+    echo "::error::$side ClusterRole does not grant get on /version, which the strict managed scan needs"
+    exit 1
+  fi
 done
 
 if ! diff -u "$work/manifest.json" "$work/chart.json"; then

tests/helm/nullify-k8s-readonly-access/must-fail/kube-system-serviceaccount.yaml

@@ -0,0 +1,5 @@
+# expect: extraSubjects ServiceAccount namespace "kube-system" is not allowed
+extraSubjects:
+  - kind: ServiceAccount
+    name: default
+    namespace: kube-system

tests/helm/nullify-k8s-readonly-access/must-fail/rule-nodes-proxy.yaml

@@ -0,0 +1,5 @@
+# expect: extraRules resource "nodes/proxy" is not allowed
+extraRules:
+  - apiGroups: [""]
+    resources: ["nodes/proxy"]
+    verbs: ["get"]

tests/helm/nullify-k8s-readonly-access/must-fail/rule-non-resource-url.yaml

@@ -0,0 +1,4 @@
+# expect: extraRules nonResourceURL "/metrics" is not allowed
+extraRules:
+  - nonResourceURLs: ["/metrics"]
+    verbs: ["get"]

tests/helm/nullify-k8s-readonly-access/must-fail/rule-pods-exec.yaml

@@ -0,0 +1,5 @@
+# expect: extraRules resource "pods/exec" is not allowed
+extraRules:
+  - apiGroups: [""]
+    resources: ["pods/exec"]
+    verbs: ["get"]

tests/helm/nullify-k8s-readonly-access/must-fail/rule-wildcard-api-group.yaml

@@ -0,0 +1,5 @@
+# expect: extraRules apiGroup "*" is not allowed
+extraRules:
+  - apiGroups: ["*"]
+    resources: ["jobs"]
+    verbs: ["list"]

tests/helm/nullify-k8s-readonly-access/must-fail/rule-wildcard-resource.yaml

@@ -0,0 +1,5 @@
+# expect: extraRules resource "*" is not allowed
+extraRules:
+  - apiGroups: [""]
+    resources: ["*"]
+    verbs: ["list"]

tests/helm/nullify-k8s-readonly-access/must-fail/system-extra-subject.yaml

@@ -1,4 +1,4 @@
-# expect: extraSubjects group "system:serviceaccounts"
+# expect: extraSubjects Group "system:serviceaccounts" must not start with "system:"
 extraSubjects:
   - apiGroup: rbac.authorization.k8s.io
     kind: Group

tests/helm/nullify-k8s-readonly-access/must-fail/system-user-subject.yaml

@@ -0,0 +1,5 @@
+# expect: extraSubjects User "System:Anonymous" must not start with "system:"
+extraSubjects:
+  - apiGroup: rbac.authorization.k8s.io
+    kind: User
+    name: "  System:Anonymous"

tests/helm/nullify-k8s-readonly-access/must-fail/unsupported-subject-kind.yaml

@@ -0,0 +1,4 @@
+# expect: extraSubjects kind "Anonymous" is not allowed
+extraSubjects:
+  - kind: Anonymous
+    name: nullify-scanner