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

BPF: Encrypted Overlay Support #31073

Merged
merged 5 commits into from Mar 27, 2024

Conversation

ldelossa
Copy link
Contributor

@ldelossa ldelossa commented Feb 29, 2024

This pull request implements the data-path side of "Encrypted Overlay" support.

"Encrypted Overlay" allows Cilium to IPSec encrypt overlay traffic and deliver the encrypted overlay traffic to a remote node via IPSec tunnel.

A subsequent PR will follow which enables this in the IPSec control plane, creating the necessary policies and states to configure the tunnel source and destination addresses correctly.

This feature is useful when an application's traffic destined for a remote host cannot be routed into the XFRM netfilter hooks for various reasons.
In these cases we can first encapsulate the traffic in VXLAN, catch it at the egress interface, and recirculate the packet back into the system, at which point the traffic looks like unicast traffic that should be routed off host (and XFRM will be applied at this point).

It's important to note that the following sysctl configs must be applied to any interfaces which tunnels traffic toward a remote node for this to work

- net.ipv4.conf.default.rp_filter = 0
- net.ipv4.conf.default.accept_local = 1
bpf: introduce encrypted overlay datapath support 

@ldelossa ldelossa added sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Feb 29, 2024
@ldelossa ldelossa requested review from a team as code owners February 29, 2024 22:15
@ldelossa ldelossa requested review from julianwiedmann, brb, a team and lambdanis and removed request for a team February 29, 2024 22:15
bpf/bpf_host.c Outdated Show resolved Hide resolved
@ldelossa ldelossa force-pushed the ldelossa/encrypted-overlay-oss branch 2 times, most recently from b5f0705 to ceb1eec Compare March 1, 2024 00:32
@ldelossa
Copy link
Contributor Author

ldelossa commented Mar 1, 2024

/test

@ldelossa ldelossa force-pushed the ldelossa/encrypted-overlay-oss branch 2 times, most recently from f2f6671 to 9433bad Compare March 1, 2024 02:27
Copy link
Member

@brb brb left a comment

Choose a reason for hiding this comment

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

@cilium/wireguard changes LGTM.

@pchaigno pchaigno added release-note/major This PR introduces major new functionality to Cilium. feature/ipsec Relates to Cilium's IPsec feature and removed release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Mar 1, 2024
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.

Neat pull request! I really like the code comments 🙂

Couple comments below. IIUC, I think there's an issue with the SPI and security identity.

bpf/bpf_host.c Outdated Show resolved Hide resolved
bpf/lib/encrypt.h Outdated Show resolved Hide resolved
bpf/lib/encrypt.h Outdated Show resolved Hide resolved
bpf/lib/trace.h Show resolved Hide resolved
Copy link
Member

@kaworu kaworu left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @ldelossa, on the Hubble side:

  1. Please add TraceReasonEncryptOverlay introduced by this PR in TraceReasonIsEncap. That way the corresponding Hubble flow won't have the IsReply field set. The test case should be similar to SRV6 encap, except that the Source should be hostEP instead (since this trace is emitted from bpf_host.c).
  2. I'm not sure whether decodeTrafficDirection will correctly infer EGRESS for the corresponding flow. Can you run hubble observe -o json and see what the TraceReasonCtReply flow looks like? If shown as INGRESS, please add a comment in decodeTrafficDirection stating that it is currently broken for this particular trace reason 🙏

bpf/lib/trace.h Show resolved Hide resolved
@ldelossa
Copy link
Contributor Author

ldelossa commented Mar 1, 2024

discussion with @joestringer lead to the following suggested changes:

  1. Create and use a reserved security ID to signal that vxlan traffic should be encapsulated and not the encryption bits.
  2. Suggested re-writing the VNI based on the source destination of the encapsulated packet before it leaves the host.

These suggestions are to mitigate any shenanigans that could occur from leaking nonsensical VXLAN ids.

@ldelossa ldelossa force-pushed the ldelossa/encrypted-overlay-oss branch from 9433bad to a01c9d1 Compare March 2, 2024 00:26
kaworu added a commit to kaworu/cilium that referenced this pull request Mar 6, 2024
cilium#30154 and
cilium#31073 introduced new datapath
trace reasons and had an impact on Hubble, but the sig-hubble team
doesn't get automatically pulled in for review.

This patch adds the sig-hubble team to review datapath_trace.go changes.

Signed-off-by: Alexandre Perrin <alex@isovalent.com>
@ldelossa ldelossa force-pushed the ldelossa/encrypted-overlay-oss branch from a01c9d1 to 27ef8dc Compare March 6, 2024 22:57
Copy link
Member

@julianwiedmann julianwiedmann 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, thank you!

@pchaigno

This comment was marked as outdated.

bpf/Makefile Show resolved Hide resolved
@ldelossa

This comment was marked as outdated.

@pchaigno
Copy link
Member

@ldelossa We just merged the IPsec fixes so there's a conflict. Ping me on Slack if you need help on that.

This commit adds a new reserved security identity for signaling overlay
traffic which must be IPSec encrypted.

When the eBPF datapath encounters an egress packet with this security
identity in an overlay header (currently only VXLan supported) it will
subject the packet to IPSec encryption and rewrite the overlay header
with the correct security identity before the packet leaves the host.

Therefore, this identity should NEVER be seen on traffic ingress or
egress the node from the network.

Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
This commits and both the agent and datapath flag required to enable the
"Encrypted Overlay" feature.

The datapath will use ENABLE_ENCRYPTED_OVERLAY feature flag.
The agent will use "encryption.ipsec.encryptOverlay"

Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
@ldelossa
Copy link
Contributor Author

ldelossa commented Mar 26, 2024

@pchaigno handling this now, the diff looks pretty simple on my end. Just a change in includes in encrypt.h

Just pushed conflict resolution.

ldelossa and others added 3 commits March 26, 2024 10:05
This commit introduces the ability to encrypt overlay traffic before it
leaves the host.

The 'cil_to_netdev' function is updated to sniff into overlay packets
(only VXLAN supported for now) and determine if the ENCRYPTED_OVERLAY_ID
security identifier is present in the overlay's header.

If it is, a new function in encrypt.h will set the appropriate packet
mark on the skb and redirect the packet to the ingress of the interface
it was egressing on.

When the packet is seen on the ingress side of the device it will be
submitted to the XFRM hooks in the output routing path and the XFRM
subsystem will encrypt the packet.

Subsequent changes to the IPSec control plane to create the appropriate
states and policies to support this are required.

Signed-off-by: ldelossa <louis.delos@isovalent.com>
Add a trace notification when we are redirecting a packet back into the
stack for XFRM encryption.

Trace example:
-> stack flow 0xc218244b , identity unknown->unknown state encrypt-overlay ifindex 0 orig-ip 0.0.0.0: 172.18.0.3:58167 -> 172.18.0.2:8472 udp

Signed-off-by: ldelossa <louis.delos@isovalent.com>
Add unit tests for new vxlan helper functions in tunnel.h

Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
@ldelossa ldelossa force-pushed the ldelossa/encrypted-overlay-oss branch from 40d4347 to bcd5eec Compare March 26, 2024 14:05
@ldelossa
Copy link
Contributor Author

/test

@ldelossa
Copy link
Contributor Author

https://github.com/cilium/cilium/actions/runs/8437738465 is failing, but this is an unrelated error.

Marking this PR as ready to merge.

@ldelossa ldelossa added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Mar 26, 2024
@pchaigno pchaigno added this pull request to the merge queue Mar 27, 2024
Merged via the queue into cilium:main with commit d00547a Mar 27, 2024
62 checks passed
@ldelossa ldelossa deleted the ldelossa/encrypted-overlay-oss branch March 27, 2024 15:15
kaworu added a commit to kaworu/cilium that referenced this pull request Mar 27, 2024
cilium#30154 and
cilium#31073 introduced new datapath
trace reasons and had an impact on Hubble, but the sig-hubble team
doesn't get automatically pulled in for review.

This patch adds the sig-hubble team to review datapath_trace.go changes.

Signed-off-by: Alexandre Perrin <alex@isovalent.com>
kaworu added a commit to kaworu/cilium that referenced this pull request Apr 10, 2024
cilium#30154 and
cilium#31073 introduced new datapath
trace reasons and had an impact on Hubble, but the sig-hubble team
doesn't get automatically pulled in for review.

This patch adds the sig-hubble team to review datapath_trace.go changes.

Signed-off-by: Alexandre Perrin <alex@isovalent.com>
kaworu added a commit to kaworu/cilium that referenced this pull request Apr 19, 2024
cilium#30154 and
cilium#31073 introduced new datapath
trace reasons and had an impact on Hubble, but the sig-hubble team
doesn't get automatically pulled in for review.

This patch adds the sig-hubble team to review datapath_trace.go changes.

Signed-off-by: Alexandre Perrin <alex@isovalent.com>
kaworu added a commit to kaworu/cilium that referenced this pull request Apr 19, 2024
cilium#30154 and
cilium#31073 introduced new datapath
trace reasons and had an impact on Hubble, but the sig-hubble team
doesn't get automatically pulled in for review.

This patch adds the sig-hubble team to review datapath_trace.go changes.

Signed-off-by: Alexandre Perrin <alex@isovalent.com>
kaworu added a commit to kaworu/cilium that referenced this pull request Apr 22, 2024
cilium#30154 and
cilium#31073 introduced new datapath
trace reasons and had an impact on Hubble, but the sig-hubble team
doesn't get automatically pulled in for review.

This patch adds the sig-hubble team to review datapath_trace.go changes.

Signed-off-by: Alexandre Perrin <alex@isovalent.com>
kaworu added a commit to kaworu/cilium that referenced this pull request Apr 23, 2024
cilium#30154 and
cilium#31073 introduced new datapath
trace reasons and had an impact on Hubble, but the sig-hubble team
doesn't get automatically pulled in for review.

This patch adds the sig-hubble team to review datapath_trace.go changes.

Signed-off-by: Alexandre Perrin <alex@isovalent.com>
kaworu added a commit to kaworu/cilium that referenced this pull request Apr 26, 2024
cilium#30154 and
cilium#31073 introduced new datapath
trace reasons and had an impact on Hubble, but the sig-hubble team
doesn't get automatically pulled in for review.

This patch adds the sig-hubble team to review datapath_trace.go changes.

Signed-off-by: Alexandre Perrin <alex@isovalent.com>
kaworu added a commit to kaworu/cilium that referenced this pull request Apr 30, 2024
cilium#30154 and
cilium#31073 introduced new datapath
trace reasons and had an impact on Hubble, but the sig-hubble team
doesn't get automatically pulled in for review.

This patch adds the sig-hubble team to review datapath_trace.go changes.

Signed-off-by: Alexandre Perrin <alex@isovalent.com>
github-merge-queue bot pushed a commit that referenced this pull request May 1, 2024
#30154 and
#31073 introduced new datapath
trace reasons and had an impact on Hubble, but the sig-hubble team
doesn't get automatically pulled in for review.

This patch adds the sig-hubble team to review datapath_trace.go changes.

Signed-off-by: Alexandre Perrin <alex@isovalent.com>
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 ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/major This PR introduces major new functionality to Cilium. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants