-
Notifications
You must be signed in to change notification settings - Fork 3.4k
ipsec: Fix key derivation regression due to boot IDs #35806
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
Merged
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
fb872a5
to
3d802bb
Compare
Global keys have been deprecated, so in the EKS workflow, we are getting a warning because we don't use per-tunnel keys yet: level=warn msg="global IPsec keys are deprecated and will be removed in v1.17. Use per-tunnel keys instead by adding a '+' sign after the SPI (15+ in your case)." This commit switches the secret to use per-tunnel IPsec keys. Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
Until this commit, we would cover the transitions between single-key and per-tunnel-key systems for IPsec. In v1.17, we will remove support for single-key. There is thus no need to cover all of those transitions anymore. All users are now expected to be (and remain) on per-tunnel-key. Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
The GitHub action for IPsec key rotations currently assumes the cluster has two nodes and that both IPv4 and IPv6 are enabled. These assumptions don't hold for the EKS conformance test where IPv6 is disabled and three nodes are created. This commit therefore adds two new arguments to the GitHub action, to support IPv6 being enabled/disabled and a variable number of nodes. Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
In the key derivation logic, commit 7cb2582 ("ipsec: utilize declarative XFRM policy/state creation") removed the inversion of local and remote node IP addresses based on XFRM direction because it's not needed anymore. We however still need to invert the local and remote boot IDs based on direction. The consequence was that key derivation in per-tunnel key mode was always broken. Kind would work because all nodes have the same boot ID on Kind. This would result in packet drops of type XfrmInStateProtoError because the computed keys end up being different on both ends of the tunnels. Fixes: 7cb2582 ("ipsec: utilize declarative XFRM policy/state creation") Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
3d802bb
to
6a9766b
Compare
/ci-gke |
/ci-aks |
tommyp1ckles
approved these changes
Nov 6, 2024
smagnani96
approved these changes
Nov 7, 2024
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.
Changes looks good, took me a bit to figure out CI -- my bad (I left a comment explaining where I was stuck).
nbusseneau
approved these changes
Nov 7, 2024
This was referenced Nov 7, 2024
This was referenced Nov 15, 2024
CI: Hubble CLI Integration Test: hubble-relay 1 pods of Deployment hubble-relay are not ready
#34994
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
feature/ipsec
Relates to Cilium's IPsec feature
kind/bug
This is a bug in the Cilium logic.
release-note/bug
This PR fixes an issue in a previous release of Cilium.
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.
See commit descriptions. This bug made it only into a pre-release for v1.17.
Fixes: #35210.