Skip to content

Commit

Permalink
Fixes #1694: Additional fix. Capture the first left-most x-forwarded-… (
Browse files Browse the repository at this point in the history 
#1704)

* Fixes #1694: Additional fix. Capture the first left-most x-forwarded-for header value

* Fixed python-checker errors

* Code review change: Set the max len of the copy buffer to 128
  • Loading branch information
@ganeshmurthy
ganeshmurthy authored Dec 23, 2024
1 parent 8920e88 commit e94ec71
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 20 deletions.
48 changes: 34 additions & 14 deletions src/observers/http2/http2_observer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
//
Expand All @@ -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[128];
strncpy(value_copy, (const char *)value, 128);
// 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;
}
Expand Down
11 changes: 6 additions & 5 deletions src/observers/private.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

};

Expand Down
47 changes: 46 additions & 1 deletion tests/system_tests_http_observer.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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],
Expand Down

0 comments on commit e94ec71

Please sign in to comment.