-
Notifications
You must be signed in to change notification settings - Fork 3.4k
cilium-cli: Add strict-mode-test v2 #38566
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
Merged
julianwiedmann
merged 1 commit into
cilium:main
from
pippolo84:pr/pippolo84/cilium-cli-strict-mode-test
Apr 2, 2025
Merged
cilium-cli: Add strict-mode-test v2 #38566
julianwiedmann
merged 1 commit into
cilium:main
from
pippolo84:pr/pippolo84/cilium-cli-strict-mode-test
Apr 2, 2025
Conversation
This file contains hidden or 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
b664334
to
b14056f
Compare
/test |
b14056f
to
fbb5ffb
Compare
/test |
9ee4575
to
274c8dd
Compare
/test |
274c8dd
to
494751b
Compare
/test |
494751b
to
c340d03
Compare
c340d03
to
5f472a0
Compare
/ci-e2e-upgrade |
5f472a0
to
79cffd2
Compare
/test |
joamaki
approved these changes
Apr 1, 2025
smagnani96
approved these changes
Apr 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, left a couple of nits.
Thanks Fabio!
The `strict-mode-test` verifies that in Wireguard strict mode, no unecrypted pod-to-pod traffic is leaked. To do that, it temporarily removes all the ipcache entries related to the echo pods IP. Doing so, the pod IP is assigned the "world" identity and since no encryption key can be found, this leads to send unencrypted traffic is dropped as expected. Since v1.18, besides the pod IPs, the ipcache BPF map now stores the pod CIDRs too. Thus, removing just the IP is not enough to have a pod without an associated encryption key in the ipcache: the key can still be found from the pod CIDR entry and all the traffic from the client pods to the echo pods is encrypted. Despite this, the test still passes (somehow accidentally), because the reply traffic from the echo pod is now dropped. Since only pod CIDRs from remote nodes are inserted into the ipcache, no entry is found for the local endpoint on the remote agent, thus the egress reply traffic from that endpoint is leaked. To restore the test as was originally intended, the commit adds a new version that removes the ipcache pod CIDR entries too. To do that it relies on the `cilium-dbg bpf ipcache match` command, to find the pod CIDRs with an exact match and restore them at the end of the test. Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
79cffd2
to
0cf892c
Compare
/test |
christarazi
approved these changes
Apr 1, 2025
This was referenced Apr 2, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
affects/v1.17
This issue affects v1.17 branch
cilium-cli
This PR contains changes related with cilium-cli
feature/wireguard
Relates to Cilium's Wireguard feature
ready-to-merge
This PR has passed all tests and received consensus from code owners to merge.
release-note/ci
This PR makes changes to the CI.
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.
The
strict-mode-test
verifies that in Wireguard strict mode, nounecrypted pod-to-pod traffic is leaked. To do that, it temporarily
removes all the ipcache entries related to the echo pods IP. Doing so,
the pod IP is assigned the "world" identity and since no encryption key
can be found, this leads to send unencrypted traffic is dropped as
expected.
Since v1.18, besides the pod IPs, the ipcache BPF map now stores the pod
CIDRs too. Thus, removing just the IP is not enough to have a pod
without an associated encryption key in the ipcache: the key can still
be found from the pod CIDR entry and all the traffic from the client
pods to the echo pods is encrypted.
Despite this, the test still passes (somehow accidentally), because the
reply traffic from the echo pod is now dropped. Since only pod CIDRs
from remote nodes are inserted into the ipcache, no entry is found for
the local endpoint on the remote agent, thus the egress reply traffic
from that endpoint is leaked.
To restore the test as was originally intended, the commit adds a new
version that removes the ipcache pod CIDR entries too. To do that it
relies on the
cilium-dbg bpf ipcache match
command, to find the podCIDRs with an exact match and restore them at the end of the test.
Example run: https://github.com/cilium/cilium/actions/runs/14173035345
Related: #38483
Blocked by: #38483 and #38579