You are viewing a plain text version of this content. The canonical link for it is here.
Posted to bugs@httpd.apache.org by bu...@apache.org on 2019/05/15 08:42:19 UTC

[Bug 63434] New: Multiple Cookie headers combined to one header line

https://bz.apache.org/bugzilla/show_bug.cgi?id=63434

            Bug ID: 63434
           Summary: Multiple Cookie headers combined to one header line
           Product: Apache httpd-2
           Version: 2.4.39
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Core
          Assignee: bugs@httpd.apache.org
          Reporter: a.abfalterer@gmail.com
  Target Milestone: ---

RFC 6265, 5.4. The Cookie Header says

"When the user agent generates an HTTP request, the user agent MUST NOT attach
more than one Cookie header field."

However, httpd combines multiple Cookie headers into on header line; e.g.

Cookie: foo1=bar1
Cookie: foo2=bar2
Cookie: foo3=bar3

becomes "Cookie: foo1=bar1, foo2=bar2, foo3=bar3" (which in turns violates
syntax definition in RFC 6265, 4.2.1. Syntax).

The call of apr_table_compress() in in
server/protocol.c:ap_get_mime_headers_core() leads to this misbehaviour

https://github.com/apache/httpd/blob/trunk/server/protocol.c#L1274

Cheers, Armin

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 63434] Multiple Cookie headers combined to one comma-separated header line

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63434

--- Comment #6 from Armin Abfalterer <a....@gmail.com> ---
Created attachment 36601
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=36601&action=edit
patch file with initialization fix

(In reply to Armin Abfalterer from comment #5)
> Created attachment 36600 [details]
> patch that turns multiple Cookie headers into single header

there is a error in the patch, a wrong initialization

cookie_hdrs_state state = {r->pool, NULL, 0};

see new patch file

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 63434] Multiple Cookie headers combined to one comma-separated header line

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63434

--- Comment #11 from Michael Kaufmann <ap...@michael-kaufmann.ch> ---
If we patch apr_table_compress(), then these related APR functions would also
need a patch: apr_table_merge(), apr_table_mergen(), apr_table_overlap()

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 63434] Multiple Cookie headers combined to one comma-separated header line

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63434

Christophe JAILLET <ch...@wanadoo.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #36600|0                           |1
        is obsolete|                            |

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 63434] Multiple Cookie headers combined to one comma-separated header line

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63434

--- Comment #8 from Yann Ylavic <yl...@gmail.com> ---
(In reply to rgagnon24 from comment #7)
> 
> Apache is compressing them with ", " into one header.  This leads to an
> improper single Cookie value that some language like PHP (or others) will
> then not be able to properly parse (as commas are allowed characters within
> the value of a cookie),

From rfc6265 (which you refer to) it is defined as:
 cookie-octet      = %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E
                       ; US-ASCII characters excluding CTLs,
                       ; whitespace, DQUOTE, comma, semicolon,
                       ; and backslash

In older RFCs (because cookies have several RFCs and the latest is not
necessarily the one used), the value was defined as a token (or quoted-string):
       token          = 1*<any CHAR except CTLs or separators>
       separators     = "(" | ")" | "<" | ">" | "@"
                      | "," | ";" | ":" | "\" | <">
                      | "/" | "[" | "]" | "?" | "="
                      | "{" | "}" | SP | HT

In the original Netscape's "Persistent Client State -- HTTP Cookies" (actually
the one mostly used):
 NAME=VALUE
    This string is a sequence of characters excluding semi-colon, comma and
    white space. If there is a need to place such data in the name or value,
    some encoding method such as URL style %XX encoding is recommended,
    though no encoding is defined or required.

So no, commas and whitespaces are not allowed in a cookie value (unless
quoted). The are allowed in the Expires attribute value, with no quoting (!),
causing all sort of special case / workarounds for Set-Cookie headers (like
never compressing them).

> and because cookie parsers in most interpreters are
> going to "split" the key-value pairs out of the single Cookie: value by
> looking for a semi-colon to split on, and then creating the key-value pairs
> from those.

These parsers need to be fixed IMHO, they should look for the comma too, a
compressed Cookie header is a single Cookie header (though semantically
equivalent in HTTP to as much headers as commas).
Yes they should not happen because, as you said, rfc6265 forbids multiple
Cookie headers in the first place, but by the way I'm afraid there is no such
thing as a cookie RFC which is honored by everyone...

> The compression routine in apache is technically not at fault (sort of)
> because it is combining multiple headers as other RFCs allow, but the issue
> it would be nice if (for the Cookie header) that httpd could play nice with
> a bad user-agent, and fix the multiple "Cookie" header mistake the UA makes,
> so that the parser behind httpd can properly see the values.

httpd enforces the HTTP protocol, not rfc6265 or any cookie specification which
is an application thing (i.e. httpd has nothing to do with cookies besides
validating that they are correct HTTP headers). If modules/applications need to
make use of the cookies they should extract them from the HTTP header, and
parse them according to HTTP (compressed-)headers semantics, and eventually
reject them if a comma is found (meaning multiple headers were received).

> When PHP now looks to find it's session data, it can't find it because the
> value for "PHPSESSID" now appears as "XXXXXXXXXXXX,
> BIGipServer~XXXXX~XXXX=XXXXX"
> 
> Worse yet, if the bad user-agent sends the headers in a different order, PHP
> won't even find a value at all for "PHPSESSID" because there will be no
> Cookie that starts like that... for example:
> 
> Cookie: BIGipServer~XXXXX~XXXX=XXXXX
> Cookie: PHPSESSID=XXXXXXXXXXXX
> 
> Would become:
> 
> Cookie: BIGipServer~XXXXX~XXXX=XXXXX, PHPSESSID=XXXXXXXXXXXX
> 
> And the only thing then present in $_COOKIE would be the key
> "BIGipServer~XXXXX~XXXX" with the value "XXXXX, PHPSESSID=XXXXXXXXXXXX"

How about fixing (mod_)PHP(-fpm)? Either to reject the request (enforcing
rfc6265, which wouldn't help in your case) or to deal with the comma correctly.
PHP cares about cookies since it provides $_COOKIE, httpd does not.

Please note that addressing this in httpd would require a quite painful
treatment for every request: first set aside Cookie headers, compress the other
headers with comma, compress the Cookie headers with semicolon and re-insert.
This all would be before knowing whether the Cookies are needed/used later or
not, so at the very least it would need to be optional.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 63434] Multiple Cookie headers combined to one comma-separated header line

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63434

--- Comment #7 from rgagnon24@gmail.com ---
I can confirm I've seen this behavior as well.  The problem stems from bad
clients under HTTP/1.1.

HTTP/2 allows for multiple "Cookie" headers, but the problem shows really when
a HTTP/1.1 user-agent incorrectly sends multiple Cookie headers.

Apache is compressing them with ", " into one header.  This leads to an
improper single Cookie value that some language like PHP (or others) will then
not be able to properly parse (as commas are allowed characters within the
value of a cookie), and because cookie parsers in most interpreters are going
to "split" the key-value pairs out of the single Cookie: value by looking for a
semi-colon to split on, and then creating the key-value pairs from those.

So RFC6265 (https://tools.ietf.org/html/rfc6265#page-7) shows how servers can
send multiple "Set-Cookie" headers (to support proxies like load balancers, etc
injecting their own cookies), but as the OP stated, 5.4 states the user agent
MUST NOT attach more than one Cookie header field" (which is correct for
HTTP/1.1)

The compression routine in apache is technically not at fault (sort of) because
it is combining multiple headers as other RFCs allow, but the issue it would be
nice if (for the Cookie header) that httpd could play nice with a bad
user-agent, and fix the multiple "Cookie" header mistake the UA makes, so that
the parser behind httpd can properly see the values.

Scenario 1: Similar example to the OP, imagine a bad UA sends back:

Cookie: foo1=hello, world
Cookie: foo2=How are you, sam?

Current httpd would compress this into:

