Skip to content
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.

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

NotSameLabel Match or Invert match for SameLabel #6424

Closed
jsalatiel opened this issue Jun 8, 2024 · 5 comments
Closed

NotSameLabel Match or Invert match for SameLabel #6424

jsalatiel opened this issue Jun 8, 2024 · 5 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@jsalatiel
Copy link

Now that antrea 2.0 has been released with new features I have started tryinng to simplify my netpolicies.
Reading the documentation I saw the SameLabels matcher and I thought that it would help me to reduce a pretty big acnp I have, but unfortunately I could not make that work because actually I would need an invert match for that. I will write an use case here using the org label ( for tenants ) that you have in documentation.

Scenario:
One has a cluster with several tenants and these tenants can never cross communicate. They can only talk to the same tenant. There is also a need of deny all in baseline tier. Dev team in any org/tenant can create ANP in application tier only, being denied any other tier or ACNP creation by a policy engine.
The cluster operator, to comply with those requirements, have created an ACNP in SecurityOps tier with the following ( antrea 1.14) :
( Since there is no dynamic matching, it has to match any org by name, so I will do some simplification in the scope of this example: 3 orgs and only egress )

apiVersion: crd.antrea.io/v1alpha1
kind: ClusterNetworkPolicy
spec:
    priority: 3
    tier: SecurityOps
    egress:
    # ORG1    
    - name: org1
      action: Drop
      appliedTo:
      - namespaceSelector:
          matchLabels:
            org: 'org1'
      to:
      - namespaceSelector:
          matchExpressions:
          - { key: env, operator: NotIn, values: [ org1 ] }
    # ORG2      
    - name: org2
      action: Drop
      appliedTo:
      - namespaceSelector:
          matchLabels:
            org: 'org2'
      to:
      - namespaceSelector:
          matchExpressions:
          - { key: env, operator: NotIn, values: [ org2 ] }
    # ORG3
    - name: org3
      action: Drop
      appliedTo:
      - namespaceSelector:
          matchLabels:
            org: 'org3'
      to:
      - namespaceSelector:
          matchExpressions:
          - { key: env, operator: NotIn, values: [ org3 ] }

That policy works pretty well:

  1. All pods are still denied by default by the deny-all policy in Baseline tier
  2. If namespace1 from org1 wants to talk to namespace2 from org1, the developers will need to create a ANP for that in application tier ( the only tier they can create policies inside)
  3. If some developer malicious try to create a ANP in Application tier to allow to talk to a namespace from another org that traffic will be blocked by the policy created by the operator in the SecurityOps tier.

The problem with the policy is that it is not scalable. Every new org needs a hardcoded in the policy file and that can grow too much.

So I decided to check if SameLabels matcher could help. First I tried directly the example from the docs:

apiVersion: crd.antrea.io/v1beta1
kind: ClusterNetworkPolicy
metadata:
  name: isolation-based-on-org
spec:
  priority: 1
  tier: securityops
  appliedTo:
    - namespaceSelector:
        matchExpressions:
          - { key: org, operator: Exists }
  egress:
    - action: Allow
      to:
        - namespaces:
            sameLabels: [org]
    - action: Deny

The traffic between orgs is indeed blocked, but the traffic inside the same org will be accepted by default ( ignoring the deny all from baseline tier )

So i tried a bit different

---

apiVersion: crd.antrea.io/v1beta1
kind: ClusterNetworkPolicy
metadata:
  name: 201-isolate-by-env
spec:
  priority: 1
  tier: SecurityOps
  appliedTo:
  - namespaceSelector:
      matchExpressions:
      - { key: org, operator: Exists }
  egress:
    - action: Pass
      to:
        - namespaces:
            sameLabels: [org]
    - action: Drop
      enableLogging: true

In this case the traffic from different orgs would be dropped and the traffic from the same org would reach the baseline tier and be dropped, the problem is that the Pass for the sameLabel will make all other tiers be skipped including any ANP in application tier created by the developers.

So I was thinking if antrea could implement a notSameLabels operator that would allow the following rule:

apiVersion: crd.antrea.io/v1beta1
kind: ClusterNetworkPolicy
metadata:
  name: 201-isolate-by-env
spec:
  priority: 1
  tier: SecurityOps
  appliedTo:
  - namespaceSelector:
      matchExpressions:
      - { key: org, operator: Exists }
  egress:
    - action: Drop
      to:
        - namespaces:
            notSameLabels: [org]

That should work dropping any traffic when "org" exists in the namespace and they do not match and since that's dynamic any new org added would work without requiring changes to the acnp.

@jsalatiel jsalatiel added the kind/feature Categorizes issue or PR as related to a new feature. label Jun 8, 2024
@antoninbas
Copy link
Contributor

@Dyanngg could you take a look at this?

@Dyanngg
Copy link
Contributor

Dyanngg commented Jun 11, 2024

@jsalatiel if you look at the documentation https://github.com/antrea-io/antrea/blob/main/docs/antrea-network-policy.md#antrea-networkpolicy, the namespace-scoped AntreaNetworkPolicy resource is actually targeted for admins as well, not for namespace developers. Given that Antrea does not know about any external policy engines, it does not make sense to grant developers access to all the Tiers Antrea-native policies offer. We envision developers creating K8s NetworkPolicies in their namespaces and the last 'Pass' policy would work in that case.
We had thought above adding "notSelf" or "notSameLabels" support, but with the current datapath design, it is either going to require fundamental changes in Antrea rule computation, or become unscalable, given that for each "not" address group we need to compute the compliment of these addresses in the entire IP space

@jsalatiel
Copy link
Author

Hi @Dyanngg, the problem with KNP ( and the reason why we always forced the developers to avoid it ) is the implicit isolation when anything selects a pod. That goes way different from what you see in common firewalls.
But I got your point.

@antoninbas
Copy link
Contributor

@jsalatiel We discussed this at our community meeting this week, and at the moment we do not believe that this is something we should add to the API, for the reasons described above.
If writing the policy manually becomes cumbersome, have you considered a programmatic approach to monitor Namespaces / org labels, and update the policy automatically, using a K8s controller pattern?

Writing a policy like this one (your example):

apiVersion: crd.antrea.io/v1alpha1
kind: ClusterNetworkPolicy
spec:
    priority: 3
    tier: SecurityOps
    egress:
    # ORG1    
    - name: org1
      action: Drop
      appliedTo:
      - namespaceSelector:
          matchLabels:
            org: 'org1'
      to:
      - namespaceSelector:
          matchExpressions:
          - { key: env, operator: NotIn, values: [ org1 ] }
    # ORG2      
    - name: org2
      action: Drop
      appliedTo:
      - namespaceSelector:
          matchLabels:
            org: 'org2'
      to:
      - namespaceSelector:
          matchExpressions:
          - { key: env, operator: NotIn, values: [ org2 ] }
    # ORG3
    - name: org3
      action: Drop
      appliedTo:
      - namespaceSelector:
          matchLabels:
            org: 'org3'
      to:
      - namespaceSelector:
          matchExpressions:
          - { key: env, operator: NotIn, values: [ org3 ] }

does not scale very well from an implementation perspective, for the same reason that this policy would not scale very well if it were supported:

apiVersion: crd.antrea.io/v1beta1
kind: ClusterNetworkPolicy
metadata:
  name: 201-isolate-by-env
spec:
  priority: 1
  tier: SecurityOps
  appliedTo:
  - namespaceSelector:
      matchExpressions:
      - { key: org, operator: Exists }
  egress:
    - action: Drop
      to:
        - namespaces:
            notSameLabels: [org]

@Dyanngg can correct me if I am wrong, but actually writing N^2 policy rules (with N being the number of organizations) may perform better assuming that N is reasonable (<100?) and assuming that you can generate / manage the policies manually. The policies would look like this:

apiVersion: crd.antrea.io/v1alpha1
kind: ClusterNetworkPolicy
spec:
    priority: 3
    tier: SecurityOps
    name: org1
    appliedTo:
    - namespaceSelector:
        matchLabels:
          org: 'org1'
    egress:
    - name: org1-to-org2
      action: Drop
      to:
      - namespaceSelector:
          matchLabels:
            org: 'org2'
    - name: org1-to-org3
      action: Drop
      to:
      - namespaceSelector:
          matchLabels:
            org: 'org3'

You would end up with N policies, each with N-1 rules. My understanding of the implementation (cc @Dyanngg @tnqn) is that with this approach you would only have N AddressGroups (one for each organization) as AddressGroups can be shared, and their total size would be O(X) with X being the number of Pods in the cluster, across all organizations. With your original example, you would also have N AddressGroups, but their total size would be O(X^2), as each AddressGroup would include all Pods not in the appliedTo organization.

Copy link
Contributor

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment, or this will be closed in 90 days

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 17, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

No branches or pull requests

3 participants