Bug 11475 - usertrack can read Cookie2 header but spec says it doesn't contain cookies
Summary: usertrack can read Cookie2 header but spec says it doesn't contain cookies
Status: CLOSED FIXED
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: mod_usertrack (show other bugs)
Version: 2.0.39
Hardware: All All
: P3 minor (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL: ftp://ftp.isi.edu/in-notes/rfc2965.txt
Keywords:
Depends on:
Blocks:
 
Reported: 2002-08-05 16:52 UTC by Chris Darroch
Modified: 2004-11-16 19:05 UTC (History)
0 users



Attachments
Netscape and RFC 2695 compliant cookie parser (9.98 KB, text/plain)
2002-08-07 16:01 UTC, Chris Darroch
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Darroch 2002-08-05 16:52:45 UTC
If the CookieStyle configuration directive is set to Cookie2 or RFC2965, then
mod_usertrack sets dcfg->style = CT_COOKIE2.  In turn, the spot_cookie()
function will then parse the Cookie2: request header, looking for the Apache
cookie:

    cookie = apr_table_get(r->headers_in,
        (dcfg->style == CT_COOKIE2 ? "Cookie2" : "Cookie"))

However, reading the RFC 2965 specification, specifically section 3.3.5,
it appears to me that the Cookie2: header is only used to indicate the
highest version of the cookie specification that the client understands.
Per 3.3.4, the actual cookie values are still sent in the Cookie: header.
(See also 9.1 and the examples under 4.1 and 4.2.)

As a further note, it seems to me -- I could be reading the spec or code
incorrectly, of course -- that the cookie parsing code in spot_cookie()
may not really work with RFCs 2109 or 2965, because it doesn't accept
commas as cookie delimiters, nor the whitespace or double-quote (")
quoted-strings allowed by those RFCs.  See 10.1.3 in RFC 2109, as well
as 4.1 and 4.3.4 in RFC 2109, and 3.1 and 3.3.4 in RFC 2965.
My apologies if I've misread something!
Comment 1 Chris Darroch 2002-08-07 15:59:22 UTC
Looking at the spot_cookie() code a bit more, I'm also suspicious that it may
be confused by both RFC 2965-style cookies (which can have quoted-string
values, with escaped characters).

Further, I think it will also be confused just by old-style cookies where
the string "Apache" (or whatever the cookie name it's looking for is) appears
in a cookie name or value somewhere before the actual cookie name/value pair
in the header.  For example, I suspect "Apache2=foo; Apache=bar" or
"foo=Apache; Apache=bar" would both cause the apr_strstr_c() call to find
the first, incorrect, occurance of "Apache".  Or, if the client has two valid
"Apache" cookies for the server, with different paths, it may send them both,
with the more-specific path first ... but since we always set the path=/, our
cookie will be the last one, not the first one.

if ((value = ap_strstr_c(cookie, dcfg->cookie_name))) {
    char *cookiebuf, *cookieend;

    value += strlen(dcfg->cookie_name) + 1;  /* Skip over the '=' */
    cookiebuf = apr_pstrdup(r->pool, value);
    cookieend = strchr(cookiebuf, ';');
    if (cookieend)
        *cookieend = '\0';      /* Ignore anything after a ; */

I don't know if this helps or not, but since I don't have time right now to
implement a complete fix, I will attach a file that parses a Cookie: header
into an APR hash of cookies, where each hash value is an APR array of
"string" structures.  The first structure in an array is the first cookie
we found with the given name (the name is the hash key that points to the
array), which should be the most-specific cookie of that name, assuming the
client is working correctly.  The last structure in an array is the last
cookie -- which is the one mod_usertrack would want, because it would have
to be the path=/ cookie.

The "string" structure contains both the string itself, null-terminated, and
the string length, which is useful for avoiding additional strlen() calls.
The cookie_get() function returns the string of the first cookie, or NULL
if there is more than one cookie with the same name.  This obviously isn't
useful for the mod_usertrack situation, where we want the last cookie in
an array.  For that, you want to do something like (assuming you know
there's at each one element in the array):

/**** DEBUG: watch out for nelts == 0 !! ****/
string = ((struct string*) val_arr->elts) + val_arr->nelts - 1;
str = string->ptr;
str_len = string->str_len;

When parsing the Cookie: header, if a $Version=1 cookie is detected
at the start (or some legal RFC 2965 variation), then the code parses
according to RFCs 2965 and 2616 (mostly); otherwise, it parses according
to the old Netscape specification.

When parsing in RFC 2965-mode, cookie names are flattened to lowercase,
since the spec calls for case-insensitive cookie names.  Quoted strings
are de-quoted and escaped characters in quoted strings are un-escaped
(except for some dubious values that RFC 2616 allows).  Unquoted cookie
values must be RFC 2616 tokens, and all cookie names must be tokens as well.
Both commas and semicolons are legal delimiters.

When parsing in Netscape-mode, cookie names and values are not altered,
except that we ignore internal whitespace and commas, because the spec
doesn't allow those at all.  Only semicolons are legal delimiters.

In general, high-bit-set octets and ASCII control characters are
stripped out, despite what RFC 2616 allows, because -- well, because
applications really shouldn't be using such stuff in an HTTP header,
should they?

The code also imposes its own #defined limits on name and value lengths.
This probably overkill given that the header is normally limited to
about 8 Kb by the DEFAULT_LIMIT_REQUEST_FIELDSIZE #define, but this
code came from another application where we didn't have such external
limits.

Plus, it's worth noting that both the Netscape and RFC 2965
specs allow clients to send 20 cookies where each one's name/value
pair is 4 Kb ... but any application that actually relied on that many
large cookies would cause Apache errors for its clients, once they
exceeded the 8 Kb header limit.  Maybe someday Cookie: headers should
be allowed to exceed the DEFAULT_LIMIT_REQUEST_FIELDSIZE?  Or else,
at least a warning about this conflict with the specs should maybe go
in the docs.

The code tries hard to avoid excess strlen()-type calls and multiple
passes over the data.  Although it would be more elegant to allocate
key[] and val[] buffers off the stack for the maximum, dump characters
into them as we find them, and then apr_palloc() just enough space
for the resultant strings, that requires at least two passes over
all the data.

Instead, this code starts by allocating a small buf_size buffer
from apr_palloc(), and then, once it's filled up, allocating double
that amount of space for the next buffer.  The doubling continues
until a reasonable maximum is reached that can always contain the
largest string we need to handle (ideally, several large strings).
This does involve memcpy() calls when we have to reallocate in the
middle of a name or value, and some wasted space, but we should
avoid a full 2* pass over all the data, and not waste too much more
space than we need.  Like I said, it's a bit of overkill just for
the Cookie: header, but I had the code on hand.

The attached code should compile, but it differs slightly from
our actual implementation usage, so I can't guarantee that it's
bug-free.

Perhaps something like this might form part of an APR-util cookie
library?  Or not ...
Comment 2 Chris Darroch 2002-08-07 16:01:06 UTC
Created attachment 2624 [details]
Netscape and RFC 2695 compliant cookie parser
Comment 3 Chris Darroch 2002-08-07 16:07:50 UTC
Oops, two extra comments: the last line of the cookie_parse_header() function
should "return APR_SUCCESS", not "return OK", and this code simply skips over
any cookies whose names start with $ when parsing in RFC 2965-mode.
An enhancement would be to put those $name values into a fancier "struct cookie"
that contains the "struct string", and push those structures onto the APR
arrays.
Comment 4 Jeff Trawick 2003-11-21 17:06:41 UTC
regarding the spotcookie problem misrecogonizing cookies: that was fixed
recently...  no comment from me on your rfc compliance comment
Comment 5 André Malo 2004-01-13 00:34:31 UTC
RFC issue Fixed in 2.1 and proposed for backport into the 2.0 and 1.3 stable
branches.

Thanks for the report and thanks for using Apache.