Closed Bug 128213 Opened 23 years ago Closed 22 years ago

Instead of an order form page I get an ecommerce security warning

Categories

(Core :: Networking: HTTP, defect)

x86
Windows 98
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: rthomas, Assigned: badami)

References

()

Details

(Whiteboard: [patch needsr/sr=])

Attachments

(1 file, 4 obsolete files)

From Bugzilla Helper: User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.8+) Gecko/20020227 BuildID: 2002022703 I am wanting to order a particular product from this website, when I go to checkout I get a page giving me a security warning, instead of the order form that I get with IE. Reproducible: Always Steps to Reproduce: 1.Go to the URL 2.Click on the green Overseas "Add to Cart" button on the right hand side of the page - you will get a page showing the contents of your shopping cart, 1 item of product. 3.Click on the Order Checkout at the top right of the page - the security warning page appears Actual Results: I get a page about a security warning Expected Results: I should have got an order form to fill in my details
-> badami for investigation
Assignee: darin → badami
Looks like we are being pretty strict wrt setting referrer headers especially wrt secure servers. Commenting out the following lines solves the problem /******* if (nsCRT::strcasecmp(referrerHost, host) != 0) { return NS_OK; } ********/ I do not see in the spec anything saying that the hosts should match to set a refererrer on secure servers. It indicates that we should never set a referrrer while going to a non-secure server from a secure server. why are we doing this ?
ah, we are doing this to prevent sending a Referer header from one secure site to another secure site. from the point of view of the browser, there is no way of knowing that first secure site is part of the same "trust domain" as the second secure site. therefore, we assume that the second secure site should not see an URL from the first secure site. this is a security measure that isn't implemented by other browsers but is implemented by mozilla as an extra measure to protect user privacy. moreover, there is a user preference to disable sending all Referer headers, as recommended by RFC2616. in this case, i think the site should be evangelized. cc'ing some folks who may have an opinion on this.
> from the point of view of the browser, there is no way of knowing that first secure site is part of the same "trust domain" as the second secure site. That is not totally correct IIRC. We should be comparing the cert of the dest site the the cert of the referer site. The code has changed alot since I was hacking on it, but PSM (NSS) does provide a way to verify that two certs match. If they don't, I believe that this is calls for user dialog.
Status: UNCONFIRMED → NEW
Ever confirmed: true
hmm... would be nice to do that.
We'vre been through this before. IE doesn't block https->https, anbd the http rfc doesn't require it either.
1. We are doing something outsie of the spec that may break a few sites. So my take is that this behavior not be the default. 2. We put in a patch that does implements 1 so that sites will work and get that in. We then track this bug wrt Dougs comments and get a proper fix in if possible by Moz1.0 else Moz1.01.
Attached patch patch as per my previous comment (obsolete) (deleted) — Splinter Review
Prevent referrer header from siteA to siteB, both sites being secure, if user so desires.
Attached patch prevent referrer only if user so desires (obsolete) (deleted) — Splinter Review
prevent referrer only if user so desires from Site A --> Site B bot being secure. Previous patch was invalid and wrong.
Attachment #72363 - Attachment is obsolete: true
Whiteboard: [patch needsr/sr=]
We aren't introducing a user dialog for this, are we? Please don't add a dialog. I'd prefer it if we left the current behavior alone (not sending referrers cross-host), at least as the default. We could add a pref to turn it off.
Attached patch YAP- yet another patch (obsolete) (deleted) — Splinter Review
Be secure by default. Allow user to override thru a preference. Added logging to help in debugging in future.
Attachment #72365 - Attachment is obsolete: true
problem: the referrer level in nsIHttpChannel is something the client of a nsIHttpChannel uses to specify the kind of referrer being set. it doesn't make sense for the client of a nsIHttpChannel to say that a "HTTPS_ALLOW_NON_MATCHING_HOSTS" referrer is being set... usually, they say that this is a link click (REFERRER_LINK_CLICK), or this is an image (REFERRER_INLINES), etc. REFERRER_HTTPS_ALLOW_NON_MATCHING_HOSTS is not a type of referrer. moreover, it seems like the setting you're after is more of a preferences setting... not a referrer type. so i think it shouldn't be exposed on nsIHttpChannel but should simply be mentioned in all.js... e.g.: pref("network.http.sendRefererHeader", 2); // 0=don't send any // 1=send only on clicks // 2=send on image requests as well // 3=send from secure site A to secure site B but this is bad because then you end up referencing 3 in the code... of course you could introduce another #define or whatever... but something just doesn't feel right about that, cuz it isn't symmetric w/ the other values. in fact, suppose we come up with yet another preference for when to send referrers... would we add it before or after this HTTPS cross-host one? it almost seems like we need another preference for this bug. a boolean "network.http.sendSecureReferrerCrossHost" pref might do... of course that's pretty wordy. suggestions?
Attached patch using a separate pref (obsolete) (deleted) — Splinter Review
Attachment #72593 - Attachment is obsolete: true
how about picking one name for this attribute and make it universal? in your patch you use "mSecureXSiteReferrer" in one place... and i like that because it is short... so how about this: "network.http.sendSecureXSiteReferrer" nsHttpHandler::SendSecureXSiteReferrer() nsHttpHandler::mSendSecureXSiteReferrer; sound good?
Attached patch with darins comments (deleted) — Splinter Review
Attachment #73188 - Attachment is obsolete: true
Comment on attachment 73517 [details] [diff] [review] with darins comments >+ PRPackedBool mSendSecureXSiteReferrer; //default is false, if true allow referrer headers between secure non-matching hosts nit: please wrap this comment, or move it above the variable declaration, so it doesn't extend so far beyond 80 chars. sr=darin
Attachment #73517 - Flags: superreview+
Attachment #73517 - Flags: review+
Two questions: * What's the point of making this a pref if there's no UI -- how will users know to fix some broken site by twiddling the magic pref in their prefs.js? Are some implementations (e.g., Netscape commercial, some embedders) going to want to set the default differently? Or are you planning to add UI? * Aren't prefs supposed to have the default values listed in all.js (at the very least, for documentation)? (I have heard different answers to this question, I admit...)
dbaron: good point... this pref should be documented in all.js. also, i'd imagine that we might one day want to add UI for the "send Referer header" prefs. at any rate, this patch is just the first step. perhaps when i work on moving some of the networking prefs into the advanced section of the preferences dialog, this preference can be added.
Comment on attachment 73517 [details] [diff] [review] with darins comments a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #73517 - Flags: approval+
Fixed with checkin D:\mozilla\netwerk\protocol\http\src>cvs commit cvs commit: Examining . ? bak Checking in nsHttpChannel.cpp; /cvsroot/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp,v <-- nsHttpChann new revision: 1.98; previous revision: 1.97 done Checking in nsHttpHandler.cpp; /cvsroot/mozilla/netwerk/protocol/http/src/nsHttpHandler.cpp,v <-- nsHttpHandl new revision: 1.52; previous revision: 1.51 done Checking in nsHttpHandler.h; /cvsroot/mozilla/netwerk/protocol/http/src/nsHttpHandler.h,v <-- nsHttpHandler new revision: 1.24; previous revision: 1.23 done
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Adding a new hidden, undocumented pref might not be the best way to fix this problem... You might want to take a look at the work I have been doing with referrers in bug #55477. I have added a UI for configuring the current referrer settings, as well as adding some additional levels of referrer control - including the ability to send the referrer only to the same host. My patch is currently blocking https->https referrers, however, mainly because I just didn't want to change the original piece of code because of potential privacy problems. But I think that removing the https->https check entirely wouldn't be unreasonable, iff the user had more options on how and when to send the referrer header. The very best solution, as mentioned above, would be to verify that the domains in the certificates match, then decide what to send as the referrer.
*** Bug 130170 has been marked as a duplicate of this bug. ***
I don't understand how adding a hidden pref "fixed" this bug. I proposed a solution in bug 141641: send the hostname (but not the path) as the referrer if the origin is https, regardless of the target.
jesse: right.. this was not a complete solution. post 1.0, i'm planning to test out the partial referrer solution.
Reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
ok, we actually allow cross site HTTPS referrers now by default, so this bug should be fixed.
Status: REOPENED → RESOLVED
Closed: 23 years ago22 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: