Skip to content

Commit

Permalink
[USMO] Fix dynamic counter table TLS leak (#24042)
Browse files Browse the repository at this point in the history
* fix dynamic_counter_table TLS leak

* re-arrange the deletion order

* created a helper function for the termination tls part

* simpler implementation

* fix cr notes

* fix typos
  • Loading branch information
amitslavin authored Mar 26, 2024
1 parent 8596c54 commit 10911a1
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 5 deletions.
12 changes: 12 additions & 0 deletions pkg/network/ebpf/c/protocols/sockfd-probes.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,18 @@ int kprobe__tcp_close(struct pt_regs *ctx) {

bpf_map_delete_elem(&sock_by_pid_fd, pid_fd);
bpf_map_delete_elem(&pid_fd_by_sock, &sk);

conn_tuple_t t;
u64 pid_tgid = bpf_get_current_pid_tgid();
if (!read_conn_tuple(&t, sk, pid_tgid, CONN_TYPE_TCP)) {
return 0;
}

// The cleanup of the map happens either during TCP termination or during the TLS shutdown event.
// TCP termination is managed by the socket filter, thus it cannot clean TLS entries,
// as it does not have access to the PID and NETNS.
// Therefore, we use tls_finish to clean the connection. While this approach is not ideal, it is the best option available to us for now.
tls_finish(ctx, &t, true);
return 0;
}

Expand Down
7 changes: 6 additions & 1 deletion pkg/network/ebpf/c/protocols/tls/https.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ static __always_inline void tls_process(struct pt_regs *ctx, conn_tuple_t *t, vo
bpf_tail_call_compat(ctx, &tls_process_progs, prog);
}

static __always_inline void tls_finish(struct pt_regs *ctx, conn_tuple_t *t) {
static __always_inline void tls_finish(struct pt_regs *ctx, conn_tuple_t *t, bool skip_http) {
conn_tuple_t final_tuple = {0};
conn_tuple_t normalized_tuple = *t;
normalize_tuple(&normalized_tuple);
Expand All @@ -123,6 +123,11 @@ static __always_inline void tls_finish(struct pt_regs *ctx, conn_tuple_t *t) {
protocol_t protocol = get_protocol_from_stack(stack, LAYER_APPLICATION);
switch (protocol) {
case PROTOCOL_HTTP:
// HTTP is a special case. As of today, regardless of TLS or plaintext traffic, we ignore the PID and NETNS while processing it.
// The termination, both for TLS and plaintext, for HTTP traffic is taken care of in the socket filter.
// Until we split the TLS and plaintext management for HTTP traffic, there are flows (such as those being called from tcp_close)
// in which we don't want to terminate HTTP traffic, but instead leave it to the socket filter.
if (skip_http) {return;}
prog = TLS_HTTP_TERMINATION;
final_tuple = normalized_tuple;
break;
Expand Down
2 changes: 1 addition & 1 deletion pkg/network/ebpf/c/protocols/tls/java/erpc_handlers.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ int kprobe_handle_close_connection(struct pt_regs *ctx) {
if (exists != NULL){
// tls_finish can launch a tail call, thus cleanup should be done before.
bpf_map_delete_elem(&java_tls_connections, &connection);
tls_finish(ctx, &connection);
tls_finish(ctx, &connection, false);
}
return 0;
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/network/ebpf/c/protocols/tls/native-tls.h
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ int uprobe__SSL_shutdown(struct pt_regs *ctx) {

// tls_finish can launch a tail call, thus cleanup should be done before.
bpf_map_delete_elem(&ssl_sock_by_ctx, &ssl_ctx);
tls_finish(ctx, t);
tls_finish(ctx, t, false);

return 0;
}
Expand Down Expand Up @@ -518,7 +518,7 @@ static __always_inline void gnutls_goodbye(struct pt_regs *ctx, void *ssl_sessio

// tls_finish can launch a tail call, thus cleanup should be done before.
bpf_map_delete_elem(&ssl_sock_by_ctx, &ssl_session);
tls_finish(ctx, t);
tls_finish(ctx, t, false);
}

// int gnutls_bye (gnutls_session_t session, gnutls_close_request_t how)
Expand Down
2 changes: 1 addition & 1 deletion pkg/network/ebpf/c/runtime/usm.c
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ int uprobe__crypto_tls_Conn_Close(struct pt_regs *ctx) {

conn_tuple_t copy = *t;
// tls_finish can launch a tail call, thus cleanup should be done before.
tls_finish(ctx, &copy);
tls_finish(ctx, &copy, false);
return 0;
}

Expand Down

0 comments on commit 10911a1

Please sign in to comment.