-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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: monitor: hubble: wireguard: add TRACE_{FROM/TO}_CRYPTO #34958
base: main
Are you sure you want to change the base?
Conversation
14ca639
to
00ceb20
Compare
80ed9a1
to
f352aa8
Compare
f352aa8
to
49cc912
Compare
/test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding from-wireguard
to trace decryption events makes sense to me. But I think we can use TRACE_{FROM,TO}_CRYPTO_DEV
in from-wireguard
and to-wireguard
instead of anywhere in bpf_host. Those two hooks see the plain-text packets which should be easy to retrieve src_id for trace.
If the above point also makes sense to you, then maybe we don't have to add new mark MARK_MAGIC_WG_DECRYPTED
.
This pull request has been automatically marked as stale because it |
This pull request has been automatically marked as stale because it |
This pull request has not seen any activity since it was marked stale. |
37d872a
to
7466413
Compare
7466413
to
9ed1102
Compare
/test |
Added new commits (6), (7) to improve tracing, plus I added comments in the code where things might not be 100% clear otherwise (ex. why we should check Could I have a 2nd look from reviewers please? 🙏 |
9ed1102
to
4cc1766
Compare
/test |
This commit introduces the trace TRACE_{FROM_TO}_CRYPTO observation point to indicate whether a packet is coming/going through the encryption/decryption path. This can turn useful for both WireGuard and IPSec, as showed in subsequent commits. These values differentiate from the current one as they will be used to update different types of metrics REASON_{EN,DE}CRYPT that will refers to different files/lines. This provides more information concerning the flow of the packet when using `cilium-dbg bpf metrics list`, `cilium-dbg monitor`, and `hubble observe. The old TRACE_REASON_ENCRYPTED (along with the REASON_DECRYPT metric) is still valid and refers to observed encrypted packets prior/after the crypto process. This patch wants to provide additional metrics and does not aim to replace those already existing. Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
This commit adds send_trace_notify into the cil_to_wireguard. With no monitor aggregations, the notification is logged using the new TRACE_TO_CRYPTO observation point. With monitor aggregation, the trace event will be displayed according to the retrieved `monitor` value. In any case, the code updated the respective bpf metric, and it shows up with `cilium bpf metrics list`, meaning that packets are going through the WireGuard encryption hook. Note that we attach the `cil_to_wireguard` program only given a set of condition. In all the other cases, no metrics updates nor trace events are expected from this program. Always-injecting wg-related program will be considered for future enhancements. An example of a trace event would look as follows: ```bash -> crypto flow 0x0 , identity unknown->unknown state unknown ifindex cilium_wg0 orig-ip 0.0.0.0: 40:00:3f:01:48:2f -> 45:00:00:54:d9:f3 UnknownEthernetType ``` The bpf metrics map would contain also the following entry: ```bash REASON DIRECTION PACKETS BYTES LINE FILE ... Interface Encrypting EGRESS 1218 103428 57 bpf_wireguard.c ... ``` Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
This commit introduces a new bpf test file to make sure that `cil_to_wireguard` correctly updates the metrics map with REASON_ENCRYPTED. Here we do not explicitly test the whole egress hook logic (red-DNAT in handle_nat_fwd) as already covered in other test files. For this reason, the remaining aspect to cover is the correct metric update. Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
This commit modifies the tracing observation point in cil_from_netdev in case the program is injected into the Wireguard device. We do that to attach `cil_from_netdev` to the ingress hook of the wg device given a set of conditions. In that case, the tracing system would still use TRACE_FROM_NETWORK as observation point: let's update it with the new TRACE_FROM_CRYPTO one (a from-network even would already be traced from the native device). This commit also adds the minimal logic in `trace.h` to handle `{FROM,TO}_CRYPTO` events either coming from bpf_wireguard.c or bpf_host.c. Using these observation points outside the expected code path (ENABLE_WIREGUARD && (IS_BPF_HOST || IS_BPF_WIREGUARD)) would result in a build bug. An example of a trace event would look as follows: ```bash <- crypto flow 0x0 , identity unknown->unknown state unknown ifindex cilium_wg0 orig-ip 0.0.0.0: 40:00:3f:01:48:2f -> 45:00:00:54:d9:f3 UnknownEthernetType ``` The bpf metrics map would contain also the following entry: ```bash REASON DIRECTION PACKETS BYTES LINE FILE ... Interface Decrypting INGRESS 7303 470892 1199 bpf_host.c ... ``` Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
This commit modifies the current tc_nodeport_l3_dev logic to be executed from the wireguard device (IS_BPF_WIREGUARD and ENABLE_WIREGUARD). Unfortunately, our Wireguard ingress hook is represented by `cil_from_netdev`, therefore here we cannot add such a check in the apposite wireguard_metrics.c file. Once we'll move to a custom `cil_from_wireguard` then we can simply test this logic there and restore the current file by decoupling-it from IS_BPF_WIREGUARD. Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
This commit adds to the current nodeport functions the possibility to retrieve and use the src_sec_id while tracing packets. This only holds true when the packet mark has the MARK_MAGIC_IDENTITY set. An example of a trace before and after this patch is reported next. Before: ```bash -> crypto flow 0x0 , identity unknown->unknown state unknown ifindex cilium_wg0 orig-ip 0.0.0.0: 40:00:3f:01:48:2f -> 45:00:00:54:d9:f3 UnknownEthernetType ``` After: ```bash -> crypto flow 0x0 , identity 21567->unknown state unknown ifindex cilium_wg0 orig-ip 0.0.0.0: 40:00:3f:01:48:2f -> 45:00:00:54:d9:f3 UnknownEthernetType ``` Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
This commit adds to the current nodeport functions the possibility to retrieve and use the src_sec_id while tracing packets. This only holds true when the packet mark has the MARK_MAGIC_OVERLAY set. An example of a trace before and after this patch is reported next. Before: ```bash -> overlay flow 0x0 , identity unknown->unknown state unknown ifindex cilium_vxlan orig-ip 0.0.0.0: 10.244.3.84 -> 10.244.1.28 EchoRequest ``` After: ```bash -> overlay flow 0x0 , identity 53498->unknown state unknown ifindex cilium_vxlan orig-ip 0.0.0.0: 10.244.3.84 -> 10.244.1.28 EchoRequest ``` Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
4cc1766
to
c845c97
Compare
/test |
bpf: monitor: hubble: add TRACE_{FROM,TO}_CRYPTO observation point
: add needed structure to support these new trace points, metrics, and hubble/monitor parsing logic for these new types of events.bpf: wireguard: add trace notification in cil_to_wireguard
: enable the TRACE_TO_CRYPTO from (1) in cil_to_wireguard (cilium_wg0 egress).bpf: wireguard: test: add wireguard egress hook metrics check
: add new bpf test file for metrics in (3).bpf: wireguard: TRACE_FROM_CRYPTO in bpf_host
: enable TRACE_FROM_CRYPTO from (1) in cil_from_netdev (cilium_wg0 ingress).bpf: wireguard: test: add wireguard ingress hook metrics check
: extend existing bpf tests for (5). Will be reverted and placed the logic in (4) when moving to a dedicated Wireguard program cil_from_wireguard.bpf: nodeport: retrieve src_sec_id when available in native routing
: enable logging the source endpoint ID when possible in the nodeport codepath.bpf: nodeport: retrieve src_sec_id when available in tunnel routing
: similar as (7) but for when using overlay.An example of output with both ingress/egress programs attached on Wireguard (native routing, KPR, nodeEncryption):
NOTE: I opened #37095 while working on this PR. As you can see from above tracing output, packets being traced from an L3 device (ex. Wireguard) are erroneously decoded. The problem has been identified and discussed in the linked issue, and will be tackled on a separate PR.
Fixes: #35280