The default bug view has changed. See this FAQ.

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

RESOLVED FIXED

Status

()

Core
Networking: HTTP
RESOLVED FIXED
15 years ago
3 years ago

People

(Reporter: Richard Thomas, Assigned: Vinay Badami)

Tracking

Trunk
x86
Windows 98
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [patch needsr/sr=], URL)

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

15 years ago
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

Comment 1

15 years ago
-> badami for investigation
Assignee: darin → badami
(Assignee)

Comment 2

15 years ago
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 ?

Comment 3

15 years ago
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.

Comment 4

15 years ago
> 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

Comment 5

15 years ago
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.
(Assignee)

Comment 7

15 years ago
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.
(Assignee)

Comment 8

15 years ago
Created attachment 72363 [details] [diff] [review]
patch as per my previous comment

Prevent referrer header from siteA to siteB, both sites being secure, if user
so desires.
(Assignee)

Comment 9

15 years ago
Created attachment 72365 [details] [diff] [review]
prevent referrer only if user so desires 

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
(Assignee)

Updated

15 years ago
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.
(Assignee)

Comment 11

15 years ago
Created attachment 72593 [details] [diff] [review]
YAP- yet another patch


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

Comment 12

15 years ago
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?
(Assignee)

Comment 13

15 years ago
Created attachment 73188 [details] [diff] [review]
using a separate pref
Attachment #72593 - Attachment is obsolete: true

Comment 14

15 years ago
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?
(Assignee)

Comment 15

15 years ago
Created attachment 73517 [details] [diff] [review]
with darins comments
Attachment #73188 - Attachment is obsolete: true

Comment 16

15 years ago
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+

Comment 17

15 years ago
Comment on attachment 73517 [details] [diff] [review]
with darins comments

r=rpotts@netscape.com
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...)

Comment 19

15 years ago
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 20

15 years ago
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+
(Assignee)

Comment 21

15 years ago
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
Last Resolved: 15 years ago
Resolution: --- → FIXED

Comment 22

15 years ago
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.

Comment 23

15 years ago
*** Bug 130170 has been marked as a duplicate of this bug. ***

Comment 24

15 years ago
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.

Comment 25

15 years ago
jesse: right.. this was not a complete solution.  post 1.0, i'm planning to test
out the partial referrer solution.

Comment 26

15 years ago
Reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 27

15 years ago
ok, we actually allow cross site HTTPS referrers now by default, so this bug
should be fixed.
Status: REOPENED → RESOLVED
Last Resolved: 15 years ago15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.