-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Pr/gray/main/fix leak detection race #32569
Draft
jschwinger233
wants to merge
8
commits into
main
Choose a base branch
from
pr/gray/main/fix-leak-detection-race
base: main
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
+420
−43
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: gray <gray.liang@isovalent.com>
Otherwise the further patch for L7 proxy + IPsec fails pod-to-world testcases. To fix #31984, we are going to change the routing for traffic from egress proxy: when tunnel is disabled, the traffic will be routed to cilium_host instead of eth0. This leads to a consequence of changing packets' source IPs: from eth0's IP to cilium_host's IP. The implication requires additional masquerading for pod to world traffic, because these packets now have cilium_host's IP as source, rather than eth0. This patch ensures iptables masquerade rules are installed for pod to world traffic, even when KPR is enabled. Signed-off-by: gray <gray.liang@isovalent.com>
Signed-off-by: gray <gray.liang@isovalent.com>
…ckets To ensure IPsec encryption for proxy forward packets, we added routing rule to push them to cilium_host. This change caused side effects for to-world traffic. This patch fixes the consequences of side effects. Proxy will perform "SNAT" for to-world packets, but the new source address is decided by routing rules. Previously, to-world packets are routed to eth0, so proxy uses eth0's address for SNAT. Now with new routing rule to push them to cilium_host instead of eth0, proxy uses cilium_host's address for SNAT as the side effect. This change makes to-world packets rely on "external" SNAT, which wasn't required because proxy's SNAT worked perfectly. We need "external" SNAT to change source address of to-world packets from cilium_host's IP to eth0's IP. As IPsec doesn't work with KPR, the "external" SNAT mechanism is iptables. However, due to kernel's implementation details, an skb won't be processed by nat POSTROUTING for twice. When the packet is routed to cilium_host, it's the first time; when forwarded from cilium_net to eth0, it's supposed to be the second time. This is because, After the first POSTROUTING traversal, skb's ct (struct nf_conn*)(skb->_nfct & ~7) has a status IPS_SRC_NAT_DONE to skip the second traversal at all. To avoid setting the IPS_SRC_NAT_DONE flag, this patch adds an iptables rule `-j CT --notrack` for skip the first round iptables ct. Signed-off-by: gray <gray.liang@isovalent.com>
Extend the conformance-ipsec-e2e GHA workflow to additionally check that we don't leak any unencrypted packets during the connectivity test. This aims to complement the validation already performed as part of the connectivity tests by the Cilium CLI. Specifically, we leverage bpftrace to analyze the packets forwarded by the bridge device (used by kind), and report those that are not encrypted. We flag packets with both the source and the destination belonging to the IPv4/6 PodCIDR, and we consider the inner headers if packets are encapsulated. In this case, we additionally skip packets originating or targeting CiliumInternalIP addresses (as these are used for node-to-pod traffic when running in tunnel mode, which is not encrypted by design). Extra checks are finally added to always include packets originating from the L7 and DNS proxies, as their source IP is not that of a pod. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
So that we can install the version we want. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
Signed-off-by: gray <gray.liang@isovalent.com>
maintainer-s-little-helper
bot
added
the
dont-merge/needs-release-note-label
The author needs to describe the release impact of these changes.
label
May 16, 2024
/ci-ipsec-e2e Result: https://github.com/cilium/cilium/actions/runs/9109346753/job/25042030204 All jobs failed due to
But no leak was detected! Try again. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
dont-merge/needs-release-note-label
The author needs to describe the release impact of these changes.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please ensure your pull request adheres to the following guidelines:
description and a
Fixes: #XXX
line if the commit addresses a particularGitHub issue.
Fixes: <commit-id>
tag, thenplease add the commit author[s] as reviewer[s] to this issue.
Fixes: #issue-number