Last Comment Bug 142803 - Port is being stored as part of cookie domain
: Port is being stored as part of cookie domain
Status: VERIFIED FIXED
:
Product: Core
Classification: Components
Component: Networking: Cookies (show other bugs)
: Trunk
: x86 Windows 2000
: P2 normal with 2 votes (vote)
: mozilla1.2beta
Assigned To: Stephen P. Morse
:
Mentors:
http://www.jlchicago.org/index.cfm?fa...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2002-05-07 09:17 PDT by Dennis Spaag
Modified: 2003-06-18 09:58 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
traffic showing cookie headers (8.71 KB, text/plain)
2002-07-09 12:23 PDT, Stephen P. Morse
no flags Details
leaving port number off when storing cookie domain (6.14 KB, patch)
2002-07-10 23:44 PDT, Stephen P. Morse
samir_bugzilla: review+
Details | Diff | Splinter Review
stop cookie leak, delete dead code (7.42 KB, patch)
2002-07-12 10:03 PDT, Stephen P. Morse
morse: review+
roc: superreview+
Details | Diff | Splinter Review
Simple JSP to test cookie behavior (320 bytes, text/plain)
2002-09-18 08:59 PDT, Dave Makower
morse: review+
morse: superreview+
Details
remove bit-rot from previous patch (6.19 KB, patch)
2002-09-18 16:19 PDT, Stephen P. Morse
no flags Details | Diff | Splinter Review

Description Dennis Spaag 2002-05-07 09:17:37 PDT
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0rc1)
Gecko/20020417
BuildID:    2002041711

SSL form checked by JavaScript, redirect appends :443 to domain name, resulting
in loss of session state.  Session state is cookie based.  Cookie not sent when
requested domain is www.jlchicago.ord:443 instead of www.jlchicago.org ... You
would have to begin the purchase process to see the error. You would not need to
actually buy anything though.  On this site you will need to enter a credit card
number to get to the point where the error occurs.  4111111111111111 will work
for that purpose.  


Reproducible: Always
Steps to Reproduce:
1. HTML Form in SSL that uses an onSubmit="return [true|false]" javascript
handler, action uses relative path.
2. Fill out form correctly, submit it.
3. page redirect includes :443 on end of domain.

Actual Results:  Loss of cookie-based session state.

Expected Results:  Should not append :443 to domain.
Comment 1 Boris Zbarsky [:bz] (still a bit busy) 2002-05-07 10:41:13 PDT
To cookies for starters.
Comment 2 Stephen P. Morse 2002-05-12 14:52:56 PDT
I tried this using both n4.x and mozilla.  In both cases, I submitted the form 
and was taken to the site

   
https://www.jlchicago.org:443/index.cfm/y/index.cfm/fa/mkp.home/cfid/716528/cfto
ken/68636654/y/index.htm

where only the digit fields (716528 and 68636654) differed between n4.x and 
mozilla.  But on both cases there was a :443 appended at the end.  So mozilla 
behaves exactly like 4.x

I then tried this with IE and in this case I got to the following site instead

https://www.jlchicago.org/index.cfm/y/index.cfm/fa/mkp.confirm/y/index.htm

Need to investigate further to determine whether the IE behavior is correct or 
the Netscape one is correct.  In any event, the site was designed to expect the 
IE behavior and only works correctly with IE but not with netscape (neither 4.x 
nor mozilla).

In any event, if this is a netscape but, it certainly is of low priority since 
it existed in n4 all this time and we lived with it.
Comment 3 Stephen P. Morse 2002-07-09 12:22:22 PDT
Attaching the relevant headers in the traffic that I captured when using the 
mozilla browser.  Note that the first 9 requests are do not involve any port, 
and the cookies are set and read.  Then request 10 through 50 are to port 443 
and do not involve any cookies.  Request 51 is also to port 443 and its response 
sets some cookies.
Comment 4 Stephen P. Morse 2002-07-09 12:23:53 PDT
Created attachment 90638 [details]
traffic showing cookie headers
Comment 5 Stephen P. Morse 2002-07-10 23:12:29 PDT
Problem is due to the fact that mozilla is including port number as part of the 
domain attribute of the cookie.  As far as I can tell, that is incorrect.  The 
RFC2109 spec states:

> The term request-host ... refers to the value the client would send to the
> server as ... the host (but not port) ... of the HTTP request line.

> 4.3.1 Interpreting Set-Cookie
> ...
> Domain Defaults to the request-host.
Comment 6 Stephen P. Morse 2002-07-10 23:43:15 PDT
Attaching patch to no longer store port as part of cookie domain attribute.

This involves the following changes:

1. getting cookies and setting cookies no longer includes url_port when calling 
ExtractUrlPart

2. Since port is no longer extracted, there is no longer any reason to strip the 
port number in various places of the code.

3. Existing cookie files might include some cookies that have port numbers.  
This will cause duplicate cookies to be sent to the server.  To solve this, 
cookies with port numbers will be discarded when reading in the cookie file.
Comment 7 Stephen P. Morse 2002-07-10 23:44:47 PDT
Created attachment 90920 [details] [diff] [review]
leaving port number off when storing cookie domain
Comment 8 Samir Gehani 2002-07-11 13:31:51 PDT
Comment on attachment 90920 [details] [diff] [review]
leaving port number off when storing cookie domain

r=sgehani
Comment 9 Daniel Veditz [:dveditz] 2002-07-11 22:20:14 PDT
Comment on attachment 90920 [details] [diff] [review]
leaving port number off when storing cookie domain

Is CookieCompare() used? I couldn't find a reference to it. If so maybe you
could zap the dead code as part of this patch (ran across it while
double-checking users of ->host).

>     /* check for bad legacy cookies (domain not starting with a dot) */
>     if (new_cookie->isDomain && *new_cookie->host != '.') {
>+      /* bad cookie, discard it */
>+      continue;
>+    }
>+
>+    /* check for bad legacy cookies (domain containing a port) */
>+    if (strchr(new_cookie->host, ':')) {
>       /* bad cookie, discard it */
>       continue;
>     }

If you're never stripping ports then won't an old cookie stored with a port
never match? in that case can we just leave it around until is expires away on
its own?

An extra strchr() times the max number of cookies at startup isn't going to
kill us. My main concern here is that both of these continues leak the
discarded cookie and its half a dozen or so allocated strings.
Comment 10 Stephen P. Morse 2002-07-12 10:02:52 PDT
> Is CookieCompare() used?

No.  Looks like it got consumed by cookie_CheckForPrevCookie at some time, and 
never got removed.  I'll yank it out now.

> If you're never stripping ports then won't an old cookie stored with a port
> never match? in that case can we just leave it around until is expires away
> on its own?

Yes, that's probably true.  It's just that we already got burned when we added 
dots to the beginning of domain names and wound up with both dotted and undotted 
cookies, both of which were being sent back to the host.  That regression led to 
the code that you see for removing legacy cookies not having the dot.  So that's 
why I felt it best to clean out these legacy cookies as well.

> My main concern here is that both of these continues leak the discarded
> cookie and its half a dozen or so allocated strings.

Good catch.  I just added code to plug the leak.  Attaching new patch.
Comment 11 Stephen P. Morse 2002-07-12 10:03:50 PDT
Created attachment 91120 [details] [diff] [review]
stop cookie leak, delete dead code
Comment 12 Stephen P. Morse 2002-07-12 10:04:41 PDT
Comment on attachment 91120 [details] [diff] [review]
stop cookie leak, delete dead code

bringing r=sgehani forward from previous patch
Comment 13 Stephen P. Morse 2002-07-12 10:11:43 PDT
One issue that I'd like people to think about before approving this patch is 
whether it is correct to ignore the port as far as cookies are concerned.  I 
think so (based on my reading of the RFC).  But it will be a significant change 
to our cookie behavior and I want to make sure it is the correct thing to do.

It might break some websites.  If they are few and minor ones, we can solve that 
with evangelism.  However I believe that IE must be ingnoring ports as well 
(otherwise how could the site in this bug report have worked with IE) in which 
case we should be safe.

There might also be security implications to not including the port test, so I'm 
cc'ing mitch on this.
Comment 14 Dan Mellem 2002-07-17 10:17:38 PDT
That's interesting. I thought that cookies are to be port-specific since there
can be different sites on different ports of the same host (example:
http://www.openvms.compaq.com and http://www.openvms.compaq.com:8000 use
different cookies).
Internally we have several development web sites all on one server with ports
80, 81, 82, etc. They all have different cookies with overlapping names (since
they all have Username, etc.). This problem isn't really significant to me but I
mention it anecdotally since at least some people may depend on the current
behavior.
Another quirk but I'd guess this one's pretty common.
Comment 15 Stephen P. Morse 2002-07-17 11:34:26 PDT
Dan, thanks for your comments.  They were most informative.

Would you agree from the reading of the RFC2109 spec that ports should be 
ignored (comment 5)?  But if appears that if we take this patch, then we will 
break your website.  If this is too widespread, then we are going to have to 
look for a different solution.

I presume that your website works with IE.  Can you look at the IE cookie file 
and see what is being stored.
Comment 16 Heikki Toivonen (remove -bugzilla when emailing directly) 2002-08-15 11:46:55 PDT
My gut feeling is that ports should always count - they could very well point to
a different server under different administration etc. I would vote to make this
an evangelism issue.

If we find a large number of sites that do not work because of our current
implementation I could reconsider, but since there are no dupes and this bug was
found relatively late, I don't think this is a common situation.
Comment 17 C. R. Oldham 2002-09-09 14:37:56 PDT
I know the following argument probably represents a small segment of the 
population, but I think Mozilla should adhere to the standard (leaving the 
port out) for 2 reasons:

1. IE does it this way.
2. If Mozilla includes the port it makes it impossible to share cookies across 
servers running on different ports.  For example, let's say a user visits a 
site running on nonstandard ports (http:8000 and https:8443 for example) and 
the site requires ssl login.  When the user is redirected to the non-ssl part 
of the site he will no longer be logged in.  And what's more, if parts of the 
site are accessible only via login and non-ssl, he won't ever be able to see 
them because he cannot login securely and have his login persist.
Comment 18 Dave Makower 2002-09-18 08:54:33 PDT
I think it is very important that Mozilla, and all other browsers, adhere to
published standards in general, and specifically to the cookie spec as laid out
in RFC2109.  Many eCommerce sites, including those that I have built, rely on
the ability to switch seamlessly between HTTP and HTTPS while keeping
cookie-based session information intact.  RFC2109 leaves no room for doubt: the
port is to be ignored in matching the domain name of the cookie.  All versions
of Internet Explorer (and pre-Mozilla Netscape) that I have tested behave
according to this spec, at least with regard to stripping the port.  Therefore,
it is incumbent upon those web sites that are out of spec to make changes to
accomodate the spec, not vice versa.
Comment 19 Dave Makower 2002-09-18 08:59:24 PDT
Created attachment 99671 [details]
Simple JSP to test cookie behavior

Install this JSP in any JSP container that supports SSL, and is using cookies
for session tracking.  Hit the page under HTTP, and then hit it under HTTPS
(using ports other than 80 and 443; e.g., 8000 and 8001).  If the browser is
behaving according to RFC2109, then the session identifiers should be the same.
 If the browser is out of spec, then the session identifiers will be different.
Comment 20 Robert O'Callahan (:roc) (email my personal email if necessary) 2002-09-18 13:25:43 PDT
So the spec, IE, and web developers all agree that the port should be ignored.
Why don't we check this patch in?

I'll sr the patch...
Comment 21 Robert O'Callahan (:roc) (email my personal email if necessary) 2002-09-18 13:27:00 PDT
Comment on attachment 91120 [details] [diff] [review]
stop cookie leak, delete dead code

sr=roc+moz
Comment 22 Stephen P. Morse 2002-09-18 15:47:41 PDT
The issue was not standard compliance or the lack of an sr that kept me from
persuing this.  Rather I had let this one lie because I was afraid of breaking
websites.  Comment 14 implied that we would break websites and this to me said
that IE does check for ports.  But from what is said in comment 17 and comment
18, it seems that IE does not check ports.  (Comment 18 is incorrect about
pre-mozilla netscape -- it was the basis for the current mozilla code so N4.x
does check ports.)

OK, if there is agreement that IE does not check ports, then I'll check this in
(as soon as I fix the bit rot).  But I'll monitor the criticisms carefully, and
if it turns out that it causes us to start breaking websites that work fine in
IE, I'll back it out immediately.
Comment 23 Robert O'Callahan (:roc) (email my personal email if necessary) 2002-09-18 15:52:30 PDT
Sounds reasonable. Check it in!
Comment 24 Dave Makower 2002-09-18 16:06:34 PDT
I stand corrected with regard to pre-Mozilla Netscape.  I just ran session-snoop.jsp (see 
my previous attachment) from Netscape Communicator 4.7 on a Mac, and Stephen 
Morse is correct: this bug has been present in Netscape since before Mozilla.  But the 
spec is clear, and IE 5 (Mac/Windows) and IE 6 (Windows) ignore the port in matching 
the cookie, as per spec.  Thanks for the attention and quick response on this issue, guys.
Comment 25 Stephen P. Morse 2002-09-18 16:19:34 PDT
Created attachment 99754 [details] [diff] [review]
remove bit-rot from previous patch
Comment 26 Stephen P. Morse 2002-09-18 17:35:09 PDT
Comment on attachment 99671 [details]
Simple JSP to test cookie behavior

