diff options
| author |   Matus Fabian <matfabia@cisco.com> | 2024-08-23 17:35:50 +0200 |
|---|---|---|
| committer |   Florin Coras <florin.coras@gmail.com> | 2024-08-23 17:59:46 +0000 |
| commit | 69123a3f670a41e31b0988583e342a7df028f136 (patch) | |
| tree | be00d3ffc3ba9769ea6d36d5c8ef2ec69a8fe9f1 | |
| parent | 4306a3e8f4f8016e7571be75b6418b77ee2f701d (diff) | |
http: status line parsing fix
Request line must only start with method name and server should
ignore at least one empty line (CRLF) received prior to the
request-line.
Type: fix
Change-Id: Ifebd992dc4c13df1a3fabfcdef9e7ee644150a21
Signed-off-by: Matus Fabian <matfabia@cisco.com>
| -rw-r--r-- | extras/hs-test/http_test.go | 27 | ||||
| -rw-r--r-- | src/plugins/http/http.c | 34 |
2 files changed, 50 insertions, 11 deletions
diff --git a/extras/hs-test/http_test.go b/extras/hs-test/http_test.go index 872f4c234b3..9fc426a88a7 100644 --- a/extras/hs-test/http_test.go +++ b/extras/hs-test/http_test.go @@ -29,7 +29,7 @@ func init() { HttpStaticMacTimeTest, HttpStaticBuildInUrlGetVersionVerboseTest, HttpVersionNotSupportedTest, HttpInvalidContentLengthTest, HttpInvalidTargetSyntaxTest, HttpStaticPathTraversalTest, HttpUriDecodeTest, HttpHeadersTest, HttpStaticFileHandlerTest, HttpStaticFileHandlerDefaultMaxAgeTest, HttpClientTest, HttpClientErrRespTest, HttpClientPostFormTest, - HttpClientPostFileTest, HttpClientPostFilePtrTest, AuthorityFormTargetTest) + HttpClientPostFileTest, HttpClientPostFilePtrTest, AuthorityFormTargetTest, HttpRequestLineTest) RegisterNoTopoSoloTests(HttpStaticPromTest, HttpTpsTest, HttpTpsInterruptModeTest, PromConcurrentConnectionsTest, PromMemLeakTest, HttpClientPostMemLeakTest, HttpInvalidClientRequestMemLeakTest) } @@ -867,7 +867,19 @@ func HttpInvalidRequestLineTest(s *NoTopoSuite) { serverAddress := s.GetInterfaceByName(TapInterfaceName).Peer.Ip4AddressString() vpp.Vppctl("http cli server") - resp, err := TcpSendReceive(serverAddress+":80", "GET / HTTP/1.1") + resp, err := TcpSendReceive(serverAddress+":80", " GET / HTTP/1.1") + s.AssertNil(err, fmt.Sprint(err)) + s.AssertContains(resp, "HTTP/1.1 400 Bad Request", "invalid request line start not allowed") + + resp, err = TcpSendReceive(serverAddress+":80", "\rGET / HTTP/1.1") + s.AssertNil(err, fmt.Sprint(err)) + s.AssertContains(resp, "HTTP/1.1 400 Bad Request", "invalid request line start not allowed") + + resp, err = TcpSendReceive(serverAddress+":80", "\nGET / HTTP/1.1") + s.AssertNil(err, fmt.Sprint(err)) + s.AssertContains(resp, "HTTP/1.1 400 Bad Request", "invalid request line start not allowed") + + resp, err = TcpSendReceive(serverAddress+":80", "GET / HTTP/1.1") s.AssertNil(err, fmt.Sprint(err)) s.AssertContains(resp, "HTTP/1.1 400 Bad Request", "invalid framing not allowed") @@ -896,6 +908,17 @@ func HttpInvalidRequestLineTest(s *NoTopoSuite) { s.AssertContains(resp, "HTTP/1.1 400 Bad Request", "'HTTP1.1' invalid http version not allowed") } +func HttpRequestLineTest(s *NoTopoSuite) { + vpp := s.GetContainerByName("vpp").VppInstance + serverAddress := s.GetInterfaceByName(TapInterfaceName).Peer.Ip4AddressString() + vpp.Vppctl("http cli server") + + resp, err := TcpSendReceive(serverAddress+":80", "\r\nGET /show/version HTTP/1.1\r\nHost:"+serverAddress+":80\r\nUser-Agent:test\r\n\r\n") + s.AssertNil(err, fmt.Sprint(err)) + s.AssertContains(resp, "HTTP/1.1 200 OK") + s.AssertContains(resp, "<html>", "html content not found") +} + func HttpInvalidTargetSyntaxTest(s *NoTopoSuite) { vpp := s.GetContainerByName("vpp").VppInstance serverAddress := s.GetInterfaceByName(TapInterfaceName).Peer.Ip4AddressString() diff --git a/src/plugins/http/http.c b/src/plugins/http/http.c index f4b330a19fc..b143893f494 100644 --- a/src/plugins/http/http.c +++ b/src/plugins/http/http.c @@ -587,10 +587,10 @@ static int http_parse_request_line (http_conn_t *hc, http_status_code_t *ec) { int i, target_len; - u32 next_line_offset; + u32 next_line_offset, method_offset; /* request-line = method SP request-target SP HTTP-version CRLF */ - i = v_find_index (hc->rx_buf, 0, 0, "\r\n"); + i = v_find_index (hc->rx_buf, 8, 0, "\r\n"); if (i < 0) { clib_warning ("request line incomplete"); @@ -609,24 +609,40 @@ http_parse_request_line (http_conn_t *hc, http_status_code_t *ec) return -1; } + /* + * RFC9112 2.2: + * In the interest of robustness, a server that is expecting to receive and + * parse a request-line SHOULD ignore at least one empty line (CRLF) + * received prior to the request-line. + */ + method_offset = hc->rx_buf[0] == '\r' && hc->rx_buf[1] == '\n' ? 2 : 0; /* parse method */ - if ((i = v_find_index (hc->rx_buf, 0, next_line_offset, "GET ")) >= 0) + if (!memcmp (hc->rx_buf + method_offset, "GET ", 4)) { HTTP_DBG (0, "GET method"); hc->method = HTTP_REQ_GET; - hc->target_path_offset = i + 4; + hc->target_path_offset = method_offset + 4; } - else if ((i = v_find_index (hc->rx_buf, 0, next_line_offset, "POST ")) >= 0) + else if (!memcmp (hc->rx_buf + method_offset, "POST ", 5)) { HTTP_DBG (0, "POST method"); hc->method = HTTP_REQ_POST; - hc->target_path_offset = i + 5; + hc->target_path_offset = method_offset + 5; } else { - clib_warning ("method not implemented: %8v", hc->rx_buf); - *ec = HTTP_STATUS_NOT_IMPLEMENTED; - return -1; + if (hc->rx_buf[method_offset] - 'A' <= 'Z' - hc->rx_buf[method_offset]) + { + clib_warning ("not method name: %8v", hc->rx_buf); + *ec = HTTP_STATUS_BAD_REQUEST; + return -1; + } + else + { + clib_warning ("method not implemented: %8v", hc->rx_buf); + *ec = HTTP_STATUS_NOT_IMPLEMENTED; + return -1; + } } /* find version */ |