Skip to content

Conversation

pchaigno
Copy link
Member

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

@pchaigno pchaigno added kind/bug This is a bug in the Cilium logic. release-note/bug This PR fixes an issue in a previous release of Cilium. area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. labels Jun 24, 2021
@pchaigno pchaigno requested review from a team and jrfastab June 24, 2021 16:57
@pchaigno pchaigno marked this pull request as draft June 29, 2021 19:51
@pchaigno pchaigno removed the request for review from jrfastab June 29, 2021 19:51
@pchaigno pchaigno force-pushed the fix-max-ipsec-spi branch 2 times, most recently from e89d81a to 43d7dd0 Compare July 13, 2021 08:49
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>
@pchaigno pchaigno force-pushed the fix-max-ipsec-spi branch from 43d7dd0 to 964a1ad Compare July 26, 2021 15:52
@pchaigno
Copy link
Member Author

test-me-please

@pchaigno pchaigno marked this pull request as ready for review July 28, 2021 19:54
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@joestringer joestringer merged commit c88244c into cilium:master Jul 29, 2021
@pchaigno pchaigno deleted the fix-max-ipsec-spi branch July 29, 2021 18:22
@odinuge
Copy link
Member

odinuge commented May 26, 2022

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. kind/bug This is a bug in the Cilium logic. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants