1320557 - No security information displayed for a specific website, using a specific Firefox profile

Status: VERIFIED FIXED

(Firefox :: Site Identity, defect, P1)

Description

[dhakir]

Attachment: firefox-bug-paypal.png

User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:50.0) Gecko/20100101 Firefox/50.0
Build ID: 20161114215630

Steps to reproduce:

1. Went to https://www.paypal.com
2. Clicked on the green padlock on the URL bar (next to it, it correctly displays "PayPal, Inc. (US)"), but nothing was displayed.
3. Clicked on Tools -> Page Info -> Security tab, but nothing is displayed (as indicated in the attached screenshot). Clicking on any of the buttons (View Certificate, View Cookies, and View Saved Passwords) does nothing (for this website, works for others).
4. Disabled all extensions and add-ons and tried again, no success.
5. Cleared all cookies for the paypal domain, still nothing.
6. Opened a Private Window from the same profile, still nothing.
7. Used a different profile, information is displayed normally (both next to the padlock, and on the Page Info tab, with website identity, privacy, technical details, etc.).



Actual results:

No information is displayed when clicking on the green padlock for that website, or on the Security tab of the Page Info window.

It only happens for this specific website (from all those I've tried, with https), and only on one specific profile (my main one).

The reason I'm filling the bug is because PayPal is a security-sensitive website, so I'd like to know if my profile could be corrupted or affected by some malware.

If there is a way to clean my profile other than starting from scratch, would gladly like to know. Otherwise, if there is a way to obtain debugging information, that would help.


Expected results:

Security information should be displayed normally in both profiles, or some error message should help explain why it can't be shown.

Comment 1

[YF (Yang)]

Try https://developer.mozilla.org/en-US/docs/Tools/Browser_Console, maybe there are some related errors.

Comment 2

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Maybe try clearing your cache? (History -> Clear Recent History -> Cache)

Comment 3

[dhakir]

Thank you for the tips. I tried clearing the cache and restarting Firefox, didn't change anything.

However, the Browser Console does show the following error whenever I click the green padlock next to the URL bar:

Error: unknown permission state

this.SitePermissions.getStateLabel resource://app/modules/SitePermissions.jsm:179:15
	gIdentityHandler._createPermissionItem chrome://browser/content/browser.js:7320:30
	gIdentityHandler.updateSitePermissions chrome://browser/content/browser.js:7297:18
	refreshIdentityPopup chrome://browser/content/browser.js:7175:5
	gIdentityHandler.handleIdentityButtonEvent chrome://browser/content/browser.js:7232:5
	onclick chrome://browser/content/browser.xul:1:1

This message does not appear on my other profile, or on other HTTPS websites I tried.

Googling 'firefox "Error: unknown permission state"' returns 2 results only, which are both copies of the SitePermissions.jsm source, so I still don't know much about it.

Comment 4

[dhakir]

Well, actually it's happening on several websites: humblebundle.com, google.com, youtube.com, always the same error message as before. But it does NOT happen on most websites: mail.google.com, play.google.com, bugzilla.mozilla.com, amazon.com, gog.com, my banking website, etc. all work fine. I don't see the logic here. Again, using a different profile, I never get any errors.

It must really be something in my profile, so if there's a way to "clean" it other than erasing it entirely, I could try that. It's an old profile that has been created months (if not years) ago. Maybe it has some old certificates that were revoked later?

Comment 5

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Maybe try History -> Clear Recent History -> Site Preferences? It looks like there's a permission entry (or more than one, actually) with an unexpected state in your profile, and the implementation doesn't handle that gracefully.

Comment 6

[dhakir]

Indeed, that did it! I'm sorry, I thought I had already cleaned everything, including Offline Website Data, but I missed the very last one.

Comment 7

[Johann Hofmann [:johannh]]

Very interesting, that error is thrown under the assumption that there will never be an unknown permission state. (What would I give for some compile-time verification). If this is actually happening to people we should probably handle this error more gracefully in the permission UI.

Might be something for a contributor to pick up :)

Comment 8

[Matthew N. [:MattN]]

Since I believe this is a regression I think it should be fixed sooner rather than later. At a minimum, an error in the permission pane shouldn't break the security pane but it's probably easiest to just fix this case in SitePermissions.jsm.

Comment 9

[Johann Hofmann [:johannh]]

We should find a way to uplift this to 52 without getting merge conflicts around the Elm branch changes that were merged into central at 53.

Comment 10

[Johann Hofmann [:johannh]]

Attachment: Bug 1320557 - Prevent broken UI from invalid permission states.

Review commit: https://reviewboard.mozilla.org/r/102866/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/102866/

Comment 11

[Johann Hofmann [:johannh]]

After looking into this I don't think it's a regression, as in, we did not recently introduce an incompatibility to invalid permission states. These states are breaking the UI since it was introduced in 2007 (bug 339102).

In fact, I'm really hesitant to make this change. This patch doesn't magically make unknown states "work", they're still lacking a label and are not in the list of available states, which means they don't get their own radio button in the permission section. It just prevents the pageinfo dialog from "freezing".

I don't see a better alternative, though. This state has very likely been introduced by an addon and we have to make it work to prevent users from not being able to access the security information at all.

Comment 12

[:Gijs (he/him)]

Comment on attachment 8824345 [details]
Bug 1320557 - Prevent broken UI from invalid permission states.

https://reviewboard.mozilla.org/r/102866/#review103356

::: browser/modules/SitePermissions.jsm:189
(Diff revision 1)
>        case this.SESSION:
>          return gStringBundle.GetStringFromName("allowForSession");
>        case this.BLOCK:
>          return gStringBundle.GetStringFromName("block");
>        default:
> -        throw new Error("unknown permission state");
> +        return null;

It feels like this should produce a fallback value like `this.UNKNOWN` instead?

Comment 13

[Johann Hofmann [:johannh]]

(In reply to :Gijs from comment #12)
> Comment on attachment 8824345 [details]
> Bug 1320557 - Prevent broken UI from invalid permission states.
> 
> https://reviewboard.mozilla.org/r/102866/#review103356
> 
> ::: browser/modules/SitePermissions.jsm:189
> (Diff revision 1)
> >        case this.SESSION:
> >          return gStringBundle.GetStringFromName("allowForSession");
> >        case this.BLOCK:
> >          return gStringBundle.GetStringFromName("block");
> >        default:
> > -        throw new Error("unknown permission state");
> > +        return null;
> 
> It feels like this should produce a fallback value like `this.UNKNOWN`
> instead?

Since the function returns a string we'd have to add a new string like "Unknown state", which comes with copy discussions and not being able to uplift. Right now returning null means the control center displays no state label (see attachment). I'm not sure if "Unknown state" would be much better than that. Also, yeah, this should never really happen unless addons.

Thanks for the review! :)

Comment 14

[Johann Hofmann [:johannh]]

Attachment: Screen Shot 2017-01-06 at 12.34.07.png

Comment 15

[:Gijs (he/him)]

Hm, OK. I'm also confused about how the permissions manager is returning stuff we don't know about. :-\

Comment 16

[Johann Hofmann [:johannh]]

Yeah, Services.perms.add should throw if you try to add an invalid permission state, really. I wonder if there's some specific reason why it doesn't.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d0f0fbb66c71
Prevent broken UI from invalid permission states. r=Gijs

Comment 18

[:Gijs (he/him)]

(In reply to Johann Hofmann [:johannh] from comment #16)
> Yeah, Services.perms.add should throw if you try to add an invalid
> permission state, really. I wonder if there's some specific reason why it
> doesn't.

File a bug and see if we can find out? :-)

Comment 19

[Ryan VanderMeulen [:RyanVM]]

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

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Seems pretty low-risk, worth uplifting to Aurora/Beta? If so, please nominate accordingly :)

Comment 22

[Johann Hofmann [:johannh]]

Comment on attachment 8824345 [details]
Bug 1320557 - Prevent broken UI from invalid permission states.

Approval Request Comment
[Feature/Bug causing the regression]: None, the problem was probably introduced by an addon.
[User impact if declined]: Security information such as site permissions and certificate information are permanently unavailable for certain sites (even the control center is inaccessible, see bug 1328195
[Is this code covered by automated tests?]: No, the resulting UI is still broken (because it lacks data), only a bit less severe. Not sure if we should test that.
[Has the fix been verified in Nightly?]: Not yet.
[Needs manual test from QE? If yes, steps to reproduce]: I don't think so.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No.
[Why is the change risky/not risky?]: It's a very small patch and it changes code that would throw an exception under normal circumstances to be a bit more forgiving.
[String changes made/needed]: None

Comment 23

[Julien Cristau [:jcristau]]

Comment on attachment 8824345 [details]
Bug 1320557 - Prevent broken UI from invalid permission states.

defend against unexpected permission states, aurora52+

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/2f4669bb4054

Comment 26

[Gerry Chang [:gchang]]

Comment on attachment 8824345 [details]
Bug 1320557 - Prevent broken UI from invalid permission states.

Fix a regression. Beta51+. Should be in 51 beta 14.

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/8c50e605ccd4

Comment 28

[Johann Hofmann [:johannh]]

(In reply to :Gijs from comment #18)
> (In reply to Johann Hofmann [:johannh] from comment #16)
> > Yeah, Services.perms.add should throw if you try to add an invalid
> > permission state, really. I wonder if there's some specific reason why it
> > doesn't.
> 
> File a bug and see if we can find out? :-)

So after looking into this for a bit I'm not sure this is a good idea anymore. As I found out in my conversation in bug 1328195 (and a Reddit thread that I can't find anymore) there's at least one popular add-on adding invalid permission states to the permission manager. We solved the UI issues around that to the point where it's not necessary to break add-on compatibility just for the sake of it. This would have been great to have from the start.

Comment 29

[Bogdan Maris, Desktop QA]

Hi Johann, does this bug need manual QA attention for verification? If yes can you please provide some guidelines? I tried to reproduce the issue described in comment 0 on old builds but with no success.

Comment 30

[Johann Hofmann [:johannh]]

Hey Bodgan,

you can presumably trigger the problem by using Self Destructing Cookies (https://addons.mozilla.org/en-US/firefox/addon/self-destructing-cookies/?src=userprofile). I haven't tried it myself but I know that this addon is responsible for at least some of the breakage.

It would be nice to verify that this is still breaking on release (50) but not on beta anymore.

Comment 31

[Bogdan Maris, Desktop QA]

(In reply to Johann Hofmann [:johannh] from comment #30)
> Hey Bodgan,
> 
> you can presumably trigger the problem by using Self Destructing Cookies
> (https://addons.mozilla.org/en-US/firefox/addon/self-destructing-cookies/
> ?src=userprofile). I haven't tried it myself but I know that this addon is
> responsible for at least some of the breakage.
> 
> It would be nice to verify that this is still breaking on release (50) but
> not on beta anymore.

I installed the add-on that you mentioned on Fx 50.0 RC, but I could not reproduce the initial problem though (I spent at least 1 hour on this). 
Do you have any other ideas on how I could reproduce?

Comment 32

[Johann Hofmann [:johannh]]

(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #31)
> (In reply to Johann Hofmann [:johannh] from comment #30)
> > Hey Bodgan,
> > 
> > you can presumably trigger the problem by using Self Destructing Cookies
> > (https://addons.mozilla.org/en-US/firefox/addon/self-destructing-cookies/
> > ?src=userprofile). I haven't tried it myself but I know that this addon is
> > responsible for at least some of the breakage.
> > 
> > It would be nice to verify that this is still breaking on release (50) but
> > not on beta anymore.
> 
> I installed the add-on that you mentioned on Fx 50.0 RC, but I could not
> reproduce the initial problem though (I spent at least 1 hour on this). 
> Do you have any other ideas on how I could reproduce?

I could reproduce the problem by downloading, going to twitter.com and selecting "Never" in the addon popup in the upper right corner.

Comment 33

[Bogdan Maris, Desktop QA]

(In reply to Johann Hofmann [:johannh] from comment #32)
> (In reply to Bogdan Maris, QA [:bogdan_maris] from comment #31)
> > (In reply to Johann Hofmann [:johannh] from comment #30)
> > > Hey Bodgan,
> > > 
> > > you can presumably trigger the problem by using Self Destructing Cookies
> > > (https://addons.mozilla.org/en-US/firefox/addon/self-destructing-cookies/
> > > ?src=userprofile). I haven't tried it myself but I know that this addon is
> > > responsible for at least some of the breakage.
> > > 
> > > It would be nice to verify that this is still breaking on release (50) but
> > > not on beta anymore.
> > 
> > I installed the add-on that you mentioned on Fx 50.0 RC, but I could not
> > reproduce the initial problem though (I spent at least 1 hour on this). 
> > Do you have any other ideas on how I could reproduce?
> 
> I could reproduce the problem by downloading, going to twitter.com and
> selecting "Never" in the addon popup in the upper right corner.

This works \o/. 
I could also reproduce on Fx 50 with these steps, I verified that using Fx 51.0 RC it does not!

Comment 34

[Johann Hofmann [:johannh]]

Awesome!