r=sgehani
sr=roc+moz
(bringing reviews forward)
Comment 27 Stephen P. Morse 2002-09-18 18:02:52 PDT
checked in.
Comment 28 Rob Jones 2002-10-31 13:57:38 PST
I've added this comment to bugzilla for bugs 99311 and 142803 but thought
it worth bringing to your attention via email as well. Hope thats ok and
hope you'll excess the somewhat long post.

Came across this bug with cookies and port numbers yesterday. Looking at
the bugzilla history it appears (not completely confirmed) that mozilla's
behaviour has been swinging from working one way to working another several
times over the last few builds. I just wanted to add my opinion that the
current behaviour in 1.2beta is correct and and as per the RFCs. I also
wanted to point out this change to those people who have 'fixed' or requested
incorrect (IMO) behaviour for previous bugs so that do not reopen the old bugs 
and cause the behaviour to change back in future releases.

The bug relates to the default domain (also called host in the cookie manager)
that cookies are stored under/sent back to and weather or not to store port
numbers if they are used.

There are two RFC for cookies, 2109 (For set-cookie) and 2965 (For set-cookie2)

In RFC 2109 
in section 4.3.1 Interpreting Set-Cookie
it states 
  "Domain Defaults to the request-host. "
And in section 2 TERMINOLOGY
it states 
 "The terms request-host and request-URI refer to the values the client 
  would send to the server as, respectively, the host (but not port) and 
  abs_path portions of the absoluteURI (http_URL) of the HTTP request line. 
  Note that request-host must be a FQHN."
 
In RFC 2965
in section 3.3.1 Interpreting Set-Cookie2 
it states
  "Domain Defaults to the effective request-host. "
it also states
  " Port    The default behavior is that a cookie MAY be 
            returned to any request-port. "
And in section 1 TERMINOLOGY
it states
  " The terms request-host and request-URI refer to the values the client 
    would send to the server as, respectively, the host (but not port) and 
    abs_path portions of the absoluteURI (http_URL) of the HTTP request line. "
  (Just like RFC 2109)


My interpretation of these is that port numbers should not be used for recording
cookie domains unless a set-cookie2 header explicitly defines port number.

When a cookie is sent to mozilla with no domain specified

It appears that the behaviour for

0.9.3 stores port number 
0.9.4 does not store the port number
0.9.5 stores port number

And the tested behaviour for

1.0.1 stores port number
1.1 stores port number
1.2b does not store port number


The first bug in bugzilla relating to this is 99311 reported by
hniksic@arsdigita.com
who to summarise requested that a cookie set by
	delirium:1280
be a seperate cookie domain to a cookie set by 
	delirium:9800

The fix for this bug was to store port number for unspecified domains.

There are then a couple of similar bugs (bug numbers 134194, 130545) around the
0.9.8/0.9.9 builds that seem related but I did not find enough info in the 
bug reports to be sure that this is same thing.

Then bug 142803 'port numbers being stored as part of cookie' reported 
by dennis@gorillapolymedia.com requesting that port numbers not be 
stored with cookies results in the correct (IMO) behaviour being 
implemented for build 1.2beta.

For The discussion for bug 142803 I agree with commet 5 that port numbers 
should not be stored according to RFC's. And comments 17 and 18 are also very 
valid. I can also confirm (from testing) that IE (5.5 & 6) does ignore ports 
in this kind of situation.

As far as breaking websites (comment 14) goes. This is similar to the symptoms
that caused hniksic@arsdigita.com to report bug 99311. It is only when a site
uses different ports and the same name that problems might arise. If a shared
server implements name based (or ip based) virtual hosting this problem can
not arise. For IP based each ip must have a different name (although multiple
names can point to same IP) so for cookies this is same as name based virtual
hosting. Each site will automatically be distinguised by its name. In my
experience the number of site that exist that use different ports and the same
name that require seperation is very small especially in comparision with
the number of of sites that use http/https transparently and require sharing.
Comment 29 benc 2003-06-18 09:58:08 PDT
VERIFIED: fixed.
Mozilla 1.4a & b, allplats.

As long as I have tested cookies (since Jan), no ports have shown up in "site"
field.

Note You need to log in before you can comment on or make changes to this bug.