Handling cookies in HTTP parser


(Eloy Brodin) #1

There are several errors parsing Cookies in HTTP protocol.

  1. If split_cookies is enabled, all the other request headers are lost.
  2. If a response has two or more cookies, the json is not correctly composed.

This could be an example with 5 cookies:

HTTP/1.0 200 OK
Date: Mon, 28 Nov 2016 08:49:56 GMT
Server: Apache/1.3.34 (Debian) PHP/4.4.4-8+etch6
X-Powered-By: PHP/4.4.4-8+etch6
Set-Cookie: name1=justname
Set-Cookie: name2=withexpire; expires=Wed, 28 Dec 2016 08:49:57 GMT
Set-Cookie: name3=withexpirepath; expires=Wed, 28 Dec 2016 08:49:57 GMT; path=/test/index.php
Set-Cookie: name4=withexpirepathdomain; expires=Wed, 28 Dec 2016 08:49:57 GMT; path=/test/index.php; domain=ZZZZZ
Set-Cookie: name5=withexpiredomainsecure; expires=Wed, 28 Dec 2016 08:49:57 GMT; path=/test; domain=ZZZZZ; secure
Content-Type: text/html; charset=iso-8859-1
X-Cache: MISS from XXXXX
X-Cache-Lookup: MISS from XXXX:YYYY
Via: 1.0 XXXX (squid)
Connection: close

With this response, cookies are overwritten (Notice that cookie's name is name5 not naname5 and the other 4 cookies are missing):

"response" : {
			"code" : 200,
			"headers" : {
				"connection" : "Keep-Alive",
				"content-length" : 21,
				"content-type" : "text/html; charset=iso-8859-1",
				"date" : "Mon, 28 Nov 2016 09:17:32 GMT",
				"keep-alive" : "timeout=15, max=100",
				"server" : "Apache/1.3.34 (Debian) PHP/4.4.4-8+etch6",
				"set-cookie" : "naname5=withexpiredomainsecure; expires=Wed, 28 Dec 2016 09:17:32 GMT; path=/test; domain=ZZZZZ; secure\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000, \u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000, \u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000",
				"transfer-encoding" : "chunked",
				"x-powered-by" : "PHP/4.4.4-8+etch6"
			},
			"phrase" : "OK"
		}

This is corrected adding a "+" in http_parser.go
off = copy(composed[off:], []byte(", "))
would be
off += copy(composed[off:], []byte(", "))

With that change we obtain:

"headers" : {
				"connection" : "Keep-Alive",
				"content-length" : 21,
				"content-type" : "text/html; charset=iso-8859-1",
				"date" : "Mon, 28 Nov 2016 09:24:03 GMT",
				"keep-alive" : "timeout=15, max=100",
				"server" : "Apache/1.3.34 (Debian) PHP/4.4.4-8+etch6",
				"set-cookie" : "name1=justname, name2=withexpire; expires=Wed, 28 Dec 2016 09:24:03 GMT, name3=withexpirepath; expires=Wed, 28 Dec 2016 09:24:03 GMT; path=/test/index.php, name4=withexpirepathdomain; expires=Wed, 28 Dec 2016 09:24:03 GMT; path=/test/index.php; domain=ZZZZZ, name5=withexpirepathdomainsecure; expires=Wed, 28 Dec 2016 09:24:03 GMT; path=/test; domain=ZZZZZ; secure",
				"transfer-encoding" : "chunked",
				"x-powered-by" : "PHP/4.4.4-8+etch6"
			},

Packetbeat windows - all_headers not working?
(Eloy Brodin) #2

After that last change, enabling split_cookies option this is the result:

"headers" : {
				"connection" : "Keep-Alive",
				"content-length" : 21,
				"content-type" : "text/html; charset=iso-8859-1",
				"date" : "Mon, 28 Nov 2016 09:53:50 GMT",
				"keep-alive" : "timeout=15, max=100",
				"server" : "Apache/1.3.34 (Debian) PHP/4.4.4-8+etch6",
				"set-cookie" : {
					"domain" : "ZZZZZ",
					"expires" : "Wed, 28 Dec 2016 09:53:50 GMT",
					"name1" : "justname, name2=withexpire",
					"path" : "/test"
				},
				"transfer-encoding" : "chunked",
				"x-powered-by" : "PHP/4.4.4-8+etch6"
			},

The composed field separator is a comma but in split_cookies function, it is used a semicolon.
In other words, field "cookie" is composed with semicolon separator and "set-cookie" with comma, but split_cookies work with semicolons.
If we set semicolon also for set-cookie field, the cookie's inner fields break the split, so it seems the splitcookie function should be different for cookie and set-cookie.


(ruflin) #3

Related PR: https://github.com/elastic/beats/pull/3065


(system) #4

This topic was automatically closed 28 days after the last reply. New replies are no longer allowed.