Skip to content

[v1.16] Reject CNPs with CIDRGroupRef used in combination with ExceptCIDRs #36561

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Dec 13, 2024
Merged

[v1.16] Reject CNPs with CIDRGroupRef used in combination with ExceptCIDRs #36561

merged 1 commit into from
Dec 13, 2024

Conversation

pippolo84
Copy link
Member

@pippolo84 pippolo84 commented Dec 12, 2024 •
edited
Loading

In CNP CIDR rules, ExceptCIDRs is supported only in combination with a CIDR, but not with a CIDR group reference.

An example of a not supported CNP is the following:

apiVersion: "cilium.io/v2"
kind: CiliumNetworkPolicy
metadata:
  name: client-egress-to-cidrgroup-deny
spec:
  endpointSelector:
    matchLabels:
      kind: client
  egressDeny:
  - toCIDRSet:
    - cidrGroupRef: cilium-test-external-cidr
      except:
        - "1.1.1.1/32"

The PR adds a specific validation step to early reject this kind of policies before trying to parse it.

Reject CNP/CCNP with CIDR rules where CIDRGroupRef is used in combination with ExceptCIDRs

@pippolo84 pippolo84 added kind/bug This is a bug in the Cilium logic. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies. labels Dec 12, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added backport/1.16 This PR represents a backport for Cilium 1.16.x of a PR that was merged to main. kind/backports This PR provides functionality previously merged into master. labels Dec 12, 2024
@pippolo84 pippolo84 marked this pull request as ready for review December 12, 2024 23:19
@pippolo84 pippolo84 requested a review from a team as a code owner December 12, 2024 23:19
@pippolo84
Copy link
Member Author

/test-backport-1.16

@pippolo84
policy: Reject CNP when ExceptCIDRs is used with CIDRGroupRef
f47f12a 
Since the usage of ExceptCIDRs is not supported in conjunction with
CIDRGroupRef, scan each CIDR rule in the policy and reject it in case a
rule specifying the unallowed combination is found.

Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
@pippolo84 pippolo84 force-pushed the pr/pippolo84/v1.16-reject-cnp-cidrgroupref-except branch from 8ac3acf to f47f12a Compare December 12, 2024 23:40
@pippolo84
Copy link
Member Author

/test-backport-1.16

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Dec 13, 2024
@tklauser tklauser added this pull request to the merge queue Dec 13, 2024
Merged via the queue into cilium:v1.16 with commit 397a2eb Dec 13, 2024
63 checks passed
@joestringer joestringer added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Dec 13, 2024
This was referenced Jan 15, 2025
Merged
Merged
@pippolo84 pippolo84 mentioned this pull request Jun 20, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christarazi christarazi christarazi approved these changes

Assignees
No one assigned
Labels
backport/1.16 This PR represents a backport for Cilium 1.16.x of a PR that was merged to main. kind/backports This PR provides functionality previously merged into master. kind/bug This is a bug in the Cilium logic. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pippolo84 @christarazi @tklauser @joestringer