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

fix(network-monitor): Include the packet payload regarless of L4 protocol #303

Merged
merged 2 commits into from
Jun 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions crates/bpf-builder/include/buffer.bpf.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,16 @@ static __always_inline int buffer_append_skb_bytes(struct buffer *buffer,
return -1;
}

// * The `< 0` case is impossible to reproduce (otherwise it would mean
// `skb->len` is somehow lower than the size of packet headers we already
// consumed from that SKB).
// It's here only to appease the eBPF verifier (using `u32` and checking
// just for `0` doesn't work).
// * The `= 0` case usually applies to SYN, SYN-ACK, ACK or any other packets
// which don't contain any payload after L4 headers.
// Therefore, we don't return any error here.
if (len <= 0) {
LOG_ERROR("Invalid offset (%zu) exceeding the packet length (%zu).",
offset, skb->len);
return -1;
return 0;
}

int r = bpf_skb_load_bytes(skb, offset, &((char *)buffer->buffer)[pos],
Expand Down
13 changes: 7 additions & 6 deletions crates/modules/network-monitor/probes.bpf.c
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,8 @@ __always_inline int process_skb(struct __sk_buff *skb,
void *data_end = (void *)(long)skb->data_end;
void *data = (void *)(long)skb->data;

buffer_index_init(&network_event->buffer, &msg_event->data);

// Parse L3 header (IPv4 / IPv6).
switch (l3_proto) {
case ETH_P_IPV4: {
Expand Down Expand Up @@ -653,19 +655,18 @@ __always_inline int process_skb(struct __sk_buff *skb,
}
break;
}

buffer_index_init(&network_event->buffer, &msg_event->data);
if (buffer_append_skb_bytes(&network_event->buffer, &msg_event->data, skb,
headers_len) < 0) {
LOG_ERROR("Failed to retrieve the packet payload. The event is going to miss the `data` part.");
}
break;
}
default:
LOG_DEBUG("ignored unsupported L4 protocol %d", l4_proto);
goto send_event;
}

if (buffer_append_skb_bytes(&network_event->buffer, &msg_event->data, skb,
headers_len) < 0) {
LOG_ERROR("Failed to retrieve the packet payload. The event is going to miss the `data` part.");
}

msg_event->data_len = skb->len - headers_len;

send_event:
Expand Down
12 changes: 4 additions & 8 deletions crates/modules/network-monitor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,11 +303,11 @@ pub mod pulsar {
let data = data
.bytes(&event.buffer)
.map_err(|err| {
log::error!("[dns] Error getting message: {}", err);
log::error!("Error getting network packet payload: {err}");
})
.ok()?;

// any valid dns data?
// Check wheter the payload contains any DNS data.
let dns = dns_parser::Packet::parse(data).ok()?;
let with_q = !dns.questions.is_empty();
let with_a = !dns.answers.is_empty();
Expand Down Expand Up @@ -557,10 +557,6 @@ pub mod test_suite {
// for TCP it's overriden on connection
let mut source = dest;
let msg = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
let data_copied = match proto {
Proto::TCP => Vec::new(),
Proto::UDP => msg.to_vec(),
};
TestRunner::with_ebpf(program)
.run(|| match proto {
Proto::UDP => {
Expand Down Expand Up @@ -592,15 +588,15 @@ pub mod test_suite {
NetworkEvent::Send,
(dst, dest.into(), "destination address"),
(src, source.into(), "source address"),
(data, data_copied.clone(), "data copy"),
(data, msg.to_vec(), "data copy"),
(data_len, msg.len() as u32, "real message len"),
(proto, proto, "protocol")
))
.expect_event(event_check!(
NetworkEvent::Receive,
(dst, source.into(), "destination address"),
(src, dest.into(), "source address"),
(data, data_copied.clone(), "data copy"),
(data, msg.to_vec(), "data copy"),
(data_len, msg.len() as u32, "real message len"),
(proto, proto, "protocol")
))
Expand Down