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

Status: RESOLVED FIXED

(Firefox :: Site Identity, defect, P3)

Description

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

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

Comment 1

[Darkspirit]

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.

Comment 2

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

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.

Comment 3

[Johann Hofmann [:johannh]]

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.

Comment 4

[Johann Hofmann [:johannh]]

Morphing this bug if you don't mind :)

Comment 5

[(no longer active)]

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...

Comment 6

[Johann Hofmann [:johannh]]

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?

Comment 8

[Hannah Peuckmann]

Attachment: Bug 1567443 - Added flag for indicating secure Chrome UI in nsIAboutModule. r=#anti-tracking-reviewers

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/177e6f28d348
Using nsIAboutModule flag instead of regex to indicate secure about page. r=pbz,webcompat-reviewers,twisniewski

Comment 10

[Cristian Tuns]

Backed out for causing mochitest failures on browser_check_identity_state.js

• Backout link
• Push with failures
• Failure Log

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1226e02d41d8
Added flag for indicating secure Chrome UI in nsIAboutModule. r=pbz,webcompat-reviewers,twisniewski

Comment 12

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/1226e02d41d8