Cookie: foo1=hello, world, foo2=How are you, sam?

Then some processor looking at the COOKIE value, would only see a single cookie
with the key "foo1" and a value of "hello, world, foo2=How are you, sam?" 

If the processor were to try and split on the ", " that the
apr_table_compress() inserted, it would not properly be able to parse the
string because the parts would be:

foo1=hello
world
foo2=How are you
sam?

However, if the "Cookie" header were given special-case treatment (as would be
needed under an HTTP/2 transaction anyhow, then the routine would create:

Cookie: foo1=hello, world;foo2=How are you, sam?

Which, current processors will properly split with semicolon delimiting that
they expect in the Cookie value.

In the case of PHP, the current problem with the apr_table_compress() in regard
to Cookie is that the resulting $_COOKIE array in that language will miss
seeing Cookies that should be present, which can lead to broken SESSION's as
the session-id used is stored in the Cookie header---like any other language.

We've seen this with incompatible UA's communicating with server banks behind
load balancers especially.

Scenario 2: UA connects to a server through a proxy like a load balancer that
adds its own cookie for state tracking, and a server that adds the cookie value
"PHPSESSID" for session tracking.

When the server initiates the response back to the UA, it will add something
like:

Set-Cookie: PHPSESSID=XXXXXXXXXXXX; path=/

On the way through the proxy, the proxy (like and F5 load balancer) adds its
own cookie:

Set-Cookie: BIGipServer~XXXXX~XXXX=XXXXX; expires=Some date; path=/; Httponly

Now, the poor broken HTTP/1.1 User-agent (which is supposed to combine those 2
headers into one, but does not), sends back:

Cookie: PHPSESSID=XXXXXXXXXXXX
Cookie: BIGipServer~XXXXX~XXXX=XXXXX

apr_table_compress() now proceeds to combine those into one header before PHP
gets it:

Cookie: PHPSESSID=XXXXXXXXXXXX, BIGipServer~XXXXX~XXXX=XXXXX

When PHP now looks to find it's session data, it can't find it because the
value for "PHPSESSID" now appears as "XXXXXXXXXXXX,
BIGipServer~XXXXX~XXXX=XXXXX"

Worse yet, if the bad user-agent sends the headers in a different order, PHP
won't even find a value at all for "PHPSESSID" because there will be no Cookie
that starts like that... for example:

Cookie: BIGipServer~XXXXX~XXXX=XXXXX
Cookie: PHPSESSID=XXXXXXXXXXXX

Would become:

Cookie: BIGipServer~XXXXX~XXXX=XXXXX, PHPSESSID=XXXXXXXXXXXX

And the only thing then present in $_COOKIE would be the key
"BIGipServer~XXXXX~XXXX" with the value "XXXXX, PHPSESSID=XXXXXXXXXXXX"

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 63434] Multiple Cookie headers combined to one comma-separated header line

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63434

Armin Abfalterer <a....@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Multiple Cookie headers     |Multiple Cookie headers
                   |combined to one header line |combined to one
                   |                            |comma-separated header line

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 63434] Multiple Cookie headers combined to one comma-separated header line

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63434

--- Comment #1 from Yann Ylavic <yl...@gmail.com> ---
(In reply to Armin Abfalterer from comment #0)
> RFC 6265, 5.4. The Cookie Header says
> 
> "When the user agent generates an HTTP request, the user agent MUST NOT
> attach more than one Cookie header field."

This says "there must not be multiple Cookie headers" (thus "multiple cookies
must be in the same header"), which you translate as "there must not be
multiple cookies in the same header"?

It seems to me that apr_table_compress() does the right thing, including for
Cookie headers..

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 63434] Multiple Cookie headers combined to one comma-separated header line

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63434

--- Comment #3 from Yann Ylavic <yl...@gmail.com> ---
So, since comma in a header is equivalent to multiple headers, do you propose
that httpd rejects (with status 4xx) any request with either multiple Cookie
header or a single one containing comma(s)?

Because turning multiple Cookie headers into a single one with semicolon(s) is
not the same HTTP request (while the comma preserves semantics), the only
possible action would be to reject.

Also, it seems to me that Cookie is an application thingy, not an HTTP one, so
why would httpd reject it if the HTTP header is valid?
With comma separated cookies, the application can detect and reject, not if
httpd changes the semantics..

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 63434] Multiple Cookie headers combined to one comma-separated header line

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63434

