From a8ffcd8bb0273d59600c6310a80b81206096c113 Mon Sep 17 00:00:00 2001 From: Ivan Nardi <12729895+IvanNardi@users.noreply.github.com> Date: Wed, 24 Nov 2021 10:46:48 +0100 Subject: Rework how hostname/SNI info is saved (#1330) Looking at `struct ndpi_flow_struct` the two bigger fields are `host_server_name[240]` (mainly for HTTP hostnames and DNS domains) and `protos.tls_quic.client_requested_server_name[256]` (for TLS/QUIC SNIs). This commit aims to reduce `struct ndpi_flow_struct` size, according to two simple observations: 1) maximum one of these two fields is used for each flow. So it seems safe to merge them; 2) even if hostnames/SNIs might be very long, in practice they are rarely longer than a fews tens of bytes. So, using a (single) large buffer is a waste of memory for all kinds of flows. If we need to truncate the name, we keep the *last* characters, easing domain matching. Analyzing some real traffic, it seems safe to assume that the vast majority of hostnames/SNIs is shorter than 80 bytes. Hostnames/SNIs are always converted to lowercase. Attention was given so as to be sure that unit-tests outputs are not affected by this change. Because of a bug, TLS/QUIC SNI were always truncated to 64 bytes (the *first* 64 ones): as a consequence, there were some "Suspicious DGA domain name" and "TLS Certificate Mismatch" false positives. --- src/lib/protocols/quic.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) (limited to 'src/lib/protocols/quic.c') diff --git a/src/lib/protocols/quic.c b/src/lib/protocols/quic.c index 358edb064..6bae70524 100644 --- a/src/lib/protocols/quic.c +++ b/src/lib/protocols/quic.c @@ -1324,7 +1324,7 @@ static void process_chlo(struct ndpi_detection_module_struct *ndpi_struct, uint32_t i; uint16_t num_tags; uint32_t prev_offset; - uint32_t tag_offset_start, offset, len, sni_len; + uint32_t tag_offset_start, offset, len; ndpi_protocol_match_result ret_match; int sni_found = 0, ua_found = 0; @@ -1356,22 +1356,20 @@ static void process_chlo(struct ndpi_detection_module_struct *ndpi_struct, crypto_data_len, tag_offset_start, prev_offset, offset, len); #endif if(memcmp(tag, "SNI\0", 4) == 0) { - sni_len = MIN(len, sizeof(flow->protos.tls_quic.client_requested_server_name) - 1); - memcpy(flow->protos.tls_quic.client_requested_server_name, - &crypto_data[tag_offset_start + prev_offset], sni_len); - flow->protos.tls_quic.client_requested_server_name[sni_len] = '\0'; + + ndpi_hostname_sni_set(flow, &crypto_data[tag_offset_start + prev_offset], len); NDPI_LOG_DBG2(ndpi_struct, "SNI: [%s]\n", - flow->protos.tls_quic.client_requested_server_name); + flow->host_server_name); ndpi_match_host_subprotocol(ndpi_struct, flow, - (char *)flow->protos.tls_quic.client_requested_server_name, - strlen((const char*)flow->protos.tls_quic.client_requested_server_name), + flow->host_server_name, + strlen(flow->host_server_name), &ret_match, NDPI_PROTOCOL_QUIC); flow->protos.tls_quic.hello_processed = 1; /* Allow matching of custom categories */ ndpi_check_dga_name(ndpi_struct, flow, - flow->protos.tls_quic.client_requested_server_name, 1); + flow->host_server_name, 1); sni_found = 1; if (ua_found) @@ -1396,7 +1394,7 @@ static void process_chlo(struct ndpi_detection_module_struct *ndpi_struct, NDPI_LOG_DBG(ndpi_struct, "Something went wrong in tags iteration\n"); /* Add check for missing SNI */ - if(flow->protos.tls_quic.client_requested_server_name[0] == '\0') { + if(flow->host_server_name[0] == '\0') { /* This is a bit suspicious */ ndpi_set_risk(ndpi_struct, flow, NDPI_TLS_MISSING_SNI); } @@ -1508,7 +1506,7 @@ static int eval_extra_processing(struct ndpi_detection_module_struct *ndpi_struc */ if((version == V_Q046 && - flow->protos.tls_quic.client_requested_server_name[0] == '\0') || + flow->host_server_name[0] == '\0') || is_ch_reassembler_pending(flow)) { NDPI_LOG_DBG2(ndpi_struct, "We have further work to do\n"); return 1; -- cgit v1.2.3