diff --git a/CHANGELOG.md b/CHANGELOG.md index df5e87ea..74d59ddf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Security +- Opt-in `ValidatingAdmissionPolicy` and binding to restrict the `node-data-broker` ServiceAccount to only updating its host Node resource, mitigating privilege escalation via node spec or annotation manipulation (audit F1 compliance). - Removed unused RBAC verbs from the Topograph API server and node-data-broker ClusterRoles (least-privilege): API server `pods` rule dropped `get` (list-only), `daemonsets` rule dropped `list` (get-only), and the Slinky `configmaps` rule dropped `list`; node-data-broker `nodes` rule dropped `list` (get/update), and the InfiniBand `daemonsets`/`pods` rules dropped `list`/`get` respectively (get-only, list-only). - node-observer ClusterRole no longer grants unused `get`; `nodes` list/watch now gated on `trigger.nodeSelector`. diff --git a/charts/topograph/templates/nodeDataBroker/validatingadmissionpolicy.yaml b/charts/topograph/templates/nodeDataBroker/validatingadmissionpolicy.yaml new file mode 100644 index 00000000..2a7bc115 --- /dev/null +++ b/charts/topograph/templates/nodeDataBroker/validatingadmissionpolicy.yaml @@ -0,0 +1,37 @@ +{{- if and .Values.nodeDataBroker.enabled .Values.nodeDataBroker.validatingAdmissionPolicy.enabled }} +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingAdmissionPolicy +metadata: + name: {{ include "nodeDataBroker.fullname" . }} + labels: + {{- include "nodeDataBroker.labels" . | nindent 4 }} +spec: + failurePolicy: Fail + matchConstraints: + resourceRules: + - apiGroups: [""] + apiVersions: ["v1"] + operations: ["UPDATE"] + resources: ["nodes"] + matchConditions: + - name: match-node-data-broker-serviceaccount + expression: >- + request.userInfo.username == 'system:serviceaccount:{{ .Release.Namespace }}:{{ include "nodeDataBroker.serviceAccountName" . }}' + validations: + - expression: >- + has(request.userInfo.extra) && + 'authentication.kubernetes.io/node-name' in request.userInfo.extra && + size(request.userInfo.extra['authentication.kubernetes.io/node-name']) > 0 && + request.name == request.userInfo.extra['authentication.kubernetes.io/node-name'][0] + message: "node-data-broker ServiceAccount is only authorized to update its own Node resource (audit F1)." +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingAdmissionPolicyBinding +metadata: + name: {{ include "nodeDataBroker.fullname" . }} + labels: + {{- include "nodeDataBroker.labels" . | nindent 4 }} +spec: + policyName: {{ include "nodeDataBroker.fullname" . }} + validationActions: [Deny] +{{- end }} diff --git a/charts/topograph/tests/node-data-broker_validatingadmissionpolicy_test.yaml b/charts/topograph/tests/node-data-broker_validatingadmissionpolicy_test.yaml new file mode 100644 index 00000000..32dc7726 --- /dev/null +++ b/charts/topograph/tests/node-data-broker_validatingadmissionpolicy_test.yaml @@ -0,0 +1,105 @@ +suite: node-data-broker validating admission policy +templates: + - templates/nodeDataBroker/validatingadmissionpolicy.yaml +release: + name: chart-ci + namespace: topograph +capabilities: + majorVersion: "1" + minorVersion: "30" +tests: + - it: is omitted by default + asserts: + - hasDocuments: + count: 0 + + - it: is omitted when the component is disabled + set: + nodeDataBroker: + enabled: false + validatingAdmissionPolicy: + enabled: true + asserts: + - hasDocuments: + count: 0 + + - it: renders both resources when enabled + set: + nodeDataBroker: + enabled: true + validatingAdmissionPolicy: + enabled: true + asserts: + - hasDocuments: + count: 2 + + - it: renders correct API versions and kinds for ValidatingAdmissionPolicy + set: + nodeDataBroker: + enabled: true + validatingAdmissionPolicy: + enabled: true + documentIndex: 0 + asserts: + - isKind: + of: ValidatingAdmissionPolicy + - equal: + path: apiVersion + value: admissionregistration.k8s.io/v1 + - equal: + path: metadata.name + value: chart-ci-topograph-node-data-broker + - equal: + path: spec.failurePolicy + value: Fail + + - it: renders correct API versions and kinds for ValidatingAdmissionPolicyBinding + set: + nodeDataBroker: + enabled: true + validatingAdmissionPolicy: + enabled: true + documentIndex: 1 + asserts: + - isKind: + of: ValidatingAdmissionPolicyBinding + - equal: + path: apiVersion + value: admissionregistration.k8s.io/v1 + - equal: + path: metadata.name + value: chart-ci-topograph-node-data-broker + - equal: + path: spec.policyName + value: chart-ci-topograph-node-data-broker + - equal: + path: spec.validationActions[0] + value: Deny + + - it: configures the CEL expression and dynamic ServiceAccount matching + set: + nodeDataBroker: + enabled: true + validatingAdmissionPolicy: + enabled: true + serviceAccount: + create: true + name: custom-node-data-broker-sa + documentIndex: 0 + asserts: + - equal: + path: spec.matchConditions[0].name + value: match-node-data-broker-serviceaccount + - equal: + path: spec.matchConditions[0].expression + value: "request.userInfo.username == 'system:serviceaccount:topograph:custom-node-data-broker-sa'" + - equal: + path: spec.validations[0].expression + value: >- + has(request.userInfo.extra) && + 'authentication.kubernetes.io/node-name' in request.userInfo.extra && + size(request.userInfo.extra['authentication.kubernetes.io/node-name']) > 0 && + request.name == request.userInfo.extra['authentication.kubernetes.io/node-name'][0] + - equal: + path: spec.validations[0].message + value: "node-data-broker ServiceAccount is only authorized to update its own Node resource (audit F1)." diff --git a/charts/topograph/values.schema.json b/charts/topograph/values.schema.json index a3ac793f..4886e849 100644 --- a/charts/topograph/values.schema.json +++ b/charts/topograph/values.schema.json @@ -228,6 +228,12 @@ "startupProbe": { "type": "object" }, "serviceAccount": { "type": "object" }, "rbac": { "type": "object" }, + "validatingAdmissionPolicy": { + "type": "object", + "properties": { + "enabled": { "type": "boolean" } + } + }, "env": { "type": "object" }, "initContainers": { "type": "array" }, "lifecycle": { "type": "object" }, diff --git a/charts/topograph/values.yaml b/charts/topograph/values.yaml index f79b4739..5c59cbbd 100644 --- a/charts/topograph/values.yaml +++ b/charts/topograph/values.yaml @@ -312,6 +312,11 @@ nodeDataBroker: rbac: create: true + # Opt-in ValidatingAdmissionPolicy to scope node writes. Requires + # Kubernetes >= 1.30 (admissionregistration.k8s.io/v1) and ServiceAccountTokenPodNodeInfo enabled. + validatingAdmissionPolicy: + enabled: false + env: {} initContainers: [] lifecycle: {} diff --git a/docs/engines/k8s.md b/docs/engines/k8s.md index f99fd23e..c2032c4d 100644 --- a/docs/engines/k8s.md +++ b/docs/engines/k8s.md @@ -302,6 +302,22 @@ Apply alongside the chart. A bundled template is under consideration. The node-observer `ClusterRole` grants `pods [list, watch]` unconditionally, `apps/daemonsets [list, watch]` when `nodeDataBroker.enabled=true`, and `nodes [list, watch]` only when `nodeObserver.topograph.trigger.nodeSelector` is set. Set `nodeObserver.rbac.create: false` to suppress the `ClusterRole`/`ClusterRoleBinding` when managing RBAC externally. +### Node Data Broker write-scoping via ValidatingAdmissionPolicy + +The node-data-broker's RBAC grants update permission on nodes cluster-wide. To enforce least-privilege scoping, the chart supports deploying an opt-in `ValidatingAdmissionPolicy` and its binding: + +```yaml +nodeDataBroker: + validatingAdmissionPolicy: + enabled: true +``` + +When enabled, the policy matches node `UPDATE` operations performed by the broker's ServiceAccount and validates that the name of the node being modified matches the node name claim bound to the requester's ServiceAccount token (`authentication.kubernetes.io/node-name`). + +**Prerequisites:** +1. A Kubernetes cluster running **Kubernetes 1.30+** with the **`ValidatingAdmissionPolicy` API enabled** (`admissionregistration.k8s.io/v1`). +2. ServiceAccount tokens carrying the node-binding identity claim (e.g., via `ServiceAccountTokenPodNodeInfo`). The policy denies updates from the broker ServiceAccount if this required node-name claim is missing from the request context. + ## Validation and Testing The Helm chart ships two layers of validation for operators.