Various fixes for corrupt gRPC URLs and wrong request directions

bpf/go_common.h

@@ -143,8 +143,11 @@ static __always_inline void server_trace_parent(void *goroutine_addr, tp_info_t
         u8 found_info = 0;
 
         if (info) {
+            connection_info_t conn = *info;
+            // Must sort here, Go connection info retains the original ordering.
+            sort_connection_info(&conn);
             bpf_dbg_printk("Looking up traceparent for connection info");
-            tp_info_pid_t *tp_p = trace_info_for_connection(info);
+            tp_info_pid_t *tp_p = trace_info_for_connection(&conn);
             if (tp_p) {                
                 if (correlated_request_with_current(tp_p)) {
                     bpf_dbg_printk("Found traceparent from trace map, another process.");
@@ -242,8 +245,12 @@ static __always_inline void get_conn_info_from_fd(void *fd_ptr, connection_info_
             // read remote
             read_ip_and_port(info->d_addr, &info->d_port, raddr_ptr);
 
-            sort_connection_info(info);
             //dbg_print_http_connection_info(info);
+
+            // IMPORTANT: Unlike kprobes, where we track the sorted connection info
+            // in Go we keep the original connection info order, since we only need it
+            // sorted when we make server requests or when we populate the trace_map for
+            // black box context propagation.
         }
     }
 }

bpf/go_grpc.c

@@ -198,6 +198,8 @@ int uprobe_server_handleStream_return(struct pt_regs *ctx) {
         __builtin_memset(&trace->conn, 0, sizeof(connection_info_t));
     }
 
+    // Server connections have port order reversed from what we want
+    swap_connection_info_order(&trace->conn);
     trace->tp = invocation->tp;
     trace->end_monotime_ns = bpf_ktime_get_ns();
     // submit the completed trace via ringbuffer
@@ -480,7 +482,7 @@ int uprobe_grpcFramerWriteHeaders(struct pt_regs *ctx) {
     void *framer = GO_PARAM1(ctx);
     u64 stream_id = (u64)GO_PARAM2(ctx);
 
-    bpf_printk("framer=%llx, stream_id=%lld, framer_w_pos %llx", framer, ((u64)stream_id), framer_w_pos);
+    bpf_dbg_printk("framer=%llx, stream_id=%lld, framer_w_pos %llx", framer, ((u64)stream_id), framer_w_pos);
 
     u32 stream_lookup = (u32)stream_id;
 

bpf/go_nethttp.c

@@ -277,6 +277,8 @@ int uprobe_ServeHTTPReturns(struct pt_regs *ctx) {
         }
     }
 
+    // Server connections have opposite order, source port is the server port
+    swap_connection_info_order(&trace->conn);
     trace->tp = invocation->tp;
     trace->content_length = invocation->content_length;
     __builtin_memcpy(trace->method, invocation->method, sizeof(trace->method));
@@ -607,7 +609,7 @@ int uprobe_http2FramerWriteHeaders(struct pt_regs *ctx) {
     void *framer = GO_PARAM1(ctx);
     u64 stream_id = (u64)GO_PARAM2(ctx);
 
-    bpf_printk("framer=%llx, stream_id=%lld", framer, ((u64)stream_id));
+    bpf_dbg_printk("framer=%llx, stream_id=%lld", framer, ((u64)stream_id));
 
     u32 stream_lookup = (u32)stream_id;
 
@@ -822,8 +824,12 @@ int uprobe_persistConnRoundTrip(struct pt_regs *ctx) {
                 tp_clone(&tp_p.tp, &invocation->tp);
                 tp_p.tp.ts = bpf_ktime_get_ns();
                 bpf_dbg_printk("storing trace_map info for black-box tracing");
-                bpf_map_update_elem(&trace_map, &conn, &tp_p, BPF_ANY);
                 bpf_map_update_elem(&ongoing_client_connections, &goroutine_addr, &conn, BPF_ANY);
+
+                // Must sort the connection info, this map is shared with kprobes which use sorted connection
+                // info always.
+                sort_connection_info(&conn);
+                bpf_map_update_elem(&trace_map, &conn, &tp_p, BPF_ANY);
             }
         }
     }

bpf/http_sock.c

@@ -106,18 +106,30 @@ int BPF_KPROBE(kprobe_tcp_rcv_established, struct sock *sk, struct sk_buff *skb)
         return 0;
     }
 
