1221771 - On about:accounts, Firefox warns that your login could be compromised.

Status: VERIFIED FIXED

(Firefox :: Firefox Accounts, defect, P1)

Description

[Mark Hammond [:markh] [:mhammond]]

Attachment: aa-login-compromised.png

STR:
* Start a Sync signin flow.
* When on about:accounts, click the globe icon in the URL bar.

Actual:
* Dropdown tells you your login could be compromised.
Expected:
* It doesn't

Comment 1

[:Margaret Leibovic]

This is probably a bug with the login manager code more than anything else.

Comment 2

[Tanvi Vyas[:tanvi]]

Is about:accounts listed as moz-safe-about? There is a bug open on the webconsole warning for about:accounts that probably has some helpful context - https://bugzilla.mozilla.org/show_bug.cgi?id=983326

Comment 3

[Mark Hammond [:markh] [:mhammond]]

As Margaret guessed, the login manager is the source of this (although it doesn't look like a bug). checkIfURIisSecure() in LoginManagerContent.js is returning false for about:accounts due to...

(In reply to Tanvi Vyas [:tanvi] from comment #2)
> Is about:accounts listed as moz-safe-about? There is a bug open on the
> webconsole warning for about:accounts that probably has some helpful context
> - https://bugzilla.mozilla.org/show_bug.cgi?id=983326

about:accounts *does not* have URI_SAFE_TO_LOAD_IN_SECURE_CONTEXT, because, as mentioned in bug 983326, that would mean it loses chrome permissions (and indeed, adding that flag prevent aboutAccounts.js from referencing Components, and thus it can't import anything or do what it needs to do.) So adding that flag isn't the simple fix.

I'm not sure where this leave us.

Comment 4

[Tanvi Vyas[:tanvi]]

I think we need to revisit the URI flags we use here:
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/passwordmgr/LoginManagerContent.jsm#1105

This method will also undergo changes in https://bugzilla.mozilla.org/show_bug.cgi?id=1217133.

Comment 5

[Chris Karlof [:ckarlof]]

IIUC, this doesn't affect Fx44. I can only repro in Fx45, and needs to be fixed for Fx45.

Comment 6

[Tanvi Vyas[:tanvi]]

(In reply to Chris Karlof [:ckarlof] from comment #5)
> IIUC, this doesn't affect Fx44. I can only repro in Fx45, and needs to be
> fixed for Fx45.

Right now the pref for insecure passwords is only set in Nightly builds, and hence the inability to reproduce on FF 44.  This bug blocks https://bugzilla.mozilla.org/show_bug.cgi?id=1221206 to turn on the warnings in dev edition.

Comment 7

[johngraciliano]

(Excuse my ignorance about Firefox internal processes, this commentary is from a theme developer.)  
I noticed the default theme fixes this problem for Firefox Developer Edition 44.0a2 with a change in chrome://browser/skin/controlcenter/panel.css, lines 7-9 from
  /* Hide all conditional elements by default. */
  :-moz-any([when-connection],[when-mixedcontent],[when-ciphers]) {
    display: none;
  }
to
  /* Hide all conditional elements by default. */
  :-moz-any([when-connection],[when-mixedcontent],[when-ciphers],[when-loginforms]) {
    display: none;
  }
Regarding this, I also found new rules in lines 32-38:
  /* Hide redundant messages based on insecure login forms presence. */
  #identity-popup[loginforms=secure] [and-when-loginforms=insecure] {
    display: none;
  }
  #identity-popup[loginforms=insecure] [and-when-loginforms=secure] {
    display: none;
  }
I STRONGLY BELIEF THEY ARE WRONG.  They should read:
  /* Hide redundant messages based on insecure login forms presence. */
  #identity-popup[loginforms=secure] [when-loginforms=insecure] {
    display: none;
  }
  #identity-popup[loginforms=insecure] [when-loginforms=secure] {
    display: none;
  }
(i.e., remove "and-").
The problem in Nightly 45.0a1 is more severe because loginforms changes from secure to insecure after the page loads.  This is real puzzling to me.  If I can trust whatever changed loginforms to insecure, then I cannot trust Firefox security.  I should be able to simply change, as I am doing in my theme:
  /* Hide redundant messages based on insecure login forms presence. */
  #identity-popup[loginforms=secure] [when-loginforms=insecure] {
    display: none;
  }
to
  /* Hide redundant messages based on insecure login forms presence. */
  #identity-popup:-moz-any([loginforms=secure],[connection=chrome]) [when-loginforms=insecure] {
    display: none;
  }
because if #identity-popup[connection=chrome] then Firefox states:
  This is a secure Nightly page.
or is is not?!?!
The remaining issue is to fix the background image for #identity-popup-security-content but that is done by moving the rules for background image with [connection=chrome] below the rules with [loginforms=insecure] so they take precedence.

Yet I still ask: Can I just fix this in my theme trusting chrome connections are safe while the bug is properly fixed elsewhere (perhaps in LoginManagerContent.jsm)?  After all, Chrome connections must to be secure. (Unless I am using highly experimental code.)

Comment 8

[johngraciliano]

I forgot that line #32 stating:
  /* Hide redundant messages based on insecure login forms presence. */
should really say:
  /* Hide contradictory messages based on insecure login forms presence. */
But this is only to make the code readable.
--johngraciliano

Comment 9

[johngraciliano]

I am sorry, I spoke out of ignorance in my previous two comments (#7 and #8), I wish I could just delete them.  Yet, I am temporarily fixing the bug in my own theme by adding into panel.css the rule:

  /* Hide contradictory messages based on insecure login forms presence. */
  #identity-popup[connection=chrome] [when-loginforms=insecure] {
    display: none;
  }

Comment 10

[Tanvi Vyas[:tanvi]]

This issue gets fixed with bug 1217766, which is on fx-team and if all goes well will merge to central soon.

Comment 11

[:Paolo Amadini]

Marking as resolved so it can be verified.

Comment 12

[Paul Silaghi, QA [:pauly]]

Verified fixed FF 46.0a2 (2016-01-25) Win 7