935753 - Firefox displays the "This is a secure Firefox page" indicator on pages served by addons

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Aaron Loessberg-Zahl]

Attachment: ff_secure.png

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20100101 Firefox/25.0 (Beta/Release)
Build ID: 20131025151332

Steps to reproduce:

I use the IT Tab addon (https://addons.mozilla.org/en-US/firefox/addon/ie-tab-2-ff-36/) to view pages on my corporate intranet using the Internet Explorer rendering engine.


Actual results:

I noticed today after updating to Firefox 25 that these pages displayed an orange Firefox emblem in the URL bar and showed "This is a secure Firefox page" when the emblem was clicked (see attached screenshot).


Expected results:

I do not expect to see this emblem on these pages, since they are in no way under the control of Mozilla.  Even if an addon is rendering the page, it could still contain anything.

Comment 1

[Justin Dolske [:Dolske]]

Erp. Sounds like something is confused, but I thought we had this UI whitelisted to only certain about: pages... I wonder if IE Tab is reusing one inappropriately?

Comment 2

[Aaron Loessberg-Zahl]

More detailed reproduction steps:

1. Open a website (e.g., www.gmail.com) in a new tab.
2. Click the IE Tab toolbar button to switch to the IE rendering engine.
3. Observe the presence of the Firefox address bar badge.

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

We don't have a whitelist, but we should add one. The code that determines if this UI is shown is at http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#6491.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

This is the list of about: pages shown on about:about:

about:
about:about
about:addons
about:app-manager
about:buildconfig
about:cache
about:config
about:crashes
about:credits
about:customizing
about:downloads
about:healthreport
about:home
about:license
about:logo
about:memory
about:mozilla
about:networking
about:newtab
about:permissions
about:plugins
about:preferences
about:privatebrowsing
about:rights
about:robots
about:sessionrestore
about:support
about:sync-log
about:sync-progress
about:sync-tabs
about:telemetry
about:welcomeback

Of the above pages, I think we should limit this identity to pages that a "secure" identifier will benefit the user. For example, about:mozilla doesn't benefit from a positive security signal. I would also like to limit the list to high-profile pages, which would reduce the size of the list to hopefully make it more manageable.

I'd like to limit this identity to the following pages:

about:addons
about:app-manager
about:config
about:crashes
about:healthreport
about:home
about:permissions
about:preferences
about:sessionrestore
about:support
about:welcomeback

(about:newtab is not included here because it is treated as a "blank" tab)

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

Comment 6

[Matthew N. [:MattN]]

Comment on attachment 829355 [details] [diff] [review]
Patch

Review of attachment 829355 [details] [diff] [review]:
-----------------------------------------------------------------

Whitelists make me sad when they get out-of-date, but I can see why we don't want to show this UI for all third-party about:* pages. 

My other idea would be to build a list of all about: pages by default at compile-time (via xpcshell) and then use a blacklist to exclude the smaller number we don't want. This is probably fine for now since we don't add new highly-visible about: pages *that* often.

r=me with consideration for the two additions below and the comment fix.

Sorry for the delay.

::: browser/base/content/browser.js
@@ +6483,3 @@
>  
>      // For some URIs like data: we can't get a host and so can't do
>      // anything useful here. Chrome URIs however get special treatment.

I think you should move "Chrome URIs however get special treatment." above the whitelist and indicate what this list represents (i.e. rationale from comment 4). For example: "Some chrome URIs are whitelisted to provide a positive security signal to the user:"

@@ +6490,5 @@
>  
> +    let chromeWhitelist = ["about:addons", "about:app-manager", "about:config",
> +                           "about:crashes", "about:healthreport", "about:home",
> +                           "about:permissions", "about:preferences", "about:sessionstore",
> +                           "about:support", "about:welcomeback"];

I would probably add "about:newaddon" (one of the ones excluded from about:about[1]) so users understand that it's Firefox asking if they want to install the add-on.

I also wonder about "about:privatebrowsing" since it has a similar role to about:home in that it's the initial page for private browsing mode.

[1] https://mxr.mozilla.org/mozilla-central/ident?i=HIDE_FROM_ABOUTABOUT

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/integration/fx-team/rev/430ebb28dd92

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Backed out for mochitest-bc failures.
https://hg.mozilla.org/integration/fx-team/rev/1fbf3444ee50

https://tbpl.mozilla.org/php/getParsedLog.php?id=30776585&tree=Fx-Team

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

Relanded with test fix and comment, https://hg.mozilla.org/integration/fx-team/rev/6fae9d6feec8

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

And pushed a follow-up because I missed the test comment to update,
https://hg.mozilla.org/integration/fx-team/rev/bf562f4eb546

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6fae9d6feec8
https://hg.mozilla.org/mozilla-central/rev/bf562f4eb546

Comment 12

[Hector Zhao [:hectorz]]

Should this fix be uplifted?