Cilium non-deterministically classifies CIDR policy matches for range with node IPs #16308
Comments
pchaigno
added
kind/community-report
|
We determined on Slack that the proper policy would be to use /cc @pchaigno |
|
Unfortunately, that isn't enough. As sometimes the traffic will match the host entity, sometimes it matches the CIDR. So to make this work you actually need to have a policy that allows for both of those. This really feels like a largish gap in the flows that Cilium policies can match. There is a similar issue with pod -> host networking (e.g if you have kube API servers on host network, which is very common). Here, there is no policy that can match if remove identities are enabled. |
christarazi
added
the
sig/policy
|
Interesting report. When Cilium detects that the IP address is associated with the host, for example assigned to a local interface, then it is expected to map that IP address to the "host" identity and avoid matching via IPs. Usually this would exclude the IP from the CIDR portion of the policy engine. The background to this is that label-based and entity-based policies are semantically richer ways of describing the connectivity that you wish to allow. So host would take precedence. Reading from the IP/CIDR-based policies docs, the relevant text is:
Curiously though, there's a bit of a conflict in the docs. Above it talks about how "special identities" (including "host") are excluded from CIDR policy. Following that paragraph we have the following list of peers that CIDR policies may apply to:
I actually think that We probably need to investigate a little bit closer the ipcache precedence rules in |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
stale
bot
added
the
stale
|
Still a pretty big important issue to us |
stale
bot
removed
the
stale
|
We face the same issue today using a regular Some trafic went to |
aanm
added
the
sig/datapath
|
Hello, we've experienced same issue with regular Same policy worked on different CNI and broke after we've migrated to cilium. |
|
This issue has been automatically marked as stale because it has not |
github-actions
bot
added
the
stale
|
This is still an issue. |
sayboras
removed
the
stale
joestringer
changed the title
This is a fix for a regression in the local addresses logic, introduced in 080857b as part of the implementation for AddressScopeMax. Addresses with the form of link-local unicast addresses began to be filtered out of the local address aggregation, causing them to labeled with the "world" identity for the sake of policy enforcement. Examples of such addresses include: 169.254.10.10 fe80::1234 This caused issues for a variety of users, whose policies allowing "host" traffic would no longer allow traffic to these addresses, forcing the use of workarounds involving CIDR policies, which is not the intended behavior for this type of address. This was a regression as of Cilium 1.12.0-rc2. One reason for this regression was that logic prior to the change looked at the address scope, whereas logic after the change looked at the address bytes, and it was found that many users had assigned addresses of the forms above but with scope global, causing them to again be filtered unconditionally. This patch factors out the local address filtering logic into a function, removes the skip over IsLinkLocalUnicast(), and adds a variety of unit tests for that function. fixes: #25242 fixes: #23910 fixes: #16308 fixes: #20055 Signed-off-by: Andrew Sauber <andrew.sauber@isovalent.com>
This is a fix for a regression in the local addresses logic, introduced in 080857b as part of the implementation for AddressScopeMax. Addresses with the form of link-local unicast addresses began to be filtered out of the local address aggregation, causing them to labeled with the "world" identity for the sake of policy enforcement. Examples of such addresses include: 169.254.10.10 fe80::1234 This caused issues for a variety of users, whose policies allowing "host" traffic would no longer allow traffic to these addresses, forcing the use of workarounds involving CIDR policies, which is not the intended behavior for this type of address. This was a regression as of Cilium 1.12.0-rc2. One reason for this regression was that logic prior to the change looked at the address scope, whereas logic after the change looked at the address bytes, and it was found that many users had assigned addresses of the forms above but with scope global, causing them to again be filtered unconditionally. This patch factors out the local address filtering logic into a function, removes the skip over IsLinkLocalUnicast(), and adds a variety of unit tests for that function. fixes: #25242 fixes: #23910 fixes: #16308 fixes: #20055 Signed-off-by: Andrew Sauber <andrew.sauber@isovalent.com>
This is a fix for a regression in the local addresses logic, introduced in 080857b as part of the implementation for AddressScopeMax. Addresses with the form of link-local unicast addresses began to be filtered out of the local address aggregation, causing them to labeled with the "world" identity for the sake of policy enforcement. Examples of such addresses include: 169.254.10.10 fe80::1234 This caused issues for a variety of users, whose policies allowing "host" traffic would no longer allow traffic to these addresses, forcing the use of workarounds involving CIDR policies, which is not the intended behavior for this type of address. This was a regression as of Cilium 1.12.0-rc2. One reason for this regression was that logic prior to the change looked at the address scope, whereas logic after the change looked at the address bytes, and it was found that many users had assigned addresses of the forms above but with scope global, causing them to again be filtered unconditionally. This patch factors out the local address filtering logic into a function, removes the skip over IsLinkLocalUnicast(), and adds a variety of unit tests for that function. fixes: #25242 fixes: #23910 fixes: #16308 fixes: #20055 Signed-off-by: Andrew Sauber <andrew.sauber@isovalent.com>
[ upstream commit 05b6d82 ] This is a fix for a regression in the local addresses logic, introduced in 080857b as part of the implementation for AddressScopeMax. Addresses with the form of link-local unicast addresses began to be filtered out of the local address aggregation, causing them to labeled with the "world" identity for the sake of policy enforcement. Examples of such addresses include: 169.254.10.10 fe80::1234 This caused issues for a variety of users, whose policies allowing "host" traffic would no longer allow traffic to these addresses, forcing the use of workarounds involving CIDR policies, which is not the intended behavior for this type of address. This was a regression as of Cilium 1.12.0-rc2. One reason for this regression was that logic prior to the change looked at the address scope, whereas logic after the change looked at the address bytes, and it was found that many users had assigned addresses of the forms above but with scope global, causing them to again be filtered unconditionally. This patch factors out the local address filtering logic into a function, removes the skip over IsLinkLocalUnicast(), and adds a variety of unit tests for that function. fixes: cilium#25242 fixes: cilium#23910 fixes: cilium#16308 fixes: cilium#20055 Signed-off-by: Andrew Sauber <andrew.sauber@isovalent.com> Signed-off-by: Tim Horner <timothy.horner@isovalent.com>
[ upstream commit 05b6d82 ] This is a fix for a regression in the local addresses logic, introduced in 080857b as part of the implementation for AddressScopeMax. Addresses with the form of link-local unicast addresses began to be filtered out of the local address aggregation, causing them to labeled with the "world" identity for the sake of policy enforcement. Examples of such addresses include: 169.254.10.10 fe80::1234 This caused issues for a variety of users, whose policies allowing "host" traffic would no longer allow traffic to these addresses, forcing the use of workarounds involving CIDR policies, which is not the intended behavior for this type of address. This was a regression as of Cilium 1.12.0-rc2. One reason for this regression was that logic prior to the change looked at the address scope, whereas logic after the change looked at the address bytes, and it was found that many users had assigned addresses of the forms above but with scope global, causing them to again be filtered unconditionally. This patch factors out the local address filtering logic into a function, removes the skip over IsLinkLocalUnicast(), and adds a variety of unit tests for that function. fixes: #25242 fixes: #23910 fixes: #16308 fixes: #20055 Signed-off-by: Andrew Sauber <andrew.sauber@isovalent.com> Signed-off-by: Tim Horner <timothy.horner@isovalent.com>
Bug report
General Information
How to reproduce the issue
Run
node-local-dns --setupiptables=falseandkubelet --cluster-dns=169.254.20.10Add a CNP that looks like this:
On first cilium run, cilium will deny traffic to node-local-dns. If the pod is deleted, the second cilium run will accept traffic.
First cilium run
cilium ip list:cilium monitor -t policy-verdict:Second cilium run
cilium monitor -t policy-verdict:Slack thread https://cilium.slack.com/archives/C1MATJ5U5/p1621963201289500
The text was updated successfully, but these errors were encountered: