gadgets: Add trace_ssl

[i-Pear]

gadgets: Add trace_ssl

Usage: capture SSL data/latency from OpenSSL, GnuTLS, NSS and libcrypto.

[i-Pear]

This gadget and the sslsnoop gadget is modified from the BCC project, which is using Apache2 license. I'm not sure whether using the Apache2 license will cause any problems?

[i-Pear]

This pull request is updated according to the comments from the sslsnoop PR.

[alban]

trace_sslsniff and trace_sslsnoop (#2746) are similar. To avoid user confusion, I think there should be only one. There could be a gadget parameter to enable or disable buf.

[i-Pear]

This PR has been merged with sslsnoop. To avoid sending raw buffer to userspace, use ig run trace_ssl:latest --record_data=false.

[alban]

Thanks!

[alban]

Thanks!

[alban]

It would be good to provide a README.md so the artifacthub page can display it.

It could be done in a following PR.

[alban]

I suggest to use gadget_should_discard_mntns_id only in the "enter" probe and not in the "exit" probe.

In most cases, the mntns will not change between "enter" and "exit". And it will be functionally equivalent because one of the bpf_map_lookup_elem below will return 0, so the "exit" probe will not do anything.

However, a container could change mntns between the 2 probes by patching libssl in the container image. The container could even do that without privileges by making use of unprivileged user namespaces. This could cause the maps to get full and the gadget would stop working properly for other containers.

And event->mntns_id could be filled with the mntns_id from the "enter" probe kept in the context map keyed by tid.

This kind of issues exist with uprobes, but in general not with kprobes (unless the kprobe is attached to sys_setns, sys_unshare or sys_clone, equivalent) because userspace code does not have control on the mntns while kernel code is running (multithreaded processes cannot change namespaces of type mount).

Alternatively, you could replace return 0; by goto out; and add the following cleanup code at the end of the function:

out:
    bpf_map_delete_elem(&ssl_bufs, &tid);
    bpf_map_delete_elem(&ssl_starttime, &tid);

Ditto for trace_uretprobe_libssl_SSL_do_handshake and probe_crypto_exit.

cc @mauriciovasquezbernal I think this tip should be in a documentation somewhere.

[i-Pear]

I can understand your concern about security. I'll remove all gadget_should_discard_mntns_id in exit probes, and replace return 0; by goto out;.

Some other ideas:

• Is this used for writing the correct mntns in events?

And event->mntns_id could be filled with the mntns_id from the "enter" probe kept in the context map keyed by tid.

• If a process changed its mntns, then all its events will not be traced? Maybe not strict enough in some security-related situations. If we attach to some kprobes which can change processes' mntns, can we still trace those processes with new mntns?

• What will happen if a process calls clone and changed its tid? For most use cases it doesn't matter, but developers may also think about it.

• Maybe I can manage to skip the ret-probes? For example, I can write (or patch) some assembly and bypass the "return" symbol. More easily, I think c++'s exception can unwind the stack directly, and bypass the ret-probe. If I do this, I can easily make the ebpf map full.

• If I patch libssl (it's indeed possible, maybe just mprotect+rw?), will the inode be the same? And will the trace point still be called? In other words, uprobe is "patching" the binary, and if the user is also patching the binary, who will win?

[alban]

In various error cases, you should still clean up the maps with goto out;:

out:
    bpf_map_delete_elem(&ssl_bufs, &tid);
    bpf_map_delete_elem(&ssl_starttime, &tid);

Ditto for trace_uretprobe_libssl_SSL_do_handshake and probe_crypto_exit.

[alban]

Thanks!

[alban]

Optional suggestion: we now have several maps indexed by the tid. I think it would be cleaner to have a single map with a struct type as value.

[alban]

I get the following:

Error: starting operator "oci": starting operator "ebpf": creating eBPF collection: program trace_uretprobe_libssl_SSL_write: load program: permission denied: 102: (85) call bpf_probe_read_user#112: R2 unbounded memory access, use 'var &= const' or 'if (var < const)' (133 line(s) omitted)

[alban]

I got it working with the following:

--- a/gadgets/trace_ssl/program.bpf.c
+++ b/gadgets/trace_ssl/program.bpf.c
@@ -175,9 +175,11 @@ static __always_inline int probe_ssl_rw_exit(struct pt_regs *ctx,
        if (mntns_ptr == 0)
                goto clean;
 
-       buf_copy_size = (size_t)MAX_BUF_SIZE < (size_t)len ?
-                               (size_t)MAX_BUF_SIZE :
-                               (size_t)len;
+       buf_copy_size = len;
+       // MAX_BUF_SIZE is a power of two, so &=MAX_BUF_SIZE-1 makes sure
+       // buf_copy_size does not go above the upper limit
+       buf_copy_size &= MAX_BUF_SIZE-1;
+
        event = gadget_reserve_buf(&events, sizeof(struct event));
        if (!event)
                goto clean;

[i-Pear]

Looks strange, it could work on my linux 6.9. Trying to rollback to 6.6

[i-Pear]

Got the same err message in 6.6, seems it's because the low version of verifier.

[i-Pear]

Although buf_copy_size can be up to MAX_BUF_SIZE, and using MAX_BUF_SIZE - 1 will waste 1 byte, I haven't found a better way to make the 6.6 verifier happy. Let's go with this.

[alban]

LGTM, thanks!