Closed Bug 1567443 Opened 5 years ago Closed 3 years ago

Move secure chrome UI indicators into nsIAboutModule and add more items to the list

Categories

(Firefox :: Site Identity, defect, P3)

70 Branch
x86_64
Windows 7
defect

Tracking

()

RESOLVED FIXED
95 Branch
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox68 --- unaffected
firefox69 --- unaffected
firefox70 --- wontfix
firefox95 --- fixed

People

(Reporter: Virtual, Assigned: h.sofie.p)

References

(Blocks 1 open bug)

Details

(5 keywords)

Attachments

(1 file)

  1. Open one of these affected internal Mozilla Firefox pages:
  • about:about
  • about:buildconfig
  • about:certificate
  • about:checkerboard
  • about:compat
  • about:debugging
  • about:devtools
  • about:library
  • about:logo
  • about:memory
  • about:mozilla
  • about:networking
  • about:performance
  • about:plugins
  • about:policies
  • about:privatebrowsing
  • about:profiles
  • about:protections
  • about:robots
  • about:serviceworkers
  • about:studies
  • about:sync-log
  • about:tabcrashed
  • about:telemetry
  • about:url-classifier
  • about:webrtc
  • can be more others
    and see that "Tracking Protections" is available on these internal Mozilla Firefox pages,
    while it shouldn't, as it doesn't even works there.

Not affected internal Mozilla Firefox pages:

  • about:addons
  • about:cache
  • about:config
  • about:crashes
  • about:downloads
  • about:license
  • about:logins
  • about:preferences
  • about:rights
  • about:support
  • can be more others

To me, this sounds much like bug 1365552.

(:Gijs from bug 1365552 comment 4)

No. Nononononoo. Very much no. about:blank and about:reader and various other pages routinely contain web content that is untrusted.

Are there cases where TP should be internally active, but not visible to the user?

Since bug 1567300 about:protections is shown as secure internal page.

(:Gijs from bug 1365552 comment 1)

This was an intentional decision in bug 1051847. For many of those pages, that still seems right - about: and about:about and about:logo and so on aren't user-exposed and so we shouldn't put the chrome icon up there.

I'm not convinced by the "it's not user-exposed" argument, but by "routinely contain web content".
I would be happy to make a patch to add at least about:mozilla, about:plugins, etc. to secureInternalUIWhitelist as well, if I get green light.

Most of these internal Mozilla Firefox pages don't contain web content in any way and lacks marking as secure internal Mozilla Firefox pages.
What's more, even about:addons/Add-ons Manager is marked as secure Mozilla Firefox internal page, but in "Recommendations" tab there is web content, so this claim is already kinda inconsistent.
And first of all, it's on Mozilla side to provide secure trusted web content to users on its own internal Mozilla Firefox pages.
Also Chromium (UnGoogled-Chromium version in my case) has all its own internal pages marked as secure Chromium pages.

So in end I'm also not convinced too and like you I also reported this in past.

But this bug is mainly about useless "Tracking Protections" button, which is available on internal Mozilla Firefox pages.

Ugh, yeah, so, I think a refactoring of this is really overdue. The secure chrome UI should just be an nsIAboutModule flag. This way we would get rid of that weird allowlist regex and it would enable folks who are adding new pages to make the choice right when they add the registration.

I also think we should expand our list of chrome UI pages, at least to stuff like about:buildconfig, about:performance, etc.

Morphing this bug if you don't mind :)

No longer blocks: protections-panel
Priority: -- → P3
Summary: "Tracking Protections" is available on internal Mozilla Firefox pages → Move secure chrome UI indicators into nsIAboutModule and add more items to the list

Now that our about pages have CSP rules, is there any reason why any page which has a CSP which amounts to restricting it to run only chrome code to not be on the allow list?

In other words, it seems better to build the UI based on the actual security properties of the page, rather than a manifest of declared expectations of their security, and have a test that enforces our expectations, to catch us when the truth gets out of sync with our expectations...

I think this is more of a design feature and less of an actual indicator of "there's only chrome code running on this page". Also we don't want to have this on about:blank and any other linkable page for obvious reasons. How about giving this out to all about: pages by default and adding a flag to explicitly opt out?

Blocks: 1702757
Assignee: nobody → hpeuckmann
Status: NEW → ASSIGNED
Attachment #9245446 - Attachment description: Bug 1567443 - Added secure_chrome_ui flag to nsIAboutModule. r=#anti-tracking-reviewers → Bug 1567443 - added check for about page, fixed test browser_identityIcon_img_url. r=#anti-tracking-reviewers
Attachment #9245446 - Attachment description: Bug 1567443 - added check for about page, fixed test browser_identityIcon_img_url. r=#anti-tracking-reviewers → Bug 1567443 - flagged more about pages as secure, added test for secure chromeUI. r=#anti-tracking-reviewers
Attachment #9245446 - Attachment description: Bug 1567443 - flagged more about pages as secure, added test for secure chromeUI. r=#anti-tracking-reviewers → Bug 1567443 - Using nsIAboutModule flag instead of regex to indicate secure about page. r=#anti-tracking-reviewers
Pushed by pzuhlcke@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/177e6f28d348 Using nsIAboutModule flag instead of regex to indicate secure about page. r=pbz,webcompat-reviewers,twisniewski

Backed out for causing mochitest failures on browser_check_identity_state.js

Flags: needinfo?(hpeuckmann)
Attachment #9245446 - Attachment description: Bug 1567443 - Using nsIAboutModule flag instead of regex to indicate secure about page. r=#anti-tracking-reviewers → Bug 1567443 - removed about debugging from test for causing problems. r=#anti-tracking-reviewers
Attachment #9245446 - Attachment description: Bug 1567443 - removed about debugging from test for causing problems. r=#anti-tracking-reviewers → Bug 1567443 - Added flag for indicating secure Chrome UI in nsIAboutModule. r=#anti-tracking-reviewers
Pushed by pzuhlcke@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1226e02d41d8 Added flag for indicating secure Chrome UI in nsIAboutModule. r=pbz,webcompat-reviewers,twisniewski
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 95 Branch
Flags: needinfo?(hpeuckmann)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: