From a0487e45ac497d82112f6f6977bbcc5f64dc23d4 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Mon, 2 Nov 2026 14:01:19 +0100 Subject: [PATCH 1/5] refactor(policy): move the tenant network guard to ValidatingPolicy Keeps every refused shape, the DELETE guard on the generated floor names and both applying-identity exemptions. The per-rule rows stay as per-check.yaml and each check runs alone against them. Part of #4995. Co-Authored-By: Claude Opus 5.5 --- .../restrict-tenant-network-policies.yaml | 691 +++++++----------- .../cluster-roles/cilium-tenant-edit.yaml | 2 +- scripts/tests/tenant-webapp-admission/main.go | 2 +- .../test-restrict-tenant-network-policies.sh | 180 +++-- ...st-tenant-controller-namespace-coverage.sh | 2 +- ...est-validate-kyverno-fixture-evaluation.sh | 36 +- tests/policy-failure-actions.json | 8 +- .../kyverno-author/kyverno-test.yaml | 16 +- .../kyverno-author/per-check.yaml | 25 + .../kyverno-test.yaml | 379 +++------- .../per-check.yaml | 314 ++++++++ .../platform-author/kyverno-test.yaml | 15 +- .../platform-author/per-check.yaml | 29 + .../tenant-scaffold/kyverno-test.yaml | 28 +- .../tenant-scaffold/per-check.yaml | 42 ++ 15 files changed, 944 insertions(+), 825 deletions(-) create mode 100644 tests/restrict-tenant-network-policies/kyverno-author/per-check.yaml create mode 100644 tests/restrict-tenant-network-policies/per-check.yaml create mode 100644 tests/restrict-tenant-network-policies/platform-author/per-check.yaml create mode 100644 tests/restrict-tenant-network-policies/tenant-scaffold/per-check.yaml diff --git a/k8s/bases/infrastructure/cluster-policies/best-practices/restrict-tenant-network-policies.yaml b/k8s/bases/infrastructure/cluster-policies/best-practices/restrict-tenant-network-policies.yaml index dc63c6fab6..27ec763746 100644 --- a/k8s/bases/infrastructure/cluster-policies/best-practices/restrict-tenant-network-policies.yaml +++ b/k8s/bases/infrastructure/cluster-policies/best-practices/restrict-tenant-network-policies.yaml @@ -87,10 +87,7 @@ # in form only too, and a short list of them covers essentially every # practical public name. The check refuses the whole shape (`*`, `*.*`, # `*.*.*`, …) rather than the one catch-all, so a fixed suffix is required -# somewhere in the pattern. A `matchName` is an exact name and unaffected — -# and the condition guards for its absence, because `regex_match` against a -# missing `matchPattern` ERRORS, and a Kyverno condition that errors never -# denies (a fail-open, measured on `allow-web-egress-named-fqdn`). +# somewhere in the pattern. A `matchName` is an exact name and unaffected. # * A `toServices` reference without the policy's explicit namespace. Cilium # treats an omitted or empty Service namespace as ANY namespace, for both # named Services and Service selectors. A fixed name or label alone therefore @@ -131,11 +128,11 @@ # allow-cnpg-operator objects. Without the second exclusion this policy # would refuse the very floor it exists to protect. # The discriminator is the applying identity, which a tenant cannot forge. -# Because the excludes match on request.userInfo the policy is admission-only -# (background: false); every escape is a create/update passing through admission, +# Because the exclusions match on request.userInfo the policy is admission-only +# (background disabled); every escape is a request passing through admission, # so enforcement is unaffected. -apiVersion: kyverno.io/v1 -kind: ClusterPolicy +apiVersion: policies.kyverno.io/v1 +kind: ValidatingPolicy metadata: name: restrict-tenant-network-policies annotations: @@ -143,7 +140,7 @@ metadata: policies.kyverno.io/category: Security, Networking policies.kyverno.io/severity: high policies.kyverno.io/subject: CiliumNetworkPolicy - policies.kyverno.io/minversion: 1.13.0 + policies.kyverno.io/minversion: 1.17.0 policies.kyverno.io/description: >- Blocks tenant-applied CiliumNetworkPolicy resources in ksail tenant namespaces from dissolving the generated default-deny floor: every @@ -159,428 +156,256 @@ metadata: unaffected, as are policies applied by flux-system's kustomize-controller or generated by kyverno's background-controller. spec: - # Admission-only: the excludes match on request.userInfo (the applying + failurePolicy: Fail + validationActions: + - Deny + # Admission-only: the exclusions match on request.userInfo (the applying # ServiceAccount), which is unavailable during background scans. - background: false - rules: - - name: cnp-ingress-sources-must-be-constrained - match: - any: - - resources: - kinds: - - CiliumNetworkPolicy - namespaceSelector: - matchExpressions: - - key: app.kubernetes.io/managed-by - operator: In - values: [ksail, kro] - exclude: - any: - - subjects: - - kind: ServiceAccount - name: kustomize-controller - namespace: flux-system - - subjects: - - kind: ServiceAccount - name: kyverno-background-controller - namespace: kyverno - validate: - # Rule-level, not the deprecated top-level spec.validationFailureAction: - # if a future Kyverno stops honouring that field, a rule with no action - # of its own defaults to Audit and these violations would be ADMITTED. - failureAction: Enforce - message: >- - Every ingress source in a tenant CiliumNetworkPolicy must be - constrained. fromNodes and fromGroups are outside the tenant namespace - boundary and must be omitted. A fromEndpoints selector with no matchLabels and no - matchExpressions allows all sources; a selector key naming a reserved - identity (`reserved:host`, or its bare/`any:` spelling), another - namespace (`k8s:io.kubernetes.pod.namespace`, - `io.cilium.k8s.namespace.labels.*`) or another cluster - (`io.cilium.k8s.policy.cluster`) reaches the same broad source — pin a - tenant label with the `k8s:` prefix instead, and name the post-Gateway - identity as `reserved:ingress` or `fromEntities: [ingress]`; only - `kube-apiserver` and the post-Gateway `ingress` identity may be named - as an entity; and a CIDR prefix shorter than /16 — or a CIDR inside - private, cluster, link-local, loopback or multicast space, or a - CIDRSet entry naming a CiliumCIDRGroup, with or without a literal cidr beside it — - reaches outside this - namespace. A rule with only toPorts and no source at all allows every - source on that port. Cilium unions allow rules, so one unconstrained - source removes this namespace's isolation regardless of the generated - default-deny. - deny: - conditions: - any: - # WHITELIST: count the fromEndpoints selectors that carry a real - # constraint and require that to be ALL of them. JMESPath treats - # `{}` and `[]` as false-y, so `matchLabels || matchExpressions` - # is true only for a selector that actually constrains something — - # which makes all four empty spellings, and any future fifth, - # fail the same way. - - key: "{{ length(request.object.spec.ingress[].fromEndpoints[] || `[]`) }}" - operator: NotEquals - value: "{{ length(request.object.spec.ingress[].fromEndpoints[] || `[]` | [?matchLabels || length(matchExpressions[?operator == 'In'] || `[]`) > `0`]) }}" - # WHITELIST OF KEYS: a selector that passed the presence check - # above is still unconstrained when what it NAMES is a reserved - # identity, another namespace or another cluster — Cilium exposes - # all three as ordinary labels (see the header). Read every key a - # selector carries, matchLabels AND matchExpressions, and count - # the selectors naming one; that count must be zero. Keys are - # matched in all three source spellings — `reserved:`/`k8s:`, the - # explicit `any:`, and no prefix at all (which Cilium defaults to - # `any:`) — so the map form and the `In` form fail the same way. - # Carved out, mirroring the entity whitelist: `reserved:kube-apiserver` - # and, on this side only, `reserved:ingress` (each with its - # bare/`any:` spelling) — the selector forms of the two entities - # the next condition permits. - # `[.]` rather than `\.` in the pattern: this scalar is - # double-quoted YAML, where a backslash escape would not survive. - # `(matchLabels && keys(matchLabels)) || `[]`` rather than - # `keys(matchLabels || `{}`)`: keys() of a missing map errors, - # and a Kyverno condition that errors never denies. - - key: "{{ length(request.object.spec.ingress[].fromEndpoints[] || `[]` | [?length([(matchLabels && keys(matchLabels)) || `[]`, matchExpressions[].key || `[]`][] | [?regex_match('^(reserved:|(any:)?(unknown|host|world|world-ipv4|world-ipv6|unmanaged|health|init|remote-node|kube-apiserver|ingress|cluster|cluster-mesh|all|none)$|(k8s:|any:)?io[.]kubernetes[.]pod[.]namespace$|(k8s:|any:)?io[.]cilium[.]k8s[.](namespace[.]labels[.]|policy[.]cluster$))', @) && @ != 'reserved:kube-apiserver' && @ != 'kube-apiserver' && @ != 'any:kube-apiserver' && @ != 'reserved:ingress' && @ != 'ingress' && @ != 'any:ingress']) > `0`]) }}" - operator: GreaterThan - value: 0 - # A rule carrying ONLY toPorts and no layer-3 stanza is a fifth - # spelling of allow-all: Cilium's own layer-4 examples pair a port - # with a source, and a port-only ingress rule permits every source - # on that port. Require each rule to name its sources at all, - # which the selector whitelist above then constrains. - - key: "{{ length(request.object.spec.ingress[] || `[]`) }}" - operator: NotEquals - value: "{{ length(request.object.spec.ingress[] || `[]` | [?fromEndpoints || fromCIDR || fromCIDRSet || fromEntities || fromNodes || fromGroups]) }}" - # Reject the fields themselves: a pinned node or provider group - # is no more namespace-scoped than an empty selector. keys() also - # catches empty lists/nulls beside another valid source. - - key: "{{ length(request.object.spec.ingress[] || `[]` | [?contains(keys(@), 'fromNodes') || contains(keys(@), 'fromGroups')]) }}" - operator: GreaterThan - value: 0 - # WHITELIST the entities a tenant may name, rather than blacklisting - # the broad ones. Cilium's entity vocabulary is open-ended — beyond - # `all`/`world`/`cluster`, `remote-node` reaches remote nodes and - # host-networked workloads and `cluster-mesh` reaches endpoints in - # meshed clusters — so an exclusion list is one entity away from being - # incomplete again. `kube-apiserver` and, here on the ingress side, the - # post-Gateway `ingress` identity are the entities a namespaced - # tenant workload has a legitimate reason to name, and it does not - # dissolve the namespace boundary. Compared with a JMESPath filter - # rather than AnyIn, for the wildcard reason given in the egress rule. - - key: "{{ length(request.object.spec.ingress[].fromEntities[] || `[]` | [?@ != 'kube-apiserver' && @ != 'ingress']) }}" - operator: GreaterThan - value: 0 - # Refuse on PREFIX LENGTH, not on prefix text. `0.0.0.0/0` and `::/0` - # are merely the shortest spellings of "everything": `0.0.0.0/1` plus - # `128.0.0.0/1`, or `::/1` plus `8000::/1`, covers an entire address - # family while matching neither literal. Refusing every prefix shorter - # than /16 closes that whole family of splits without enumerating any of - # them. A bare address carries no `/` and is unaffected. - - key: "{{ length(request.object.spec.ingress[].fromCIDR[] || `[]` | [?regex_match('^[^/]*/0*([0-9]|1[0-5])$', @)]) }}" - operator: GreaterThan - value: 0 - - key: "{{ length(request.object.spec.ingress[].fromCIDRSet[].cidr || `[]` | [?regex_match('^[^/]*/0*([0-9]|1[0-5])$', @)]) }}" - operator: GreaterThan - value: 0 - # DENY-LIST of address space that is NON-GLOBAL by definition, - # read from the address text. The floor above refuses every entry - # shorter than /16, so each remaining entry is either inside one - # of these blocks or disjoint from it — the leading octets (or - # hextets) decide, and no CIDR arithmetic is needed (JMESPath has - # none). IPv4: 0/8 · 10/8, 172.16/12, 192.168/16 (private — the - # node network, the pod network and the Service CIDR all live in - # 10/8) · 100.64/10 (CGNAT) · 127/8 · 169.254/16 (link-local; the - # cloud metadata endpoint) · 198.18/15 (benchmarking) · 224/4 and - # 240/4 (multicast, reserved). IPv6: ::/8 (unspecified, loopback, - # IPv4-mapped/-compatible and the NAT64 prefix — every - # IPv4-embedding spelling) · 2002::/16 (6to4, embeds an IPv4 - # destination) · fc00::/7 (ULA) · fe80::/10 and fec0::/10 - # (link-/site-local) · ff00::/8 (multicast). `(?i)`: hex digits - # may be upper-case. Repetition is spelled with `?`, never a - # `{n}` quantifier, so the `{{ }}` variable delimiters stay the - # only braces in the scalar. - - key: "{{ length(request.object.spec.ingress[].fromCIDR[] || `[]` | [?regex_match('(?i)^((0|10|127)[.]|100[.](6[4-9]|[7-9][0-9]|1[01][0-9]|12[0-7])[.]|169[.]254[.]|172[.](1[6-9]|2[0-9]|3[01])[.]|192[.]168[.]|198[.]1[89][.]|(22[4-9]|2[3-5][0-9])[.]|0*[0-9a-f]?[0-9a-f]?:|2002:|f[cd][0-9a-f][0-9a-f]:|fe[89a-f][0-9a-f]:|ff[0-9a-f][0-9a-f]:)', @)]) }}" - operator: GreaterThan - value: 0 - - key: "{{ length(request.object.spec.ingress[].fromCIDRSet[].cidr || `[]` | [?regex_match('(?i)^((0|10|127)[.]|100[.](6[4-9]|[7-9][0-9]|1[01][0-9]|12[0-7])[.]|169[.]254[.]|172[.](1[6-9]|2[0-9]|3[01])[.]|192[.]168[.]|198[.]1[89][.]|(22[4-9]|2[3-5][0-9])[.]|0*[0-9a-f]?[0-9a-f]?:|2002:|f[cd][0-9a-f][0-9a-f]:|fe[89a-f][0-9a-f]:|ff[0-9a-f][0-9a-f]:)', @)]) }}" - operator: GreaterThan - value: 0 - # A CIDRSet entry naming a GROUP instead of a cidr — `cidrGroupRef` - # or `cidrGroupSelector`, a cluster-scoped CiliumCIDRGroup — - # satisfied the presence check and reached neither the floor nor - # the deny-list, because both read `.cidr` and a projection drops - # the entry that has none. A tenant cannot create groups but can - # name a platform one, and an empty `cidrGroupSelector` names every - # group the platform ever adds. WHITELIST: count the entries that - # carry a literal `cidr` and require that to be all of them. - - key: "{{ length(request.object.spec.ingress[].fromCIDRSet[] || `[]`) }}" - operator: NotEquals - value: "{{ length(request.object.spec.ingress[].fromCIDRSet[].cidr || `[]`) }}" - # …and refuse the group fields OUTRIGHT. Cilium applies every - # populated source in a CIDRSet entry, so an admitted `cidr` beside a - # `cidrGroupRef` — or beside an empty `cidrGroupSelector`, which - # names every group — still grants the group's ranges, and the - # count above admits it (one entry on both sides). Keyed on the - # field NAMES, because an empty selector is falsy to JMESPath. - - key: "{{ length(request.object.spec.ingress[].fromCIDRSet[] || `[]` | [?contains(keys(@), 'cidrGroupRef') || contains(keys(@), 'cidrGroupSelector')]) }}" - operator: NotEquals - value: 0 - - name: cnp-egress-destinations-must-be-constrained - match: - any: - - resources: - kinds: - - CiliumNetworkPolicy - namespaceSelector: - matchExpressions: - - key: app.kubernetes.io/managed-by - operator: In - values: [ksail, kro] - exclude: - any: - - subjects: - - kind: ServiceAccount - name: kustomize-controller - namespace: flux-system - - subjects: - - kind: ServiceAccount - name: kyverno-background-controller - namespace: kyverno - validate: - # Rule-level, not the deprecated top-level spec.validationFailureAction: - # if a future Kyverno stops honouring that field, a rule with no action - # of its own defaults to Audit and these violations would be ADMITTED. - failureAction: Enforce - message: >- - Every egress destination in a tenant CiliumNetworkPolicy must be - constrained. toNodes and toGroups are outside the tenant namespace - boundary and must be omitted. A toEndpoints selector with no matchLabels and no - matchExpressions allows all destinations; a selector key naming a - reserved identity (`reserved:world`, or its bare/`any:` spelling), - another namespace (`k8s:io.kubernetes.pod.namespace`, - `io.cilium.k8s.namespace.labels.*`) or another cluster - (`io.cilium.k8s.policy.cluster`) reaches the same broad destination — - pin a tenant label with the `k8s:` prefix instead; `kube-apiserver` is - the only entity a tenant may name; and a CIDR prefix shorter than /16 - — or a CIDR inside private, cluster, link-local, loopback or multicast - space, or a CIDRSet entry naming a CiliumCIDRGroup, with or without a - literal cidr beside it — reaches outside this - namespace, as do a rule with only toPorts, a toFQDNs pattern of `*`, - and a toServices entry that names no service or does not explicitly - set namespace to this policy's own namespace. An omitted or empty - Service namespace matches Services across namespaces. - Name the destinations this workload actually needs. - deny: - conditions: - any: - - key: "{{ length(request.object.spec.egress[].toEndpoints[] || `[]`) }}" - operator: NotEquals - value: "{{ length(request.object.spec.egress[].toEndpoints[] || `[]` | [?matchLabels || length(matchExpressions[?operator == 'In'] || `[]`) > `0`]) }}" - # Selector KEY whitelist — same shape and reasoning as the ingress - # rule. The only carve-out here is `reserved:kube-apiserver` (with - # its bare/`any:` spelling), mirroring the egress entity whitelist; - # `reserved:ingress` is a SOURCE identity and has no egress case. - - key: "{{ length(request.object.spec.egress[].toEndpoints[] || `[]` | [?length([(matchLabels && keys(matchLabels)) || `[]`, matchExpressions[].key || `[]`][] | [?regex_match('^(reserved:|(any:)?(unknown|host|world|world-ipv4|world-ipv6|unmanaged|health|init|remote-node|kube-apiserver|ingress|cluster|cluster-mesh|all|none)$|(k8s:|any:)?io[.]kubernetes[.]pod[.]namespace$|(k8s:|any:)?io[.]cilium[.]k8s[.](namespace[.]labels[.]|policy[.]cluster$))', @) && @ != 'reserved:kube-apiserver' && @ != 'kube-apiserver' && @ != 'any:kube-apiserver']) > `0`]) }}" - operator: GreaterThan - value: 0 - # Port-only egress rules permit every destination on that port — - # see the ingress note above. - - key: "{{ length(request.object.spec.egress[] || `[]`) }}" - operator: NotEquals - value: "{{ length(request.object.spec.egress[] || `[]` | [?toEndpoints || toCIDR || toCIDRSet || toEntities || toNodes || toGroups || toServices || toFQDNs]) }}" - # Same field-presence boundary as ingress, including an empty - # field beside a permitted destination. - - key: "{{ length(request.object.spec.egress[] || `[]` | [?contains(keys(@), 'toNodes') || contains(keys(@), 'toGroups')]) }}" - operator: GreaterThan - value: 0 - # A toFQDNs destination counts as named above, so the catch-all - # pattern needs refusing on its own — `*` matches every name. - # Counted with a JMESPath equality filter rather than AnyIn: Kyverno - # expands `*` as a WILDCARD in operator comparisons, so an AnyIn - # written against it also refuses a perfectly ordinary - # `*.example.com` (measured). JMESPath `==` is an exact string - # compare, so only the catch-all itself matches. - - key: "{{ length(request.object.spec.egress[].toFQDNs[] || `[]` | [?matchPattern && regex_match('^[*]([.][*])*$', matchPattern)]) }}" - operator: GreaterThan - value: 0 - # Entity whitelist and CIDR prefix-length floor — see the ingress rule - # for why each is a whitelist rather than a list of broad spellings. - - key: "{{ length(request.object.spec.egress[].toEntities[] || `[]` | [?@ != 'kube-apiserver']) }}" - operator: GreaterThan - value: 0 - - key: "{{ length(request.object.spec.egress[].toCIDR[] || `[]` | [?regex_match('^[^/]*/0*([0-9]|1[0-5])$', @)]) }}" - operator: GreaterThan - value: 0 - - key: "{{ length(request.object.spec.egress[].toCIDRSet[].cidr || `[]` | [?regex_match('^[^/]*/0*([0-9]|1[0-5])$', @)]) }}" - operator: GreaterThan - value: 0 - # Protected address-range DENY-LIST and the literal-cidr - # whitelist for CIDRSet entries — same pattern and reasoning as - # the ingress rule. - - key: "{{ length(request.object.spec.egress[].toCIDR[] || `[]` | [?regex_match('(?i)^((0|10|127)[.]|100[.](6[4-9]|[7-9][0-9]|1[01][0-9]|12[0-7])[.]|169[.]254[.]|172[.](1[6-9]|2[0-9]|3[01])[.]|192[.]168[.]|198[.]1[89][.]|(22[4-9]|2[3-5][0-9])[.]|0*[0-9a-f]?[0-9a-f]?:|2002:|f[cd][0-9a-f][0-9a-f]:|fe[89a-f][0-9a-f]:|ff[0-9a-f][0-9a-f]:)', @)]) }}" - operator: GreaterThan - value: 0 - - key: "{{ length(request.object.spec.egress[].toCIDRSet[].cidr || `[]` | [?regex_match('(?i)^((0|10|127)[.]|100[.](6[4-9]|[7-9][0-9]|1[01][0-9]|12[0-7])[.]|169[.]254[.]|172[.](1[6-9]|2[0-9]|3[01])[.]|192[.]168[.]|198[.]1[89][.]|(22[4-9]|2[3-5][0-9])[.]|0*[0-9a-f]?[0-9a-f]?:|2002:|f[cd][0-9a-f][0-9a-f]:|fe[89a-f][0-9a-f]:|ff[0-9a-f][0-9a-f]:)', @)]) }}" - operator: GreaterThan - value: 0 - - key: "{{ length(request.object.spec.egress[].toCIDRSet[] || `[]`) }}" - operator: NotEquals - value: "{{ length(request.object.spec.egress[].toCIDRSet[].cidr || `[]`) }}" - # Group fields refused outright — see the ingress rule. - - key: "{{ length(request.object.spec.egress[].toCIDRSet[] || `[]` | [?contains(keys(@), 'cidrGroupRef') || contains(keys(@), 'cidrGroupSelector')]) }}" - operator: NotEquals - value: 0 - # A toServices entry satisfied the presence check above on its - # mere existence, so the service reference itself was never asked - # what it meant. Cilium resolves each selected Service's backends - # into endpoint selectors, so an unconstrained reference reaches - # arbitrary backends WITHOUT passing the toEndpoints whitelist — - # measured admitted. Same whitelist shape as toEndpoints: count the - # entries that actually name something and require that to be all - # of them, so every empty spelling fails the same way. - - key: "{{ length(request.object.spec.egress[].toServices[] || `[]`) }}" - operator: NotEquals - value: "{{ length(request.object.spec.egress[].toServices[] || `[]` | [?k8sService.serviceName || k8sServiceSelector.selector.matchLabels || length(k8sServiceSelector.selector.matchExpressions[?operator == 'In'] || `[]`) > `0`]) }}" - # Cilium's serviceRefMatches/serviceSelectorMatches treat an empty - # namespace as a wildcard, not as the policy namespace. Compare - # each populated reference against the admission namespace; the - # two checks cannot hide a foreign reference beside a local one. - # JMESPath equality is literal (Kyverno Equals accepts wildcards). - - key: "{{ length(request.object.spec.egress[].toServices[].k8sService || `[]`) }}" - operator: NotEquals - value: "{{ length(request.object.spec.egress[].toServices[].k8sService || `[]` | [?namespace == '{{ request.namespace }}']) }}" - - key: "{{ length(request.object.spec.egress[].toServices[].k8sServiceSelector || `[]`) }}" - operator: NotEquals - value: "{{ length(request.object.spec.egress[].toServices[].k8sServiceSelector || `[]` | [?namespace == '{{ request.namespace }}']) }}" - - name: cnp-must-not-opt-out-of-default-deny - match: - any: - - resources: - kinds: - - CiliumNetworkPolicy - namespaceSelector: - matchExpressions: - - key: app.kubernetes.io/managed-by - operator: In - values: [ksail, kro] - exclude: - any: - - subjects: - - kind: ServiceAccount - name: kustomize-controller - namespace: flux-system - - subjects: - - kind: ServiceAccount - name: kyverno-background-controller - namespace: kyverno - validate: - # Rule-level, not the deprecated top-level spec.validationFailureAction: - # if a future Kyverno stops honouring that field, a rule with no action - # of its own defaults to Audit and these violations would be ADMITTED. - failureAction: Enforce - message: >- - A tenant CiliumNetworkPolicy must not set enableDefaultDeny to false. - The namespace's whitelist mode is what the generated default-deny - establishes; opting out of it is never something a tenant allow-list - needs to do. - deny: - conditions: - any: - # to_string() rather than a `|| default` guard: JMESPath treats the - # boolean false as false-y, so `x || 'unset'` collapses an explicit - # false into the unset case and the check would never fire. - # to_string(null) is "null" and to_string(false) is "false". - - key: "{{ to_string(request.object.spec.enableDefaultDeny.ingress) }}" - operator: Equals - value: "false" - - key: "{{ to_string(request.object.spec.enableDefaultDeny.egress) }}" - operator: Equals - value: "false" - - name: cnp-generated-policy-names-are-reserved - match: - any: - - resources: - kinds: - - CiliumNetworkPolicy - names: - - default-deny - - allow-dns - - allow-cnpg-operator - namespaceSelector: - matchExpressions: - - key: app.kubernetes.io/managed-by - operator: In - values: [ksail, kro] - exclude: - any: - - subjects: - - kind: ServiceAccount - name: kustomize-controller - namespace: flux-system - - subjects: - - kind: ServiceAccount - name: kyverno-background-controller - namespace: kyverno - validate: - # Rule-level, not the deprecated top-level spec.validationFailureAction: - # if a future Kyverno stops honouring that field, a rule with no action - # of its own defaults to Audit and these violations would be ADMITTED. - failureAction: Enforce - message: >- - `default-deny`, `allow-dns` and `allow-cnpg-operator` are generated and - re-synchronised by the - platform (cluster-policies/best-practices/add-default-deny.yaml) and - are reserved names. Add your allow-list as a separate policy — Cilium - unions allow rules, so a policy of your own is additive and does not - need to replace the floor. - deny: {} - - name: cnp-must-use-the-singular-spec-container - match: - any: - - resources: - kinds: - - CiliumNetworkPolicy - namespaceSelector: - matchExpressions: - - key: app.kubernetes.io/managed-by - operator: In - values: [ksail, kro] - exclude: - any: - - subjects: - - kind: ServiceAccount - name: kustomize-controller - namespace: flux-system - - subjects: - - kind: ServiceAccount - name: kyverno-background-controller - namespace: kyverno - validate: - # Rule-level, not the deprecated top-level spec.validationFailureAction: - # if a future Kyverno stops honouring that field, a rule with no action - # of its own defaults to Audit and these violations would be ADMITTED. - failureAction: Enforce - message: >- - A tenant CiliumNetworkPolicy must carry its rules under `spec`, not the - plural `specs` container. Every boundary check in this policy reads - `spec`, so rules hidden under `specs` would reach admission unexamined. - Move the rules into `spec`; a policy needing several rule sets can be - split into several policies, which Cilium unions anyway. - deny: - # CiliumNetworkPolicy accepts EITHER a single `spec` or a plural - # `specs` list, and the two are equivalent to Cilium. Every condition - # in the rules above reads `request.object.spec.*`, so a rule placed - # under `specs` is invisible to all of them and is admitted whatever it - # contains — a complete bypass, not a missing spelling. (This - # repository already handles both shapes elsewhere, in - # scripts/tests/test-crossplane-egress-policy.sh.) - # - # Refusing the container outright is the WHITELIST form of the fix, and - # is deliberately preferred over mirroring every condition across both - # paths: mirroring doubles each check and re-opens the whole boundary - # the first time a future condition is added to one path and not the - # other. A tenant allow-list has no need for `specs`. - conditions: - any: - - key: "{{ length(request.object.specs || `[]`) }}" - operator: GreaterThan - value: 0 + evaluation: + background: + enabled: false + matchConstraints: + namespaceSelector: + matchExpressions: + - key: app.kubernetes.io/managed-by + operator: In + values: [ksail, kro] + resourceRules: + # DELETE is here for the reserved names only: without it a tenant could + # remove the generated floor. Every other check reads the submitted body, + # which a DELETE does not have, so it accepts. + - apiGroups: ["cilium.io"] + apiVersions: ["*"] + operations: ["CREATE", "UPDATE", "DELETE"] + resources: ["ciliumnetworkpolicies"] + matchConditions: + - name: not-platform-applied + expression: >- + request.userInfo.username != 'system:serviceaccount:flux-system:kustomize-controller' + - name: not-generated-by-kyverno + expression: >- + request.userInfo.username != 'system:serviceaccount:kyverno:kyverno-background-controller' + variables: + - name: isDelete + expression: "request.operation == 'DELETE'" + - name: policyName + expression: "variables.isDelete ? oldObject.metadata.name : object.metadata.name" + - name: spec + expression: "variables.isDelete ? dyn({}) : object.?spec.orValue(dyn({}))" + - name: ingress + expression: "variables.spec.?ingress.orValue([])" + - name: egress + expression: "variables.spec.?egress.orValue([])" + # A selector key naming a reserved identity, another namespace or another + # cluster. Keys are matched in all three source spellings: `reserved:`/`k8s:`, + # the explicit `any:`, and no prefix at all (which Cilium defaults to `any:`). + - name: broadSelectorKey + expression: >- + '^(reserved:|(any:)?(unknown|host|world|world-ipv4|world-ipv6|unmanaged|health|init|remote-node|kube-apiserver|ingress|cluster|cluster-mesh|all|none)$|(k8s:|any:)?io[.]kubernetes[.]pod[.]namespace$|(k8s:|any:)?io[.]cilium[.]k8s[.](namespace[.]labels[.]|policy[.]cluster$))' + # Refuse on PREFIX LENGTH, not on prefix text: `0.0.0.0/1` plus `128.0.0.0/1` + # covers an address family while matching neither literal. A bare address + # carries no `/` and is unaffected. + - name: shortPrefix + expression: "'^[^/]*/0*([0-9]|1[0-5])$'" + # The protected ranges from the header, read from the address text. The floor + # above refuses every entry shorter than /16, so each remaining entry is either + # inside one of these blocks or disjoint from it: the leading octets (or + # hextets) decide, and no CIDR arithmetic is needed. IPv4: 0/8, 10/8, 172.16/12, + # 192.168/16, 100.64/10, 127/8, 169.254/16, 198.18/15, 224/4 and 240/4. IPv6: + # ::/8, 2002::/16, fc00::/7, fe80::/10, fec0::/10 and ff00::/8. + - name: protectedRange + expression: >- + '(?i)^((0|10|127)[.]|100[.](6[4-9]|[7-9][0-9]|1[01][0-9]|12[0-7])[.]|169[.]254[.]|172[.](1[6-9]|2[0-9]|3[01])[.]|192[.]168[.]|198[.]1[89][.]|(22[4-9]|2[3-5][0-9])[.]|0*[0-9a-f]?[0-9a-f]?:|2002:|f[cd][0-9a-f][0-9a-f]:|fe[89a-f][0-9a-f]:|ff[0-9a-f][0-9a-f]:)' + + # ── Ingress ────────────────────────────────────────────────────────────── + - name: fromEndpoints + expression: "variables.ingress.map(rule, rule.?fromEndpoints.orValue([])).flatten()" + # WHITELIST OF OPERATORS: a selector counts as constrained only with + # matchLabels or an `In` expression, so every empty spelling and every other + # operator fails the same way. + - name: fromEndpointsConstrained + expression: >- + variables.fromEndpoints.all(selector, + size(selector.?matchLabels.orValue({})) > 0 || + selector.?matchExpressions.orValue([]).exists(item, item.?operator.orValue('') == 'In')) + # Both forms are read, so the `In` form cannot smuggle a key the map form + # refuses. Carved out, mirroring the entity whitelist: `kube-apiserver` and, + # on this side only, `ingress`, each in its reserved/bare/`any:` spelling. + - name: fromEndpointKeysAllowed + expression: >- + variables.fromEndpoints.map(selector, + selector.?matchLabels.orValue({}).map(key, key) + + selector.?matchExpressions.orValue([]).map(item, item.?key.orValue(''))).flatten() + .all(key, !key.matches(variables.broadSelectorKey) || + key in ['reserved:kube-apiserver', 'kube-apiserver', 'any:kube-apiserver', + 'reserved:ingress', 'ingress', 'any:ingress']) + # A rule with no layer-3 stanza permits every source on its port. An empty + # stanza names nothing, so it does not count. + - name: ingressRulesNameSources + expression: >- + variables.ingress.all(rule, + ['fromEndpoints', 'fromCIDR', 'fromCIDRSet', 'fromEntities', 'fromNodes', 'fromGroups'] + .exists(field, field in rule && rule[field] != null && size(rule[field]) > 0)) + # The fields themselves are refused, even empty or null beside a valid source. + - name: ingressOmitsNodesAndGroups + expression: "variables.ingress.all(rule, !('fromNodes' in rule) && !('fromGroups' in rule))" + - name: fromEntitiesAllowed + expression: >- + variables.ingress.map(rule, rule.?fromEntities.orValue([])).flatten() + .all(entity, entity in ['kube-apiserver', 'ingress']) + - name: fromCIDRSets + expression: "variables.ingress.map(rule, rule.?fromCIDRSet.orValue([])).flatten()" + # Every CIDRSet entry must carry a literal cidr and name no CiliumCIDRGroup: + # Cilium applies every populated source in an entry, so a group beside a cidr + # still grants the group's ranges. + - name: fromCIDRSetsLiteral + expression: >- + variables.fromCIDRSets.all(entry, 'cidr' in entry && entry.cidr != null && + !('cidrGroupRef' in entry) && !('cidrGroupSelector' in entry)) + - name: fromAddressesAllowed + expression: >- + (variables.ingress.map(rule, rule.?fromCIDR.orValue([])).flatten() + + variables.fromCIDRSets.filter(entry, 'cidr' in entry && entry.cidr != null).map(entry, entry.cidr)) + .all(address, !address.matches(variables.shortPrefix) && !address.matches(variables.protectedRange)) + + # ── Egress ─────────────────────────────────────────────────────────────── + - name: toEndpoints + expression: "variables.egress.map(rule, rule.?toEndpoints.orValue([])).flatten()" + - name: toEndpointsConstrained + expression: >- + variables.toEndpoints.all(selector, + size(selector.?matchLabels.orValue({})) > 0 || + selector.?matchExpressions.orValue([]).exists(item, item.?operator.orValue('') == 'In')) + # `reserved:ingress` is a SOURCE identity and has no egress case. + - name: toEndpointKeysAllowed + expression: >- + variables.toEndpoints.map(selector, + selector.?matchLabels.orValue({}).map(key, key) + + selector.?matchExpressions.orValue([]).map(item, item.?key.orValue(''))).flatten() + .all(key, !key.matches(variables.broadSelectorKey) || + key in ['reserved:kube-apiserver', 'kube-apiserver', 'any:kube-apiserver']) + - name: egressRulesNameDestinations + expression: >- + variables.egress.all(rule, + ['toEndpoints', 'toCIDR', 'toCIDRSet', 'toEntities', 'toNodes', 'toGroups', 'toServices', 'toFQDNs'] + .exists(field, field in rule && rule[field] != null && size(rule[field]) > 0)) + - name: egressOmitsNodesAndGroups + expression: "variables.egress.all(rule, !('toNodes' in rule) && !('toGroups' in rule))" + # Every label a wildcard (`*`, `*.*`, …) names no destination. A matchName is + # an exact name and unaffected. + - name: toFQDNsNameADestination + expression: >- + variables.egress.map(rule, rule.?toFQDNs.orValue([])).flatten() + .all(fqdn, !fqdn.?matchPattern.orValue('').matches('^[*]([.][*])*$')) + - name: toEntitiesAllowed + expression: >- + variables.egress.map(rule, rule.?toEntities.orValue([])).flatten() + .all(entity, entity == 'kube-apiserver') + - name: toCIDRSets + expression: "variables.egress.map(rule, rule.?toCIDRSet.orValue([])).flatten()" + - name: toCIDRSetsLiteral + expression: >- + variables.toCIDRSets.all(entry, 'cidr' in entry && entry.cidr != null && + !('cidrGroupRef' in entry) && !('cidrGroupSelector' in entry)) + - name: toAddressesAllowed + expression: >- + (variables.egress.map(rule, rule.?toCIDR.orValue([])).flatten() + + variables.toCIDRSets.filter(entry, 'cidr' in entry && entry.cidr != null).map(entry, entry.cidr)) + .all(address, !address.matches(variables.shortPrefix) && !address.matches(variables.protectedRange)) + - name: toServices + expression: "variables.egress.map(rule, rule.?toServices.orValue([])).flatten()" + # Cilium resolves a selected Service's backends into endpoint selectors, so an + # unconstrained reference reaches arbitrary backends without passing the + # toEndpoints whitelist. + - name: toServicesNameSomething + expression: >- + variables.toServices.all(service, + service.?k8sService.?serviceName.orValue('') != '' || + size(service.?k8sServiceSelector.?selector.?matchLabels.orValue({})) > 0 || + service.?k8sServiceSelector.?selector.?matchExpressions.orValue([]) + .exists(item, item.?operator.orValue('') == 'In')) + # Cilium treats an empty Service namespace as a wildcard, not as the policy's + # namespace. Each populated reference is compared on its own, so a local one + # cannot hide a foreign one beside it. + - name: toServicesStayInNamespace + expression: >- + variables.toServices.all(service, + (!('k8sService' in service) || service.k8sService.?namespace.orValue('') == request.namespace) && + (!('k8sServiceSelector' in service) || service.k8sServiceSelector.?namespace.orValue('') == request.namespace)) + validations: + - expression: >- + variables.fromEndpointsConstrained && variables.fromEndpointKeysAllowed && + variables.ingressRulesNameSources && variables.ingressOmitsNodesAndGroups && + variables.fromEntitiesAllowed && variables.fromCIDRSetsLiteral && variables.fromAddressesAllowed + message: >- + Every ingress source in a tenant CiliumNetworkPolicy must be + constrained. fromNodes and fromGroups are outside the tenant namespace + boundary and must be omitted. A fromEndpoints selector with no matchLabels and no + matchExpressions allows all sources; a selector key naming a reserved + identity (`reserved:host`, or its bare/`any:` spelling), another + namespace (`k8s:io.kubernetes.pod.namespace`, + `io.cilium.k8s.namespace.labels.*`) or another cluster + (`io.cilium.k8s.policy.cluster`) reaches the same broad source — pin a + tenant label with the `k8s:` prefix instead, and name the post-Gateway + identity as `reserved:ingress` or `fromEntities: [ingress]`; only + `kube-apiserver` and the post-Gateway `ingress` identity may be named + as an entity; and a CIDR prefix shorter than /16 — or a CIDR inside + private, cluster, link-local, loopback or multicast space, or a + CIDRSet entry naming a CiliumCIDRGroup, with or without a literal cidr beside it — + reaches outside this + namespace. A rule with only toPorts and no source at all allows every + source on that port. Cilium unions allow rules, so one unconstrained + source removes this namespace's isolation regardless of the generated + default-deny. + - expression: >- + variables.toEndpointsConstrained && variables.toEndpointKeysAllowed && + variables.egressRulesNameDestinations && variables.egressOmitsNodesAndGroups && + variables.toFQDNsNameADestination && variables.toEntitiesAllowed && + variables.toCIDRSetsLiteral && variables.toAddressesAllowed && + variables.toServicesNameSomething && variables.toServicesStayInNamespace + message: >- + Every egress destination in a tenant CiliumNetworkPolicy must be + constrained. toNodes and toGroups are outside the tenant namespace + boundary and must be omitted. A toEndpoints selector with no matchLabels and no + matchExpressions allows all destinations; a selector key naming a + reserved identity (`reserved:world`, or its bare/`any:` spelling), + another namespace (`k8s:io.kubernetes.pod.namespace`, + `io.cilium.k8s.namespace.labels.*`) or another cluster + (`io.cilium.k8s.policy.cluster`) reaches the same broad destination — + pin a tenant label with the `k8s:` prefix instead; `kube-apiserver` is + the only entity a tenant may name; and a CIDR prefix shorter than /16 + — or a CIDR inside private, cluster, link-local, loopback or multicast + space, or a CIDRSet entry naming a CiliumCIDRGroup, with or without a + literal cidr beside it — reaches outside this + namespace, as do a rule with only toPorts, a toFQDNs pattern of `*`, + and a toServices entry that names no service or does not explicitly + set namespace to this policy's own namespace. An omitted or empty + Service namespace matches Services across namespaces. + Name the destinations this workload actually needs. + - expression: >- + variables.spec.?enableDefaultDeny.?ingress.orValue(true) != false && + variables.spec.?enableDefaultDeny.?egress.orValue(true) != false + message: >- + A tenant CiliumNetworkPolicy must not set enableDefaultDeny to false. + The namespace's whitelist mode is what the generated default-deny + establishes; opting out of it is never something a tenant allow-list + needs to do. + - expression: "!(variables.policyName in ['default-deny', 'allow-dns', 'allow-cnpg-operator'])" + message: >- + `default-deny`, `allow-dns` and `allow-cnpg-operator` are generated and + re-synchronised by the + platform (cluster-policies/best-practices/add-default-deny.yaml) and + are reserved names. Add your allow-list as a separate policy — Cilium + unions allow rules, so a policy of your own is additive and does not + need to replace the floor. + # CiliumNetworkPolicy accepts EITHER a single `spec` or a plural `specs` list. + # Every check above reads `spec`, so a rule under `specs` is invisible to them. + # Refusing the container is preferred over mirroring every check across both + # paths, which re-opens the boundary the first time one is added to one path. + - expression: "variables.isDelete || size(object.?specs.orValue([])) == 0" + message: >- + A tenant CiliumNetworkPolicy must carry its rules under `spec`, not the + plural `specs` container. Every boundary check in this policy reads + `spec`, so rules hidden under `specs` would reach admission unexamined. + Move the rules into `spec`; a policy needing several rule sets can be + split into several policies, which Cilium unions anyway. diff --git a/k8s/bases/infrastructure/cluster-roles/cilium-tenant-edit.yaml b/k8s/bases/infrastructure/cluster-roles/cilium-tenant-edit.yaml index 9bf0bd03ff..763d79436b 100644 --- a/k8s/bases/infrastructure/cluster-roles/cilium-tenant-edit.yaml +++ b/k8s/bases/infrastructure/cluster-roles/cilium-tenant-edit.yaml @@ -7,7 +7,7 @@ # These verbs also let a tenant change any namespaced CiliumNetworkPolicy, and # Cilium combines matching allow rules, so RBAC alone does not make the # namespace boundary hold. Admission does: the -# `restrict-tenant-network-policies` ClusterPolicy requires every tenant ingress +# `restrict-tenant-network-policies` ValidatingPolicy requires every tenant ingress # source and egress destination to carry a real constraint, refuses the broad # `all`/`world`/`cluster` entities and the world-equivalent CIDRs, and reserves # the generated `default-deny` and `allow-dns` names — so these verbs stay diff --git a/scripts/tests/tenant-webapp-admission/main.go b/scripts/tests/tenant-webapp-admission/main.go index 57a05ef51d..8a0e880319 100644 --- a/scripts/tests/tenant-webapp-admission/main.go +++ b/scripts/tests/tenant-webapp-admission/main.go @@ -124,7 +124,7 @@ func evaluate(f fixture, policies map[string]*policiesv1beta1.ValidatingPolicy) if target == nil { target = f.OldObject } - resources := map[string]string{"WebApp": "webapps", "Deployment": "deployments", "Service": "services", "HTTPRoute": "httproutes", "ExternalSecret": "externalsecrets", "SecretStore": "secretstores", "PushSecret": "pushsecrets", "ConfigMap": "configmaps"} + resources := map[string]string{"CiliumNetworkPolicy": "ciliumnetworkpolicies", "WebApp": "webapps", "Deployment": "deployments", "Service": "services", "HTTPRoute": "httproutes", "ExternalSecret": "externalsecrets", "SecretStore": "secretstores", "PushSecret": "pushsecrets", "ConfigMap": "configmaps"} gvk := target.GroupVersionKind() resource, ok := resources[gvk.Kind] if !ok || gvk.Version == "" { diff --git a/scripts/tests/test-restrict-tenant-network-policies.sh b/scripts/tests/test-restrict-tenant-network-policies.sh index 039c34dd42..7256b612e2 100755 --- a/scripts/tests/test-restrict-tenant-network-policies.sh +++ b/scripts/tests/test-restrict-tenant-network-policies.sh @@ -5,13 +5,14 @@ script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" repo_root="$(cd "${script_dir}/../.." && pwd)" policy="${repo_root}/k8s/bases/infrastructure/cluster-policies/best-practices/restrict-tenant-network-policies.yaml" tests="${repo_root}/tests/restrict-tenant-network-policies" -output_file="$(mktemp)" -trap 'rm -f "${output_file}"' EXIT +work="$(mktemp -d)" +output_file="${work}/output" +trap 'rm -rf "${work}"' EXIT -# `kyverno test` treats a rule that matches nothing as Excluded, and an Excluded -# row satisfies `result: fail` too — so the declarative test can pass vacuously -# if a rule silently stops matching. Exercise the policy directly as a second -# gate, asserting the exact allow/deny counts rather than an exit code. +# `kyverno test` treats a resource the policy does not evaluate as Excluded, and an +# Excluded row satisfies `result: fail` too — so the declarative test can pass +# vacuously if the policy silently stops matching. Exercise the policy directly as a +# second gate, asserting the exact allow/deny counts rather than an exit code. apply() { local resources="$1" values="$2" userinfo="$3" kyverno apply "${policy}" \ @@ -34,7 +35,7 @@ expect_summary() { # additive allow-lists admitted. A drop in the fail count means a shape that # used to be refused now admits. apply "${tests}/resources.yaml" "${tests}/values.yaml" "${tests}/user-info.yaml" -expect_summary "pass: 309, fail: 78, warn: 0, error: 0, skip: 0" \ +expect_summary "pass: 18, fail: 78, warn: 0, error: 0, skip: 0" \ "tenant CiliumNetworkPolicy boundary" # 2. Carve-out: kyverno's background controller owns the generated floor. These @@ -42,77 +43,164 @@ expect_summary "pass: 309, fail: 78, warn: 0, error: 0, skip: 0" \ # regression here would block the platform from creating its own default-deny. apply "${tests}/kyverno-author/resources.yaml" \ "${tests}/kyverno-author/values.yaml" "${tests}/kyverno-author/user-info.yaml" -expect_summary "pass: 0, fail: 0, warn: 0, error: 0, skip: 0" \ +expect_summary "pass: 0, fail: 0, warn: 0, error: 0, skip: 3" \ "generated default-deny/allow-dns/allow-cnpg-operator must remain creatable by kyverno" # 3. CONTROL for 2: the identical bodies, in the identical namespace, submitted -# by the TENANT. Only the applying identity differs. Without this the zero -# above is indistinguishable from the policy simply not matching. All three -# bodies fail the reserved-name rule; allow-dns ALSO fails the egress rule, -# because it reaches CoreDNS by naming kube-system through the namespace -# label, and allow-cnpg-operator ALSO fails the ingress rule for naming -# cnpg-system — the cross-namespace selector a tenant is refused (#3399, -# #3655). default-deny ALSO fails both the ingress and the egress rule: its -# one empty rule per direction names no source or destination, which those -# rules refuse because a sourceless rule WITH toPorts allows every peer on -# that port. Without toPorts, as here, Cilium allows nothing (#3501), so the -# refusal is conservative, not a hole. The generated floor is exempt from all -# of that only through its applying identity, which is the point. +# by the TENANT. Only the applying identity differs, and all three are refused. +# Besides the reserved name, allow-dns reaches CoreDNS by naming kube-system +# through the namespace label and allow-cnpg-operator names cnpg-system, the +# cross-namespace selector a tenant is refused (#3399, #3655). default-deny's +# one empty rule per direction names no source or destination, which is refused +# because a sourceless rule WITH toPorts allows every peer on that port. Without +# toPorts, as here, Cilium allows nothing (#3501), so the refusal is conservative, +# not a hole. The generated floor is exempt only through its applying identity. apply "${tests}/kyverno-author/resources.yaml" \ "${tests}/kyverno-author/values.yaml" "${tests}/user-info.yaml" -expect_summary "pass: 8, fail: 7, warn: 0, error: 0, skip: 0" \ +expect_summary "pass: 0, fail: 3, warn: 0, error: 0, skip: 0" \ "control: a tenant must NOT be able to author the reserved generated names" # 4. Carve-out: platform-authored policies applied by flux-system's # kustomize-controller stay permitted even when broad. apply "${tests}/platform-author/resources.yaml" \ "${tests}/platform-author/values.yaml" "${tests}/platform-author/user-info.yaml" -expect_summary "pass: 0, fail: 0, warn: 0, error: 0, skip: 0" \ +expect_summary "pass: 0, fail: 0, warn: 0, error: 0, skip: 1" \ "platform-authored policies must stay permitted" -# 5. CONTROL for 4: the identical broad body submitted by the TENANT is refused -# by both the ingress and the egress rule. +# 5. CONTROL for 4: the identical broad body submitted by the TENANT is refused. apply "${tests}/platform-author/resources.yaml" \ "${tests}/platform-author/values.yaml" "${tests}/user-info.yaml" -expect_summary "pass: 2, fail: 2, warn: 0, error: 0, skip: 0" \ +expect_summary "pass: 0, fail: 1, warn: 0, error: 0, skip: 0" \ "control: a tenant must NOT be able to submit the broad platform body" # 6. Node/provider fields are forbidden even when empty beside a valid rule. apply "${tests}/node-group-resources.yaml" "${tests}/values.yaml" "${tests}/user-info.yaml" -expect_summary "pass: 48, fail: 16, warn: 0, error: 0, skip: 0" \ +expect_summary "pass: 0, fail: 16, warn: 0, error: 0, skip: 0" \ "tenant node/provider selectors must remain forbidden" # 7. The day-one policy platform-tenant-template ships must stay admissible # under the tenant's own identity. Tightening this boundary has made a freshly # generated tenant inadmissible three times, each found only by the -# template's scheduled run (#3652); this makes the next one fail here. Four -# rules evaluate it, and none may refuse it. +# template's scheduled run (#3652); this makes the next one fail here. apply "${tests}/tenant-scaffold/resources.yaml" \ "${tests}/tenant-scaffold/values.yaml" "${tests}/tenant-scaffold/user-info.yaml" -expect_summary "pass: 4, fail: 0, warn: 0, error: 0, skip: 0" \ +expect_summary "pass: 1, fail: 0, warn: 0, error: 0, skip: 0" \ "the platform-tenant-template scaffold must be admissible on day one" -# 8. Static action guard. Every count above is a RULE result; none of them says -# what admission actually does on a violation. Enforcement lives in each -# rule's own `failureAction`, because the deprecated top-level -# spec.validationFailureAction defaults an action-less rule to Audit — which -# would admit every shape the counts above prove is "denied", with the whole -# suite still green. Assert the action itself, not just the verdicts. -rule_count="$(yq '.spec.rules | length' "${policy}")" -enforced="$(yq '[.spec.rules[] | select(.validate.failureAction == "Enforce")] | length' "${policy}")" -top_level="$(yq '.spec.validationFailureAction // "absent"' "${policy}")" -if [[ "${rule_count}" -lt 1 ]]; then - echo "::error::no rules found in ${policy}" +# 8. Static action guard. Every count above is a policy verdict; none of them says +# what admission does on a violation. Audit would admit every shape the counts +# above prove is refused, with the whole suite still green. +if ! yq -o=json '.' "${policy}" | jq -e ' + .kind == "ValidatingPolicy" and .spec.failurePolicy == "Fail" and + .spec.validationActions == ["Deny"] and .spec.evaluation.background.enabled == false and + .spec.matchConstraints.resourceRules == [{ + apiGroups: ["cilium.io"], apiVersions: ["*"], operations: ["CREATE", "UPDATE", "DELETE"], + resources: ["ciliumnetworkpolicies"]}] +' >/dev/null; then + echo "::error::scope, operations or fail-closed disposition of ${policy} changed" exit 1 fi -if [[ "${enforced}" != "${rule_count}" ]]; then - echo "::error::only ${enforced}/${rule_count} rules set validate.failureAction: Enforce" - exit 1 -fi -if [[ "${top_level}" != "absent" ]]; then - echo "::error::spec.validationFailureAction is deprecated; enforcement must be rule-level" + +# 9. One result per resource cannot say which check refused it, and a resource can +# fail more than one. Run each check alone against the rows recorded for it in +# per-check.yaml, and require those rows to go red when it accepts everything. +checks=( + cnp-ingress-sources-must-be-constrained + cnp-egress-destinations-must-be-constrained + cnp-must-not-opt-out-of-default-deny + cnp-generated-policy-names-are-reserved + cnp-must-use-the-singular-spec-container +) +if [[ "$(yq '.spec.validations | length' "${policy}")" != "${#checks[@]}" ]]; then + echo "::error::the policy's checks and the ${#checks[@]} names here differ" exit 1 fi +for index in "${!checks[@]}"; do + check="${checks[${index}]}" + fail_rows=0 + for rows_file in "${tests}/per-check.yaml" "${tests}"/*/per-check.yaml; do + fixture="$(dirname "${rows_file}")" + case_dir="${work}/${index}-$(basename "${fixture}")" + mkdir -p "${case_dir}" + CHECK="${check}" yq -o=json '.results | map(select(.rule == strenv(CHECK)))' "${rows_file}" \ + >"${case_dir}/rows.json" + [[ "$(jq 'length' "${case_dir}/rows.json")" -gt 0 ]] || continue + refused="$(jq '[.[] | select(.result == "fail") | .resources[]] | length' "${case_dir}/rows.json")" + fail_rows=$((fail_rows + refused)) + yq -o=json '{"resources": .resources, "variables": .variables, "userinfo": .userinfo}' "${rows_file}" \ + >"${case_dir}/inputs.json" + for variant in kept weakened; do + target="${case_dir}/${variant}" + mkdir -p "${target}" + find "${fixture}" -maxdepth 1 -type f -name '*.yaml' ! -name 'kyverno-test.yaml' ! -name 'per-check.yaml' \ + -exec cp {} "${target}/" \; + if [[ "${variant}" == kept ]]; then + INDEX="${index}" yq '.spec.validations = [.spec.validations[env(INDEX)]]' "${policy}" >"${target}/policy.yaml" + else + INDEX="${index}" yq '.spec.validations = [.spec.validations[env(INDEX)] | .expression = "true"]' \ + "${policy}" >"${target}/policy.yaml" + fi + jq --slurpfile inputs "${case_dir}/inputs.json" \ + '{apiVersion: "cli.kyverno.io/v1alpha1", kind: "Test", metadata: {name: "check"}, + policies: ["policy.yaml"], resources: $inputs[0].resources, + variables: $inputs[0].variables, userinfo: $inputs[0].userinfo, + results: map(del(.rule))}' "${case_dir}/rows.json" >"${target}/kyverno-test.yaml" + done + if ! kyverno test "${case_dir}/kept" --remove-color >"${case_dir}/kept.log" 2>&1; then + cat "${case_dir}/kept.log" + echo "::error::${check}: its rows in ${rows_file} do not hold when it runs alone" + exit 1 + fi + if [[ "${refused}" -gt 0 ]] && kyverno test "${case_dir}/weakened" --remove-color >"${case_dir}/weakened.log" 2>&1; then + echo "::error::${check}: its fail rows in ${rows_file} still hold with the check removed" + exit 1 + fi + done + if [[ "${fail_rows}" -eq 0 ]]; then + echo "::error::${check}: no fixture proves this check refuses anything" + exit 1 + fi + echo "ok: ${check} (${fail_rows} refused)" +done + +# 10. The CLI cannot send a DELETE or an UPDATE's old object, so these go through the +# pinned admission engine. A tenant holds delete on CiliumNetworkPolicy, so +# without the DELETE rows a tenant could remove the generated floor. +tenant='system:serviceaccount:tenant-ns:tenant' +kyverno_sa='system:serviceaccount:kyverno:kyverno-background-controller' +flux_sa='system:serviceaccount:flux-system:kustomize-controller' +cnp() { + jq -cn --arg name "$1" '{apiVersion: "cilium.io/v2", kind: "CiliumNetworkPolicy", + metadata: {name: $name, namespace: "tenant-ns"}, + spec: {endpointSelector: {}, ingress: [{fromEndpoints: [{matchLabels: {app: "web"}}]}]}}' +} +control() { + local label="$1" actor="$2" operation="$3" name="$4" expected="$5" object='null' old='null' + [[ "${operation}" == DELETE ]] || object="$(cnp "${name}")" + [[ "${operation}" == CREATE ]] || old="$(cnp "${name}")" + jq -cn --arg label "${label}" --arg actor "${actor}" --arg operation "${operation}" \ + --arg expected "${expected}" --argjson object "${object}" --argjson old "${old}" ' + {label: $label, policy: "restrict-tenant-network-policies", actor: $actor, operation: $operation, + expected: $expected, namespaces: [{metadata: {name: "tenant-ns", labels: {"app.kubernetes.io/managed-by": "ksail"}}}]} + + (if $object == null then {} else {object: $object} end) + + (if $old == null then {} else {oldObject: $old} end)' +} +{ + control tenant-deletes-own-policy "${tenant}" DELETE allow-web pass + for reserved in default-deny allow-dns allow-cnpg-operator; do + control "tenant-deletes-${reserved}" "${tenant}" DELETE "${reserved}" fail + control "tenant-updates-${reserved}" "${tenant}" UPDATE "${reserved}" fail + control "kyverno-deletes-${reserved}" "${kyverno_sa}" DELETE "${reserved}" skip + control "flux-deletes-${reserved}" "${flux_sa}" DELETE "${reserved}" skip + done + control tenant-updates-own-policy "${tenant}" UPDATE allow-web pass +} >"${work}/controls.jsonl" +( + cd "${repo_root}/scripts/tests/tenant-webapp-admission" + go test -mod=readonly -count=1 -timeout 60s ./... + go build -mod=readonly -o "${work}/admission" . +) +"${work}/admission" "${policy}" <"${work}/controls.jsonl" echo "Tenant CiliumNetworkPolicy policy enforced the expected boundary, and both" echo "platform carve-outs were proven to depend on the applying identity." diff --git a/scripts/tests/test-tenant-controller-namespace-coverage.sh b/scripts/tests/test-tenant-controller-namespace-coverage.sh index 45143f2bfa..c5779b9e07 100755 --- a/scripts/tests/test-tenant-controller-namespace-coverage.sh +++ b/scripts/tests/test-tenant-controller-namespace-coverage.sh @@ -26,7 +26,7 @@ done <<'CASES' restrict-tenant-secret-stores 5 7 restrict-tenant-issuer-refs 4 3 restrict-tenant-route-hostnames 3 13 -restrict-tenant-network-policies 309 78 +restrict-tenant-network-policies 18 78 CASES [[ "$failures" == 0 ]] || exit 1 echo 'PASS: all four tenant admission families evaluate their full controls under ksail and kro ownership' diff --git a/scripts/tests/test-validate-kyverno-fixture-evaluation.sh b/scripts/tests/test-validate-kyverno-fixture-evaluation.sh index 4fa6979088..ed9837dd09 100755 --- a/scripts/tests/test-validate-kyverno-fixture-evaluation.sh +++ b/scripts/tests/test-validate-kyverno-fixture-evaluation.sh @@ -93,40 +93,40 @@ if kyverno_passes "shorthand kinds" "${root}"; then fi # #3152: a deleted rule leaves its rows Excluded. -root="$(copy deleted-rule restrict-tenant-network-policies)" -yq -i 'del(.spec.rules[] | select(.name == "cnp-ingress-sources-must-be-constrained"))' \ - "${root}/${policies}/restrict-tenant-network-policies.yaml" +root="$(copy deleted-rule restrict-arc-retirement-listener-pod)" +yq -i 'del(.spec.rules[] | select(.name == "fence-retired-listener-pod"))' \ + "${root}/${policies}/restrict-arc-retirement.yaml" if kyverno_passes "deleted rule" "${root}"; then - expect_fail "deleted rule" "${root}" 1 "names rule cnp-ingress-sources-must-be-constrained of policy restrict-tenant-network-policies, which the policies it loads do not define" + expect_fail "deleted rule" "${root}" 1 "names rule fence-retired-listener-pod of policy restrict-arc-retirement, which the policies it loads do not define" fi # #3152: a renamed rule is the same break under another name. -root="$(copy renamed-rule restrict-tenant-network-policies)" -yq -i '(.spec.rules[] | select(.name == "cnp-ingress-sources-must-be-constrained") | .name) = "cnp-ingress-sources"' \ - "${root}/${policies}/restrict-tenant-network-policies.yaml" +root="$(copy renamed-rule restrict-arc-retirement-listener-pod)" +yq -i '(.spec.rules[] | select(.name == "fence-retired-listener-pod") | .name) = "fence-retired-listener"' \ + "${root}/${policies}/restrict-arc-retirement.yaml" if kyverno_passes "renamed rule" "${root}"; then - expect_fail "renamed rule" "${root}" 1 "names rule cnp-ingress-sources-must-be-constrained" + expect_fail "renamed rule" "${root}" 1 "names rule fence-retired-listener-pod" fi # A rule named only by `skip` rows passes the Excluded check, so only the # existence check can catch its removal. -root="$(copy skip-only restrict-tenant-network-policies)" +root="$(copy skip-only restrict-arc-retirement-listener-pod)" mkdir -p "${root}/skip-only" -mv "${root}/tests/restrict-tenant-network-policies" "${root}/skip-only/" -yq -i '.results |= map(select(.result == "skip"))' "${root}/skip-only/restrict-tenant-network-policies/kyverno-test.yaml" +mv "${root}/tests/restrict-arc-retirement-listener-pod" "${root}/skip-only/" +yq -i '.results |= map(select(.result == "skip"))' "${root}/skip-only/restrict-arc-retirement-listener-pod/kyverno-test.yaml" expect_pass "a skip-only fixture whose rule exists" "${root}" skip-only -yq -i 'del(.spec.rules[] | select(.name == "cnp-ingress-sources-must-be-constrained"))' \ - "${root}/${policies}/restrict-tenant-network-policies.yaml" +yq -i 'del(.spec.rules[] | select(.name == "fence-retired-listener-pod"))' \ + "${root}/${policies}/restrict-arc-retirement.yaml" if kyverno_passes "skip-only rule removed" "${root}" skip-only; then - expect_fail "skip-only rule removed" "${root}" 1 "names rule cnp-ingress-sources-must-be-constrained" skip-only + expect_fail "skip-only rule removed" "${root}" 1 "names rule fence-retired-listener-pod" skip-only fi # #3392: a results entry naming a resource the test never loads gets no row at all. -root="$(copy renamed-resource restrict-tenant-network-policies)" -yq -i '(.results[].resources[] | select(. == "tenant-ns/attack-toservices-foreign-named")) = "tenant-ns/attack-toservices-foreign-renamed"' \ - "${root}/tests/restrict-tenant-network-policies/kyverno-test.yaml" +root="$(copy renamed-resource restrict-arc-retirement-listener-pod)" +yq -i '(.results[].resources[] | select(. == "arc-systems/listener-restored")) = "arc-systems/listener-renamed"' \ + "${root}/tests/restrict-arc-retirement-listener-pod/kyverno-test.yaml" if kyverno_passes "renamed resource" "${root}"; then - expect_fail "renamed resource" "${root}" 1 "tenant-ns/attack-toservices-foreign-renamed, but kyverno ran no assertion for it" + expect_fail "renamed resource" "${root}" 1 "arc-systems/listener-renamed, but kyverno ran no assertion for it" fi # A generate row names the object it generates, which need not share its trigger's name. diff --git a/tests/policy-failure-actions.json b/tests/policy-failure-actions.json index d8cbb1b646..4be6d04ffc 100644 --- a/tests/policy-failure-actions.json +++ b/tests/policy-failure-actions.json @@ -19,11 +19,6 @@ "restrict-github-team-management/teamrepositories-maintainers-capped-below-admin": "Enforce", "restrict-homepage-service-groups/declared-service-group": "Enforce", "restrict-tenant-issuer-refs": "Enforce", - "restrict-tenant-network-policies/cnp-ingress-sources-must-be-constrained": "Enforce", - "restrict-tenant-network-policies/cnp-egress-destinations-must-be-constrained": "Enforce", - "restrict-tenant-network-policies/cnp-must-not-opt-out-of-default-deny": "Enforce", - "restrict-tenant-network-policies/cnp-generated-policy-names-are-reserved": "Enforce", - "restrict-tenant-network-policies/cnp-must-use-the-singular-spec-container": "Enforce", "restrict-tenant-route-hostnames/wedding-app-approved-hostnames": "Enforce", "restrict-tenant-route-hostnames/ascoachingogvaner-approved-hostnames": "Enforce", "restrict-tenant-route-hostnames/unlisted-tenant-namespace-has-no-approved-hostnames": "Enforce", @@ -48,5 +43,6 @@ "restrict-arc-retirement/fence-retired-secret": "Enforce", "restrict-arc-retirement/preserve-retirement-journal": "Enforce", "restrict-arc-retirement/preserve-retiring-namespace": "Enforce", - "restrict-arc-openbao-listener-settings": "Enforce" + "restrict-arc-openbao-listener-settings": "Enforce", + "restrict-tenant-network-policies": "Enforce" } diff --git a/tests/restrict-tenant-network-policies/kyverno-author/kyverno-test.yaml b/tests/restrict-tenant-network-policies/kyverno-author/kyverno-test.yaml index 093eeda178..eed05270a8 100644 --- a/tests/restrict-tenant-network-policies/kyverno-author/kyverno-test.yaml +++ b/tests/restrict-tenant-network-policies/kyverno-author/kyverno-test.yaml @@ -1,4 +1,8 @@ --- +# One row per resource: fail when any check refuses it. Which check refuses it is +# in per-check.yaml, which scripts/tests/test-restrict-tenant-network-policies.sh +# runs. The policy uses a namespaceSelector, so the CLI needs the namespace labels +# declared explicitly. apiVersion: cli.kyverno.io/v1alpha1 kind: Test metadata: @@ -11,15 +15,9 @@ resources: variables: values.yaml userinfo: user-info.yaml results: - # The background controller creates and re-synchronises the generated floor. - # Both objects carry a reserved name, so without this carve-out the policy - # would refuse the very default-deny it exists to protect. The sibling shell - # test proves the same two bodies ARE denied under the tenant identity, so - # these skips cannot be a namespace-label mismatch in disguise. - policy: restrict-tenant-network-policies - rule: cnp-generated-policy-names-are-reserved - resources: - - tenant-ns/default-deny - - tenant-ns/allow-dns kind: CiliumNetworkPolicy result: skip + resources: + - tenant-ns/allow-dns + - tenant-ns/default-deny diff --git a/tests/restrict-tenant-network-policies/kyverno-author/per-check.yaml b/tests/restrict-tenant-network-policies/kyverno-author/per-check.yaml new file mode 100644 index 0000000000..093eeda178 --- /dev/null +++ b/tests/restrict-tenant-network-policies/kyverno-author/per-check.yaml @@ -0,0 +1,25 @@ +--- +apiVersion: cli.kyverno.io/v1alpha1 +kind: Test +metadata: + name: restrict-tenant-network-policies-kyverno-author +policies: + - >- + ../../../k8s/bases/infrastructure/cluster-policies/best-practices/restrict-tenant-network-policies.yaml +resources: + - resources.yaml +variables: values.yaml +userinfo: user-info.yaml +results: + # The background controller creates and re-synchronises the generated floor. + # Both objects carry a reserved name, so without this carve-out the policy + # would refuse the very default-deny it exists to protect. The sibling shell + # test proves the same two bodies ARE denied under the tenant identity, so + # these skips cannot be a namespace-label mismatch in disguise. + - policy: restrict-tenant-network-policies + rule: cnp-generated-policy-names-are-reserved + resources: + - tenant-ns/default-deny + - tenant-ns/allow-dns + kind: CiliumNetworkPolicy + result: skip diff --git a/tests/restrict-tenant-network-policies/kyverno-test.yaml b/tests/restrict-tenant-network-policies/kyverno-test.yaml index 41f26837bd..6a25b64506 100644 --- a/tests/restrict-tenant-network-policies/kyverno-test.yaml +++ b/tests/restrict-tenant-network-policies/kyverno-test.yaml @@ -1,4 +1,8 @@ --- +# One row per resource: fail when any check refuses it. Which check refuses it is +# in per-check.yaml, which scripts/tests/test-restrict-tenant-network-policies.sh +# runs. The policy uses a namespaceSelector, so the CLI needs the namespace labels +# declared explicitly. apiVersion: cli.kyverno.io/v1alpha1 kind: Test metadata: @@ -9,306 +13,131 @@ policies: resources: - resources.yaml - node-group-resources.yaml -# The policy uses a namespaceSelector, so the CLI needs the namespace labels -# declared explicitly or every case is reported as Excluded — which satisfies -# `result: fail` too and would pass vacuously. The companion shell test -# (scripts/tests/test-restrict-tenant-network-policies.sh) is the second gate: -# it runs `kyverno apply` and asserts the exact allow/deny counts, so a rule -# that stopped matching cannot go unnoticed here. variables: values.yaml userinfo: user-info.yaml results: - # ── Every semantically empty source selector, one spelling per case ───────── - policy: restrict-tenant-network-policies - rule: cnp-ingress-sources-must-be-constrained - resources: - - tenant-ns/ingress-empty-object - - tenant-ns/ingress-empty-matchlabels - - tenant-ns/ingress-empty-matchexpressions - - tenant-ns/ingress-both-fields-empty - - tenant-ns/ingress-vacuous-beside-constrained - - tenant-ns/ingress-vacuous-in-second-rule - - tenant-ns/ingress-entity-all - - tenant-ns/ingress-entity-cluster - - tenant-ns/ingress-bare-l4 - - tenant-ns/ingress-entity-remote-node - - tenant-ns/ingress-cidr-split-v4 - kind: CiliumNetworkPolicy - result: fail - - policy: restrict-tenant-network-policies - rule: cnp-ingress-sources-must-be-constrained - resources: - - tenant-ns/allow-api-to-web - - tenant-ns/allow-web-egress-db - - tenant-ns/allow-web-egress-specific-cidr - - tenant-ns/allow-web-egress-named-fqdn - - tenant-ns/allow-web-egress-subdomain-fqdn - - tenant-ns/allow-all-own-pods-from-api - - tenant-ns/allow-web-ingress-from-gateway - kind: CiliumNetworkPolicy - result: pass - # ── Egress destinations ──────────────────────────────────────────────────── - - policy: restrict-tenant-network-policies - rule: cnp-egress-destinations-must-be-constrained - resources: - - tenant-ns/egress-empty-object - - tenant-ns/egress-entity-world - - tenant-ns/egress-cidr-world - - tenant-ns/egress-cidrset-world-v6 - - tenant-ns/egress-bare-l4 - - tenant-ns/egress-fqdn-wildcard - - tenant-ns/egress-entity-cluster-mesh - - tenant-ns/egress-cidrset-split-v6 - kind: CiliumNetworkPolicy - result: fail - - policy: restrict-tenant-network-policies - rule: cnp-egress-destinations-must-be-constrained - resources: - - tenant-ns/allow-api-to-web - - tenant-ns/allow-web-egress-db - - tenant-ns/allow-web-egress-specific-cidr - - tenant-ns/allow-web-egress-named-fqdn - - tenant-ns/allow-web-egress-subdomain-fqdn - - tenant-ns/allow-all-own-pods-from-api - kind: CiliumNetworkPolicy - result: pass - # ── Opting out of the namespace's default-deny ───────────────────────────── - - policy: restrict-tenant-network-policies - rule: cnp-must-not-opt-out-of-default-deny - resources: - - tenant-ns/opt-out-default-deny-ingress - - tenant-ns/opt-out-default-deny-egress kind: CiliumNetworkPolicy result: fail - - policy: restrict-tenant-network-policies - rule: cnp-must-not-opt-out-of-default-deny - resources: - - tenant-ns/allow-api-to-web - - tenant-ns/allow-web-egress-db - kind: CiliumNetworkPolicy - result: pass - # ── Ownership: the generated floor's names are reserved ──────────────────── - - policy: restrict-tenant-network-policies - rule: cnp-generated-policy-names-are-reserved resources: - - tenant-ns/default-deny - - tenant-ns/allow-dns - tenant-ns/allow-cnpg-operator - kind: CiliumNetworkPolicy - result: fail - # ── A platform namespace is out of scope entirely ────────────────────────── - - policy: restrict-tenant-network-policies - rule: cnp-ingress-sources-must-be-constrained - resources: - - platform-ns/platform-allow-all - kind: CiliumNetworkPolicy - result: skip - # ── Shapes that PASS a presence check but are semantically unbounded ─────── - # Each of these was measured ADMITTED before the whitelist forms landed: the - # per-entry checks all saw "something is there" and never asked what it meant. - - policy: restrict-tenant-network-policies - rule: cnp-egress-destinations-must-be-constrained - resources: + - tenant-ns/allow-dns + - tenant-ns/attack-cidr-cgnat-egress + - tenant-ns/attack-cidr-every-slash16-v4-egress + - tenant-ns/attack-cidr-every-slash16-v6-egress + - tenant-ns/attack-cidr-ipv4-mapped-v6-egress + - tenant-ns/attack-cidr-link-local-metadata-egress + - tenant-ns/attack-cidr-link-local-v6-ingress + - tenant-ns/attack-cidr-loopback-egress + - tenant-ns/attack-cidr-multicast-egress + - tenant-ns/attack-cidr-node-private-network-ingress + - tenant-ns/attack-cidr-pod-network-egress - tenant-ns/attack-cidr-prefix-floor-slash8 + - tenant-ns/attack-cidr-private-172-cidrset-egress + - tenant-ns/attack-cidr-reserved-beside-legitimate-egress + - tenant-ns/attack-cidr-six-to-four-egress + - tenant-ns/attack-cidr-ula-v6-egress + - tenant-ns/attack-cidr-ula-v6-uppercase-egress - tenant-ns/attack-cidr-zero-padded-prefix + - tenant-ns/attack-cidrset-group-ref-beside-cidr-egress + - tenant-ns/attack-cidrset-group-ref-beside-cidr-ingress + - tenant-ns/attack-cidrset-group-ref-egress + - tenant-ns/attack-cidrset-group-ref-ingress + - tenant-ns/attack-cidrset-group-selector-beside-cidr-egress + - tenant-ns/attack-cidrset-group-selector-beside-cidr-ingress + - tenant-ns/attack-cidrset-group-selector-egress - tenant-ns/attack-fqdn-wildcard-depths - kind: CiliumNetworkPolicy - result: fail - - policy: restrict-tenant-network-policies - rule: cnp-ingress-sources-must-be-constrained - resources: - - tenant-ns/attack-selector-exists-universal - kind: CiliumNetworkPolicy - result: fail - # ── The plural rule container, which every `spec`-reading condition misses ── - - policy: restrict-tenant-network-policies - rule: cnp-must-use-the-singular-spec-container - resources: - - tenant-ns/attack-specs-container-egress-world - kind: CiliumNetworkPolicy - result: fail - - policy: restrict-tenant-network-policies - rule: cnp-must-use-the-singular-spec-container - resources: - - tenant-ns/allow-api-to-web - - tenant-ns/allow-web-egress-db - kind: CiliumNetworkPolicy - result: pass - # ── toServices: a reference that names nothing is still "present" ────────── - # Every one of these satisfied the egress presence check on the mere - # existence of a toServices key, and reached arbitrary Service backends - # without passing the toEndpoints selector whitelist. - - policy: restrict-tenant-network-policies - rule: cnp-egress-destinations-must-be-constrained - resources: - - tenant-ns/attack-toservices-empty-selector - - tenant-ns/attack-toservices-empty-matchlabels - - tenant-ns/attack-toservices-unnamed-service - - tenant-ns/attack-toservices-empty-entry - - tenant-ns/attack-toservices-vacuous-beside-constrained - kind: CiliumNetworkPolicy - result: fail - # CONTROL: the constrained spellings stay admitted. - - policy: restrict-tenant-network-policies - rule: cnp-egress-destinations-must-be-constrained - resources: - - tenant-ns/allow-web-egress-named-service - - tenant-ns/allow-web-egress-selected-service - kind: CiliumNetworkPolicy - result: pass - # ── Selector KEYS that name an identity, a namespace or a cluster ────────── - # Every one of these is non-empty and uses only whitelisted operators, so the - # presence and operator checks read it as constrained. Cilium exposes reserved - # identities, the pod namespace and the cluster name as selectable labels, so - # each reaches a destination the entity whitelist or the namespace scoping - # refuses. All were measured ADMITTED before the key check landed. - - policy: restrict-tenant-network-policies - rule: cnp-egress-destinations-must-be-constrained - resources: - - tenant-ns/attack-selector-reserved-world-egress - - tenant-ns/attack-selector-bare-world-egress + - tenant-ns/attack-fromgroups-empty-list + - tenant-ns/attack-fromgroups-empty-selector + - tenant-ns/attack-fromgroups-null + - tenant-ns/attack-fromgroups-populated-selector + - tenant-ns/attack-fromnodes-empty-list + - tenant-ns/attack-fromnodes-empty-selector + - tenant-ns/attack-fromnodes-null + - tenant-ns/attack-fromnodes-populated-selector - tenant-ns/attack-selector-any-host-egress - - tenant-ns/attack-selector-reserved-expression-egress + - tenant-ns/attack-selector-bare-namespace-ingress + - tenant-ns/attack-selector-bare-world-egress + - tenant-ns/attack-selector-cluster-name-egress - tenant-ns/attack-selector-cross-namespace-egress - tenant-ns/attack-selector-cross-namespace-expression-egress + - tenant-ns/attack-selector-cross-namespace-ingress + - tenant-ns/attack-selector-exists-universal - tenant-ns/attack-selector-namespace-labels-egress - - tenant-ns/attack-selector-cluster-name-egress - tenant-ns/attack-selector-reserved-beside-constrained-egress - kind: CiliumNetworkPolicy - result: fail - - policy: restrict-tenant-network-policies - rule: cnp-ingress-sources-must-be-constrained - resources: + - tenant-ns/attack-selector-reserved-expression-egress - tenant-ns/attack-selector-reserved-host-ingress - - tenant-ns/attack-selector-cross-namespace-ingress - - tenant-ns/attack-selector-bare-namespace-ingress - kind: CiliumNetworkPolicy - result: fail - # CONTROL: the ingress-side `reserved:ingress` carve-out, and source-prefixed - # ordinary keys (including a k8s label named like a reserved identity), stay - # admitted by BOTH rules — a key check that refused these would refuse the - # spelling its own message recommends. - - policy: restrict-tenant-network-policies - rule: cnp-ingress-sources-must-be-constrained - resources: - - tenant-ns/allow-web-ingress-from-ingress-selector - - tenant-ns/allow-web-egress-k8s-prefixed-labels - - tenant-ns/allow-web-egress-to-kube-apiserver-selector - kind: CiliumNetworkPolicy - result: pass - - policy: restrict-tenant-network-policies - rule: cnp-egress-destinations-must-be-constrained - resources: - - tenant-ns/allow-web-ingress-from-ingress-selector - - tenant-ns/allow-web-egress-k8s-prefixed-labels - - tenant-ns/allow-web-egress-to-kube-apiserver-selector - kind: CiliumNetworkPolicy - result: pass - # ── CIDR entries inside NON-GLOBAL address space, and group references ───── - # Each clears the /16 prefix floor and was measured ADMITTED before the - # deny-list landed. The boundary is the cluster's own address space, the - # host, the metadata endpoint, and the link-local/loopback/multicast blocks; - # any set covering an address family contains one of them. - - policy: restrict-tenant-network-policies - rule: cnp-egress-destinations-must-be-constrained - resources: - - tenant-ns/attack-cidr-pod-network-egress - - tenant-ns/attack-cidr-private-172-cidrset-egress - - tenant-ns/attack-cidr-link-local-metadata-egress - - tenant-ns/attack-cidr-loopback-egress - - tenant-ns/attack-cidr-cgnat-egress - - tenant-ns/attack-cidr-multicast-egress - - tenant-ns/attack-cidr-ipv4-mapped-v6-egress - - tenant-ns/attack-cidr-ula-v6-egress - - tenant-ns/attack-cidr-ula-v6-uppercase-egress - - tenant-ns/attack-cidr-six-to-four-egress - - tenant-ns/attack-cidr-every-slash16-v4-egress - - tenant-ns/attack-cidr-every-slash16-v6-egress - - tenant-ns/attack-cidr-reserved-beside-legitimate-egress - - tenant-ns/attack-cidrset-group-ref-egress - - tenant-ns/attack-cidrset-group-selector-egress - - tenant-ns/attack-cidrset-group-ref-beside-cidr-egress - - tenant-ns/attack-cidrset-group-selector-beside-cidr-egress - kind: CiliumNetworkPolicy - result: fail - - policy: restrict-tenant-network-policies - rule: cnp-ingress-sources-must-be-constrained - resources: - - tenant-ns/attack-cidr-node-private-network-ingress - - tenant-ns/attack-cidr-link-local-v6-ingress - - tenant-ns/attack-cidrset-group-ref-ingress - - tenant-ns/attack-cidrset-group-ref-beside-cidr-ingress - - tenant-ns/attack-cidrset-group-selector-beside-cidr-ingress - kind: CiliumNetworkPolicy - result: fail - # CONTROL: CIDRs outside the protected ranges stay admitted by BOTH rules: - # a public IPv6 destination, documentation ranges, boundary neighbours and - # a CIDRSet entry with a literal cidr plus `except`. - - policy: restrict-tenant-network-policies - rule: cnp-egress-destinations-must-be-constrained - resources: - - tenant-ns/allow-web-egress-public-v6 - - tenant-ns/allow-web-egress-documentation-cidrset-except - - tenant-ns/allow-web-ingress-from-documentation-cidr - - tenant-ns/allow-web-egress-outside-protected-ranges - kind: CiliumNetworkPolicy - result: pass - - policy: restrict-tenant-network-policies - rule: cnp-ingress-sources-must-be-constrained - resources: - - tenant-ns/allow-web-egress-public-v6 - - tenant-ns/allow-web-egress-documentation-cidrset-except - - tenant-ns/allow-web-ingress-from-documentation-cidr - - tenant-ns/allow-web-egress-outside-protected-ranges - kind: CiliumNetworkPolicy - result: pass - # Every Service reference must pin the policy namespace. - - policy: restrict-tenant-network-policies - rule: cnp-egress-destinations-must-be-constrained - resources: - - tenant-ns/attack-toservices-foreign-named - - tenant-ns/attack-toservices-foreign-selected - - tenant-ns/attack-toservices-omitted-named - - tenant-ns/attack-toservices-omitted-selected + - tenant-ns/attack-selector-reserved-world-egress + - tenant-ns/attack-specs-container-egress-world + - tenant-ns/attack-togroups-empty-list + - tenant-ns/attack-togroups-empty-selector + - tenant-ns/attack-togroups-null + - tenant-ns/attack-togroups-populated-selector + - tenant-ns/attack-tonodes-empty-list + - tenant-ns/attack-tonodes-empty-selector + - tenant-ns/attack-tonodes-null + - tenant-ns/attack-tonodes-populated-selector + - tenant-ns/attack-toservices-empty-entry + - tenant-ns/attack-toservices-empty-matchlabels - tenant-ns/attack-toservices-empty-namespace-named - tenant-ns/attack-toservices-empty-namespace-selected + - tenant-ns/attack-toservices-empty-selector - tenant-ns/attack-toservices-foreign-beside-own - tenant-ns/attack-toservices-foreign-in-second-rule - - tenant-ns/attack-toservices-own-name-foreign-selector - tenant-ns/attack-toservices-foreign-name-own-selector - kind: CiliumNetworkPolicy - result: fail - # Explicit local namespaces work for selectors and another tenant. + - tenant-ns/attack-toservices-foreign-named + - tenant-ns/attack-toservices-foreign-selected + - tenant-ns/attack-toservices-omitted-named + - tenant-ns/attack-toservices-omitted-selected + - tenant-ns/attack-toservices-own-name-foreign-selector + - tenant-ns/attack-toservices-unnamed-service + - tenant-ns/attack-toservices-vacuous-beside-constrained + - tenant-ns/default-deny + - tenant-ns/egress-bare-l4 + - tenant-ns/egress-cidr-world + - tenant-ns/egress-cidrset-split-v6 + - tenant-ns/egress-cidrset-world-v6 + - tenant-ns/egress-empty-object + - tenant-ns/egress-entity-cluster-mesh + - tenant-ns/egress-entity-world + - tenant-ns/egress-fqdn-wildcard + - tenant-ns/ingress-bare-l4 + - tenant-ns/ingress-both-fields-empty + - tenant-ns/ingress-cidr-split-v4 + - tenant-ns/ingress-empty-matchexpressions + - tenant-ns/ingress-empty-matchlabels + - tenant-ns/ingress-empty-object + - tenant-ns/ingress-entity-all + - tenant-ns/ingress-entity-cluster + - tenant-ns/ingress-entity-remote-node + - tenant-ns/ingress-vacuous-beside-constrained + - tenant-ns/ingress-vacuous-in-second-rule + - tenant-ns/opt-out-default-deny-egress + - tenant-ns/opt-out-default-deny-ingress - policy: restrict-tenant-network-policies - rule: cnp-egress-destinations-must-be-constrained - resources: - - tenant-ns/allow-web-egress-selected-service-expression - - other-tenant/allow-other-tenant-egress-named-service kind: CiliumNetworkPolicy result: pass - - policy: restrict-tenant-network-policies - rule: cnp-ingress-sources-must-be-constrained resources: - - tenant-ns/attack-fromnodes-empty-selector - - tenant-ns/attack-fromnodes-populated-selector - - tenant-ns/attack-fromnodes-empty-list - - tenant-ns/attack-fromnodes-null - - tenant-ns/attack-fromgroups-empty-selector - - tenant-ns/attack-fromgroups-populated-selector - - tenant-ns/attack-fromgroups-empty-list - - tenant-ns/attack-fromgroups-null - kind: CiliumNetworkPolicy - result: fail + - other-tenant/allow-other-tenant-egress-named-service + - tenant-ns/allow-all-own-pods-from-api + - tenant-ns/allow-api-to-web + - tenant-ns/allow-web-egress-db + - tenant-ns/allow-web-egress-documentation-cidrset-except + - tenant-ns/allow-web-egress-k8s-prefixed-labels + - tenant-ns/allow-web-egress-named-fqdn + - tenant-ns/allow-web-egress-named-service + - tenant-ns/allow-web-egress-outside-protected-ranges + - tenant-ns/allow-web-egress-public-v6 + - tenant-ns/allow-web-egress-selected-service + - tenant-ns/allow-web-egress-selected-service-expression + - tenant-ns/allow-web-egress-specific-cidr + - tenant-ns/allow-web-egress-subdomain-fqdn + - tenant-ns/allow-web-egress-to-kube-apiserver-selector + - tenant-ns/allow-web-ingress-from-documentation-cidr + - tenant-ns/allow-web-ingress-from-gateway + - tenant-ns/allow-web-ingress-from-ingress-selector - policy: restrict-tenant-network-policies - rule: cnp-egress-destinations-must-be-constrained - resources: - - tenant-ns/attack-tonodes-empty-selector - - tenant-ns/attack-tonodes-populated-selector - - tenant-ns/attack-tonodes-empty-list - - tenant-ns/attack-tonodes-null - - tenant-ns/attack-togroups-empty-selector - - tenant-ns/attack-togroups-populated-selector - - tenant-ns/attack-togroups-empty-list - - tenant-ns/attack-togroups-null kind: CiliumNetworkPolicy - result: fail + result: skip + resources: + - platform-ns/platform-allow-all diff --git a/tests/restrict-tenant-network-policies/per-check.yaml b/tests/restrict-tenant-network-policies/per-check.yaml new file mode 100644 index 0000000000..41f26837bd --- /dev/null +++ b/tests/restrict-tenant-network-policies/per-check.yaml @@ -0,0 +1,314 @@ +--- +apiVersion: cli.kyverno.io/v1alpha1 +kind: Test +metadata: + name: restrict-tenant-network-policies +policies: + - >- + ../../k8s/bases/infrastructure/cluster-policies/best-practices/restrict-tenant-network-policies.yaml +resources: + - resources.yaml + - node-group-resources.yaml +# The policy uses a namespaceSelector, so the CLI needs the namespace labels +# declared explicitly or every case is reported as Excluded — which satisfies +# `result: fail` too and would pass vacuously. The companion shell test +# (scripts/tests/test-restrict-tenant-network-policies.sh) is the second gate: +# it runs `kyverno apply` and asserts the exact allow/deny counts, so a rule +# that stopped matching cannot go unnoticed here. +variables: values.yaml +userinfo: user-info.yaml +results: + # ── Every semantically empty source selector, one spelling per case ───────── + - policy: restrict-tenant-network-policies + rule: cnp-ingress-sources-must-be-constrained + resources: + - tenant-ns/ingress-empty-object + - tenant-ns/ingress-empty-matchlabels + - tenant-ns/ingress-empty-matchexpressions + - tenant-ns/ingress-both-fields-empty + - tenant-ns/ingress-vacuous-beside-constrained + - tenant-ns/ingress-vacuous-in-second-rule + - tenant-ns/ingress-entity-all + - tenant-ns/ingress-entity-cluster + - tenant-ns/ingress-bare-l4 + - tenant-ns/ingress-entity-remote-node + - tenant-ns/ingress-cidr-split-v4 + kind: CiliumNetworkPolicy + result: fail + - policy: restrict-tenant-network-policies + rule: cnp-ingress-sources-must-be-constrained + resources: + - tenant-ns/allow-api-to-web + - tenant-ns/allow-web-egress-db + - tenant-ns/allow-web-egress-specific-cidr + - tenant-ns/allow-web-egress-named-fqdn + - tenant-ns/allow-web-egress-subdomain-fqdn + - tenant-ns/allow-all-own-pods-from-api + - tenant-ns/allow-web-ingress-from-gateway + kind: CiliumNetworkPolicy + result: pass + # ── Egress destinations ──────────────────────────────────────────────────── + - policy: restrict-tenant-network-policies + rule: cnp-egress-destinations-must-be-constrained + resources: + - tenant-ns/egress-empty-object + - tenant-ns/egress-entity-world + - tenant-ns/egress-cidr-world + - tenant-ns/egress-cidrset-world-v6 + - tenant-ns/egress-bare-l4 + - tenant-ns/egress-fqdn-wildcard + - tenant-ns/egress-entity-cluster-mesh + - tenant-ns/egress-cidrset-split-v6 + kind: CiliumNetworkPolicy + result: fail + - policy: restrict-tenant-network-policies + rule: cnp-egress-destinations-must-be-constrained + resources: + - tenant-ns/allow-api-to-web + - tenant-ns/allow-web-egress-db + - tenant-ns/allow-web-egress-specific-cidr + - tenant-ns/allow-web-egress-named-fqdn + - tenant-ns/allow-web-egress-subdomain-fqdn + - tenant-ns/allow-all-own-pods-from-api + kind: CiliumNetworkPolicy + result: pass + # ── Opting out of the namespace's default-deny ───────────────────────────── + - policy: restrict-tenant-network-policies + rule: cnp-must-not-opt-out-of-default-deny + resources: + - tenant-ns/opt-out-default-deny-ingress + - tenant-ns/opt-out-default-deny-egress + kind: CiliumNetworkPolicy + result: fail + - policy: restrict-tenant-network-policies + rule: cnp-must-not-opt-out-of-default-deny + resources: + - tenant-ns/allow-api-to-web + - tenant-ns/allow-web-egress-db + kind: CiliumNetworkPolicy + result: pass + # ── Ownership: the generated floor's names are reserved ──────────────────── + - policy: restrict-tenant-network-policies + rule: cnp-generated-policy-names-are-reserved + resources: + - tenant-ns/default-deny + - tenant-ns/allow-dns + - tenant-ns/allow-cnpg-operator + kind: CiliumNetworkPolicy + result: fail + # ── A platform namespace is out of scope entirely ────────────────────────── + - policy: restrict-tenant-network-policies + rule: cnp-ingress-sources-must-be-constrained + resources: + - platform-ns/platform-allow-all + kind: CiliumNetworkPolicy + result: skip + # ── Shapes that PASS a presence check but are semantically unbounded ─────── + # Each of these was measured ADMITTED before the whitelist forms landed: the + # per-entry checks all saw "something is there" and never asked what it meant. + - policy: restrict-tenant-network-policies + rule: cnp-egress-destinations-must-be-constrained + resources: + - tenant-ns/attack-cidr-prefix-floor-slash8 + - tenant-ns/attack-cidr-zero-padded-prefix + - tenant-ns/attack-fqdn-wildcard-depths + kind: CiliumNetworkPolicy + result: fail + - policy: restrict-tenant-network-policies + rule: cnp-ingress-sources-must-be-constrained + resources: + - tenant-ns/attack-selector-exists-universal + kind: CiliumNetworkPolicy + result: fail + # ── The plural rule container, which every `spec`-reading condition misses ── + - policy: restrict-tenant-network-policies + rule: cnp-must-use-the-singular-spec-container + resources: + - tenant-ns/attack-specs-container-egress-world + kind: CiliumNetworkPolicy + result: fail + - policy: restrict-tenant-network-policies + rule: cnp-must-use-the-singular-spec-container + resources: + - tenant-ns/allow-api-to-web + - tenant-ns/allow-web-egress-db + kind: CiliumNetworkPolicy + result: pass + # ── toServices: a reference that names nothing is still "present" ────────── + # Every one of these satisfied the egress presence check on the mere + # existence of a toServices key, and reached arbitrary Service backends + # without passing the toEndpoints selector whitelist. + - policy: restrict-tenant-network-policies + rule: cnp-egress-destinations-must-be-constrained + resources: + - tenant-ns/attack-toservices-empty-selector + - tenant-ns/attack-toservices-empty-matchlabels + - tenant-ns/attack-toservices-unnamed-service + - tenant-ns/attack-toservices-empty-entry + - tenant-ns/attack-toservices-vacuous-beside-constrained + kind: CiliumNetworkPolicy + result: fail + # CONTROL: the constrained spellings stay admitted. + - policy: restrict-tenant-network-policies + rule: cnp-egress-destinations-must-be-constrained + resources: + - tenant-ns/allow-web-egress-named-service + - tenant-ns/allow-web-egress-selected-service + kind: CiliumNetworkPolicy + result: pass + # ── Selector KEYS that name an identity, a namespace or a cluster ────────── + # Every one of these is non-empty and uses only whitelisted operators, so the + # presence and operator checks read it as constrained. Cilium exposes reserved + # identities, the pod namespace and the cluster name as selectable labels, so + # each reaches a destination the entity whitelist or the namespace scoping + # refuses. All were measured ADMITTED before the key check landed. + - policy: restrict-tenant-network-policies + rule: cnp-egress-destinations-must-be-constrained + resources: + - tenant-ns/attack-selector-reserved-world-egress + - tenant-ns/attack-selector-bare-world-egress + - tenant-ns/attack-selector-any-host-egress + - tenant-ns/attack-selector-reserved-expression-egress + - tenant-ns/attack-selector-cross-namespace-egress + - tenant-ns/attack-selector-cross-namespace-expression-egress + - tenant-ns/attack-selector-namespace-labels-egress + - tenant-ns/attack-selector-cluster-name-egress + - tenant-ns/attack-selector-reserved-beside-constrained-egress + kind: CiliumNetworkPolicy + result: fail + - policy: restrict-tenant-network-policies + rule: cnp-ingress-sources-must-be-constrained + resources: + - tenant-ns/attack-selector-reserved-host-ingress + - tenant-ns/attack-selector-cross-namespace-ingress + - tenant-ns/attack-selector-bare-namespace-ingress + kind: CiliumNetworkPolicy + result: fail + # CONTROL: the ingress-side `reserved:ingress` carve-out, and source-prefixed + # ordinary keys (including a k8s label named like a reserved identity), stay + # admitted by BOTH rules — a key check that refused these would refuse the + # spelling its own message recommends. + - policy: restrict-tenant-network-policies + rule: cnp-ingress-sources-must-be-constrained + resources: + - tenant-ns/allow-web-ingress-from-ingress-selector + - tenant-ns/allow-web-egress-k8s-prefixed-labels + - tenant-ns/allow-web-egress-to-kube-apiserver-selector + kind: CiliumNetworkPolicy + result: pass + - policy: restrict-tenant-network-policies + rule: cnp-egress-destinations-must-be-constrained + resources: + - tenant-ns/allow-web-ingress-from-ingress-selector + - tenant-ns/allow-web-egress-k8s-prefixed-labels + - tenant-ns/allow-web-egress-to-kube-apiserver-selector + kind: CiliumNetworkPolicy + result: pass + # ── CIDR entries inside NON-GLOBAL address space, and group references ───── + # Each clears the /16 prefix floor and was measured ADMITTED before the + # deny-list landed. The boundary is the cluster's own address space, the + # host, the metadata endpoint, and the link-local/loopback/multicast blocks; + # any set covering an address family contains one of them. + - policy: restrict-tenant-network-policies + rule: cnp-egress-destinations-must-be-constrained + resources: + - tenant-ns/attack-cidr-pod-network-egress + - tenant-ns/attack-cidr-private-172-cidrset-egress + - tenant-ns/attack-cidr-link-local-metadata-egress + - tenant-ns/attack-cidr-loopback-egress + - tenant-ns/attack-cidr-cgnat-egress + - tenant-ns/attack-cidr-multicast-egress + - tenant-ns/attack-cidr-ipv4-mapped-v6-egress + - tenant-ns/attack-cidr-ula-v6-egress + - tenant-ns/attack-cidr-ula-v6-uppercase-egress + - tenant-ns/attack-cidr-six-to-four-egress + - tenant-ns/attack-cidr-every-slash16-v4-egress + - tenant-ns/attack-cidr-every-slash16-v6-egress + - tenant-ns/attack-cidr-reserved-beside-legitimate-egress + - tenant-ns/attack-cidrset-group-ref-egress + - tenant-ns/attack-cidrset-group-selector-egress + - tenant-ns/attack-cidrset-group-ref-beside-cidr-egress + - tenant-ns/attack-cidrset-group-selector-beside-cidr-egress + kind: CiliumNetworkPolicy + result: fail + - policy: restrict-tenant-network-policies + rule: cnp-ingress-sources-must-be-constrained + resources: + - tenant-ns/attack-cidr-node-private-network-ingress + - tenant-ns/attack-cidr-link-local-v6-ingress + - tenant-ns/attack-cidrset-group-ref-ingress + - tenant-ns/attack-cidrset-group-ref-beside-cidr-ingress + - tenant-ns/attack-cidrset-group-selector-beside-cidr-ingress + kind: CiliumNetworkPolicy + result: fail + # CONTROL: CIDRs outside the protected ranges stay admitted by BOTH rules: + # a public IPv6 destination, documentation ranges, boundary neighbours and + # a CIDRSet entry with a literal cidr plus `except`. + - policy: restrict-tenant-network-policies + rule: cnp-egress-destinations-must-be-constrained + resources: + - tenant-ns/allow-web-egress-public-v6 + - tenant-ns/allow-web-egress-documentation-cidrset-except + - tenant-ns/allow-web-ingress-from-documentation-cidr + - tenant-ns/allow-web-egress-outside-protected-ranges + kind: CiliumNetworkPolicy + result: pass + - policy: restrict-tenant-network-policies + rule: cnp-ingress-sources-must-be-constrained + resources: + - tenant-ns/allow-web-egress-public-v6 + - tenant-ns/allow-web-egress-documentation-cidrset-except + - tenant-ns/allow-web-ingress-from-documentation-cidr + - tenant-ns/allow-web-egress-outside-protected-ranges + kind: CiliumNetworkPolicy + result: pass + # Every Service reference must pin the policy namespace. + - policy: restrict-tenant-network-policies + rule: cnp-egress-destinations-must-be-constrained + resources: + - tenant-ns/attack-toservices-foreign-named + - tenant-ns/attack-toservices-foreign-selected + - tenant-ns/attack-toservices-omitted-named + - tenant-ns/attack-toservices-omitted-selected + - tenant-ns/attack-toservices-empty-namespace-named + - tenant-ns/attack-toservices-empty-namespace-selected + - tenant-ns/attack-toservices-foreign-beside-own + - tenant-ns/attack-toservices-foreign-in-second-rule + - tenant-ns/attack-toservices-own-name-foreign-selector + - tenant-ns/attack-toservices-foreign-name-own-selector + kind: CiliumNetworkPolicy + result: fail + # Explicit local namespaces work for selectors and another tenant. + - policy: restrict-tenant-network-policies + rule: cnp-egress-destinations-must-be-constrained + resources: + - tenant-ns/allow-web-egress-selected-service-expression + - other-tenant/allow-other-tenant-egress-named-service + kind: CiliumNetworkPolicy + result: pass + - policy: restrict-tenant-network-policies + rule: cnp-ingress-sources-must-be-constrained + resources: + - tenant-ns/attack-fromnodes-empty-selector + - tenant-ns/attack-fromnodes-populated-selector + - tenant-ns/attack-fromnodes-empty-list + - tenant-ns/attack-fromnodes-null + - tenant-ns/attack-fromgroups-empty-selector + - tenant-ns/attack-fromgroups-populated-selector + - tenant-ns/attack-fromgroups-empty-list + - tenant-ns/attack-fromgroups-null + kind: CiliumNetworkPolicy + result: fail + - policy: restrict-tenant-network-policies + rule: cnp-egress-destinations-must-be-constrained + resources: + - tenant-ns/attack-tonodes-empty-selector + - tenant-ns/attack-tonodes-populated-selector + - tenant-ns/attack-tonodes-empty-list + - tenant-ns/attack-tonodes-null + - tenant-ns/attack-togroups-empty-selector + - tenant-ns/attack-togroups-populated-selector + - tenant-ns/attack-togroups-empty-list + - tenant-ns/attack-togroups-null + kind: CiliumNetworkPolicy + result: fail diff --git a/tests/restrict-tenant-network-policies/platform-author/kyverno-test.yaml b/tests/restrict-tenant-network-policies/platform-author/kyverno-test.yaml index 2e855d33cb..3f9dbf42f6 100644 --- a/tests/restrict-tenant-network-policies/platform-author/kyverno-test.yaml +++ b/tests/restrict-tenant-network-policies/platform-author/kyverno-test.yaml @@ -1,4 +1,8 @@ --- +# One row per resource: fail when any check refuses it. Which check refuses it is +# in per-check.yaml, which scripts/tests/test-restrict-tenant-network-policies.sh +# runs. The policy uses a namespaceSelector, so the CLI needs the namespace labels +# declared explicitly. apiVersion: cli.kyverno.io/v1alpha1 kind: Test metadata: @@ -11,19 +15,8 @@ resources: variables: values.yaml userinfo: user-info.yaml results: - # The apps-layer controller keeps the paved road for placing broad, - # platform-owned policies in a managed tenant namespace. This body is refused - # when a TENANT submits it — the sibling shell test asserts that control, so - # these skips cannot be mistaken for the policy simply not matching. - policy: restrict-tenant-network-policies - rule: cnp-ingress-sources-must-be-constrained - resources: - - tenant-ns/platform-allow-observability kind: CiliumNetworkPolicy result: skip - - policy: restrict-tenant-network-policies - rule: cnp-egress-destinations-must-be-constrained resources: - tenant-ns/platform-allow-observability - kind: CiliumNetworkPolicy - result: skip diff --git a/tests/restrict-tenant-network-policies/platform-author/per-check.yaml b/tests/restrict-tenant-network-policies/platform-author/per-check.yaml new file mode 100644 index 0000000000..2e855d33cb --- /dev/null +++ b/tests/restrict-tenant-network-policies/platform-author/per-check.yaml @@ -0,0 +1,29 @@ +--- +apiVersion: cli.kyverno.io/v1alpha1 +kind: Test +metadata: + name: restrict-tenant-network-policies-platform-author +policies: + - >- + ../../../k8s/bases/infrastructure/cluster-policies/best-practices/restrict-tenant-network-policies.yaml +resources: + - resources.yaml +variables: values.yaml +userinfo: user-info.yaml +results: + # The apps-layer controller keeps the paved road for placing broad, + # platform-owned policies in a managed tenant namespace. This body is refused + # when a TENANT submits it — the sibling shell test asserts that control, so + # these skips cannot be mistaken for the policy simply not matching. + - policy: restrict-tenant-network-policies + rule: cnp-ingress-sources-must-be-constrained + resources: + - tenant-ns/platform-allow-observability + kind: CiliumNetworkPolicy + result: skip + - policy: restrict-tenant-network-policies + rule: cnp-egress-destinations-must-be-constrained + resources: + - tenant-ns/platform-allow-observability + kind: CiliumNetworkPolicy + result: skip diff --git a/tests/restrict-tenant-network-policies/tenant-scaffold/kyverno-test.yaml b/tests/restrict-tenant-network-policies/tenant-scaffold/kyverno-test.yaml index 5fbb32eece..a63cb36a8e 100644 --- a/tests/restrict-tenant-network-policies/tenant-scaffold/kyverno-test.yaml +++ b/tests/restrict-tenant-network-policies/tenant-scaffold/kyverno-test.yaml @@ -1,4 +1,8 @@ --- +# One row per resource: fail when any check refuses it. Which check refuses it is +# in per-check.yaml, which scripts/tests/test-restrict-tenant-network-policies.sh +# runs. The policy uses a namespaceSelector, so the CLI needs the namespace labels +# declared explicitly. apiVersion: cli.kyverno.io/v1alpha1 kind: Test metadata: @@ -11,32 +15,8 @@ resources: variables: values.yaml userinfo: user-info.yaml results: - # A tenant generated from platform-tenant-template must be admissible on day - # one, under the tenant's own identity, by every rule that evaluates it (the - # reserved-name rule matches only the generated names, so it has no row). The - # sibling shell test asserts the exact pass count, so a rule that stopped - # matching cannot read as a pass here. - policy: restrict-tenant-network-policies - rule: cnp-ingress-sources-must-be-constrained - resources: - - tenant-ns/tenant-ns - kind: CiliumNetworkPolicy - result: pass - - policy: restrict-tenant-network-policies - rule: cnp-egress-destinations-must-be-constrained - resources: - - tenant-ns/tenant-ns kind: CiliumNetworkPolicy result: pass - - policy: restrict-tenant-network-policies - rule: cnp-must-not-opt-out-of-default-deny resources: - tenant-ns/tenant-ns - kind: CiliumNetworkPolicy - result: pass - - policy: restrict-tenant-network-policies - rule: cnp-must-use-the-singular-spec-container - resources: - - tenant-ns/tenant-ns - kind: CiliumNetworkPolicy - result: pass diff --git a/tests/restrict-tenant-network-policies/tenant-scaffold/per-check.yaml b/tests/restrict-tenant-network-policies/tenant-scaffold/per-check.yaml new file mode 100644 index 0000000000..5fbb32eece --- /dev/null +++ b/tests/restrict-tenant-network-policies/tenant-scaffold/per-check.yaml @@ -0,0 +1,42 @@ +--- +apiVersion: cli.kyverno.io/v1alpha1 +kind: Test +metadata: + name: restrict-tenant-network-policies-tenant-scaffold +policies: + - >- + ../../../k8s/bases/infrastructure/cluster-policies/best-practices/restrict-tenant-network-policies.yaml +resources: + - resources.yaml +variables: values.yaml +userinfo: user-info.yaml +results: + # A tenant generated from platform-tenant-template must be admissible on day + # one, under the tenant's own identity, by every rule that evaluates it (the + # reserved-name rule matches only the generated names, so it has no row). The + # sibling shell test asserts the exact pass count, so a rule that stopped + # matching cannot read as a pass here. + - policy: restrict-tenant-network-policies + rule: cnp-ingress-sources-must-be-constrained + resources: + - tenant-ns/tenant-ns + kind: CiliumNetworkPolicy + result: pass + - policy: restrict-tenant-network-policies + rule: cnp-egress-destinations-must-be-constrained + resources: + - tenant-ns/tenant-ns + kind: CiliumNetworkPolicy + result: pass + - policy: restrict-tenant-network-policies + rule: cnp-must-not-opt-out-of-default-deny + resources: + - tenant-ns/tenant-ns + kind: CiliumNetworkPolicy + result: pass + - policy: restrict-tenant-network-policies + rule: cnp-must-use-the-singular-spec-container + resources: + - tenant-ns/tenant-ns + kind: CiliumNetworkPolicy + result: pass From 5e8ad3285d1caecfe3a98e9bab62eed7912f03da Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Tue, 3 Nov 2026 17:56:25 +0100 Subject: [PATCH 2/5] fix(policy): preserve unchanged legacy network policy updates --- .../restrict-tenant-network-policies.yaml | 17 ++++- .../test-restrict-tenant-network-policies.sh | 67 +++++++++++++++++++ 2 files changed, 81 insertions(+), 3 deletions(-) diff --git a/k8s/bases/infrastructure/cluster-policies/best-practices/restrict-tenant-network-policies.yaml b/k8s/bases/infrastructure/cluster-policies/best-practices/restrict-tenant-network-policies.yaml index 06e2845eef..52f80ae2ae 100644 --- a/k8s/bases/infrastructure/cluster-policies/best-practices/restrict-tenant-network-policies.yaml +++ b/k8s/bases/infrastructure/cluster-policies/best-practices/restrict-tenant-network-policies.yaml @@ -123,6 +123,11 @@ # exact namespace, including DELETE, without depending on namespace labels. # Reserved names refuse tenant UPDATE even when an old platform-authored body # already fails that rule for the tenant identity. +# Existing ordinary policies can predate this guard. A metadata-only UPDATE +# keeps the same identity and singular spec, so it adds no network access and +# remains admissible while those policies are migrated (#5823). CREATE and any +# changed invalid spec remain refused. Reserved names, plural specs and the +# exact staging hold are checked independently of this compatibility case. # # General scope and carve-outs: generated tenant namespaces (labelled # app.kubernetes.io/managed-by: ksail or kro), plus the exact staging hold above, @@ -197,6 +202,12 @@ spec: namespaceObject.?metadata.?labels[?'app.kubernetes.io/managed-by'].orValue('') in ['ksail', 'kro'] - name: isDelete expression: "request.operation == 'DELETE'" + - name: isUnchangedPolicyUpdate + expression: >- + request.operation == 'UPDATE' && oldObject != null && object != null && + object.metadata.name == oldObject.metadata.name && + object.metadata.namespace == oldObject.metadata.namespace && + has(object.spec) && has(oldObject.spec) && object.spec == oldObject.spec - name: policyName expression: "variables.isDelete ? oldObject.metadata.name : object.metadata.name" - name: spec @@ -343,7 +354,7 @@ spec: (!('k8sServiceSelector' in service) || service.k8sServiceSelector.?namespace.orValue('') == request.namespace)) validations: - expression: >- - !variables.isTenantNamespace || + !variables.isTenantNamespace || variables.isUnchangedPolicyUpdate || variables.fromEndpointsConstrained && variables.fromEndpointKeysAllowed && variables.ingressRulesNameSources && variables.ingressOmitsNodesAndGroups && variables.fromEntitiesAllowed && variables.fromCIDRSetsLiteral && variables.fromAddressesAllowed @@ -368,7 +379,7 @@ spec: source removes this namespace's isolation regardless of the generated default-deny. - expression: >- - !variables.isTenantNamespace || + !variables.isTenantNamespace || variables.isUnchangedPolicyUpdate || variables.toEndpointsConstrained && variables.toEndpointKeysAllowed && variables.egressRulesNameDestinations && variables.egressOmitsNodesAndGroups && variables.toFQDNsNameADestination && variables.toEntitiesAllowed && @@ -394,7 +405,7 @@ spec: Service namespace matches Services across namespaces. Name the destinations this workload actually needs. - expression: >- - !variables.isTenantNamespace || + !variables.isTenantNamespace || variables.isUnchangedPolicyUpdate || variables.spec.?enableDefaultDeny.?ingress.orValue(true) != false && variables.spec.?enableDefaultDeny.?egress.orValue(true) != false message: >- diff --git a/scripts/tests/test-restrict-tenant-network-policies.sh b/scripts/tests/test-restrict-tenant-network-policies.sh index 4cf502cafb..efbc5a7cb9 100755 --- a/scripts/tests/test-restrict-tenant-network-policies.sh +++ b/scripts/tests/test-restrict-tenant-network-policies.sh @@ -204,6 +204,21 @@ hold_control() { + (if $operation == "DELETE" then {} else {object: $resource} end) + (if $operation == "CREATE" then {} else {oldObject: $resource} end)' } +existing_control() { + local label="$1" operation="$2" expected="$3" old="$4" object="$5" + jq -cn --arg label "${label}" --arg actor "${tenant}" --arg operation "${operation}" \ + --arg expected "${expected}" --argjson old "${old}" --argjson object "${object}" ' + {label: $label, policy: "restrict-tenant-network-policies", actor: $actor, operation: $operation, + expected: $expected, namespaces: [{metadata: {name: "tenant-ns", labels: {"app.kubernetes.io/managed-by": "ksail"}}}]} + + (if $object == null then {} else {object: $object} end) + + (if $old == null then {} else {oldObject: $old} end)' +} +existing="$(yq -o=json '.' "${tests}/platform-author/resources.yaml")" +valid="$(cnp platform-allow-observability)" +legacy_ingress="$(jq -c 'del(.spec.egress)' <<<"${existing}")" +legacy_egress="$(jq -c 'del(.spec.ingress)' <<<"${existing}")" +legacy_default_deny="$(jq -c '.spec.enableDefaultDeny.ingress = false' <<<"${valid}")" +legacy_plural="$(jq -c '.specs = [.spec]' <<<"${existing}")" { control tenant-deletes-own-policy "${tenant}" DELETE allow-web pass for reserved in default-deny allow-dns allow-cnpg-operator; do @@ -213,6 +228,32 @@ hold_control() { control "flux-deletes-${reserved}" "${flux_sa}" DELETE "${reserved}" skip done control tenant-updates-own-policy "${tenant}" UPDATE allow-web pass + # Existing tenant policies can predate the guard. Reconciliation may update + # metadata without changing their rules; that must neither create a new + # violating policy nor authorize a different invalid body. Reserved names, + # plural specs and the rollout hold retain their independent refusals. + existing_control legacy-policy-create CREATE fail null "${existing}" + existing_control legacy-policy-unchanged-update UPDATE pass "${existing}" "${existing}" + existing_control legacy-policy-metadata-update UPDATE pass "${existing}" \ + "$(jq -c '.metadata.annotations.reconciled = "true"' <<<"${existing}")" + existing_control legacy-policy-changed-ingress UPDATE fail "${existing}" \ + "$(jq -c '.spec.ingress = [{fromEntities: ["world"]}]' <<<"${existing}")" + existing_control legacy-policy-changed-egress UPDATE fail "${existing}" \ + "$(jq -c '.spec.egress = [{toEntities: ["world"]}]' <<<"${existing}")" + existing_control valid-policy-cannot-become-invalid UPDATE fail "${valid}" "${existing}" + existing_control legacy-policy-can-be-repaired UPDATE pass "${existing}" "${valid}" + existing_control legacy-policy-needs-old-spec UPDATE fail \ + "$(jq -c 'del(.spec)' <<<"${existing}")" "${existing}" + existing_control legacy-ingress-unchanged UPDATE pass "${legacy_ingress}" "${legacy_ingress}" + existing_control legacy-egress-unchanged UPDATE pass "${legacy_egress}" "${legacy_egress}" + existing_control legacy-default-deny-unchanged UPDATE pass "${legacy_default_deny}" "${legacy_default_deny}" + existing_control valid-policy-cannot-disable-default-deny UPDATE fail "${valid}" "${legacy_default_deny}" + existing_control legacy-plural-specs-still-refused UPDATE fail "${legacy_plural}" "${legacy_plural}" + existing_control legacy-policy-cannot-change-name UPDATE fail "${existing}" \ + "$(jq -c '.metadata.name = "another-policy"' <<<"${existing}")" + existing_control legacy-policy-needs-same-namespace UPDATE fail \ + "$(jq -c '.metadata.namespace = "other-tenant"' <<<"${existing}")" "${existing}" + existing_control legacy-policy-delete DELETE pass "${existing}" null # The committed rollout hold is protected independently of namespace labels. # UPDATE carries the same already-invalid old body; DELETE carries only oldObject. for managed in kro ''; do @@ -243,5 +284,31 @@ hold_control() { ) "${work}/admission" "${policy}" <"${work}/controls.jsonl" +# Prove that the UPDATE compatibility does not swallow new violations or +# borrow an unchanged spec from a different object. A compiler error is not a +# successful negative control: require the corresponding wrong decision. +for mutation in any-update missing-identity; do + if [[ "${mutation}" == any-update ]]; then + expression="request.operation == 'UPDATE'" + refused=legacy-policy-changed-ingress + else + expression='request.operation == "UPDATE" && oldObject != null && object != null && has(oldObject.spec) && has(object.spec) && object.spec == oldObject.spec' + refused=legacy-policy-cannot-change-name + fi + EXPRESSION="${expression}" yq \ + '(.spec.variables[] | select(.name == "isUnchangedPolicyUpdate").expression) = strenv(EXPRESSION)' \ + "${policy}" >"${work}/${mutation}.yaml" + if "${work}/admission" "${work}/${mutation}.yaml" <"${work}/controls.jsonl" >"${work}/${mutation}.log" 2>&1; then + echo "::error::${mutation}: the weakened UPDATE compatibility still passes its controls" + exit 1 + fi + if ! grep -Fq "${refused}: expected fail, failed=false error=false" "${work}/${mutation}.log"; then + cat "${work}/${mutation}.log" >&2 + echo "::error::${mutation}: did not demonstrate the intended wrong admission" + exit 1 + fi + echo "ok: UPDATE compatibility refuses ${mutation} weakening" +done + echo "Tenant CiliumNetworkPolicy policy enforced the expected boundary, and both" echo "platform carve-outs were proven to depend on the applying identity." From c3a195897ab85d1acf030a67c628c963bdebe402 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Mon, 9 Nov 2026 09:19:11 +0100 Subject: [PATCH 3/5] test(policy): retarget the shorthand-kinds control at a ClusterPolicy The tenant network guard is a ValidatingPolicy on this branch, so the CiliumNetworkPolicy kind edit no longer applied and the negative control passed nothing. Break the ExternalSecret kind of restrict-arc-retirement instead, and fail if the edit stops applying. Co-Authored-By: Claude Opus 5.5 --- scripts/tests/test-validate-kyverno-fixture-evaluation.sh | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/scripts/tests/test-validate-kyverno-fixture-evaluation.sh b/scripts/tests/test-validate-kyverno-fixture-evaluation.sh index 0bdcce48ca..441ff57706 100755 --- a/scripts/tests/test-validate-kyverno-fixture-evaluation.sh +++ b/scripts/tests/test-validate-kyverno-fixture-evaluation.sh @@ -85,9 +85,11 @@ else fi # #3145: kubectl's shorthand kind makes every rule match nothing. -root="$(copy shorthand-kinds restrict-tenant-network-policies)" -sed -i.bak -E 's#- CiliumNetworkPolicy$#- CiliumNetworkPolicy.cilium.io#' \ - "${root}/${policies}/restrict-tenant-network-policies.yaml" +root="$(copy shorthand-kinds restrict-arc-retirement-credential-sync-rewritten)" +sed -i.bak -E 's#kinds: \[external-secrets\.io/v1/ExternalSecret\]#kinds: [ExternalSecret.external-secrets.io]#' \ + "${root}/${policies}/restrict-arc-retirement.yaml" +grep -qF 'kinds: [ExternalSecret.external-secrets.io]' "${root}/${policies}/restrict-arc-retirement.yaml" || + fail "shorthand kinds: the edit no longer applies to restrict-arc-retirement.yaml" if kyverno_passes "shorthand kinds" "${root}"; then expect_fail "shorthand kinds" "${root}" 1 "declares result: fail (kyverno reported it Excluded)" fi From 830f3c548a8bc7cb15cf4502637e45a1d0d5991c Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Mon, 9 Nov 2026 10:01:57 +0100 Subject: [PATCH 4/5] fix(policy): allow only positive operators on carved-out reserved selector keys Cilium skips namespace injection for an endpoint selector that names a `reserved:` key, so a `NotIn` or `DoesNotExist` expression on the carved-out kube-apiserver/ingress keys beside an ordinary label selected that label in every namespace. Require `In` or `Exists` for those keys on both the ingress and egress side, add an attack fixture for each, and correct the test comments that still described a namespaceSelector. Co-Authored-By: Claude Opus 5.5 --- .../restrict-tenant-network-policies.yaml | 24 ++++++++--- .../test-restrict-tenant-network-policies.sh | 2 +- .../kyverno-author/kyverno-test.yaml | 4 +- .../kyverno-test.yaml | 6 ++- .../per-check.yaml | 7 +++- .../platform-author/kyverno-test.yaml | 4 +- .../resources.yaml | 40 +++++++++++++++++++ .../tenant-scaffold/kyverno-test.yaml | 4 +- 8 files changed, 75 insertions(+), 16 deletions(-) diff --git a/k8s/bases/infrastructure/cluster-policies/best-practices/restrict-tenant-network-policies.yaml b/k8s/bases/infrastructure/cluster-policies/best-practices/restrict-tenant-network-policies.yaml index 52f80ae2ae..1fa01157ed 100644 --- a/k8s/bases/infrastructure/cluster-policies/best-practices/restrict-tenant-network-policies.yaml +++ b/k8s/bases/infrastructure/cluster-policies/best-practices/restrict-tenant-network-policies.yaml @@ -251,6 +251,10 @@ spec: # Both forms are read, so the `In` form cannot smuggle a key the map form # refuses. Carved out, mirroring the entity whitelist: `kube-apiserver` and, # on this side only, `ingress`, each in its reserved/bare/`any:` spelling. + # A carved-out key may only be matched positively (`In` or `Exists`): Cilium + # skips namespace injection for a selector naming a `reserved:` key, so a + # `NotIn`/`DoesNotExist` expression beside an ordinary label matches that + # label in every namespace. - name: fromEndpointKeysAllowed expression: >- variables.fromEndpoints.map(selector, @@ -258,7 +262,10 @@ spec: selector.?matchExpressions.orValue([]).map(item, item.?key.orValue(''))).flatten() .all(key, !key.matches(variables.broadSelectorKey) || key in ['reserved:kube-apiserver', 'kube-apiserver', 'any:kube-apiserver', - 'reserved:ingress', 'ingress', 'any:ingress']) + 'reserved:ingress', 'ingress', 'any:ingress']) && + variables.fromEndpoints.all(selector, selector.?matchExpressions.orValue([]).all(item, + !item.?key.orValue('').matches(variables.broadSelectorKey) || + item.?operator.orValue('') in ['In', 'Exists'])) # A rule with no layer-3 stanza permits every source on its port. An empty # stanza names nothing, so it does not count. - name: ingressRulesNameSources @@ -297,13 +304,17 @@ spec: size(selector.?matchLabels.orValue({})) > 0 || selector.?matchExpressions.orValue([]).exists(item, item.?operator.orValue('') == 'In')) # `reserved:ingress` is a SOURCE identity and has no egress case. + # Carved-out keys are positive-only here too (see fromEndpointKeysAllowed). - name: toEndpointKeysAllowed expression: >- variables.toEndpoints.map(selector, selector.?matchLabels.orValue({}).map(key, key) + selector.?matchExpressions.orValue([]).map(item, item.?key.orValue(''))).flatten() .all(key, !key.matches(variables.broadSelectorKey) || - key in ['reserved:kube-apiserver', 'kube-apiserver', 'any:kube-apiserver']) + key in ['reserved:kube-apiserver', 'kube-apiserver', 'any:kube-apiserver']) && + variables.toEndpoints.all(selector, selector.?matchExpressions.orValue([]).all(item, + !item.?key.orValue('').matches(variables.broadSelectorKey) || + item.?operator.orValue('') in ['In', 'Exists'])) - name: egressRulesNameDestinations expression: >- variables.egress.all(rule, @@ -368,7 +379,9 @@ spec: `io.cilium.k8s.namespace.labels.*`) or another cluster (`io.cilium.k8s.policy.cluster`) reaches the same broad source — pin a tenant label with the `k8s:` prefix instead, and name the post-Gateway - identity as `reserved:ingress` or `fromEntities: [ingress]`; only + identity as `reserved:ingress` or `fromEntities: [ingress]`; a + carved-out reserved key in matchExpressions must use `In` or `Exists`, + because a negative operator there lifts the namespace pin; only `kube-apiserver` and the post-Gateway `ingress` identity may be named as an entity; and a CIDR prefix shorter than /16 — or a CIDR inside private, cluster, link-local, loopback or multicast space, or a @@ -394,8 +407,9 @@ spec: another namespace (`k8s:io.kubernetes.pod.namespace`, `io.cilium.k8s.namespace.labels.*`) or another cluster (`io.cilium.k8s.policy.cluster`) reaches the same broad destination — - pin a tenant label with the `k8s:` prefix instead; `kube-apiserver` is - the only entity a tenant may name; and a CIDR prefix shorter than /16 + pin a tenant label with the `k8s:` prefix instead; a carved-out + `kube-apiserver` key in matchExpressions must use `In` or `Exists`; + `kube-apiserver` is the only entity a tenant may name; and a CIDR prefix shorter than /16 — or a CIDR inside private, cluster, link-local, loopback or multicast space, or a CIDRSet entry naming a CiliumCIDRGroup, with or without a literal cidr beside it — reaches outside this diff --git a/scripts/tests/test-restrict-tenant-network-policies.sh b/scripts/tests/test-restrict-tenant-network-policies.sh index efbc5a7cb9..24e0500189 100755 --- a/scripts/tests/test-restrict-tenant-network-policies.sh +++ b/scripts/tests/test-restrict-tenant-network-policies.sh @@ -41,7 +41,7 @@ expect_summary() { # used to be refused now admits. The unmanaged namespace control is an explicit # match-condition skip; a namespaceSelector previously omitted its result. apply "${tests}/resources.yaml" "${tests}/values.yaml" "${tests}/user-info.yaml" -expect_summary "pass: 18, fail: 78, warn: 0, error: 0, skip: 1" \ +expect_summary "pass: 18, fail: 80, warn: 0, error: 0, skip: 1" \ "tenant CiliumNetworkPolicy boundary" # 2. Carve-out: kyverno's background controller owns the generated floor. These diff --git a/tests/restrict-tenant-network-policies/kyverno-author/kyverno-test.yaml b/tests/restrict-tenant-network-policies/kyverno-author/kyverno-test.yaml index eed05270a8..202c0e880b 100644 --- a/tests/restrict-tenant-network-policies/kyverno-author/kyverno-test.yaml +++ b/tests/restrict-tenant-network-policies/kyverno-author/kyverno-test.yaml @@ -1,8 +1,8 @@ --- # One row per resource: fail when any check refuses it. Which check refuses it is # in per-check.yaml, which scripts/tests/test-restrict-tenant-network-policies.sh -# runs. The policy uses a namespaceSelector, so the CLI needs the namespace labels -# declared explicitly. +# runs. The policy reads namespace labels through namespaceObject in matchConditions, +# so the CLI needs the namespace labels declared explicitly. apiVersion: cli.kyverno.io/v1alpha1 kind: Test metadata: diff --git a/tests/restrict-tenant-network-policies/kyverno-test.yaml b/tests/restrict-tenant-network-policies/kyverno-test.yaml index 6a25b64506..87a5f53d0c 100644 --- a/tests/restrict-tenant-network-policies/kyverno-test.yaml +++ b/tests/restrict-tenant-network-policies/kyverno-test.yaml @@ -1,8 +1,8 @@ --- # One row per resource: fail when any check refuses it. Which check refuses it is # in per-check.yaml, which scripts/tests/test-restrict-tenant-network-policies.sh -# runs. The policy uses a namespaceSelector, so the CLI needs the namespace labels -# declared explicitly. +# runs. The policy reads namespace labels through namespaceObject in matchConditions, +# so the CLI needs the namespace labels declared explicitly. apiVersion: cli.kyverno.io/v1alpha1 kind: Test metadata: @@ -65,8 +65,10 @@ results: - tenant-ns/attack-selector-exists-universal - tenant-ns/attack-selector-namespace-labels-egress - tenant-ns/attack-selector-reserved-beside-constrained-egress + - tenant-ns/attack-selector-reserved-does-not-exist-egress - tenant-ns/attack-selector-reserved-expression-egress - tenant-ns/attack-selector-reserved-host-ingress + - tenant-ns/attack-selector-reserved-not-in-ingress - tenant-ns/attack-selector-reserved-world-egress - tenant-ns/attack-specs-container-egress-world - tenant-ns/attack-togroups-empty-list diff --git a/tests/restrict-tenant-network-policies/per-check.yaml b/tests/restrict-tenant-network-policies/per-check.yaml index 41f26837bd..6c7985ee93 100644 --- a/tests/restrict-tenant-network-policies/per-check.yaml +++ b/tests/restrict-tenant-network-policies/per-check.yaml @@ -9,8 +9,9 @@ policies: resources: - resources.yaml - node-group-resources.yaml -# The policy uses a namespaceSelector, so the CLI needs the namespace labels -# declared explicitly or every case is reported as Excluded — which satisfies +# The policy reads namespace labels through namespaceObject in matchConditions, +# so the CLI needs the namespace labels declared explicitly or every case is +# reported as Excluded — which satisfies # `result: fail` too and would pass vacuously. The companion shell test # (scripts/tests/test-restrict-tenant-network-policies.sh) is the second gate: # it runs `kyverno apply` and asserts the exact allow/deny counts, so a rule @@ -174,12 +175,14 @@ results: - tenant-ns/attack-selector-namespace-labels-egress - tenant-ns/attack-selector-cluster-name-egress - tenant-ns/attack-selector-reserved-beside-constrained-egress + - tenant-ns/attack-selector-reserved-does-not-exist-egress kind: CiliumNetworkPolicy result: fail - policy: restrict-tenant-network-policies rule: cnp-ingress-sources-must-be-constrained resources: - tenant-ns/attack-selector-reserved-host-ingress + - tenant-ns/attack-selector-reserved-not-in-ingress - tenant-ns/attack-selector-cross-namespace-ingress - tenant-ns/attack-selector-bare-namespace-ingress kind: CiliumNetworkPolicy diff --git a/tests/restrict-tenant-network-policies/platform-author/kyverno-test.yaml b/tests/restrict-tenant-network-policies/platform-author/kyverno-test.yaml index 3f9dbf42f6..4521d48cd6 100644 --- a/tests/restrict-tenant-network-policies/platform-author/kyverno-test.yaml +++ b/tests/restrict-tenant-network-policies/platform-author/kyverno-test.yaml @@ -1,8 +1,8 @@ --- # One row per resource: fail when any check refuses it. Which check refuses it is # in per-check.yaml, which scripts/tests/test-restrict-tenant-network-policies.sh -# runs. The policy uses a namespaceSelector, so the CLI needs the namespace labels -# declared explicitly. +# runs. The policy reads namespace labels through namespaceObject in matchConditions, +# so the CLI needs the namespace labels declared explicitly. apiVersion: cli.kyverno.io/v1alpha1 kind: Test metadata: diff --git a/tests/restrict-tenant-network-policies/resources.yaml b/tests/restrict-tenant-network-policies/resources.yaml index 9f188c0941..025e4cac2a 100644 --- a/tests/restrict-tenant-network-policies/resources.yaml +++ b/tests/restrict-tenant-network-policies/resources.yaml @@ -832,6 +832,46 @@ spec: - matchLabels: io.cilium.k8s.policy.cluster: other-cluster --- +# ATTACK: a carved-out reserved key matched NEGATIVELY beside an ordinary label. +# Cilium skips namespace injection for a selector that names a `reserved:` key, +# so `DoesNotExist` here selects `app: db` in every namespace. +apiVersion: cilium.io/v2 +kind: CiliumNetworkPolicy +metadata: + name: attack-selector-reserved-does-not-exist-egress + namespace: tenant-ns +spec: + endpointSelector: + matchLabels: + app: web + egress: + - toEndpoints: + - matchLabels: + app: db + matchExpressions: + - key: reserved:kube-apiserver + operator: DoesNotExist +--- +# ATTACK, ingress side: the same escape through `NotIn` on `reserved:ingress`. +apiVersion: cilium.io/v2 +kind: CiliumNetworkPolicy +metadata: + name: attack-selector-reserved-not-in-ingress + namespace: tenant-ns +spec: + endpointSelector: + matchLabels: + app: web + ingress: + - fromEndpoints: + - matchLabels: + app: client + matchExpressions: + - key: reserved:ingress + operator: NotIn + values: + - "" +--- # ATTACK: a reserved identity sitting BESIDE a properly pinned selector. Cilium # unions allow rules, so the broad entry decides — the check has to be per # selector, not "does this policy pin any ordinary label". diff --git a/tests/restrict-tenant-network-policies/tenant-scaffold/kyverno-test.yaml b/tests/restrict-tenant-network-policies/tenant-scaffold/kyverno-test.yaml index a63cb36a8e..5ecd67b038 100644 --- a/tests/restrict-tenant-network-policies/tenant-scaffold/kyverno-test.yaml +++ b/tests/restrict-tenant-network-policies/tenant-scaffold/kyverno-test.yaml @@ -1,8 +1,8 @@ --- # One row per resource: fail when any check refuses it. Which check refuses it is # in per-check.yaml, which scripts/tests/test-restrict-tenant-network-policies.sh -# runs. The policy uses a namespaceSelector, so the CLI needs the namespace labels -# declared explicitly. +# runs. The policy reads namespace labels through namespaceObject in matchConditions, +# so the CLI needs the namespace labels declared explicitly. apiVersion: cli.kyverno.io/v1alpha1 kind: Test metadata: From e78ffa53ea9d4b54a237be751a9808bb90d9f44c Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Mon, 9 Nov 2026 10:19:43 +0100 Subject: [PATCH 5/5] test(policy): count the two new reserved-operator attacks in the controller coverage pin Co-Authored-By: Claude Opus 5.5 --- scripts/tests/test-tenant-controller-namespace-coverage.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/tests/test-tenant-controller-namespace-coverage.sh b/scripts/tests/test-tenant-controller-namespace-coverage.sh index 06661e4f83..2526373baf 100755 --- a/scripts/tests/test-tenant-controller-namespace-coverage.sh +++ b/scripts/tests/test-tenant-controller-namespace-coverage.sh @@ -26,7 +26,7 @@ done <<'CASES' restrict-tenant-secret-stores 5 7 0 restrict-tenant-issuer-refs 4 3 0 restrict-tenant-route-hostnames 3 13 0 -restrict-tenant-network-policies 18 78 1 +restrict-tenant-network-policies 18 80 1 CASES [[ "$failures" == 0 ]] || exit 1 echo 'PASS: all four tenant admission families evaluate their full controls under ksail and kro ownership'