From 559667a342a365fdfd942679b5d1542ad2ea512c Mon Sep 17 00:00:00 2001 From: Simon Dugas Date: Tue, 12 Dec 2023 09:11:57 -0500 Subject: [PATCH 1/2] response: add http tunneling test case This test case shows unexpected state transitions when processing http tunnels and the emission of body data. --- test/files/101-connect-tunnel.t | 20 ++++++++++++++++++++ test/test_main.cpp | 29 +++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 test/files/101-connect-tunnel.t diff --git a/test/files/101-connect-tunnel.t b/test/files/101-connect-tunnel.t new file mode 100644 index 00000000..de6f3d0e --- /dev/null +++ b/test/files/101-connect-tunnel.t @@ -0,0 +1,20 @@ +>>> +CONNECT www.ssllabs.com:443 HTTP/1.0 +Host: www.ssllabs.com:443 + +abcdef + +<<< +HTTP/1.1 200 OK +Server: Apache/2.2 + +ABCDEF +ABCDEF + +>>> +abcdef +abcdef + +<<< +ABCDEF +ABCDEF diff --git a/test/test_main.cpp b/test/test_main.cpp index 73b86868..3bc6b7f7 100644 --- a/test/test_main.cpp +++ b/test/test_main.cpp @@ -2145,4 +2145,33 @@ TEST_F(ConnectionParsing, ResponseBodyData) { delete user_data; } + +TEST_F(ConnectionParsing, ConnectTunnel) { + htp_config_register_response_body_data(cfg, callback_RESPONSE_BODY_DATA); + + int rc = test_run(home, "101-connect-tunnel.t", cfg, &connp); + ASSERT_GE(rc, 0); + + ASSERT_EQ(1, htp_list_size(connp->conn->transactions)); + + htp_tx_t *tx = (htp_tx_t *) htp_list_get(connp->conn->transactions, 0); + ASSERT_TRUE(tx != NULL); + + EXPECT_TRUE(htp_tx_is_complete(tx)); + + EXPECT_EQ(0, bstr_cmp_c(tx->request_method, "CONNECT")); + + EXPECT_EQ(200, tx->response_status_number); + EXPECT_EQ(0, tx->request_entity_len); + EXPECT_EQ(0, tx->request_message_len); + EXPECT_EQ(-1, tx->request_content_length); + EXPECT_EQ(0, tx->response_entity_len); + EXPECT_EQ(0, tx->response_message_len); + EXPECT_EQ(-1, tx->response_content_length); + + struct ResponseBodyDataCallback *user_data = (struct ResponseBodyDataCallback *) htp_tx_get_user_data(tx); + ASSERT_TRUE(user_data); + ASSERT_EQ(0, user_data->data.size()); +} + #endif From 4d33275b00154a160d60e3091124c9b5a98ddbc4 Mon Sep 17 00:00:00 2001 From: Simon Dugas Date: Tue, 12 Dec 2023 09:13:59 -0500 Subject: [PATCH 2/2] response: fix connect tunneling bug The response was emitting partial body data depending on how you fed the parser with inbound and outbound data chunks. It seems the intended behavior is to not emit body data if HTP_STREAM_TUNNEL will eventually be entered. The fix was to allow htp_connp_REQ_CONNECT_WAIT_RESPONSE to resume in order to enter the HTP_STREAM_TUNNEL state or complete the request. The tunneling transaction was also incomplete because the request side wasn't being finalized after entering HTP_STREAM_TUNNEL. See test case for example. --- htp/htp_request.c | 3 +++ htp/htp_response.c | 14 +++++++++++--- htp/htp_util.c | 1 + 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/htp/htp_request.c b/htp/htp_request.c index 9fddbd8f..76173958 100644 --- a/htp/htp_request.c +++ b/htp/htp_request.c @@ -364,6 +364,9 @@ htp_status_t htp_connp_REQ_CONNECT_PROBE_DATA(htp_connp_t *connp) { #endif connp->in_status = HTP_STREAM_TUNNEL; connp->out_status = HTP_STREAM_TUNNEL; + + // set the final state to eventually complete the transaction + connp->in_state = htp_connp_REQ_FINALIZE; } // not calling htp_connp_req_clear_buffer, we're not consuming the data diff --git a/htp/htp_response.c b/htp/htp_response.c index 121004c7..ab1621fa 100644 --- a/htp/htp_response.c +++ b/htp/htp_response.c @@ -1166,9 +1166,17 @@ htp_status_t htp_connp_RES_FINALIZE(htp_connp_t *connp) { } if (htp_treat_response_line_as_body(data, bytes_left)) { - // Interpret remaining bytes as body data - htp_log(connp, HTP_LOG_MARK, HTP_LOG_WARNING, 0, "Unexpected response body"); - htp_status_t rc = htp_tx_res_process_body_data_ex(connp->out_tx, data, bytes_left); + // Interpret remaining bytes as body data only if inbound processing + // was not suspended. Otherwise, yield back to inbound processing in + // case we've suspended because of a CONNECT transaction and are about + // to enter a tunneled state where we won't process body data. + htp_status_t rc = HTP_DATA_OTHER; + + if (connp->in_status != HTP_STREAM_DATA_OTHER) { + htp_log(connp, HTP_LOG_MARK, HTP_LOG_WARNING, 0, "Unexpected response body"); + rc = htp_tx_res_process_body_data_ex(connp->out_tx, data, bytes_left); + } + htp_connp_res_clear_buffer(connp); return rc; } diff --git a/htp/htp_util.c b/htp/htp_util.c index 936e22b0..1dfde7b2 100644 --- a/htp/htp_util.c +++ b/htp/htp_util.c @@ -2097,6 +2097,7 @@ char *htp_connp_in_state_as_string(htp_connp_t *connp) { if (connp->in_state == htp_connp_REQ_HEADERS) return "REQ_HEADERS"; if (connp->in_state == htp_connp_REQ_CONNECT_CHECK) return "REQ_CONNECT_CHECK"; if (connp->in_state == htp_connp_REQ_CONNECT_WAIT_RESPONSE) return "REQ_CONNECT_WAIT_RESPONSE"; + if (connp->in_state == htp_connp_REQ_CONNECT_PROBE_DATA) return "REQ_CONNECT_PROBE_DATA"; if (connp->in_state == htp_connp_REQ_BODY_DETERMINE) return "REQ_BODY_DETERMINE"; if (connp->in_state == htp_connp_REQ_BODY_IDENTITY) return "REQ_BODY_IDENTITY"; if (connp->in_state == htp_connp_REQ_BODY_CHUNKED_LENGTH) return "REQ_BODY_CHUNKED_LENGTH";