From 49cc912f712d7f93a72ee1c6edd37ca0856a9210 Mon Sep 17 00:00:00 2001 From: Simone Magnani Date: Fri, 27 Sep 2024 09:55:19 +0200 Subject: [PATCH] add trace event for WireGuard en/decrypted packets This commit introduces the minimal trace events for WireGuard-related packets. On ingress packets, upon receiving the decrypted packet either `bpf_host` or `bpf_overlay`, we emit a `TRACE_FROM_CRYPTO_DEV` even with `TRACE_REASON_ENCRYPTED`, to indicate that the packet comes from a crypto device because it was encrypted and needed decryption. Egress packets on `bpf_host` that need to be encrypted will instead be logged with a `TRACE_TO_CRYPTO_DEV` and the `TRACE_REASON_ENCRYPTED` value. This will enable an output as follows. WireGuard with Overlay: ```bash 1:<- endpoint 1180 flow 0x0 , identity 7765->unknown state unknown ifindex 0 orig-ip 0.0.0.0: 10.244.1.122 -> 10.244.2.208 EchoRequest 2:-> overlay flow 0x0 , identity 7765->62609 state new ifindex cilium_vxlan orig-ip 0.0.0.0: 10.244.1.122 -> 10.244.2.208 EchoRequest 3:-> crypto_dev encrypted flow 0x0 , identity host->unknown state new ifindex cilium_wg0 orig-ip 0.0.0.0: 172.18.0.3:50877 -> 172.18.0.4:8472 udp 4:-> network flow 0xbc383006 , identity host->unknown state unknown ifindex eth0 orig-ip 0.0.0.0: 172.18.0.3:51871 -> 172.18.0.4:51871 udp 5:<- network flow 0xbc383006 , identity unknown->unknown state unknown ifindex eth0 orig-ip 0.0.0.0: 172.18.0.4:51871 -> 172.18.0.3:51871 udp 6:<- crypto_dev encrypted flow 0x0 , identity 62609->unknown state new ifindex cilium_wg0 orig-ip 0.0.0.0: 10.244.2.208 -> 10.244.1.122 EchoReply 7:<- overlay flow 0x0 , identity 62609->unknown state unknown ifindex cilium_vxlan orig-ip 0.0.0.0: 10.244.2.208 -> 10.244.1.122 EchoReply 8:-> endpoint 1180 flow 0x0 , identity 62609->7765 state reply ifindex lxc67ef7ac0e8eb orig-ip 10.244.2.208: 10.244.2.208 -> 10.244.1.122 EchoReply ``` Wireguard with Native Routing: ```bash 1:<- endpoint 2255 flow 0x0 , identity 15205->unknown state unknown ifindex 0 orig-ip 0.0.0.0: 10.244.1.125 -> 10.244.2.59 EchoRequest 2:-> stack flow 0x0 , identity 15205->33119 state new ifindex 0 orig-ip 0.0.0.0: 10.244.1.125 -> 10.244.2.59 EchoRequest 3:-> 0: 10.244.1.125 -> 10.244.2.59 EchoRequest 4:-> crypto_dev encrypted flow 0x0 , identity 15205->unknown state new ifindex cilium_wg0 orig-ip 0.0.0.0: 10.244.1.125 -> 10.244.2.59 EchoRequest 5:-> network flow 0x0 , identity host->unknown state unknown ifindex eth0 orig-ip 0.0.0.0: 172.18.0.2:51871 -> 172.18.0.4:51871 udp 6:-<- network flow 0x0 , identity unknown->unknown state unknown ifindex eth0 orig-ip 0.0.0.0: 172.18.0.4:51871 -> 172.18.0.2:51871 udp 7:<- crypto_dev encrypted flow 0x0 , identity world->unknown state unknown ifindex cilium_wg0 orig-ip 0.0.0.0: 10.244.2.59 -> 10.244.1.125 EchoReply 8:-> endpoint 2255 flow 0x0 , identity 33119->15205 state reply ifindex lxc6b5542b90ce7 orig-ip 10.244.2.59: 10.244.2.59 -> 10.244.1.125 EchoReply ``` Inspecting the bpf metrics from a Ciliun node (also collected in sysdumps) after an encrypted Echo Request/Reply between two endpoints will produce the following output: ```bash root@cilium-testing-worker:/home/cilium# cilium bpf metrics list REASON DIRECTION PACKETS BYTES LINE FILE ... Interface Decrypted INGRESS 1 98 1150 bpf_host.c Interface Encrypted EGRESS 1 98 1510 bpf_host.c ... ``` For clarity: the `TRACE_REASON_ENCRYPTED` is used even if in the trace points introduced by this patch the packets have already been decrypted (TRACE_FROM_CRYPTO_DEV) or still need to be encrypted (TRACE_TO_CRYPTO_DEV). To make full use of this value and use it where packets are actually encrypted we'd need additional parsing logic that could be an overkill since it is just for tracing. 1. For egress packets, `bpf_host` would be able to see packets marked as `MARK_MAGIC_WG_ENCRYPTED`, no further logic would be needed. A new `TRACE_FROM_CRYPTO_DEV` here could be displayed with the encrypted bit. 2. For ingress packets, we should inspect L4 and check whether is UDP and sport=dport=WIREGUARD_PORT, so that we can then trace both the network packet with the encrypted bit and display a `TRACE_TO_CRYPTO_DEV` with also the encrypted bit. As far as we don't need to add this additional logic for other reasons, I wouldn't add complexity in the datapath just for tracing. Looking in next steps, in the userspace, Hubble/Cilium-dbg could better interpret these CRYPTO_DEV events they're receiving and elaborate a more complete output themselves. Useful discussions: #4801 (Hubble/Cilium-dbg vxlan parsing logic for overlay) Fixes: #34659 Signed-off-by: Simone Magnani --- bpf/bpf_host.c | 30 ++++++++++++++++++++++++------ bpf/bpf_overlay.c | 6 ++++++ 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/bpf/bpf_host.c b/bpf/bpf_host.c index d19a4496180d61..2f1cb96b136cfb 100644 --- a/bpf/bpf_host.c +++ b/bpf/bpf_host.c @@ -1061,8 +1061,12 @@ static __always_inline int handle_l2_announcement(struct __ctx_buff *ctx) static __always_inline int do_netdev(struct __ctx_buff *ctx, __u16 proto, const bool from_host) { - enum trace_point trace = from_host ? TRACE_FROM_HOST : + enum trace_point obs_point = from_host ? TRACE_FROM_HOST : TRACE_FROM_NETWORK; + struct trace_ctx trace = { + .reason = TRACE_REASON_UNKNOWN, + .monitor = TRACE_PAYLOAD_LEN, + }; __u32 __maybe_unused identity = 0; __u32 __maybe_unused ipcache_srcid = 0; void __maybe_unused *data, *data_end; @@ -1102,9 +1106,19 @@ do_netdev(struct __ctx_buff *ctx, __u16 proto, const bool from_host) if (from_host) { __u32 magic; +#ifdef ENABLE_WIREGUARD + /* Checking for MARK_MAGIC_WG_DECRYPTED before calling inherit_identity_from_host, + * which resets the ctx->mark. + */ + if ((ctx->mark & MARK_MAGIC_WG_DECRYPTED) == MARK_MAGIC_WG_DECRYPTED) { + trace.reason = TRACE_REASON_ENCRYPTED; + obs_point = TRACE_FROM_CRYPTO_DEV; + } +#endif + magic = inherit_identity_from_host(ctx, &identity); if (magic == MARK_MAGIC_PROXY_INGRESS || magic == MARK_MAGIC_PROXY_EGRESS) - trace = TRACE_FROM_PROXY; + obs_point = TRACE_FROM_PROXY; #if defined(ENABLE_L7_LB) if (magic == MARK_MAGIC_PROXY_EGRESS_EPID) { @@ -1132,8 +1146,8 @@ do_netdev(struct __ctx_buff *ctx, __u16 proto, const bool from_host) #endif /* ENABLE_IPSEC */ } - send_trace_notify(ctx, trace, identity, UNKNOWN_ID, TRACE_EP_ID_UNKNOWN, - ctx->ingress_ifindex, TRACE_REASON_UNKNOWN, TRACE_PAYLOAD_LEN); + send_trace_notify(ctx, obs_point, identity, UNKNOWN_ID, TRACE_EP_ID_UNKNOWN, + ctx->ingress_ifindex, trace.reason, trace.monitor); bpf_clear_meta(ctx); @@ -1491,10 +1505,14 @@ int cil_to_netdev(struct __ctx_buff *ctx __maybe_unused) */ if ((ctx->mark & MARK_MAGIC_WG_ENCRYPTED) != MARK_MAGIC_WG_ENCRYPTED) { ret = wg_maybe_redirect_to_encrypt(ctx, proto); - if (ret == CTX_ACT_REDIRECT) + if (ret == CTX_ACT_REDIRECT) { + send_trace_notify(ctx, TRACE_TO_CRYPTO_DEV, src_sec_identity, + dst_sec_identity, TRACE_EP_ID_UNKNOWN, WG_IFINDEX, + TRACE_REASON_ENCRYPTED, 0); return ret; - else if (IS_ERR(ret)) + } else if (IS_ERR(ret)) { goto drop_err; + } } #if defined(ENCRYPTION_STRICT_MODE) diff --git a/bpf/bpf_overlay.c b/bpf/bpf_overlay.c index d73c8fc66a172d..660dbe265c4c30 100644 --- a/bpf/bpf_overlay.c +++ b/bpf/bpf_overlay.c @@ -659,6 +659,12 @@ int cil_from_overlay(struct __ctx_buff *ctx) break; } +#ifdef ENABLE_WIREGUARD + if ((ctx->mark & MARK_MAGIC_WG_DECRYPTED) == MARK_MAGIC_WG_DECRYPTED) + send_trace_notify(ctx, TRACE_FROM_CRYPTO_DEV, src_sec_identity, UNKNOWN_ID, + TRACE_EP_ID_UNKNOWN, WG_IFINDEX, TRACE_REASON_ENCRYPTED, 0); +#endif + #ifdef ENABLE_IPSEC if (is_esp(ctx, proto)) send_trace_notify(ctx, TRACE_FROM_OVERLAY, src_sec_identity, UNKNOWN_ID,