From 3e5b068fedd01ecb2a747aba9a4c6b6926ff7405 Mon Sep 17 00:00:00 2001 From: Yannick Dylla <17772145+ydylla@users.noreply.github.com> Date: Mon, 1 Jul 2024 00:24:35 +0200 Subject: [PATCH] fix: http matcher only signaling false if it is sure http is impossible --- modules/l4http/httpmatcher.go | 15 ++++++++++----- modules/l4http/httpmatcher_test.go | 19 ++++++++++++++++++- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/modules/l4http/httpmatcher.go b/modules/l4http/httpmatcher.go index c80fa7b..b99f6e1 100644 --- a/modules/l4http/httpmatcher.go +++ b/modules/l4http/httpmatcher.go @@ -82,8 +82,9 @@ func (m MatchHTTP) Match(cx *layer4.Connection) (bool, error) { var err error data := cx.MatchingBytes() - if !m.isHttp(data) { - return false, nil + match, err := m.isHttp(data) + if !match { + return match, err } // use bufio reader which exactly matches the size of prefetched data, @@ -122,11 +123,15 @@ func (m MatchHTTP) Match(cx *layer4.Connection) (bool, error) { return m.matcherSets.AnyMatch(req), nil } -func (m MatchHTTP) isHttp(data []byte) bool { +func (m MatchHTTP) isHttp(data []byte) (bool, error) { // try to find the end of a http request line, for example " HTTP/1.1\r\n" i := bytes.IndexByte(data, 0x0a) // find first new line + if i == -1 { + // no line break found yet so we signal we need more data + return false, layer4.ErrConsumedAllPrefetchedBytes + } if i < 10 { - return false + return false, nil } // assume only \n line ending start := i - 9 // position of space in front of HTTP @@ -136,7 +141,7 @@ func (m MatchHTTP) isHttp(data []byte) bool { start -= 1 end -= 1 } - return bytes.Compare(data[start:end], []byte(" HTTP/")) == 0 + return bytes.Compare(data[start:end], []byte(" HTTP/")) == 0, nil } // Parses information from a http2 request with prior knowledge (RFC 7540 Section 3.4) diff --git a/modules/l4http/httpmatcher_test.go b/modules/l4http/httpmatcher_test.go index 19d1e0a..8c4efb5 100644 --- a/modules/l4http/httpmatcher_test.go +++ b/modules/l4http/httpmatcher_test.go @@ -252,48 +252,65 @@ func TestMatchHTTP_isHttp(t *testing.T) { name string data []byte shouldMatch bool + expectedErr error }{ { name: "http/1.1-only-lf", data: []byte("GET /foo/bar?aaa=bbb HTTP/1.1\nHost: localhost:10443\n\n"), shouldMatch: true, + expectedErr: nil, }, { name: "http/1.1-cr-lf", data: []byte("GET /foo/bar?aaa=bbb HTTP/1.1\r\nHost: localhost:10443\r\n\r\n"), shouldMatch: true, + expectedErr: nil, }, { name: "http/1.0-cr-lf", data: []byte("GET /foo/bar?aaa=bbb HTTP/1.0\r\nHost: localhost:10443\r\n\r\n"), shouldMatch: true, + expectedErr: nil, }, { name: "http/2.0-cr-lf", data: []byte("PRI * HTTP/2.0\r\n\r\n"), shouldMatch: true, + expectedErr: nil, }, { name: "dummy-short", data: []byte("dum\n"), shouldMatch: false, + expectedErr: nil, }, { name: "dummy-long", data: []byte("dummydummydummy\n"), shouldMatch: false, + expectedErr: nil, }, { name: "http/1.1-without-space-in-front", data: []byte("HTTP/1.1\n"), shouldMatch: false, + expectedErr: nil, + }, + { + name: "get-with-incomplete-request-line", + data: []byte("GET /with/incomplete/request-line"), + shouldMatch: false, + expectedErr: layer4.ErrConsumedAllPrefetchedBytes, }, } { t.Run(tc.name, func(t *testing.T) { - matched := MatchHTTP{}.isHttp(tc.data) + matched, err := MatchHTTP{}.isHttp(tc.data) if matched != tc.shouldMatch { t.Fatalf("test %v | matched: %v != shouldMatch: %v", tc.name, matched, tc.shouldMatch) } + if err != tc.expectedErr { + t.Fatalf("test %v | err: %v != expectedErr: %v", tc.name, err, tc.expectedErr) + } }) } }