Skip to content

Commit

Permalink
Add support for space in get request (#5495)
Browse files Browse the repository at this point in the history
This fix an issue when the http request contains a space instead of
breaking the line with `bytes.fields` we are finding the start and the end
of the URI using the `METHOD` verb and the `HTTP/{VERSION}`. This will
allow packet beat to record theses request instead of ignoring them.

Fixes: #4974
  • Loading branch information
ph authored and Steffen Siering committed Nov 9, 2017
1 parent 67e05fc commit 16dffd3
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ https://github.com/elastic/beats/compare/v6.0.0-beta2...master[Check the HEAD di
- Fix http status phrase parsing not allow spaces. {pull}5312[5312]
- Fix missing length check in the PostgreSQL module. {pull}5457[5457]
- Fix panic in ACK handler if event is dropped on blocked queue {issue}5524[5524]
- Fix http parse to allow to parse get request with space in the URI. {pull}5495[5495]

*Winlogbeat*

Expand Down
23 changes: 14 additions & 9 deletions packetbeat/protos/http/http_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"time"
"unicode"

"github.com/elastic/beats/libbeat/common"
"github.com/elastic/beats/libbeat/common/streambuf"
Expand Down Expand Up @@ -74,8 +75,9 @@ var (

constCRLF = []byte("\r\n")

constClose = []byte("close")
constKeepAlive = []byte("keep-alive")
constClose = []byte("close")
constKeepAlive = []byte("keep-alive")
constHTTPVersion = []byte("HTTP/")

nameContentLength = []byte("content-length")
nameContentType = []byte("content-type")
Expand Down Expand Up @@ -145,7 +147,7 @@ func (*parser) parseHTTPLine(s *stream, m *message) (cont, ok, complete bool) {
}
return false, false, false
}
if bytes.Equal(fline[0:5], []byte("HTTP/")) {
if bytes.Equal(fline[0:5], constHTTPVersion) {
//RESPONSE
m.isRequest = false
version = fline[5:8]
Expand All @@ -160,20 +162,23 @@ func (*parser) parseHTTPLine(s *stream, m *message) (cont, ok, complete bool) {
}
} else {
// REQUEST
slices := bytes.Fields(fline)
if len(slices) != 3 {
afterMethodIdx := bytes.IndexFunc(fline, unicode.IsSpace)
afterRequestURIIdx := bytes.LastIndexFunc(fline, unicode.IsSpace)

// Make sure we have the VERB + URI + HTTP_VERSION
if afterMethodIdx == -1 || afterRequestURIIdx == -1 || afterMethodIdx == afterRequestURIIdx {
if isDebug {
debugf("Couldn't understand HTTP request: %s", fline)
}
return false, false, false
}

m.method = common.NetString(slices[0])
m.requestURI = common.NetString(slices[1])
m.method = common.NetString(fline[:afterMethodIdx])
m.requestURI = common.NetString(fline[afterMethodIdx+1 : afterRequestURIIdx])

if bytes.Equal(slices[2][:5], []byte("HTTP/")) {
if bytes.Equal(fline[afterRequestURIIdx+1:afterRequestURIIdx+len(constHTTPVersion)+1], constHTTPVersion) {
m.isRequest = true
version = slices[2][5:]
version = fline[afterRequestURIIdx+len(constHTTPVersion)+1:]
} else {
if isDebug {
debugf("Couldn't understand HTTP version: %s", fline)
Expand Down
39 changes: 39 additions & 0 deletions packetbeat/protos/http/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,45 @@ func TestEatBodyChunkedWaitCRLF(t *testing.T) {
}
}

func TestHttpParser_requestURIWithSpace(t *testing.T) {
if testing.Verbose() {
logp.LogInit(logp.LOG_DEBUG, "", false, true, []string{"http", "httpdetailed"})
}

http := httpModForTests(nil)
http.hideKeywords = []string{"password", "pass"}
http.parserConfig.sendHeaders = true
http.parserConfig.sendAllHeaders = true

// Non URL-encoded string, RFC says it should be encoded
data1 := "GET http://localhost:8080/test?password=two secret HTTP/1.1\r\n" +
"Host: www.google.com\r\n" +
"Connection: keep-alive\r\n" +
"User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_4) AppleWebKit/537.1 (KHTML, like Gecko) Chrome/21.0.1180.75 Safari/537.1\r\n" +
"Accept: */*\r\n" +
"X-Chrome-Variations: CLa1yQEIj7bJAQiftskBCKS2yQEIp7bJAQiptskBCLSDygE=\r\n" +
"Referer: http://www.google.com/\r\n" +
"Accept-Encoding: gzip,deflate,sdch\r\n" +
"Accept-Language: en-US,en;q=0.8\r\n" +
"Content-Type: application/x-www-form-urlencoded\r\n" +
"Content-Length: 23\r\n" +
"Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.3\r\n" +
"Cookie: PREF=ID=6b67d166417efec4:U=69097d4080ae0e15:FF=0:TM=1340891937:LM=1340891938:S=8t97UBiUwKbESvVX; NID=61=sf10OV-t02wu5PXrc09AhGagFrhSAB2C_98ZaI53-uH4jGiVG_yz9WmE3vjEBcmJyWUogB1ZF5puyDIIiB-UIdLd4OEgPR3x1LHNyuGmEDaNbQ_XaxWQqqQ59mX1qgLQ\r\n" +
"\r\n" +
"username=ME&pass=twosecret"
tp := newTestParser(http, data1)

msg, ok, complete := tp.parse()
assert.True(t, ok)
assert.True(t, complete)
rawMsg := tp.stream.data[tp.stream.message.start:tp.stream.message.end]
path, params, err := http.extractParameters(msg, rawMsg)
assert.Nil(t, err)
assert.Equal(t, "/test", path)
assert.Equal(t, string(msg.requestURI), "http://localhost:8080/test?password=two secret")
assert.False(t, strings.Contains(params, "two secret"))
}

func TestHttpParser_censorPasswordURL(t *testing.T) {
if testing.Verbose() {
logp.LogInit(logp.LOG_DEBUG, "", false, true, []string{"http", "httpdetailed"})
Expand Down

0 comments on commit 16dffd3

Please sign in to comment.