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

✨ Add IPv6 support to TCP kprobes #13

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

bserdar
Copy link
Contributor

@bserdar bserdar commented Dec 18, 2023

Mysql uses an ipv6 socket to accept connections. Because of this, the ebpf code completely ignored data captured from MySQL. This PR changes the ebpf logic to accept both ipv4 and ipv6 sockets, but captures only ipv4 addresses from them.

… capture still only works for ipv4 addresses, but these changes enable capturing packets from mysql, because mysql uses an ipv6 socket to accept connections, which are then sent to openssl functions that ultimately write to ipv4 addresses.
Copy link
Member

@mertyildiran mertyildiran left a comment

Choose a reason for hiding this comment

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

The changes are unrelated to MySQL. The changes are about adding IPv6 support to TCP kprobes.

bpf/common.c Outdated
if (flags == NULL) {
return 0;
if (flags != NULL) {
chunk->flags |= (*flags & FLAGS_IS_CLIENT_BIT);
Copy link
Member

Choose a reason for hiding this comment

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

What happens if flags is NULL?

bpf/common.c Outdated
@@ -103,7 +101,7 @@ static __always_inline void output_ssl_chunk(struct pt_regs *ctx, struct ssl_inf
if (!add_address_to_chunk(ctx, chunk, id, chunk->fd, info)) {
// Without an address, we drop the chunk because there is not much to do with it in Go
//
return;
//return;
Copy link
Member

Choose a reason for hiding this comment

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

What the reason of commenting out this return statement?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will revert. No longer necessary to comment this out.

@@ -13,7 +13,7 @@ Copyright (C) Kubeshark
#define IPV4_ADDR_LEN (16)

struct accept_info {
__u32* addrlen;
__u32* addrlen;
Copy link
Member

Choose a reason for hiding this comment

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

Don't change the indentation.

packet_sorter.go Outdated
err = writer.WriteFileHeader(uint32(misc.Snaplen), layers.LinkTypeEthernet)
if err != nil {
log.Error().Err(err).Msg("While writing the PCAP header:")
}
// Do not write PCAP header. This is a named pipe, and a worker
// might be waiting to read from it already
Copy link
Member

Choose a reason for hiding this comment

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

Revert.

@@ -70,6 +70,7 @@ void sys_enter_read(struct sys_enter_read_write_ctx *ctx) {

if (infoPtr != NULL) {
fd_tracepoints_handle_openssl(ctx, id, infoPtr, &openssl_read_context, ORIGIN_SYS_ENTER_READ_CODE);
return;
Copy link
Member

Choose a reason for hiding this comment

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

Why return?

@@ -87,6 +88,7 @@ void sys_enter_write(struct sys_enter_read_write_ctx *ctx) {

if (infoPtr != NULL) {
fd_tracepoints_handle_openssl(ctx, id, infoPtr, &openssl_write_context, ORIGIN_SYS_ENTER_WRITE_CODE);
return;
Copy link
Member

Choose a reason for hiding this comment

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

Why return?

Comment on lines 14 to 29
struct sock_common scommon;
err=bpf_probe_read(&scommon,sizeof(scommon),&sk->__sk_common);
if (err!=0) {
log_error(ctx, LOG_ERROR_READING_SOCKET_INFO, id, err, 0l);
return -1;
}
// Do not capture anything but inet and inet6. Inet4 traffic can
// also appear as inet6 here.
if(scommon.skc_family!=AF_INET && scommon.skc_family!=AF_INET6) {
return -1;
}
// ipv4 addresses are populated even if skc_family=AF_INET6
//
// daddr, saddr and dport are in network byte order (big endian)
// sport is in host byte order
__be32 saddr;
__be32 daddr;
__be16 dport;
__u16 sport;

err = bpf_probe_read(&saddr, sizeof(saddr), (void *)&sk->__sk_common.skc_rcv_saddr);
if (err != 0) {
log_error(ctx, LOG_ERROR_READING_SOCKET_SADDR, id, err, 0l);
return -1;
}
err = bpf_probe_read(&daddr, sizeof(daddr), (void *)&sk->__sk_common.skc_daddr);
if (err != 0) {
log_error(ctx, LOG_ERROR_READING_SOCKET_DADDR, id, err, 0l);
return -1;
}
err = bpf_probe_read(&dport, sizeof(dport), (void *)&sk->__sk_common.skc_dport);
if (err != 0) {
log_error(ctx, LOG_ERROR_READING_SOCKET_DPORT, id, err, 0l);
return -1;
}
err = bpf_probe_read(&sport, sizeof(sport), (void *)&sk->__sk_common.skc_num);
if (err != 0) {
log_error(ctx, LOG_ERROR_READING_SOCKET_SPORT, id, err, 0l);
return -1;
}

address_info_ptr->daddr = daddr;
address_info_ptr->saddr = saddr;
address_info_ptr->dport = dport;
address_info_ptr->sport = bpf_htons(sport);
address_info_ptr->daddr = scommon.skc_daddr;
Copy link
Member

Choose a reason for hiding this comment

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

Indentation?
https://github.com/MaJerle/c-code-style?tab=readme-ov-file#general-rules ?

Explain how it adds IPv6 support?

@mertyildiran mertyildiran changed the title Mysql packet capture ✨ Add IPv6 support to TCP kprobes Jan 3, 2024
@mertyildiran
Copy link
Member

Don't use forks. Push branches to https://github.com/kubeshark/tracer

@bserdar
Copy link
Contributor Author

bserdar commented Jan 9, 2024

Don't use forks. Push branches to https://github.com/kubeshark/tracer

I do not have push access to this repository

@bserdar
Copy link
Contributor Author

bserdar commented Jan 9, 2024

The changes are unrelated to MySQL. The changes are about adding IPv6 support to TCP kprobes.

These changes are insufficient for IPv6 support. It only makes the existing code more forgiving by handling the case where the socket is created for IPv6 but the underlying traffic is still IPv4. This needs more work to capture IPv6 traffic. I agree that it is no longer MySQL specific, but it is not to add IPv6 support either.

@mertyildiran mertyildiran requested review from iluxa and mertyildiran and removed request for mertyildiran January 29, 2024 20:36
bpf/tcp_kprobes.c Outdated Show resolved Hide resolved
bpf/tcp_kprobes.c Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants