CFP-42904: Rule evaluation order with multiple network policies#84
CFP-42904: Rule evaluation order with multiple network policies#84TheBeeZee wants to merge 1 commit intocilium:mainfrom
Conversation
525ddc6 to
7b995c2
Compare
cilium/CFP-42904-policy-ordering.md
Outdated
| @@ -0,0 +1,113 @@ | |||
| # CFP-39646: Decouple internal policy engine from external API | |||
There was a problem hiding this comment.
| # CFP-39646: Decouple internal policy engine from external API | |
| # CFP-42904: Rule evaluation order with multiple network policies |
|
cc: @cilium/sig-policy |
jrajahalme
left a comment
There was a problem hiding this comment.
Some early notes about proposed priority overlaps.
7b995c2 to
a349b83
Compare
43042cc to
e7f7392
Compare
|
I have realized that PR 42784 only support 1<<24 - 1 priorities, because the upper 8 bits of the Precedence field are used to map the ProxyPort. With this in mind, the maximum supported priority is 1.67 million, which is less than the original proposal in this design. I have updated the design with much more conservative numeric ranges, 100000 for each tier. |
|
|
||
| There are three levels of ordering: | ||
|
|
||
| ### Tier |
There was a problem hiding this comment.
One question came up during the community meeting today: Is the intended persona for all Kubernetes ClusterNetworkPolicy resources also the cluster-wide administrator?
There was a problem hiding this comment.
Yes, but there could be multiple admins. That's one of the reasons why the CNPs are prioritized.
|
|
||
| The interaction between ClusterNetworkPolicy and v1.NetworkPolicy is well defined in the ClusterNetworkPolicy CRD and is described above. | ||
|
|
||
| CiliumNetworkPolicy and CiliumClusterwideNetworkPolicy do not support explicit rule ordering. Currently, CiliumNetworkPolicy and CiliumClusterwideNetworkPolicy rules are ordered implicitly: Deny rules take precedence over Allow rules, and more specific L4/L7 rules take precedence over less specific ones. Otherwise, all rules effectively have the same priority. To maintain backward compatibility, CiliumNetworkPolicy and CiliumClusterwideNetworkPolicy will be placed within the NetworkPolicy tier. |
There was a problem hiding this comment.
and more specific L4/L7 rules take precedence over less specific ones
nit, I think this is a bit simplified and glosses over a bit of the nuance of how these rules work. I'm not sure whether the subtlety is material to the overall CFP, but I'll clarify just in case.
- If there is a deny rule, the traffic is denied. True. This takes precedence.
- If there is an allow rule, that traffic is allowed if not denied by (1).
- If there is an allow rule with L7 filtering, then all traffic on the target port+protocol is sent to the L7 processor, then the traffic is allowed based on the union of the L3/L4 and L7 components.
Implication of the last rule is that if there is an (L3+)L4+L7 rule with a fairly limited "allow" (such as only allow GET /public), then there is another L3/L4 rule which also allows the same L3/L4 destinations as the L7 rule, then all traffic on the overlapping L3+L4 is allowed. This includes other L7 destinations not explicitly listed by the L7 rule.
I'm not sure if I would necessarily describe that as "L7 rules take precedence" rather than "allow rules ensure that traffic described by the allow rule is allowed, unless otherwise explicitly denied by another rule" + "the presence of an L7 match field in the policy rule enables all traffic on the same port+proto to be subject to L7 processing, including metrics and events generation"
(From a pure API design perspective these side effects / implicit behaviors are not perfectly captured. They are important to the desired operation of Cilium though, so breaking them could cause surprising behavior.)
There was a problem hiding this comment.
I'll note that this behaviour is the case today with any mix of CNP, CCNP and KNP policies. There could be a CiliumNetworkPolicy or CiliumClusterwideNetworkPolicy rule with an L7 allow, then another overlapping allow in a Kubernetes NetworkPolicy and this would cause the traffic on the overlapping L3/L4 to generate L7 metrics, potentially with specific URLs/methods/etc.
There was a problem hiding this comment.
(Footnote: This whole time, I was operating under the assumption that a L7 allow "cut off" a L4 allow. I just verified and, yes, an overlapping L7 and L3 will compound)
|
|
||
| #### Cons | ||
|
|
||
| * Backwards incompatible. Currently CCNP has the same priority as k8s NP and CilumNetworkPolicy. |
There was a problem hiding this comment.
OK, I guess this is where the earlier discussion comes into play. The merging behavior of L7 rules between CNP and CCNP implies the same precedence level, so if CCNP was moved to a higher precedence level without retaining the rules at the same precedence level as the CNP/KNP rules then this behavior would be broken.
There was a problem hiding this comment.
(I emphasized moved here as I was pondering whether there's another alternative path where CCNP rules are copied to apply both at highest precedence and at the same precedence as NP/CNP... interesting thought whether that might help to solve some API compatibility concerns raised in other threads. More thought necessary to decide if that's helpful or viable. Definitely more complicated, which I don't love.. but do I love it less than breaking compatibility? I'm not sure.)
There was a problem hiding this comment.
After some further thought, "copying" doesn't solve the problem I was concerned about, which is ensuring that the merging of L7 properties performs the same as today. Probably there's too much complexity with that option to seriously consider it.
| #### Cons | ||
|
|
||
| * Backwards incompatible. Currently CCNP has the same priority as k8s NP and CilumNetworkPolicy. | ||
| * Relative priority vs ClusterNetworkPolicy is unclear. Should it be higher, lower or equal? |
There was a problem hiding this comment.
One aspect is this: Today, if a cluster administrator wishes to implement a blanket denylist, they can create that denylist with CCNP and there is no way to override that denylist. You could imagine using this for DoS mitigation, geolocation compliance, or just general network segmentation use cases.
The implication of putting Kubernetes Cluster Network Policy/KCNP at a higher precedence than CCNP means that it is now possible to override the intent of the CCNP.
If we are requiring that the persona who manages CCNP and KCNP are the same persona with the same threat model and so on, then I could imagine we simply document this to say that it is your responsibility as a cluster administrator to understand this difference and that if you opt to allow KCNP resources to be created in the cluster, then you are also responsible for (a) locking access down and (b) ensuring the cluster is matching your intended deny posture.
There was a problem hiding this comment.
I would not expect a cluster to have both CCNP and KCNP. Existing users of CCNP can continue using that, if they wish. New users can choose KCNP and skip CCNP altogether.
Combining both in a single cluster invites trouble, as the complexity of determining what the actual effective policy is, becomes quite high. I expect KCNP and CCNP to co-exist only temporary during a migration from CCNP to KCNP.
If we really want to encourage permanent CCNP and KCNP co-existence, then there are several solutions:
- Add explicit priority support to CCNP.
- Add a knob that can switch CCNP from being lower than KCNP to being higher than KCNP, although that then breaks KNP/CNP semantics.
Given that the target audience of KCNP is largely the same as CCNP, is there truly an issue here?
There was a problem hiding this comment.
Given that the personas are the same, I think this is more of a docs & user education issue than a technical issue to solve. I'm inclined to agree that mixing KCNP+CCNP at the same time will introduce unnecessary complexity to both maintenance and usage in the clusters.
There was a problem hiding this comment.
Raising a point that was previously discussed in the policy sync - I'm curious how a service provider may be able to implement a blanket denylist in this model, while not impeding cluster admin's choices for policies. Today, CCNP deny policies effectively are a blanket deny. Is it equivalent to have one or more admin tier policies of priority 0?
There was a problem hiding this comment.
Perhaps an alternative way to phrase my question is, what happens if there are conflicting rules of equal priority? Does deny take precedence?
There was a problem hiding this comment.
Deny still takes precedence at the same priority level.
There was a problem hiding this comment.
Some service providers are using the commandline flag to preload high priority deny policies. I think that's probably a reasonable mechanism. I understand this doesn't make the policies viewable via standard k8s mechanisms, but unless we invent yet another policy resource just for service providers, I don't think we could set appropriate RBAC for that. Probably that is more effort than it's worth if SPs can just sideload the policies and restrict tampering with the cilium-agent daemonset.
| The interaction between ClusterNetworkPolicy and v1.NetworkPolicy is well defined in the ClusterNetworkPolicy CRD and is described above. | ||
|
|
||
| CiliumNetworkPolicy and CiliumClusterwideNetworkPolicy do not support explicit rule ordering. Currently, CiliumNetworkPolicy and CiliumClusterwideNetworkPolicy rules are ordered implicitly: Deny rules take precedence over Allow rules, and more specific L4/L7 rules take precedence over less specific ones. Otherwise, all rules effectively have the same priority. To maintain backward compatibility, CiliumNetworkPolicy and CiliumClusterwideNetworkPolicy will be placed within the NetworkPolicy tier. | ||
|
|
There was a problem hiding this comment.
I'll start a fresh thread here but it's relating to the observability use case, and is related to the L7 precedence discussion earlier.
Today if a user wants to gain L7 visibility into their environment, we have this documentation: https://docs.cilium.io/en/stable/observability/visibility/#layer-7-protocol-visibility . If you create these rules then Cilium will emit events with Layer 7 information.
I infer from this proposal that KCNP would take precedence over CNP/CCNP, and that "allows" at the higher precedence would not overlap with the L7 allows at the lower precedence (ie CNP/CCNP). This would mean that if a cluster administrator creates an explicit allow rule (as opposed to a "pass" rule), then this could disrupt L7 visibility that a namespace owner creates for their own purposes.
Probably the implication of this is just that we should recommend to cluster administrators to use "allow" sparingly in the Admin tier, preferring "pass" statements instead; then leave allow/deny to the baseline tier.
Most critically however, this means that if a cluster admin created a KCNP with an allow for DNS traffic, this could break Cilium's toFQDNs rules unless we otherwise somehow cover this use case. I recognize that KCNP provides some ability to match on domain names, however in Cilium's case we must somehow identify DNS traffic, ensure the DNS server is trusted, and then process the DNS responses in order to enable the domain-based allows to work. Until now, this is all configured through CNP or CCNP.
There was a problem hiding this comment.
Given the KCNP semantics, I expect it to be mostly used for DENY rules. As you noted, instead of ALLOW the admin would use PASS to seize control to lower-priority policies.
A user that wishes to use L7 policies should probably just stick to CCNP and forget about KCNP.
There was a problem hiding this comment.
I think that's more or less fine, modulo domainNames handling in KCNP. We need to think about L7 to support that API field.
There was a problem hiding this comment.
Indeed, I would expect that ALLOW in any sort of administrative tiers would be used for required access such as security scanners.
We should recommend in the documentation that ALLOW be generally restricted to the baseline tier.
|
|
||
| ### Key Question: Should CiliumClusterwideNetworkPolicy be able to override ClusterNetworkPolicy rules? | ||
|
|
||
| ### Option: Increase priority of CiliumClusterwideNetworkPolicy |
There was a problem hiding this comment.
I'm not confident enough to argue that this is the right interpretation (yet?), but one idea I had early on during this effort was that CCNP should just be considered the highest precedence. This way, deny or L7 processing rules (such as for a DNS server) could remain in place whether you use KCNP or not, and would not change behavior based on the content of KCNP rules.
This doesn't solve all the concerns I highlighted elsewhere, but it would mitigate some potential usability flaws if the user wanted to use both (or even migrate towards KCNP).
There was a problem hiding this comment.
If CCNP is highest precedence, then by definition it is higher than CNP. This then breaks compatibility with existing behavior.
There was a problem hiding this comment.
Just to help share understanding, when you say "breaks compatibility", do you have an example ruleset in mind of the kind of breakage you expect?
There was a problem hiding this comment.
A configurable tier for CCNP seems like a logical next step. Either Admin, where it would take priority zero and thus be non-overridable, or even split the tiers (ClusterAdmin, then Admin, then Policy)
Signed-off-by: Blaz Zupan <blaz@google.com>
e7f7392 to
7358459
Compare
| * The Default tier will only contain a single "allow all" or "deny all" fallback rule. | ||
| * The proposed priority range is well under the supported maximum (1^24 - 1). | ||
|
|
||
| Each ClusterNetworkPolicy rule will be mapped to a Cilium priority by multiplying the CNP priority with 25 and adding the zero-based relative order of the rules within each to the priority. For example: |
There was a problem hiding this comment.
In the next day or so, I will be merging a PR that makes priority a float. That means it can be negative or decimal. It also adds a separate ordering pass, meaning that the internal datapath ordering can be completely distinct from any user-facing API.
|
@TheBeeZee are you looking for more input on this CFP? |
This CFP defines the order of evaluation for network policy rules when ClusterNetworkPolicy (formerly AdminNetworkPolicy), Kubernetes NetworkPolicy, and CiliumNetworkPolicy or CiliumClusterwideNetworkPolicy are all present in a cluster.