Skip to content

Commit

Permalink
Add support for space in get request
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: elastic#4974
  • Loading branch information
ph committed Nov 9, 2017
1 parent 67e05fc commit af9f703
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 af9f703

Please sign in to comment.