-
Notifications
You must be signed in to change notification settings - Fork 3.4k
ipsec: Fix off-by-one error on max keyID #16647
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
Conversation
e89d81a
to
43d7dd0
Compare
We encoded the SPI (aka keyID) on 4 bits [1] in the xfrm and packet marks. The maximum value is therefore 15 and not 16. This commit fixes the check on the maximum keyID value. Note the documentation for IPsec key rotation already has the correct value [2] so there shouldn't be any users with an incorrect keyID. 1 - https://github.com/cilium/cilium/blob/v1.10.1/pkg/datapath/linux/ipsec/ipsec_linux.go#L147-L150 2 - https://docs.cilium.io/en/v1.10/gettingstarted/encryption-ipsec/#key-rotation Fixes: b698972 ("cilium: ipsec, support rolling updates") Signed-off-by: Paul Chaignon <paul@cilium.io>
43d7dd0
to
964a1ad
Compare
test-me-please |
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.
🚀
Hi @pchaigno / @joestringer Can we mark this PR for backport as well, since we hit this issues with the old docs pre #19893. We have this cherry-picked in our internal v1.9 release, but if we can get this into the v1.10 (and maybe also v1.9), that would help limit the number of changes we have to cherry-pick Thanks! |
We encoded the SPI (aka keyID) on 4 bits in the xfrm and packet marks. The maximum value is therefore 15 and not 16. This pull request fixes the check on the maximum keyID value.
Note the documentation for IPsec key rotation already has the correct value so there shouldn't be any users with an incorrect keyID.
Fixes: #7450