Skip to content
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

ipsec: Install default-drop XFRM policy sooner #25257

Merged
merged 1 commit into from
May 9, 2023

Conversation

pchaigno
Copy link
Member

@pchaigno pchaigno commented May 4, 2023

We currently install the default-drop XFRM policy when we install the XFRM policies and states for the local node. It is however possible for us to start installing XFRM policies and states for remote nodes before we handle the local one.

The default-drop XFRM policy is a safety measure for when we move XFRM policies around. Because we don't always have a way to atomically update XFRM policies, it's possible that we end up with a very short time where no encryption XFRM OUT policy is matching a subset of traffic. The default-drop policy ensures that we drop such traffic instead of letting it leave the node as plain-text.

We therefore want this default-drop XFRM policy to be installed before we update any other other XFRM policy. This pull request therefore moves its installation before any other XFRM update instead of before just local-node XFRM updates.

Fixes: #24773.

@pchaigno pchaigno added area/encryption Impacts encryption support such as IPSec, WireGuard, or kTLS. release-note/misc This PR makes changes that have no direct user impact. needs-backport/1.11 needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels May 4, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.11.17 May 4, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.13.3 May 4, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.12.10 May 4, 2023
We currently install the default-drop XFRM policy when we install the
XFRM policies and states for the local node. It is however possible for
us to start installing XFRM policies and states for remote nodes before
we handle the local one.

The default-drop XFRM policy is a safety measure for when we move XFRM
policies around. Because we don't always have a way to atomically update
XFRM policies, it's possible that we end up with a very short time where
no encryption XFRM OUT policy is matching a subset of traffic. The
default-drop policy ensures that we drop such traffic instead of letting
it leave the node as plain-text.

We therefore want this default-drop XFRM policy to be installed before
we update any other other XFRM policy. This commit therefore moves its
installation before any other XFRM update instead of before just
local-node XFRM updates.

Fixes: 7d44f37 ("ipsec: Catch-default default drop policy for encryption")
Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
@pchaigno pchaigno force-pushed the ipsec-install-default-drop-sooner branch from 339b7e9 to abd4669 Compare May 4, 2023 09:59
@pchaigno
Copy link
Member Author

pchaigno commented May 4, 2023

/test

@pchaigno pchaigno marked this pull request as ready for review May 5, 2023 13:04
@pchaigno pchaigno requested review from a team as code owners May 5, 2023 13:04
@pchaigno pchaigno requested a review from jibi May 5, 2023 13:04
Copy link
Member

@jibi jibi left a comment

Choose a reason for hiding this comment

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

other other in the commit message (as checkpatch is complaining) otherwise LGTM

@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 May 9, 2023
@pchaigno pchaigno merged commit 2045d59 into cilium:main May 9, 2023
55 of 57 checks passed
@pchaigno pchaigno deleted the ipsec-install-default-drop-sooner branch May 9, 2023 09:48
@YutaroHayakawa YutaroHayakawa mentioned this pull request May 10, 2023
14 tasks
@YutaroHayakawa YutaroHayakawa added backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. and removed needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels May 10, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.13 in 1.13.3 May 10, 2023
@YutaroHayakawa YutaroHayakawa mentioned this pull request May 10, 2023
7 tasks
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.12 in 1.12.10 May 10, 2023
@jrajahalme jrajahalme mentioned this pull request May 11, 2023
5 tasks
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.11 in 1.11.17 May 11, 2023
@jrajahalme jrajahalme added backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. and removed backport-pending/1.11 labels May 12, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.11 to Backport done to v1.11 in 1.11.17 May 12, 2023
@YutaroHayakawa YutaroHayakawa added backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. and removed backport-pending/1.12 labels May 15, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.12 to Backport done to v1.12 in 1.12.10 May 15, 2023
@pchaigno pchaigno added backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. and removed backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. labels Jun 20, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.13 to Backport done to v1.13 in 1.13.3 Jun 20, 2023
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. backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. 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
No open projects
1.11.17
Backport done to v1.11
1.12.10
Backport done to v1.12
1.13.3
Backport done to v1.13
Development

Successfully merging this pull request may close these issues.

None yet

4 participants