Skip to content

Commit

Permalink
add trace event for WireGuard en/decrypted packets
Browse files Browse the repository at this point in the history
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 <simone.magnani@isovalent.com>
  • Loading branch information
smagnani96 committed Sep 30, 2024
1 parent d3f6556 commit 49cc912
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 6 deletions.
30 changes: 24 additions & 6 deletions bpf/bpf_host.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions bpf/bpf_overlay.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 49cc912

Please sign in to comment.