Giter Club home page Giter Club logo

Comments (17)

danwinship avatar danwinship commented on August 30, 2024 4

Can we keep this as simple as "Denies should win over allows"?

That really only works if you first compile the Rules in each ANP down into a flat form, a la Cilium. (Policies can have interleaved Allow and Deny rules; you can't just take the Deny rules from a policy and apply them before the Allow rules and get the right result.)


I'm currently thinking the rule should be: "Every ANP should have a unique priority value. If two (or more) ANPs have the same priority value, and match the same connection, then only one of them will be applied to the connection. (It is implementation-defined which one.)"

Any of these implementations would obey that rule:

  • If two ANPs have the same priority then evaluate them in alphabetical order by name.
  • If two ANPs have the same priority then flip a coin at packet processing time to decide which one to evaluate first.
  • If two ANPs have the same priority then "Denies should win over allows". (Or more precisely, if either of them would Deny a packet, then Deny the packet, else if either of them would Accept the packet then Accept the packet, else if either of them would Pass the packet then Pass the packet, otherwise do nothing).
  • If two ANPs have the same priority then interleave the processing of their individual Rules like unsynchronized multithreaded code. YOLO!

If you have two same-priority ANPs, where ANP X says "Allow all to A", and ANP Y says "Allow all to B", then the result must be that all traffic is allowed to both A and B.

If you have two same-priority ANPs, where ANP X says "Allow all to A", and ANP Y says "Allow all to A and B", then the result must be that all traffic is allowed to both A and B. However, if you enable logging/tracing/whatever, it's implementation-defined whether packets to A show up as having been accepted by ANP X or by ANP Y, whereas in the previous case, traffic to A would always show up as having been accepted by ANP X.

If you have two same-priority ANPs, where ANP X says "Allow to A and Deny everywhere else", and ANP Y says "Allow to B and Deny everywhere else", then a conforming implementation could do any of:

  • Allow to A, Deny everywhere else (ie, always process ANP X first)
  • Allow to B, Deny everywhere else (ie, always process ANP Y first)
  • Allow to A and B, Deny everywhere else ("allow wins", "interleave X and Y", "apply X first for packets to A; and Y first for all other packets")
  • Sometimes-but-not-always Allow to A, sometimes-but-not-always Allow to B, always Deny everywhere else. (random)
  • Deny everywhere. ("deny wins")

from network-policy-api.

danwinship avatar danwinship commented on August 30, 2024 1

Why can't we say that this is implementation-specific? AFAIK, here are some current, real vendor implementations:

Those are vendor implementations of vendor APIs. And if users want to use those vendor APIs, they already can! They don't need anything from us.

Having a standard k8s API is only beneficial to users if it's standard.

There are plenty of edge cases and undocumented behavior in K8s netpol beyond this issue

I am trying to get rid of as many as possible of them in ANP. (This is one of the big reasons we don't yet have cluster ingress policy; because it's really unclear how to do that in a way that doesn't end up full of implementation-specific behavior.)

from network-policy-api.

fasaxc avatar fasaxc commented on August 30, 2024 1

On the call, we identified quite significant differences between the "natural" implementations for each vendor but we did agree on a lot. I think we were moving towards agreement that:

  • Having multiple policies at the same priority is inevitable (can't prevent it in the API therefore it will happen and users would complain if we blocked it anyway).
  • If the policies do not "overlap" at runtime, there is no problem, we can all enforce the policies and the semantics are intuitive.
  • We can all make sure that one of the rules from the overlapping policies is applied at runtime (so no undefined behaviour in the C language sense of "may crash, or send packets to the moon").
  • Overlap depends on the policy and the workloads in the cluster so it's a highly dynamic thing and it cannot be calculated from the policies alone (so we can't try to enforce non-overlap).

The different dataplanes break ties in very different ways that "make sense" for their implementations.

  • Calico's internal prioritisation includes the policy name to break ties so it breaks ties at the policy level.
  • (IIUC) Cilium's approach is to group all individual rules by priority and then to prefer deny rules at a given priority.
  • Antrea allows policies at same priority but I didn't quite catch if there was a tie-breaker or if it was non-deterministic.
  • OVN errors out right now but may be changing. Has limits on number of priority levels they can use for this.

My 2p: we should at least document the things that we can agree on. I suspect that agreeing on a single tie breaker may be very hard given all our constraints. It may be worth documenting that some vendors prioritise on rules and some prioritise policies.

FWIW, from experience with calico users, overlapping policies is not a common source of issues; worst case, there's an obvious self-directed fix: just use priorities and you can get the behaviour you want.

from network-policy-api.

tssurya avatar tssurya commented on August 30, 2024

hmm so in OVNKube we took the decision of removing the "undefined" behaviour around this and saying "You cannot do this. No two anp's can be created at the same priority and we error out (so only the 1st one gets created its a bit buggy today because we use caches not the kapi to figure this out to save that list operation for scale reasons but then we just also document, don't do this.)". But seems like we want to actually enforce "they are both applied" ?

from network-policy-api.

astoycos avatar astoycos commented on August 30, 2024

Hrm Is there any use cases for having two distinctly separate rules at the same priority? i.e could we allow a single ANP to have multiple Subject/Rule mapping and then actually say "Users are not allowed to create two ANPs at the same priority"

Or is the problem more so that there's no mechanism for us to actually stop users from creating two ANPs at the same priority?

from network-policy-api.

tssurya avatar tssurya commented on August 30, 2024

Hrm Is there any use cases for having two distinctly separate rules at the same priority? i.e could we allow a single ANP to have multiple Subject/Rule mapping and then actually say "Users are not allowed to create two ANPs at the same priority"

yea I can't think of any use case for why I want two ANPs at same prio unless I actually have 100+ ingress and egress rules I want at same priority, so you now need two ANPs at same prio but then again since we have precedence within the single ANP it doesn't make sense because you don't now have a way to compare the two ANPs' rules

Or is the problem more so that there's no mechanism for us to actually stop users from creating two ANPs at the same priority?

I think its this really. If we can perhaps have a CEL expression to ensure priority field is unique across the crds maybe that is something we want to explore?

from network-policy-api.

tssurya avatar tssurya commented on August 30, 2024

btw I know this might not be accessible to all because its a slack link, but I wanted to put this here for posterity that I tried to add this validation a few months ago: https://redhat-internal.slack.com/archives/CB48XQ4KZ/p1691419205773149 but couldn't and api team confirmed there is no way to do this for stuff outside the metadata.

from network-policy-api.

danwinship avatar danwinship commented on August 30, 2024

and we error out

oh, you can't do that. Customers will complain. We tried it with a few things in openshift-sdn (like "you can't have more than one EgressNetworkPolicy object per Namespace and if you do, neither of them works") and that led to us having to figure out a better way to do it in ovn-k EgressFirewall ("an EgressFirewall object must be named 'default'")

from network-policy-api.

danwinship avatar danwinship commented on August 30, 2024

Or is the problem more so that there's no mechanism for us to actually stop users from creating two ANPs at the same priority?

This. We can't prevent it from happening, so it doesn't matter if there's a good use case for it or not. It needs clear semantics.

And I think there are good use cases for "two people create ANPs without coordination between them". If there is any scenario where it makes sense to include an ANP as part of a helm chart, third-party OLM operator, etc, to ensure some default set of permissions around a component, then you have to assume that eventually two components will ship ANPs with the same priority.

from network-policy-api.

aojea avatar aojea commented on August 30, 2024

why not sort lexicographical , first priority number, second name

from network-policy-api.

danwinship avatar danwinship commented on August 30, 2024

We don't want people to depend on any specific behavior; if there is any situation where someone needs to depend on the relative ordering of two policies with the same priority, then that means that we have failed and priority was not a good API and we need something else.

(eg, there was discussion in the KEP about having a combined Action and Priority, so like first all the "VeryHighPriorityDeny" rules from all policies take effect, then all the "HighPriorityAllow" rules from all policies, then "MediumPriorityDeny", then "LowPriorityAllow", then "VeryLowPriorityDeny". Or vice versa. Nobody could agree on whether the highest priority should be "allow" or "deny". But anyway, that approach would avoid the "conflicting ANPs" problem because any two rules with the same priority would also have the same action and so could be evaluated in either order.)

(I'm not suggesting we should switch to that. I hope we can make priority work.)

from network-policy-api.

tssurya avatar tssurya commented on August 30, 2024

And I think there are good use cases for "two people create ANPs without coordination between them". Iif there is any scenario where it makes sense to include an ANP as part of a helm chart, third-party OLM operator, etc, to ensure some default set of permissions around a component, then you have to assume that eventually two components will ship ANPs with the same priority.

ok I found a use case for this :D (someone actually asked me this and that's how I found a use case for this); its useful to create two ANPs at same priority currently because we offer no support for tenancy currently, example I have:
tenant: coke and tenant: pepsi labels on coke(nsC,D) and pepsi (nsA,B) tenants. I want to simply express "allow to myself and deny to others". Today we'd need to do two different ANPs for this since subject is pretty much pinned here without tenancy and we don't have relational isSame,notSame peers anymore:

ANP1:

  subject:
     namespaces:
        tenant: coke
  peer:
    ingress:
        - allow
          namespaces:
             tenant: coke

ANP2:

  subject:
     namespaces:
        tenant: pepsi
  peer:
    ingress:
        - allow
          namespaces:
             tenant: pepsi

finally the default deny BANP:

  subject:
     namespaces: {}
  peer:
    ingress:
        - deny
          namespaces: {}

now if I have a 100 tenants, I can very easily just create that ANP at same prio they don't overlap at all..if not I will be wasting one prio per ANP which makes no sense.
So currently this is useful because its not like we can express subject: {} and allow to myself and deny to others at the moment, so users will be forced to do one ANP per tenant.

from network-policy-api.

tssurya avatar tssurya commented on August 30, 2024

and we error out

oh, you can't do that. Customers will complain. We tried it with a few things in openshift-sdn (like "you can't have more than one EgressNetworkPolicy object per Namespace and if you do, neither of them works") and that led to us having to figure out a better way to do it in ovn-k EgressFirewall ("an EgressFirewall object must be named 'default'")

ah good to know! glad we spoke before we GA-ed this :D I am considering removing this in OVNK and just supporting multiple ANPs at same prio but being very careful around wordings in docs like keep them non-overlapping for best results etc.

from network-policy-api.

nathanjsweet avatar nathanjsweet commented on August 30, 2024

It's unacceptable to say that the behavior when you have two ANPs with the same priority is completely undefined.

Why can't we say that this is implementation-specific? AFAIK, here are some current, real vendor implementations:

  • Error out (yes, customers might complain, but why isn't that the vendor's issue, why is it this SIG's responsibility?).
  • Have further precedence logic that is documented.
  • Have further precedence logic that is not documented.

I haven't seen a good argument in this discussion that pertains to API consistency only in this thread. There are plenty of edge cases and undocumented behavior in K8s netpol beyond this issue, is there more context that I'm not getting as to why this issue must must be consistent across vendors? Or do we just need more explicit documentation as to the different ways vendors will handle this problem?

from network-policy-api.

aojea avatar aojea commented on August 30, 2024

I want to reinforce what Dan says, API must define the semantics, not only the fields ...

from network-policy-api.

nathanjsweet avatar nathanjsweet commented on August 30, 2024

Can we keep this as simple as "Denies should win over allows"? That seems intuitive to me. A deny-rule, from a security perspective, is more important than an allow. Any other precedence rules after this are unnecessary, right? Even in the case of a broad-deny (for example, a simple deny-all for a specific selector) and a very specific allow (for example, an allow with a specific port and protocol) the deny should win. Are there any other cases? To the extent that a deny and allow are disjoint the allow-rule can still allow traffic.

from network-policy-api.

nathanjsweet avatar nathanjsweet commented on August 30, 2024

I'm currently thinking the rule should be: "Every ANP should have a unique priority value. If two (or more) ANPs have the same priority value, and match the same connection, then only one of them will be applied to the connection. (It is implementation-defined which one.)"

This works.

from network-policy-api.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.