Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #1694: Additional fix. Capture the first left-most x-forwarded-… #1704

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 34 additions & 14 deletions src/observers/http2/http2_observer.c
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[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;
}
Expand Down
11 changes: 6 additions & 5 deletions src/observers/private.h
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
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
Loading