+    bpf_dbg_printk("=== tcp_rcv_established id=%d ===", id);
+
     pid_connection_info_t info = {};
 
     if (parse_sock_info(sk, &info.conn)) {
+        //u16 orig_dport = info.conn.d_port;
+        //dbg_print_http_connection_info(&info.conn);
         sort_connection_info(&info.conn);
         info.pid = pid_from_pid_tgid(id);        
-        //dbg_print_http_connection_info(&info.conn);
 
         http_connection_metadata_t meta = {};
         task_pid(&meta.pid);
         meta.type = EVENT_HTTP_REQUEST;
         bpf_map_update_elem(&filtered_connections, &info, &meta, BPF_NOEXIST); // On purpose BPF_NOEXIST, we don't want to overwrite data by accept or connect
-        bpf_map_update_elem(&pid_tid_to_conn, &id, &info, BPF_ANY); // to support SSL on missing handshake
+
+        // This is a current limitation for port ordering detection for SSL.
+        // tcp_rcv_established flip flops the ports and we can't tell if it's client or server call.
+        // If the source port for a client call is lower, we'll get this wrong.
+        // TODO: Need to fix this. 
+        ssl_pid_connection_info_t pid_info = {
+            .conn = info,
+            .orig_dport = info.conn.s_port,
+        };
+        bpf_map_update_elem(&pid_tid_to_conn, &id, &pid_info, BPF_ANY); // to support SSL on missing handshake, respect the original info if there
     }
 
     return 0;
@@ -157,15 +169,20 @@ int BPF_KRETPROBE(kretprobe_sys_accept4, uint fd)
     pid_connection_info_t info = {};
 
     if (parse_accept_socket_info(args, &info.conn)) {
-        sort_connection_info(&info.conn);
+        u16 orig_dport = info.conn.d_port;
         //dbg_print_http_connection_info(&info.conn);
+        sort_connection_info(&info.conn);
         info.pid = pid_from_pid_tgid(id);
 
         http_connection_metadata_t meta = {};
         task_pid(&meta.pid);
         meta.type = EVENT_HTTP_REQUEST;
         bpf_map_update_elem(&filtered_connections, &info, &meta, BPF_ANY); // On purpose BPF_ANY, we want to overwrite stale
-        bpf_map_update_elem(&pid_tid_to_conn, &id, &info, BPF_ANY); // to support SSL on missing handshake
+        ssl_pid_connection_info_t pid_info = {
+            .conn = info,
+            .orig_dport = orig_dport,
+        };
+        bpf_map_update_elem(&pid_tid_to_conn, &id, &pid_info, BPF_ANY); // to support SSL on missing handshake
     }
 
 cleanup:
@@ -225,15 +242,20 @@ int BPF_KRETPROBE(kretprobe_sys_connect, int fd)
 
     if (parse_connect_sock_info(args, &info.conn)) {
         bpf_dbg_printk("=== connect ret id=%d, pid=%d ===", id, pid_from_pid_tgid(id));
-        sort_connection_info(&info.conn);
+        u16 orig_dport = info.conn.d_port;
         //dbg_print_http_connection_info(&info.conn);
+        sort_connection_info(&info.conn);
         info.pid = pid_from_pid_tgid(id);
 
         http_connection_metadata_t meta = {};
         task_pid(&meta.pid);
         meta.type = EVENT_HTTP_CLIENT;
         bpf_map_update_elem(&filtered_connections, &info, &meta, BPF_ANY); // On purpose BPF_ANY, we want to overwrite stale
-        bpf_map_update_elem(&pid_tid_to_conn, &id, &info, BPF_ANY); // to support SSL 
+        ssl_pid_connection_info_t pid_info = {
+            .conn = info,
+            .orig_dport = orig_dport,
+        };
+        bpf_map_update_elem(&pid_tid_to_conn, &id, &pid_info, BPF_ANY); // to support SSL 
     }
 
 cleanup:
