245406 - Display domain in status bar for secure sites

Status: RESOLVED FIXED

(Firefox :: General, enhancement, P1)

Description

[Gervase Markham [:gerv]]

Although we've got some protection in place for username/password-based URL
spoofing (bug 232567), that doesn't cover all the possibilities. We need to have
ever-present UI that, on secure sites, makes it clear what site any window, even
a "chromeless" one, actually comes from.

My assertion (as discussed with Darin and Ben by email) is that we need to make
the status bar compulsory (like IE) and add the hostname to it when we have a
secure connection. This means that people who "look for the lock" will get used
to seeing the hostname of their bank right next door.

I'm about to attach an implementation of this for Firefox, and would appreciate
feedback on whether this is a) necessary, and b) the right way to do it.

Gerv

Comment 1

[Gervase Markham [:gerv]]

Attachment: Patch v.1

Initial implementation.

Gerv

Comment 2

[Lorenzo Colitti]

Good idea!

Just one concern: might phishers be able to work around this using frames? The
problem with frames, of course, is that there's no place to show the UI
containing the domain name of each frame.

For example, might a phisher send you to a banking site which uses frames and
then use javascript, or possibly bug 103638, to replace the main frame with a
spoofed version?

Comment 3

[[e-mail]]

Comment on attachment 149878 [details] [diff] [review]
Patch v.1

Rather than combining this.securityDomain.removeAttribute("label"); with your
CSS I would have thought that using this.securityDomain.hidden = true; would
have been more straightforward. Also there might be a flaw in your exception
logic (always assuming that an exception is expected) - if you switch between
secure sites and you get your expected exception then your domain label doesn't
change.

Comment 4

[Michiel van Leeuwen (email: mvl+moz@)]

Why only for secure sites? for example, posting fake news articles in a faked
site can be just as bad.

Comment 5

[Daniel Veditz [:dveditz]]

I agree, we should put the host in the status bar ALWAYS if we're going to do
this at all, and we should never let windows be created by non-privileged
content without the status bar.

Re comment 2: frames aren't as much a problem. If evil.com hosted your bank in a
frame the status bar would say [evil.com] as it should. If you need details
about frames page-info is useful, but we don't have enough real-estate to be
showing all that info on a permanent basis. The outer site, though, we should be
able to cover.

This approach will help protect /some/ folks, but I've seen a bunch of recent
ebay and paypal phishes where the link takes you to some believable domains that
people would probably accept even when seeing it (ebay-support.com), and even
some obviously wrong domains (somerandom.ru/~memberfoo)

Especially important for secure sites, but if it's not there all the time people
won't know to look for it.

Comment 6

[Grey Nicholson (they/them)]

(In reply to comment #5)
> Especially important for secure sites, but if it's not there all the time people
> won't know to look for it.

If the domain name is to be an accompaniment to the lock icon, I don't think it
makes sense to always show it (it'd just be in the way much of the time).

How about we display the lock icon (and the domain name) on every page which
contains a form (i.e. every page which would generate a "the information you are
submitting is not secure" message)? Of course, if the page is not secure there
should be a large red cross (or similar) over the lock.

Comment 7

[Gervase Markham [:gerv]]

Dan: the idea is that people compare what they see with what they normally see
when visiting their bank. So if it normally says "barclays.co.uk", and now it
says "wibble.com/~foo", they should be suspicious. If it says
"barclays-support.co.uk", that's a bit harder - but it's still different to normal.

Greg: I'm actually surprised Firefox doesn't display the lock all the time;
Mozilla does. Presumably that was a conscious design decision, but I'd be
interested in the rationale.

If people thought it was best, we could certainly have the text there all the
time. (The idea of the current implementation was that we only displayed a site
name when we were sure - and that means a secure connection.)

Gerv

Comment 8

[Boris Zbarsky [:bzbarsky]]

A secure connection on its own is not enough (think DNS poisoning).  We could be
somewhat sure with an appropriate cert... but precious few places have those.

Comment 9

[Jason Lustig]

(In reply to comment #7)
> If people thought it was best, we could certainly have the text there all the
> time. (The idea of the current implementation was that we only displayed a site
> name when we were sure - and that means a secure connection.)

I think that we should definitely have it there if it's a secure site; in
addition we could have it there if there was a phishing attempt. Say, for
example a spoofed news article or anything. That way we could get any phishing
out of the way.

Phishing in a frame is another issue... I'm not sure how to deal with that.

Comment 10

[Gervase Markham [:gerv]]

bz: I would have thought this solution avoids the DNS poisoning problem. If the
name "www.mybank.com" actually ends up somewhere else, and an SSL cert is
offered, the name on the cert won't match the name we think we are using, and
the browser will complain. Isn't that right?

Gerv

Comment 11

[Boris Zbarsky [:bzbarsky]]

If a cert is offered, sure.  I covered that in comment 8, no?

Comment 12

[Doron Rosenberg (IBM)]

I think it would be best to show the host name next to the lock icon as one
statusbar "panel", since people are supposedley trained to look at the icon.

Comment 13

[Michael Lefevre]

For the various phishes I've seen recently, making the status bar compulsory
would defeat most of them, and I think that just means changing the value of a
pref. That'd be a change that could happen for Firefox 1.0 and even Mozilla 1.7.
Could that be done independently of this bug?

Some of the current phishes don't even bother to redirect to a secure site in
the window under their own pop-up, so people evidently don't even look for the
lock, let alone figure out if they're on the right domain. Another issue with
hostnames is simple obfuscation - www.mozi11a.org and such like.

Comment 14

[Gervase Markham [:gerv]]

doron is right; the next version of the patch will do that.

michael: today's phishes may not use SSL, but you can be sure tomorrow's will.
The only way to keep people safe long term is to make it clear exactly what site
they are on.

To that end, I'm toying with the idea of changing the background colour of the
panel to a colour value based on a hash of the domain name. So, any changing of
L to 1 or O to 0 would change the colour also. But that's a different bug.

Gerv

Comment 15

[Ben Bucksch (:BenB)]

This is very similar to what I did in bug 228524. There, I also showed the
"Organisation" field of the cert.

I personally think all this (lock icon, unobscured domain name, maybe Org name)
should show in the toolbar. I don't think many people look for or know the lock
icon at all, they look at the urlbar only at best.

bz: I thought a "secure connection" is (here) always SSL, with a valid cert, and
with domain name of the originally loaded URL matching the "common name" in the
cert. One of the main points of a "secure connection" is to prevent DNS
poisoning, man-in-the-middle etc., not?

Comment 16

[Michael Lefevre]

work related to this is happening for Firefox in bug 244025. if anything is
going to happen with this bug, seems to me it should be co-ordinated. Given that
the patch is for Firefox, I'm not sure the Browser product is the right place
for this...

Comment 17

[Chris Cook]

I thought it might be a different bug so I created a new one but some comments
here (eg. #6) touch on it.

bug 247872 requests some kind of notification if the current page is NOT secure
but DOES post to a secure URL. Some authentication pages do this. I believe it
is worth consideration.

Comment 18

[Robert Accettura [:raccettura]]

Considering these attacks are big news, this should block RC1.  Would be nice to
show yet another great security innovation by the Mozilla Foundation.

Esecially considering Mozilla's being marketed by the media as the 'secure
alternative to IE'.  These gains are big right now.

Comment 19

[Gervase Markham [:gerv]]

Attachment: Patch B v.1

Here's an updated patch for the Aviary branch, now that the security UI
situation has settled down a bit. 

We put the domain name of the site in the status bar, to the left of and in the
same box as the lock icon, if and only if the connection is secure.

The patch is designed to be as safe as possible. If in doubt, it doesn't put
the domain in the bar. It piggybacks on the existing code for changing the lock
icon, which (hopefully!) always changes the state at the correct time.

Tested on https://www.paypal.com and Bank of America; tested with multiple tabs
and windows.

Gerv

Comment 20

[Gervase Markham [:gerv]]

Comment on attachment 153161 [details] [diff] [review]
Patch B v.1

Requesting review.

Gerv

Comment 21

[Gervase Markham [:gerv]]

Note: making the status bar always-on is (in my view) also important for this,
but it's a different thing (a matter of policy), and should be handled in a
separate bug.

Also, in the future I'd like to try out my idea of changing the foreground or
background colour based on a hash of the text (to make the difference between
barclays and barc1ays more clear) but that will also be a separate bug.

Gerv

Comment 22

[Gervase Markham [:gerv]]

Attachment: Screenshot of Patch B v.1

Comment 23

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 153161 [details] [diff] [review]
Patch B v.1

>   <binding id="statusbarpanel-iconic" display="xul:button"
>            extends="chrome://global/content/bindings/general.xml#statusbarpanel">
>     <content>
>+      <children/>
>       <xul:image class="statusbarpanel-icon" xbl:inherits="src"/>
>     </content>
>   </binding>

hm... how about creating a statusbarpanel-iconic-text binding instead that
places the text like so:

<content>
  <xul:image/>
  <xul:label/>
</content>

...

and then using a style rule for your specific usage (#security-button) that
sets the "-moz-box-direction" property to "reverse" so that the appearance is
<xul:label/><xul:image/> 

That way we avoid a special case in the binding and creating something more
generally useful, without too much additional complexity. Then you can use the
same securityButton variable in the browser.js code, and just set .label and
.src on the same element, rather than creating two elements

> 
>+    this.securityDomain  = document.getElementById("security-domain-label");

See above for comment about using a single binding/variable.

Let's see another patch... let me know when it's ready and I'll review.

Comment 24

[Gervase Markham [:gerv]]

Attachment: Patch B v.2

Works exactly the same as v.1, but rearchitected as suggested by ben.

Gerv

Comment 25

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 154045 [details] [diff] [review]
Patch B v.2

>+  <binding id="statusbarpanel-iconic-text" display="xul:button">
>+    <content>
>+      <xul:image class="statusbarpanel-icon" xbl:inherits="src"/>
>+      <xul:label class="statusbarpanel-text" xbl:inherits="value"/>
>+    </content>
>+  </binding>

Can we get the same set of properties available on this as on the others?

>+        try
>+        {
>+          this.securityButton.setAttribute("value",
>+                                         gBrowser.contentWindow.location.host);
>+        } catch(exception) {}

formatting glitches, do 

try {

and insert 2 spaces before |gBrowser|

both here, and several lines down. 

Everything else looks good.

Comment 26

[Gervase Markham [:gerv]]

Attachment: Patch B v.3

Fixed as requested - at least, I think I've done what you asked - I may not
have understood your first comment. I've made my binding inherit from
"statusbarpanel" and used "label" instead of "value" to be consistent with
that.

Gerv

Comment 27

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 154271 [details] [diff] [review]
Patch B v.3

OK. r=[e-mail]

Comment 28

[Gervase Markham [:gerv]]

Checked in.

Checking in browser/base/content/browser.xul;
/cvsroot/mozilla/browser/base/content/browser.xul,v  <--  browser.xul
new revision: 1.228.2.2.2.22; previous revision: 1.228.2.2.2.21
done
Checking in browser/base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.296.2.3.2.52; previous revision: 1.296.2.3.2.51
done
Checking in browser/themes/pinstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/pinstripe/browser/browser.css,v  <--  browser.css
new revision: 1.1.2.3; previous revision: 1.1.2.2
done
Checking in browser/themes/winstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/browser.css,v  <--  browser.css
new revision: 1.1.2.15; previous revision: 1.1.2.14
done

Gerv

Comment 29

[Doug Wright]

I've just downloaded a sweetlou 'hourly' build to check this out - and
something's  gone amiss - although the address bar is gold with the lock icon,
the lock icon isn't displayed in the status bar at all anymore - although
hovering over the area still displays a 'verified by verisign' tooltip.

Comment 30

[Gervase Markham [:gerv]]

That's odd... I rebuilt and checked it was working immediately before I checked in.

What sites are you trying? It works for me on https://www.paypal.com .

Gerv

Comment 31

[Gervase Markham [:gerv]]

Er... I only checked half the patch. <looks highly embarrassed> Thanks for the
heads-up!

Checking in toolkit/content/xul.css;
/cvsroot/mozilla/toolkit/content/xul.css,v  <--  xul.css
new revision: 1.35.6.3; previous revision: 1.35.6.2
done
Checking in ./toolkit/content/widgets/general.xml;
/cvsroot/mozilla/toolkit/content/widgets/general.xml,v  <--  general.xml
new revision: 1.4.18.1; previous revision: 1.4
done

Gerv

Comment 32

[Pavel Penaz]

Strange, in newest Sweetlou build I still cannot see the lock icon in statusbar.
Also even though "www.paypal.com" is finally in the statusbar, it unfortunately
does not fit the statusbar and covers other statusbar buttons..

Comment 33

[Jesse Ruderman]

Gerv, Tinderbox and Bonsai don't show your second set of checkins.

Comment 34

[Gervase Markham [:gerv]]

jesse: when I pull a completely clean tree, the second half of my checkin is
present. And LXR says it's there too:
http://lxr.mozilla.org/aviarybranch/source/toolkit/content/xul.css#857
http://lxr.mozilla.org/aviarybranch/source/toolkit/content/widgets/general.xml#256

I don't know why it's not showing up on bonsai and tinderbox - perhaps it's
related to the recent delays in mozilla.org mail. I don't know how it's all
plumbed together on the back end. 

There are no "sweetlou" builds for Linux as far as I know:
http://ftp.mozilla.org/pub/mozilla.org/mozilla/nightly/experimental/sweetlou/
so I can't see what the issue is with tablet's build.

Gerv

Comment 35

[Jesse Ruderman]

WFM in my own branch build, which includes the site name in the status bar.

Comment 36

[Doug Wright]

Attachment: Screenshot of problem

Screenshot of the problem experienced by Tablet. This is from a Windows build
(sweetlou).

I can't see the 2nd set of checkins either - but they've had some effect
because I didn't have the text in the status bar before.

Comment 37

[Steffen Wilberg]

I'm seeing the same as shown in the screenshot: The host name is displayed on
top of the othe buttons, and the lock symbol is not displayed in the status bar.
Using WinXP. Reopening.

Gerv's second set of checkins are in my tree.

Comment 38

[Steffen Wilberg]

Attachment: fix the button width on Winstripe

The button should be a bit wider than 20px when displaying the host name :)

Remaining glitch:
On sites where a broken lock is displayed, like https://www.dab-bank.com/, the
icon is moved to the right. If there's no width specified, the icon position is
correct, but the button is extended to the left, i.e. a blank space is
displayed left of the icon.

Comment 39

[Steffen Wilberg]

Comment on attachment 154450 [details] [diff] [review]
fix the button width on Winstripe

I'd like to check this in today since it looks a bit silly on secure sites, see
the screenshot.

Comment 40

[Thomas Stache]

I can confirm this patch fixes Winstripe for levels high and low on WinXP Luna.

BTW: Testing two other themes (Qute and Mostly Crystal) which don't use the 20px
fixed width the domain is displayed OK, though on the right side of the lock icon.

Comment 41

[Steffen Wilberg]

Thomas, thanks for confirming.
Theme authors need to add |-moz-box-direction: reverse;|, like Gerv did to
Winstripe and Pinstripe. See comment 23.

Comment 42

[Gervase Markham [:gerv]]

r=gerv on Steffen's patch, but now there are tree approvals it needs Ben to look
at it. You'll need to send him email @mozilla.org to get his attention. Is it
worth opening a new bug?

The domain is placed to the left using -moz-box-reverse (as suggested by Ben);
theme authors who wish it to be on the left will need to add that to their themes.

I also see the blank space to the left of the icon; this seems to be caused by
the left and right margins on the empty label. I'm not quite sure why they are
appearing when the icon appears but not when it's not... perhaps someone with
better XUL knowledge can explain that.

Gerv

Comment 43

[Steffen Wilberg]

I hoped setting the approval flag was enough?
I don't think we should open another bug, because the problem is closely related
to the original bug, and because this bug has a blocking1.0PR+ flag.

Comment 44

[Gervase Markham [:gerv]]

Now that we have approval flags, that might be enough - I'm not sure. 

But the 1.0PR blocking flag on this bug is not magically transferable. One might
argue that it's obvious that the problem this checkin caused also blocks 1.0PR -
in which case you nominate your new bug, and expect the triage team to agree :-)

Still, we can stay here if you like. I doesn't matter too much.

We _could_ fix the label margins problem by styling it to have zero margins when
it's empty, but I'm not sure that's the right fix; this may be symptomatic of
some deeper misunderstanding/mistake.

Gerv

Comment 45

[Jesse Ruderman]

Bug 253288 covers the missing lock icon.

Comment 46

[Jesse Ruderman]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040727
Firefox/0.9.1+

Everything works fine in "Windows Classic style", but the lock icon is missing
and stuff overlaps if I switch to "Windows XP style" (Luna).

Comment 47

[Steffen Wilberg]

Comment on attachment 154450 [details] [diff] [review]
fix the button width on Winstripe

Okay, I'm moving this to bug 253288.

Comment 48

[Steffen Wilberg]

Marking as fixed again. See bug 253288 for the remaining issues.

Comment 49

[Grey Nicholson (they/them)]

Can we have a pref to turn this off? I don't find it useful and the LinkToolbar
screws up the display anyway (in addition to bug 253288).

Comment 50

[Bill Gianopoulos [:WG9s]]

I do not understand the usefulness of this at all.  If the certificate name does
not mtch the URL name, should not this code display the name from the
certificate?  so that the lock and the secured y <CA> ends up bing next to the
domain name that the CA said the certificate belongs to?

Comment 51

[Gervase Markham [:gerv]]

William said: "I do not understand the usefulness of this at all.  If the
certificate name doesnot mtch the URL name, should not this code display the
name from the certificate?"

If the Cert name doesn't match the URL name, won't the lock be broken (and the
domain name absent)? Or are you talking about the case where you get a "domain
mismatch" popup and say "well, do it anyway"? We might not have thought about
that one...

I access the domain of the site in the same way that the Security tab on Page
Info does... Is that also getting this wrong?

Gerv

Comment 52

[Bill Gianopoulos [:WG9s]]

re comment #51.  Yes it is the issue that after you accept a ceriticate name
mismatch error that the hostname portion of the URL is displayed next to the
lock Icon.  I suppose I could argue on both sides of this one and it kind of
depends on what is trying to be accomplished by displaying the hastname next to
the lock.

Should it be the hostname from the URL or the hostname on the certificate.

What I thought was misleading was having a hostname that does not match what is
on the cerfitiacte next to a lock Icon which when you over over it sayis the
certificate was signed by xxxxx.  The 2 things in combination implying that
xxxxx certified that the host anme was that presented next to the lock.

Comment 53

[Gervase Markham [:gerv]]

William: you are absolutely right - the current behaviour is wrong in this case.
Please open a bug about it, and assign it to me.

Gerv

Comment 54

[Bill Gianopoulos [:WG9s]]

I submitted bug 254745.  It will not permit me to assign it to you.

Comment 55

[Ryan Polk (Quark)]

setting fixed-aviary1.0 for bugfixes checked into branch, for searching purposes.
sorry for bugspam.