Skip to content

Conversation

smagnani96
Copy link
Contributor

IPSec relies on XFRM states and policies to ensure encrypted pod-to-pod connectivity. In the XFRM state:

  • seq is the incoming sequence number counter. It is used by the kernel
    to track and validate received packets. When a packet arrives, its
    sequence number is compared to the expected range to detect replays
    or out-of-order packets. Together this the oseq value, it helps implement
    the anti-replay window to prevent attackers from resending previously captured packets.
  • oseq is the outgoing sequence number counter. It is used when sending
    packets protected by IPsec. Each outbound IPsec packet is assigned an
    incrementing oseq value. The oseq ensures unique sequence numbers for
    each packet, which the receiver uses to validate the order and detect replays.

In a SA between two nodes A and B, the seq/oseq values in the XFRM state A must match the oseq/seq values in node B, and vice versa. If that is not the case, users would experience the XfrmInStateProtoError error, with no IPSec connectivity between the two nodes.

We noticed that a Cilium user might end up in this situation in both the following cases, as stated in the doc changes:

  1. KVStore Mode (e.g., etcd): When a Cilium agent connects too late to the
    newly created KVStore, it may miss the node delete and create events
    for entries that were restored or reinitialized. This results in staling
    XFRM state, causing permanent network disruption. Please do note that
    operations such as etcd removal and recreation are not supported.
  2. CRD Mode: a similar issue may occur when a CiliumNode resource is deleted
    and the Cilium agent DaemonSet is restarted. While other agents will
    recreate fresh XFRM states for the new CiliumNode, the restarted agent
    may continue to hold obsolete XFRM states referencing all peer nodes.

The identified mitigation strategy for these scenario is a proper IPSec key rotation, which would cause all the states to be consistently recreated in all Cilium agents.

@smagnani96 smagnani96 added kind/enhancement This would improve or streamline existing functionality. area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. release-note/misc This PR makes changes that have no direct user impact. feature/ipsec Relates to Cilium's IPsec feature labels May 26, 2025
@smagnani96 smagnani96 force-pushed the pr/smagnani96/doc-kvstore-ipsec branch from 59cc003 to cbeb3eb Compare May 26, 2025 16:33
@smagnani96 smagnani96 requested a review from pchaigno May 27, 2025 16:52
@smagnani96
Copy link
Contributor Author

/test

@smagnani96 smagnani96 marked this pull request as ready for review May 28, 2025 11:07
@smagnani96 smagnani96 requested review from a team as code owners May 28, 2025 11:07
@smagnani96 smagnani96 requested a review from joestringer May 28, 2025 11:07
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.

I provided a few general tips to follow the docs-structure style that we are aiming for in the Cilium docs. I will defer on the technical content to the relevant code owner. See the comments below for more details.

@joestringer joestringer added the needs-backport/1.17 This PR / issue needs backporting to the v1.17 branch label May 28, 2025
@smagnani96 smagnani96 self-assigned this Jun 5, 2025
@smagnani96 smagnani96 force-pushed the pr/smagnani96/doc-kvstore-ipsec branch from cbeb3eb to 2e15efa Compare June 6, 2025 09:03
@smagnani96
Copy link
Contributor Author

I provided a few general tips to follow the docs-structure style that we are aiming for in the Cilium docs. I will defer on the technical content to the relevant code owner. See the comments below for more details.

Thanks @joestringer. I had updated accordingly to your suggestions, let me know if I can further enhance this 😃

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.

Looks good to me for @cilium/docs-structure .

@joestringer
Copy link
Member

/test

Copy link
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

I think this needs to be a reworded a bit. As it stands, it seems likely to worry unaffected users and to be unactionnable for affected users.

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 11, 2025
@pchaigno pchaigno added dont-merge/bad-bot To prevent MLH from marking ready-to-merge. and removed ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Jun 11, 2025
@joestringer
Copy link
Member

@pchaigno if you are requesting changes, please consider using the "request changes" response in GitHub. This helps to make the intent very clear both to the contributor and also to the bot that sets the ready-to-merge label.

@smagnani96 smagnani96 force-pushed the pr/smagnani96/doc-kvstore-ipsec branch from 2e15efa to 52be236 Compare June 17, 2025 14:39
@smagnani96 smagnani96 requested a review from pchaigno June 17, 2025 14:40
@smagnani96
Copy link
Contributor Author

/test

Copy link
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

That's a lot better IMO! One small suggestion below.

IPSec relies on XFRM states and policies to ensure encrypted pod-to-pod
connectivity. In the XFRM state:

* seq is the incoming sequence number counter. It is used by the kernel
  to track and validate received packets. When a packet arrives, its
  sequence number is compared to the expected range to detect replays
  or out-of-order packets. Together this the oseq value, it helps implement
  the anti-replay window to prevent attackers from resending previously captured packets.
* oseq is the outgoing sequence number counter. It is used when sending
  packets protected by IPsec. Each outbound IPsec packet is assigned an
  incrementing oseq value. The oseq ensures unique sequence numbers for
  each packet, which the receiver uses to validate the order and detect replays.

In a SA between two nodes A and B, the seq/oseq values in the XFRM state
A must match the oseq/seq values in node B, and vice versa. If that is not
the case, users would experience the `XfrmInStateProtoError` error, with
no IPSec connectivity between the two nodes.

We noticed that a Cilium user might end up in this situation in both the
following cases, as stated in the doc changes:

1. KVStore Mode (e.g., etcd): if a Cilium agent connects too late to the
   newly created KVStore, it may miss the node delete and create events
   for entries that were restored or reinitialized. This results in staling
   XFRM state, causing permanent network disruption.
2. KVStore Mode: if a Cilium agent is down for prolonged time, the
   corresponding node entry in the kvstore will be deleted due to lease
   expiration (15m), resulting in stale XFRM states.
3. CRD Mode: a similar issue may occur when a CiliumNode resource is deleted
   and the Cilium agent DaemonSet is restarted. While other agents will
   recreate fresh XFRM states for the new CiliumNode, the restarted agent
   may continue to hold obsolete XFRM states referencing all peer nodes.

The identified mitigation strategy for these scenario is an IPSec
key rotation, which would cause all the states to be consistently
recreated in all Cilium agents.

Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
@smagnani96 smagnani96 force-pushed the pr/smagnani96/doc-kvstore-ipsec branch from 52be236 to 65a115d Compare June 19, 2025 15:36
@pchaigno pchaigno enabled auto-merge June 19, 2025 15:55
@smagnani96
Copy link
Contributor Author

/test

@smagnani96 smagnani96 removed the dont-merge/bad-bot To prevent MLH from marking ready-to-merge. label Jun 23, 2025
@pchaigno pchaigno added this pull request to the merge queue Jun 23, 2025
Merged via the queue into main with commit 8103535 Jun 23, 2025
65 checks passed
@pchaigno pchaigno deleted the pr/smagnani96/doc-kvstore-ipsec branch June 23, 2025 10:40
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 23, 2025
@viktor-kurchenko viktor-kurchenko added backport-pending/1.17 The backport for Cilium 1.17.x for this PR is in progress. and removed needs-backport/1.17 This PR / issue needs backporting to the v1.17 branch labels Jun 23, 2025
@github-actions github-actions bot added backport-done/1.17 The backport for Cilium 1.17.x for this PR is done. and removed backport-pending/1.17 The backport for Cilium 1.17.x for this PR is in progress. labels Jun 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. backport-done/1.17 The backport for Cilium 1.17.x for this PR is done. feature/ipsec Relates to Cilium's IPsec feature kind/enhancement This would improve or streamline existing functionality. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants