1195757 - [Control Center] about:cache shows "Connection is not secure"

Status: VERIFIED FIXED

(Firefox :: General, defect, P1)

Description

[Paul Silaghi, QA [:pauly]]

43.0a1 (2015-08-17), Win 7 x64

STR:
Open about:cache

Actual results:
"Connection is not secure"

Expected results:
"This page is stored on your computer" like any other about:about pages

Comment 1

[Tanvi Vyas[:tanvi]]

Discussed in triage.  From my understanding, there are a couple ways that we determine that this is a local file.  One is a whitelist and one is https://dxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#7136.

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

Did a quick check and about:cache seems to resolve to about:cache, not sure why that is.

Comment 3

[Brian Grinstead [:bgrins]]

(In reply to Tim Taubert [:ttaubert] from comment #2)
> Did a quick check and about:cache seems to resolve to about:cache, not sure
> why that is.

about:cache seems to be handled directly in C++ on the platform, so it won't have a file path on disk: https://dxr.mozilla.org/mozilla-central/source/netwerk/protocol/about/nsAboutCache.cpp#76

Comment 4

[:Gijs (he/him)]

Attachment: MozReview Request: Bug 1195757 - treat about:cache as chrome, make checks on about: pages stricter, catch exceptions from NetUtil.jsm, r?ttaubert

Bug 1195757 - fix about:cache security state, r?ttaubert

Comment 5

[:Gijs (he/him)]

This is awful code because about:cache also uses about:cache?foo=bar if you mess with the tickboxes etc., and for the about: protocol we actually don't meaningfully separate out the query string, so I hacked something up that is really terrible, please forgive me. :-\

(also, won't work for e.g. view-source:about:cache, maybe? Not sure if we care about that case or not...)

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8657827 [details]
MozReview Request: Bug 1195757 - treat about:cache as chrome, make checks on about: pages stricter, catch exceptions from NetUtil.jsm, r?ttaubert

https://reviewboard.mozilla.org/r/18429/#review16557

::: browser/base/content/browser.js:7213
(Diff revision 1)
> +    if (this._hardcodedLocalFileURIs.some(localURI => uri.spec.startsWith(localURI))) {
> +      return true;
> +    }

What do you think about simply checking whether the scheme is still "about" after we resolved the URI? That should only ever be true for internal URIs constructed natively. OTOH, it might be easier to just add it to the existing whitelist of internal pages.

Comment 7

[:Gijs (he/him)]

(In reply to Tim Taubert [:ttaubert] from comment #6)
> Comment on attachment 8657827 [details]
> MozReview Request: Bug 1195757 - fix about:cache security state, r?ttaubert
> 
> https://reviewboard.mozilla.org/r/18429/#review16557
> 
> ::: browser/base/content/browser.js:7213
> (Diff revision 1)
> > +    if (this._hardcodedLocalFileURIs.some(localURI => uri.spec.startsWith(localURI))) {
> > +      return true;
> > +    }
> 
> What do you think about simply checking whether the scheme is still "about"
> after we resolved the URI? That should only ever be true for internal URIs
> constructed natively.

Add-ons could implement such a page and get remote data, right? I don't know to what degree we care about not giving the right information in that case, but I guess I would prefer to err on the side of caution because it's security information.

> OTOH, it might be easier to just add it to the
> existing whitelist of internal pages.

I'll do that.

Comment 8

[:Gijs (he/him)]

Comment on attachment 8657827 [details]
MozReview Request: Bug 1195757 - treat about:cache as chrome, make checks on about: pages stricter, catch exceptions from NetUtil.jsm, r?ttaubert

Bug 1195757 - treat about:cache as chrome, make checks on about: pages stricter, catch exceptions from NetUtil.jsm, r?ttaubert

Comment 9

[:Gijs (he/him)]

Filed bug 1203055 about view-source and friends.

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8657827 [details]
MozReview Request: Bug 1195757 - treat about:cache as chrome, make checks on about: pages stricter, catch exceptions from NetUtil.jsm, r?ttaubert

https://reviewboard.mozilla.org/r/18429/#review16683

::: browser/base/content/browser.js:6930
(Diff revision 2)
> -    let whitelist = /^about:(accounts|addons|app-manager|config|crashes|customizing|downloads|healthreport|home|license|newaddon|permissions|preferences|privatebrowsing|rights|sessionrestore|support|welcomeback)/i;
> -    let isChromeUI = uri.schemeIs("about") && whitelist.test(uri.spec);
> +    let whitelist = /^(?:accounts|addons|app-manager|cache|config|crashes|customizing|downloads|healthreport|home|license|newaddon|permissions|preferences|privatebrowsing|rights|sessionrestore|support|welcomeback)(?:[?#]|$)/i;
> +    let isChromeUI = uri.schemeIs("about") && whitelist.test(uri.path);

Thought about doing that too when I last looked at the code, thanks!

Comment 11

[:Gijs (he/him)]

Oops, this dropped off my radar. :-(

Comment 12

[Pulsebot]

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

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/db681cbdc432

Comment 14

[Paul Silaghi, QA [:pauly]]

(In reply to Paul Silaghi, QA [:pauly] from comment #0)
> Expected results:
> "This page is stored on your computer" like any other about:about pages
Control Center on about:cache shows now "This is a secure Nightly page", but I guess that's ok due to comment 3.
Verified fixed FF 43.0a1 (2015-09-15) Win 7.