--- Comment #5 from Armin Abfalterer <a....@gmail.com> ---
Created attachment 36600
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=36600&action=edit
patch that turns multiple Cookie headers into single header

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 63434] Multiple Cookie headers combined to one comma-separated header line

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63434

Christophe JAILLET <ch...@wanadoo.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |PatchAvailable

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 63434] Multiple Cookie headers combined to one comma-separated header line

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63434

--- Comment #10 from Michael Kaufmann <ap...@michael-kaufmann.ch> ---
Created attachment 37185
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=37185&action=edit
Merge cookie headers with semicolons in apr_table_compress()

I have attached a patch that modifies apr_table_compress() to merge "Cookie"
headers with semicolons instead of commas.

Because that's a function of APR, things are a bit complicated. Maybe the new
behavior can be enabled with a flag, or maybe a copy of the
apr_table_compress() function could be created in httpd, with the bugfix.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 63434] Multiple Cookie headers combined to one comma-separated header line

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63434

--- Comment #4 from Armin Abfalterer <a....@gmail.com> ---
(In reply to Yann Ylavic from comment #3)
> So, since comma in a header is equivalent to multiple headers, do you
> propose that httpd rejects (with status 4xx) any request with either
> multiple Cookie header or a single one containing comma(s)?
> 
> Because turning multiple Cookie headers into a single one with semicolon(s)
> is not the same HTTP request (while the comma preserves semantics), the only
> possible action would be to reject.

I'd propose either to reject a request with multiple Cookie headers or to turn
multiple Cookie headers into one where each cookie-pair is separated by
semicolon.

In any case I'd propose to reject a request with comma separated cookie-pairs
in a Cookie header.

> Also, it seems to me that Cookie is an application thingy, not an HTTP one,
> so why would httpd reject it if the HTTP header is valid?
> With comma separated cookies, the application can detect and reject, not if
> httpd changes the semantics..

In my opinion separated cookie pairs are a HTTP protocol violation so httpd
should not allow this at all; e.g. such request should not hit backend servers
when mod_proxy is in use

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 63434] Multiple Cookie headers combined to one comma-separated header line

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63434

--- Comment #2 from Armin Abfalterer <a....@gmail.com> ---
(In reply to Yann Ylavic from comment #1)

> This says "there must not be multiple Cookie headers" (thus "multiple
> cookies must be in the same header"), which you translate as "there must not
> be multiple cookies in the same header"?

No, multiple cookies are fine in the same header... but separated by semicolon,
not comma.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org


[Bug 63434] Multiple Cookie headers combined to one comma-separated header line

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=63434

Michael Kaufmann <ap...@michael-kaufmann.ch> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |apache-bugzilla@michael-kau
                   |                            |fmann.ch

--- Comment #9 from Michael Kaufmann <ap...@michael-kaufmann.ch> ---
The current release of Apache Tomcat rejects "Cookie" headers that contain a
comma. But it accepts multiple "Cookie" headers.

Tomcat understands this request:
Cookie: a=b; c=d
Cookie: e=f; g=h

Apache httpd may be used as a reverse proxy, and Tomcat ignores this merged
header:
Cookie: a=b; c=d, e=f; g=h

Tomcat understands this header, merged with "; " instead of ", ":
Cookie: a=b; c=d; e=f; g=h

Note that the major browsers don't cut cookies at commas. For example. browsers
parse this as a single cookie "a" with the value "b, c=d":
Set-Cookie: a=b, c=d

And browsers will send this back to the server like this:
Cookie: a=b, c=d

So in reality, the "Cookie" header should not be split at ",". I think that
merging multiple "Cookie" headers with "; " would be correct.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org