Closed
Bug 142803
Opened 22 years ago
Closed 22 years ago
Port is being stored as part of cookie domain
Categories
(Core :: Networking: Cookies, defect, P2)
Tracking
()
VERIFIED
FIXED
mozilla1.2beta
People
(Reporter: dennis, Assigned: morse)
References
()
Details
Attachments
(3 files, 2 obsolete files)
8.71 KB,
text/plain
|
Details | |
320 bytes,
text/plain
|
morse
:
review+
morse
:
superreview+
|
Details |
6.19 KB,
patch
|
Details | Diff | Splinter Review |
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•22 years ago
|
||
To cookies for starters.
Assignee: Matti → morse
Status: UNCONFIRMED → NEW
Component: Browser-General → Cookies
Ever confirmed: true
QA Contact: imajes-qa → tever
Assignee | ||
Comment 2•22 years ago
|
||
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.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.2beta
Assignee | ||
Updated•22 years ago
|
Priority: -- → P2
Target Milestone: mozilla1.2beta → mozilla1.1beta
Assignee | ||
Comment 3•22 years ago
|
||
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.
Assignee | ||
Comment 4•22 years ago
|
||
Assignee | ||
Comment 5•22 years ago
|
||
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.
Assignee | ||
Comment 6•22 years ago
|
||
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.
Assignee | ||
Comment 7•22 years ago
|
||
Assignee | ||
Updated•22 years ago
|
Summary: Form checked by JavaScript, redirect appends :443 to domain name, resulting in loss of session state → Port is being stored as part of cookie domain
Comment 8•22 years ago
|
||
Comment on attachment 90920 [details] [diff] [review] leaving port number off when storing cookie domain r=sgehani
Attachment #90920 -
Flags: review+
Comment 9•22 years ago
|
||
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.
Attachment #90920 -
Flags: needs-work+
Assignee | ||
Comment 10•22 years ago
|
||
> 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.
Assignee | ||
Comment 11•22 years ago
|
||
Attachment #90920 -
Attachment is obsolete: true
Assignee | ||
Comment 12•22 years ago
|
||
Comment on attachment 91120 [details] [diff] [review] stop cookie leak, delete dead code bringing r=sgehani forward from previous patch
Attachment #91120 -
Flags: review+
Assignee | ||
Comment 13•22 years ago
|
||
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•22 years ago
|
||
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.
Assignee | ||
Comment 15•22 years ago
|
||
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.
Assignee | ||
Updated•22 years ago
|
Target Milestone: mozilla1.1beta → mozilla1.2beta
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•22 years ago
|
||
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•22 years ago
|
||
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•22 years ago
|
||
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.
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 on attachment 91120 [details] [diff] [review] stop cookie leak, delete dead code sr=roc+moz
Attachment #91120 -
Flags: superreview+
Assignee | ||
Comment 22•22 years ago
|
||
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.
Sounds reasonable. Check it in!
Comment 24•22 years ago
|
||
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.
Assignee | ||
Comment 25•22 years ago
|
||
Attachment #91120 -
Attachment is obsolete: true
Assignee | ||
Comment 26•22 years ago
|
||
Comment on attachment 99671 [details]
Simple JSP to test cookie behavior
r=sgehani
sr=roc+moz
(bringing reviews forward)
Attachment #99671 -
Flags: superreview+
Attachment #99671 -
Flags: review+
Assignee | ||
Comment 27•22 years ago
|
||
checked in.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 28•22 years ago
|
||
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•21 years ago
|
||
VERIFIED: fixed. Mozilla 1.4a & b, allplats. As long as I have tested cookies (since Jan), no ports have shown up in "site" field.
Status: RESOLVED → VERIFIED
QA Contact: tever → cookieqa
You need to log in
before you can comment on or make changes to this bug.
Description
•