@@ -243,6 +265,29 @@ int BPF_KRETPROBE(kretprobe_sys_connect, int fd)
 
 // Main HTTP read and write operations are handled with tcp_sendmsg and tcp_recvmsg 
 
+static __always_inline void *is_ssl_connection(u64 id, pid_connection_info_t *conn) {
+    void *ssl = 0;
+    // Checks if it's sandwitched between active SSL handshake, read or write uprobe/uretprobe
+    void **s = bpf_map_lookup_elem(&active_ssl_handshakes, &id);
+    if (s) {
+        ssl = *s;
+    } else {
+        ssl_args_t *ssl_args = bpf_map_lookup_elem(&active_ssl_read_args, &id);
+        if (!ssl_args) {
+            ssl_args = bpf_map_lookup_elem(&active_ssl_write_args, &id);
+        }
+        if (ssl_args) {
+            ssl = (void *)ssl_args->ssl;
+        }
+    }            
+
+    if (ssl) {
+        return ssl;
+    }
+
+    return bpf_map_lookup_elem(&active_ssl_connections, conn);
+}
+
 // The size argument here will be always the total response size.
 // However, the return value of tcp_sendmsg tells us how much it sent. When the
 // response is large it will get chunked, so we have to use a kretprobe to
@@ -265,50 +310,46 @@ int BPF_KPROBE(kprobe_tcp_sendmsg, struct sock *sk, struct msghdr *msg, size_t s
     };
 
     if (parse_sock_info(sk, &s_args.p_conn.conn)) {
-        //dbg_print_http_connection_info(&info.conn); // commented out since GitHub CI doesn't like this call
+        u16 orig_dport = s_args.p_conn.conn.d_port;
+        //dbg_print_http_connection_info(&s_args.p_conn.conn); // commented out since GitHub CI doesn't like this call
         sort_connection_info(&s_args.p_conn.conn);
         s_args.p_conn.pid = pid_from_pid_tgid(id);
 
+        void *ssl = is_ssl_connection(id, &s_args.p_conn);
         if (size > 0) {
-            void *iovec_ptr = find_msghdr_buf(msg);
-            if (iovec_ptr) {
-                u64 sock_p = (u64)sk;
-                bpf_map_update_elem(&active_send_args, &id, &s_args, BPF_ANY);
-                bpf_map_update_elem(&active_send_sock_args, &sock_p, &s_args, BPF_ANY);
-                handle_buf_with_connection(&s_args.p_conn, iovec_ptr, size, NO_SSL, TCP_SEND);
-                // if (size < KPROBES_LARGE_RESPONSE_LEN) {
-                //     bpf_dbg_printk("Maybe we need to finish the request");
-                //     finish_possible_delayed_http_request(&s_args.p_conn);
-                // }
+            if (!ssl) {
+                void *iovec_ptr = find_msghdr_buf(msg);
+                if (iovec_ptr) {
+                    u64 sock_p = (u64)sk;
+                    bpf_map_update_elem(&active_send_args, &id, &s_args, BPF_ANY);
+                    bpf_map_update_elem(&active_send_sock_args, &sock_p, &s_args, BPF_ANY);
+                    handle_buf_with_connection(&s_args.p_conn, iovec_ptr, size, NO_SSL, TCP_SEND, orig_dport);
+                    // if (size < KPROBES_LARGE_RESPONSE_LEN) {
+                    //     bpf_dbg_printk("Maybe we need to finish the request");
+                    //     finish_possible_delayed_http_request(&s_args.p_conn);
+                    // }
+                } else {
+                    bpf_dbg_printk("can't find iovec ptr in msghdr, not tracking sendmsg");
+                }
             } else {
-                bpf_dbg_printk("can't find iovec ptr in msghdr, not tracking sendmsg");
-            }
-        }
-
-        void *ssl = 0;
-        // Checks if it's sandwitched between active SSL handshake, read or write uprobe/uretprobe
-        void **s = bpf_map_lookup_elem(&active_ssl_handshakes, &id);
-        if (s) {
-            ssl = *s;
-        } else {
-            ssl_args_t *ssl_args = bpf_map_lookup_elem(&active_ssl_read_args, &id);
-            if (!ssl_args) {
-                ssl_args = bpf_map_lookup_elem(&active_ssl_write_args, &id);
-            }
-            if (ssl_args) {
-                ssl = (void *)ssl_args->ssl;
+                bpf_dbg_printk("tcp_sendmsg for identified SSL connection, ignoring...");
             }
         }
 
         if (!ssl) {
             return 0;
         }
+
         bpf_dbg_printk("=== kprobe SSL tcp_sendmsg=%d sock=%llx ssl=%llx ===", id, sk, ssl);
-        pid_connection_info_t *conn = bpf_map_lookup_elem(&ssl_to_conn, &ssl);
+        ssl_pid_connection_info_t *conn = bpf_map_lookup_elem(&ssl_to_conn, &ssl);
         if (conn) {
-            finish_possible_delayed_http_request(conn);
+            finish_possible_delayed_tls_http_request(&conn->conn, ssl);
         }
-        bpf_map_update_elem(&ssl_to_conn, &ssl, &s_args.p_conn, BPF_ANY);
+        ssl_pid_connection_info_t ssl_conn = {
+            .conn = s_args.p_conn,
+            .orig_dport = orig_dport,
+        };
+        bpf_map_update_elem(&ssl_to_conn, &ssl, &ssl_conn, BPF_ANY);
     }
 
     return 0;
@@ -367,7 +408,6 @@ int BPF_KPROBE(kprobe_tcp_close, struct sock *sk, long timeout) {
 
     ensure_sent_event(id, &sock_p);
 
-
     pid_connection_info_t info = {};
 
     if (parse_sock_info(sk, &info.conn)) {
@@ -435,10 +475,18 @@ int BPF_KRETPROBE(kretprobe_tcp_recvmsg, int copied_len) {
     pid_connection_info_t info = {};
 
     if (parse_sock_info((struct sock *)args->sock_ptr, &info.conn)) {
-        sort_connection_info(&info.conn);
+        u16 orig_dport = info.conn.d_port;
         //dbg_print_http_connection_info(&info.conn);
+        sort_connection_info(&info.conn);
         info.pid = pid_from_pid_tgid(id);
-        handle_buf_with_connection(&info, (void *)args->iovec_ptr, copied_len, NO_SSL, TCP_RECV);
+
+        void *ssl = is_ssl_connection(id, &info);
+
+        if (!ssl) {
+            handle_buf_with_connection(&info, (void *)args->iovec_ptr, copied_len, NO_SSL, TCP_RECV, orig_dport);
+        } else {
+            bpf_dbg_printk("tcp_recvmsg for an identified SSL connection, ignoring...");
+        }
     }
 
 done:
@@ -466,9 +514,6 @@ int socket__http_filter(struct __sk_buff *skb) {
     if (!tcp_close(&tcp) && tcp_empty(&tcp, skb)) {
         return 0;
     }
-
-    // sorting must happen here, before we check or set dups
-    sort_connection_info(&conn);
 
     // we don't want to read the whole buffer for every packed that passes our checks, we read only a bit and check if it's truly HTTP request/response.
     unsigned char buf[MIN_HTTP_SIZE] = {0};
@@ -480,7 +525,10 @@ int socket__http_filter(struct __sk_buff *skb) {
     }
 
     u8 packet_type = 0;
-    if (is_http(buf, len, &packet_type)) { // we must check tcp_close second, a packet can be a close and a response
+    if (is_http(buf, len, &packet_type)) { // we must check tcp_close second, a packet can be a close and a response      
+        //dbg_print_http_connection_info(&conn); // commented out since GitHub CI doesn't like this call
+        sort_connection_info(&conn);
+
         http_info_t info = {0};
         info.conn_info = conn;
 
@@ -581,6 +629,7 @@ int BPF_KPROBE(kprobe_sys_exit, int status) {
     if (s_args) {
         bpf_dbg_printk("Checking if we need to finish the request per thread id");
         finish_possible_delayed_http_request(&s_args->p_conn);
+        bpf_map_delete_elem(&active_ssl_connections, &s_args->p_conn);
     }
 
     bpf_map_delete_elem(&clone_map, &task);