From a738887fb55b8f7f591041ca097c195334c153ef Mon Sep 17 00:00:00 2001 From: ganeshmurthy Date: Thu, 19 Dec 2024 10:42:37 -0500 Subject: [PATCH] Fixes #1694: Additional fix. Capture the first left-most x-forwarded-for header value --- src/observers/http2/http2_observer.c | 48 ++++++++++++++++++++-------- src/observers/private.h | 11 ++++--- tests/system_tests_http_observer.py | 47 ++++++++++++++++++++++++++- 3 files changed, 86 insertions(+), 20 deletions(-) diff --git a/src/observers/http2/http2_observer.c b/src/observers/http2/http2_observer.c index a50248315..53ddd6411 100644 --- a/src/observers/http2/http2_observer.c +++ b/src/observers/http2/http2_observer.c @@ -191,18 +191,6 @@ static int on_data_recv_callback(qd_http2_decoder_connection_t *conn_state, return 0; } -static void set_stream_vflow_attribute(qdpo_transport_handle_t *transport_handle, uint32_t stream_id, vflow_attribute_t attribute_type, const char *string_value) -{ - qd_http2_stream_info_t *stream_info = 0; - qd_error_t error = get_stream_info_from_hashtable(transport_handle, &stream_info, stream_id); - if (error == QD_ERROR_NOT_FOUND) { - qd_log(LOG_HTTP2_DECODER, QD_LOG_ERROR, "[C%"PRIu64"] set_stream_vflow_attribute could not find in the hashtable, stream_id=%" PRIu32, transport_handle->conn_id, stream_id); - } - else { - vflow_set_string(stream_info->vflow, attribute_type, string_value); - } -} - /** * This callback is called for every single header. * We only care about the http method and the response status code. @@ -223,7 +211,13 @@ static int on_header_recv_callback(qd_http2_decoder_connection_t *conn_state, if (strcmp(HTTP_METHOD, (const char *)name) == 0) { // Set the http method (GET, POST, PUT, DELETE etc) on the stream's vflow object. qd_log(LOG_HTTP2_DECODER, QD_LOG_DEBUG, "[C%"PRIu64"] on_header_recv_callback - HTTP_METHOD=%s, stream_id=%" PRIu32, transport_handle->conn_id, (const char *)value, stream_id); - set_stream_vflow_attribute(transport_handle, stream_id, VFLOW_ATTRIBUTE_METHOD, (const char *)value); + qd_error_t error = get_stream_info_from_hashtable(transport_handle, &stream_info, stream_id); + if (error == QD_ERROR_NOT_FOUND) { + qd_log(LOG_HTTP2_DECODER, QD_LOG_ERROR, "[C%"PRIu64"] set_stream_vflow_attribute could not find in the hashtable, stream_id=%" PRIu32, transport_handle->conn_id, stream_id); + } + else { + vflow_set_string(stream_info->vflow, VFLOW_ATTRIBUTE_METHOD, (const char *)value); + } } else if (strcmp(HTTP_STATUS, (const char *)name) == 0) { qd_error_t error = get_stream_info_from_hashtable(transport_handle, &stream_info, stream_id); // @@ -246,7 +240,33 @@ static int on_header_recv_callback(qd_http2_decoder_connection_t *conn_state, } } } else if (strcmp(X_FORWARDED_FOR, (const char *)name) == 0) { - set_stream_vflow_attribute(transport_handle, stream_id, VFLOW_ATTRIBUTE_SOURCE_HOST, (const char *)value); + qd_error_t error = get_stream_info_from_hashtable(transport_handle, &stream_info, stream_id); + if (error == QD_ERROR_NOT_FOUND) { + qd_log(LOG_HTTP2_DECODER, QD_LOG_ERROR, "[C%"PRIu64"] set_stream_vflow_attribute could not find in the hashtable, stream_id=%" PRIu32, transport_handle->conn_id, stream_id); + } + else { + if (!stream_info->x_forwarded_for) { + // + // We will capture the very first x-forwarded-for header and ignore the other x-forwarded for headers in the same request. + // Say a request has the following three x-forwarded-for headers + // + // X-Forwarded-For: 2001:db8:85a3:8d3:1319:8a2e:370:7348, 197.1.773.201 + // X-Forwarded-For: 195.0.223.001 + // X-Forwarded-For: 203.0.113.195, 2007:db5:85a3:8d3:1319:8a2e:370:3221 + // + // We will obtain the value of the first X-Forwarded-For (2001:db8:85a3:8d3:1319:8a2e:370:7348, 197.1.773.201) and ignore the + // other two X-Forwarded-For headers. + // The first X-Forwarded-For header is comma separated list, we will obtain the leftmost (the first) value (2001:db8:85a3:8d3:1319:8a2e:370:7348) in the list + + // const uint8_t *value passed into this function is guaranteed to be NULL-terminated + char value_copy[valuelen+1]; + strcpy(value_copy, (const char *)value); + // Get the first token (left-most) + char *first_token = strtok(value_copy, ","); + vflow_set_string(stream_info->vflow, VFLOW_ATTRIBUTE_SOURCE_HOST, first_token); + stream_info->x_forwarded_for = true; + } + } } return 0; } diff --git a/src/observers/private.h b/src/observers/private.h index 2a8c78b8a..00d7df603 100644 --- a/src/observers/private.h +++ b/src/observers/private.h @@ -73,11 +73,12 @@ typedef struct qd_http2_stream_info_t qd_http2_stream_info_t; struct qd_http2_stream_info_t { DEQ_LINKS(qd_http2_stream_info_t); - qd_http2_decoder_connection_t *conn_state; // Reference to the stream's connection state information - vflow_record_t *vflow; // stream level vanflow. this is a vanflow per request - uint32_t stream_id; // stream_id of the stream. - uint64_t bytes_in; - uint64_t bytes_out; + qd_http2_decoder_connection_t *conn_state; // Reference to the stream's connection state information + vflow_record_t *vflow; // stream level vanflow. this is a vanflow per request + uint32_t stream_id; // stream_id of the stream. + uint64_t bytes_in; // bytes received from client on this stream + uint64_t bytes_out; // bytes sent to the client on this stream + bool x_forwarded_for; // True if the x-forwarded-for header has already been received }; diff --git a/tests/system_tests_http_observer.py b/tests/system_tests_http_observer.py index 864a6b81b..c5d31d8f3 100644 --- a/tests/system_tests_http_observer.py +++ b/tests/system_tests_http_observer.py @@ -513,7 +513,7 @@ def test_head_request(self): success = retry(lambda: snooper_thread.match_records(expected)) self.assertTrue(success, f"Failed to match records {snooper_thread.get_results()}") - # Pass traffic: + # Pass traffic with one X-Forwarded-For header: _, out, _ = run_local_curl(get_address(self.router_qdra), args=['--head', '--header', 'X-Forwarded-For: 192.168.0.2']) self.assertIn('HTTP/2 200', out) self.assertIn('content-type: text/html', out) @@ -532,6 +532,51 @@ def test_head_request(self): success = retry(lambda: snooper_thread.match_records(expected)) self.assertTrue(success, f"Failed to match records {snooper_thread.get_results()}") + # Pass traffic with comma separated X-Forwarded-For header: + _, out, _ = run_local_curl(get_address(self.router_qdra), args=['--head', '--header', 'X-Forwarded-For: 192.168.0.1, 203.168.2.2']) + self.assertIn('HTTP/2 200', out) + self.assertIn('content-type: text/html', out) + + # Expect a TCP flow/counter-flow and one HTTP/2 flow + expected = { + "QDR": [ + ('BIFLOW_APP', {'PROTOCOL': 'HTTP/2', + 'METHOD': 'HEAD', + 'RESULT': '200', + 'SOURCE_HOST': '192.168.0.1', + 'STREAM_ID': ANY_VALUE, + 'END_TIME': ANY_VALUE}) + ] + } + success = retry(lambda: snooper_thread.match_records(expected)) + self.assertTrue(success, f"Failed to match records {snooper_thread.get_results()}") + + # Pass traffic with many comma separated X-Forwarded-For headers: + _, out, _ = run_local_curl(get_address(self.router_qdra), + args=['--head', '--header', + 'X-Forwarded-For: 192.168.1.7, 203.168.2.2', + '--header', + 'X-Forwarded-For: 2001:db8:85a3:8d3:1319:8a2e:370:7348, 207.168.2.2', + '--header', + 'X-Forwarded-For: 2003:db9:85a3:8d5:1318:8a2e:370:6322, 207.168.2.1']) + self.assertIn('HTTP/2 200', out) + self.assertIn('content-type: text/html', out) + + # Expect a TCP flow/counter-flow and one HTTP/2 flow + expected = { + "QDR": [ + ('BIFLOW_APP', {'PROTOCOL': 'HTTP/2', + 'METHOD': 'HEAD', + 'RESULT': '200', + 'SOURCE_HOST': '192.168.1.7', + 'STREAM_ID': ANY_VALUE, + 'END_TIME': ANY_VALUE}) + ] + } + success = retry(lambda: snooper_thread.match_records(expected)) + self.assertTrue(success, f"Failed to match records {snooper_thread.get_results()}") + + def test_get_image_jpg(self): # Run curl 127.0.0.1:port --output images/test.jpg --http2-prior-knowledge snooper_thread = VFlowSnooperThread(self.router_qdra.addresses[0],