1179961 - Use a lock with a strikethrough for HTTP pages that have Password Fields in the Control Center

Status: VERIFIED FIXED

(Firefox :: Security, defect, P1)

Description

[Tanvi Vyas[:tanvi]]

HTTP websites have no business asking for passwords.  The passwords can be intercepted by man-in-the-middle attackers and eavesdroppers.  Given that password reuse is very prevalent, when an HTTP website asks for a password, it is compromising the users security on its site and any other site where the user shares the same password.

When we detect that an HTTP page has a password field, we should show a grey lock with a strike through instead of the grey globe.  If the user clicks the grey globe we can add text to the Control Center.

This is somewhat related to bug https://bugzilla.mozilla.org/show_bug.cgi?id=748193 but not exactly.  bug 748193 is about adding a doorhanger.  This bug changing the existing globe icon without popping anything up to the user automatically.

This also moves us a step further in the HTTP deprecation plan https://blog.mozilla.org/security/2015/04/30/deprecating-non-secure-http/

Comment 1

[Tanvi Vyas[:tanvi]]

This is actually not too hard to implement.

We can set a webprogresslistener flag in https://mxr.mozilla.org/mozilla-central/source/toolkit/components/passwordmgr/InsecurePasswordUtils.jsm#127.  Then we can check it in browser.js:checkIdentity() https://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#6732 and set the appropriate mode.  With a few more changes in browser.js, the mode will translate to the strikethrough icon.  We can also add specific text to http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/browser.properties#328 that can be used in the control center to educate users who want to better understand the risks.  And we can also add a learn more link that points to an MDN or SUMO page.

Comment 2

[Ian Macdonald]

As I have requested previously, please provide evidence of man-in-the-middle attacks being a statistically significant route to password theft. All the evidence available on security-related websites that I have visited, indicates that compromised servers or desktops are the key concern. 

The problem I see is that telling an inexperienced coder that the security risk to a password has been removed by implementing https, is dangerous misinformation. It may lead to developers assuming that they no longer need to implement password protection in their software.

Comment 3

[Jesse Ruderman]

A lock strikethrough is hard to notice and easy to ignore, judging by the mixed success of mixed content warnings. And it's irrelevant if the page contains a login form that you weren't planning on using. (For example, reddit has a login form at the top of every page, although they are https-only now.)

Another possibility is to show a warning when you click the login form. Firefox could even check whether the site supports HTTPS, and suggest navigating there to log in. The warning could be in the form of an autocomplete dropdown entry (along with HTTP saved passwords, if any).

I'm not sure how that would work if you already have a password saved, since in that case you don't have to click the username or password field. I guess we could disable 0-click autofill on HTTP sites. That would also mitigate the attack where a MITM silently slurps *all* your HTTP passwords by quickly navigating to each HTTP site.

Comment 4

[Tanvi Vyas[:tanvi]]

(In reply to Ian Macdonald from comment #2)
> As I have requested previously, please provide evidence of man-in-the-middle
> attacks being a statistically significant route to password theft. All the
> evidence available on security-related websites that I have visited,
> indicates that compromised servers or desktops are the key concern. 
> 
I don't have such statistics and don't know how I would obtain them.  Password sent in the clear over the internet are leaked and readable by eavesdroppers.  I'm not going to try and get into the business of quantifying security risk.  Compromised servers and desktops are huge security issues and other security measures should be taken to protect those machines (ex: Firefox requires click to play for vulnerable versions of plugins).

> The problem I see is that telling an inexperienced coder that the security
> risk to a password has been removed by implementing https, is dangerous
> misinformation. It may lead to developers assuming that they no longer need
> to implement password protection in their software.
All that using https does is protect passwords in transit.  Its up to the developer to do the rest on their servers.  There will be a "learn more" page that will explain this.

Comment 5

[Tanvi Vyas[:tanvi]]

(In reply to Jesse Ruderman from comment #3)
> A lock strikethrough is hard to notice and easy to ignore, judging by the
> mixed success of mixed content warnings. And it's irrelevant if the page
> contains a login form that you weren't planning on using. (For example,
> reddit has a login form at the top of every page, although they are
> https-only now.)
> 
Yes, I know.  There will be cases where the warning is irrelevant to a user who never intends to login.

> Another possibility is to show a warning when you click the login form.
> Firefox could even check whether the site supports HTTPS, and suggest
> navigating there to log in. The warning could be in the form of an
> autocomplete dropdown entry (along with HTTP saved passwords, if any).
> 
This is bug https://bugzilla.mozilla.org/show_bug.cgi?id=748193, which we could do and land in developer edition as a start.

> I'm not sure how that would work if you already have a password saved, since
> in that case you don't have to click the username or password field. I guess
> we could disable 0-click autofill on HTTP sites. That would also mitigate
> the attack where a MITM silently slurps *all* your HTTP passwords by quickly
> navigating to each HTTP site.
That is bug https://bugzilla.mozilla.org/show_bug.cgi?id=1118511.  Maybe with this bug, that bug will get the nudge that it needs and we will get sign off to remove autofill for HTTP pages.

Note that I won't have much time to work on these this quarter.

Comment 6

[Ian Macdonald]

(In reply to Tanvi Vyas [:tanvi] from comment #4)
> I don't have such statistics and don't know how I would obtain them... 
> I'm not going to try and get into the business of quantifying security risk.

Donning my accountant's hat... 

Then, since the costs of special hosting and signed certificates for every small forum or whatever that uses a login would certainly extend into many millions, possibly billions of dollars globally, I think you need to provide more than a bare hypothesis as justification. Where is your empirical, quantitative evidence to the effect that the risk justifies the expenditure? 

Bear in mind also that the hypothetical risk would ONLY exist where no password hashing is used, thus we are talking about mitigating the effects a small subset of poorly-written software, by way of forcing additional costs upon ALL forum webmasters.

Comment 7

[Ash Grigas]

Note - the strikethrough is already built on this bug https://bugzilla.mozilla.org/show_bug.cgi?id=834830 by Tim Taubert for mixed content design updates. The new icon and strikethrough also applying to pages over http with a password field would reuse his implementation.

Comment 8

[Tanvi Vyas[:tanvi]]

(In reply to Ian Macdonald from comment #6)
> (In reply to Tanvi Vyas [:tanvi] from comment #4)
> > I don't have such statistics and don't know how I would obtain them... 
> > I'm not going to try and get into the business of quantifying security risk.
> 
> Donning my accountant's hat... 
> 
> Then, since the costs of special hosting and signed certificates for every
> small forum or whatever that uses a login would certainly extend into many
> millions, possibly billions of dollars globally, I think you need to provide
> more than a bare hypothesis as justification. Where is your empirical,
> quantitative evidence to the effect that the risk justifies the expenditure? 
> 
Let's Encrypt plans to make the certificate cost free - https://letsencrypt.org/

> Bear in mind also that the hypothetical risk would ONLY exist where no
> password hashing is used, thus we are talking about mitigating the effects a
> small subset of poorly-written software, by way of forcing additional costs
> upon ALL forum webmasters.
This is not true. Client side or javascript hashing won't prevent a man-in-the-middle from extracting the password the user types into an HTTP website.

https://developer.mozilla.org/en-US/docs/Web/Security/Insecure_passwords

Comment 9

[Ian Macdonald]

A free cert would be a help, but then you still have the issue that most hosting services charge extra for https or storing certs. 

> This is not true. Client side or javascript hashing won't prevent
> a man-in-the-middle from extracting the password the user types into an HTTP website.

Please explain. I don't see how that can be the case. If it's encrypted.. it's encrypted. 

The big advantage of client-side encryption is that it also protects against Man At The Receiving End (or Robot At The Receiving End) attacks.

Comment 10

[Zack Weinberg (:zwol)]

(In reply to Ian Macdonald from comment #9)
> > This is not true. Client side or javascript hashing won't prevent
> > a man-in-the-middle from extracting the password the user types into an HTTP website.
> 
> Please explain. I don't see how that can be the case. If it's encrypted..
> it's encrypted.

If password-encryption code is sent to the client over unencrypted HTTP, the man in the middle can modify it to leak the password.

Even if the man in the middle doesn't modify the code, they can learn the session cookie, which is good enough to impersonate the user at least for a little while.

(Wouldn't it be nice if client-to-server authentication weren't all built around passwords and bearer tokens?)

Comment 11

[Ian Macdonald]

(In reply to Zack Weinberg (:zwol) from comment #10)
> If password-encryption code is sent to the client over unencrypted HTTP, the
> man in the middle can modify it to leak the password.

So what you're saying is that a MITM could strip the encryption/hashing component from the webpage data, such that the client would then inadvertently send the password in plaintext?  

I guess that is theoretically possible, although proof of concept would be needed. How would the MITM identify the code alterations to make, bearing in mind these would differ between sites? 

As for session cookies having vulnerabilites, well, that is really a case of poor Web standards leading to **** security. Speaking of which, one of the worst issues here is that of automatic session restore gizmos creating a situation where if the browser crashes or the user forgets to log out of a website, a malicious person seeing this happen can simply restart the browser and impersonate the previous logon. 

> (Wouldn't it be nice if client-to-server authentication weren't all built
> around passwords and bearer tokens?)

I agree completely. Web auth is an unmitigated disaster area. Why the hell-in-blazes, in 2015, do we have to use a platform which is intrinsically incapable of doing these things properly and securely, such that the app coder has to use all kinds of bolt-ons to stand-in for the missing bits? To anyone used to coding for auth in a LAN environment, it comes as a shock to see just how 1980's it is.

Comment 12

[Jesse Ruderman]

General discussion of HTTP deprecation is off-topic for this bug. Try https://groups.google.com/d/msg/mozilla.dev.platform/xaGffxAM-hs/9JiStoGhHY0J instead.

Comment 13

[Tanvi Vyas[:tanvi]]

Assume http://foo.com/login.html has a form with a password field.

The user goes to foo.com and clicks login.  Their browser makes a network request to http://foo.com/login.html.  Ideally, that request is sent to their router, then to the ISP, through the internet to the foo.com server.  The foo.com server then attempts to send a response back to the browser, with html that includes a form and password field.  The man-in-the-middle can intercept the response back to the browser and change it so that the password the user enters is extracted and sent to their server in the background.  The attacker can modify or completely replace the html in many ways without the user even realizing something has gone wrong.  Let's assume they add javascript to the page that collects the users keystrokes into the password field and then sends those keystrokes to the attacker's server.  Now the attacker has the users actual password, regardless of whether http://foo.com employed client or server side hashing.

Now assume that an attacker doesn't know how to conduct a man-in-the-middle attack and just does passive eavesdropping.  If http://foo.com/login.html submits the password over HTTP (without any client side hashing techniques), the password can be read in the clear through any of the hops from the browser to the foo.com server.

This is why passwords should never be requested on HTTP pages and why we should put a lock with a strikethrough on HTTP pages that ask for passwords.  It communicates to the user that it is not safe to enter a sensitive password on this website.

Comment 14

[Ian Macdonald]

Tanvi, What you are describing is essentially the mechanism by which compromised webservers harvest passwords. Send a popup to the client telling the user that their login has expired, and demand a re-login. Since the login hasn't actually expired there is no need for  the popup to do anything other than leak the password, and the user continues browsing unaware of what's just happened.

Although this could be implemented as a MITM attack, far easier to do so by hacking the host server. Particularly as https offers NO protection in the latter case. 

As a suggestion, perhaps better to implement a new password field with a different type, perhaps 'passwordx' and once established, deprecate the 'password' field type and post a big red warning if one is ever encountered. The 'passwordx' fieldtype would provide a basic degree of  encryption/hashing. Its functionality could perhaps be extended to allow for a salt to be set by Javascript or in css, for example, or even up to a full public key exchange. Bottom line is that it would not be possible to configure it to send plaintext, so all the hacker gets is a symbolic representation of the password which is probably of no use on other websites.  

This would counter both risks instead of just the one.

Comment 15

[:Paolo Amadini]

Attachment: Login form present on HTTP page

I'm uploading a few screenshots of the new Control Center states when insecure login forms are present, because the top level page is HTTP or because active mixed content is loaded.

Comment 16

[:Paolo Amadini]

Attachment: Login form present on HTTP page, details view

Comment 17

[:Paolo Amadini]

Attachment: Login form present on HTTP subframe of HTTPS page

Comment 18

[:Paolo Amadini]

Attachment: Login form present on HTTP subframe of HTTPS page, details view

Comment 19

[:Paolo Amadini]

All cases use the two strings already reviewed from bug 1193339 comment 5.

The Mixed Active Content Loaded case was not in the UX bug. Analogously to how the pure HTTP case was specificed, and to avoid a very verbose panel, in the Mixed Active Content Loaded case I've also had the insecure logins form message replace the generic message that indicated that various kind of data could be compromised.

Comment 20

[:Paolo Amadini]

I'll post the current version for code review, we may want tests though.

This is based on patches in bug 1193341, bug 1204486, and bug 1205715.

Comment 21

[:Paolo Amadini]

Attachment: MozReview Request: Bug 1179961 - Use a lock with a strikethrough for HTTP pages that have password fields in the Control Center. r=ttaubert,bgrins

Bug 1179961 - Use a lock with a strikethrough for HTTP pages that have password fields in the Control Center. r=ttaubert

Comment 22

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8662455 [details]
MozReview Request: Bug 1179961 - Use a lock with a strikethrough for HTTP pages that have password fields in the Control Center. r=ttaubert,bgrins

https://reviewboard.mozilla.org/r/19615/#review17707

::: browser/base/content/browser.js:6983
(Diff revision 1)
> +  refreshForInsecureLoginForms() {
> +    // Check this._uri because we don't want to refresh the user interface if
> +    // this is called before the first page load in the window for any reason.
> +    if (!this._uri) {
> +      Cu.reportError("Unexpected early call to refreshForInsecureLoginForms.");
> +      return;

Does this happen or this just a precautionary measure? Just wondering if we should rather swallow the "failure"?

Comment 23

[Tanvi Vyas[:tanvi]]

https://reviewboard.mozilla.org/r/19613/#review17931

::: browser/base/content/browser.js:7090
(Diff revision 1)
> -      if (this._isMixedActiveContentLoaded) {
> +      if (LoginManagerParent.hasInsecureLoginForms(gBrowser.selectedBrowser)) {

Consider this edge case:
*HTTP top level page
*Includes password field
*Includes HTTPS frame that tries to load mixed active content and we block it.

In this case, we want to show that lock with the strikethrough.  If the user drills down in the control center, they should see that the reason for the unfavorable UI is the password field.  But they should also see a way to disable mixed content blocking (as well as re-enable it).  You may need to add a test case for this; you should certainly test it locally.  I've created this test case for you - http://people.mozilla.org/~tvyas/mixedgrandiframe_with_password.html

On the other hand, we can talk to UX and see if we just don't want to provide a mixed content override for this test case.  It make break the page for the user in a way they can't fix, but is this edge case worth the code complexity?

::: browser/components/controlcenter/content/panel.inc.xul:124
(Diff revision 1)
> +        <description when-loginforms="insecure">&identity.description.insecureLoginForms;</description>

Do you need to add an and-when-connection="not-secure" here?

::: browser/components/controlcenter/content/panel.inc.xul:149
(Diff revision 1)
> +        <!-- Insecure login forms -->

Isn't this the same thing you added at line 123?

Comment 24

[:Paolo Amadini]

(In reply to Tanvi Vyas [:tanvi] from comment #23)
> I've
> created this test case for you -
> http://people.mozilla.org/~tvyas/mixedgrandiframe_with_password.html

Thanks a lot! I've checked this and it seems to be handled correctly, both the insecure password and mixed content blocked sections are shown.

Do you think you can create a variant of <https://people.mozilla.org/~tvyas/password/frame_password.html> that loads the frame always over HTTP? I'm using devtools to change the iframe target manually, but having a specific test case might be easier for manual QA as well.

Is there an index of all these test cases?

> Do you need to add an and-when-connection="not-secure" here?

I think it's implied, isn't it?

> Isn't this the same thing you added at line 123?

Fixed, a merge issue maybe.

Comment 25

[Tanvi Vyas[:tanvi]]

(In reply to :Paolo Amadini from comment #24)
> (In reply to Tanvi Vyas [:tanvi] from comment #23)
> > I've
> > created this test case for you -
> > http://people.mozilla.org/~tvyas/mixedgrandiframe_with_password.html
> 
> Thanks a lot! I've checked this and it seems to be handled correctly, both
> the insecure password and mixed content blocked sections are shown.
> 
> Do you think you can create a variant of
> <https://people.mozilla.org/~tvyas/password/frame_password.html> that loads
> the frame always over HTTP? I'm using devtools to change the iframe target
> manually, but having a specific test case might be easier for manual QA as
> well.
> 
Yes, I've created http://people.mozilla.org/~tvyas/password/http_frame_password.html for you.

> Is there an index of all these test cases?
> 
No, I don't have an index of the password test cases.  Just the mixed content test cases https://people.mozilla.com/~tvyas/AllMixedTests.html, and even that is a bit out of date.  If there were a way to expose my /password directory on people.mozilla.org automatically, that would be nice.

> > Do you need to add an and-when-connection="not-secure" here?
> 
> I think it's implied, isn't it?
> 
I'm not sure.  What if you have an HTTPS page with an HTTP iframe that has an insecure password field.  I guess in that case you want to show the degraded UI also, even though connection is broken and not "not-secure".

Comment 26

[:Paolo Amadini]

Comment on attachment 8662455 [details]
MozReview Request: Bug 1179961 - Use a lock with a strikethrough for HTTP pages that have password fields in the Control Center. r=ttaubert,bgrins

Bug 1179961 - Use a lock with a strikethrough for HTTP pages that have password fields in the Control Center. r=ttaubert

Comment 27

[:Paolo Amadini]

Comment on attachment 8662455 [details]
MozReview Request: Bug 1179961 - Use a lock with a strikethrough for HTTP pages that have password fields in the Control Center. r=ttaubert,bgrins

Now with tests! Tryserver build:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6ca1b19fce78

Comment 28

[:Paolo Amadini]

(In reply to Tanvi Vyas [:tanvi] from comment #25)
> What if you have an HTTPS page with an HTTP iframe that has
> an insecure password field.  I guess in that case you want to show the
> degraded UI also, even though connection is broken and not "not-secure".

I think this case is handled correctly, I also added a test case specifically for this. Let me know if there is anything I may have missed here.

Comment 29

[:Paolo Amadini]

(In reply to Tim Taubert [:ttaubert] from comment #22)
> ::: browser/base/content/browser.js:6983
> (Diff revision 1)
> > +  refreshForInsecureLoginForms() {
> > +    // Check this._uri because we don't want to refresh the user interface if
> > +    // this is called before the first page load in the window for any reason.
> > +    if (!this._uri) {
> > +      Cu.reportError("Unexpected early call to refreshForInsecureLoginForms.");
> > +      return;
> 
> Does this happen or this just a precautionary measure? Just wondering if we
> should rather swallow the "failure"?

I don't think this will happen in practice, in other words I've written this like an assertion. I'm on the fence on whether to write the message to the Console. I've thought it's possible that some surrounding code would change and this message might help understand the issue.

Comment 30

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8662455 [details]
MozReview Request: Bug 1179961 - Use a lock with a strikethrough for HTTP pages that have password fields in the Control Center. r=ttaubert,bgrins

Hey Brian, can you please take a look at this? I don't have a lot of time to work on control center right now. Thanks!

Comment 31

[:Paolo Amadini]

Updated since I missed some tests expecting no mixed content blocking messages. New tryserver build:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6a92e9683508

Comment 32

[:Paolo Amadini]

Comment on attachment 8662455 [details]
MozReview Request: Bug 1179961 - Use a lock with a strikethrough for HTTP pages that have password fields in the Control Center. r=ttaubert,bgrins

Bug 1179961 - Use a lock with a strikethrough for HTTP pages that have password fields in the Control Center. r=ttaubert,bgrins

Comment 33

[:Paolo Amadini]

There's a Linux-only failure in browser/base/content/test/general/browser_bug906190.js that occurs now that I've added the code that opens the security subview. Here's the summary, one log and one screenshot:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6a92e9683508
https://treeherder.mozilla.org/logviewer.html#?job_id=12144606&repo=try
http://mozilla-releng-blobs.s3.amazonaws.com/blobs/try/sha512/5499bb151446ef07eeb59178a4334c99c039704e599d60c3be83f91d13327f5dc1b6c274e3b42578457bfc32489f43f78fcdab18d2391215d6267bf91e406424

Christopher, did you see something similar when writing this test? The test has been changed a lot since the original implementation, so this may be a new thing after all.

Maybe I can also try if the CSS checks work without opening the subview. Will try and set up a new Linux build later today.

Comment 35

[:Paolo Amadini]

(In reply to :Paolo Amadini from comment #33)
> There's a Linux-only failure in browser/base/content/test/general/browser_bug906190.js

I've reproduced it, seems to be related to popup hiding and focus. There's a context menu that remains open during the test, but apparently it's not opened by the specific one that fails. The failure to continue at some point might be a focus issue related to it. In any case it appears that the whole test would have to be rewritten to accommodate for asynchronous behavior, introducing more waits at key points during popup hiding and maybe the subview opening as well.

Fixing the test might be laborious since it's written without Tasks.

I'm considering just disabling it on Linux.

Comment 36

[Brian Grinstead [:bgrins]]

Comment on attachment 8662455 [details]
MozReview Request: Bug 1179961 - Use a lock with a strikethrough for HTTP pages that have password fields in the Control Center. r=ttaubert,bgrins

https://reviewboard.mozilla.org/r/19615/#review18997

::: browser/base/content/browser.js:7134
(Diff revision 3)
> +        this._identityBox.classList.add("insecureLoginForms");

Looks like this class never gets removed.  Probably due to this, if I set a type="password" back to type="text" on a page icon doesn't go away.

::: browser/base/content/test/general/browser_insecureLoginForms.js:32
(Diff revision 3)
> +      let identityBoxImage = gBrowser.ownerGlobal

For HTTP pages can you check to make sure that the icon goes away when the password field is removed on the page and re-added when it comes back?

::: browser/base/content/test/general/browser_insecureLoginForms.js:35
(Diff revision 3)
> +      is(identityBoxImage,

Can you also test the CC icon URL after opening it up?

::: browser/components/controlcenter/content/panel.inc.xul:153
(Diff revision 3)
> +                     and-when-loginforms="insecure">&identity.description.activeLoaded; <label observes="identity-popup-mcb-learn-more"/></description>

and-when-loginforms="insecure" seems to never be targeted by CSS.  Is this meant to be when-loginforms="insecure"?

Comment 37

[Tanvi Vyas[:tanvi]]

(In reply to :Paolo Amadini from comment #35)
> I'm considering just disabling it on Linux.

Please don't disable this test.  Do you know why this failure is existing with your patches but doesn't show up without it?  The ordering of the tests in the mochitest suite?

Comment 38

[Christoph Kerschbaumer [:ckerschb]]

(In reply to :Paolo Amadini from comment #33)
> There's a Linux-only failure in
> browser/base/content/test/general/browser_bug906190.js that occurs now that
> I've added the code that opens the security subview. Here's the summary, one
> log and one screenshot:
> 
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=6a92e9683508
> https://treeherder.mozilla.org/logviewer.html#?job_id=12144606&repo=try
> http://mozilla-releng-blobs.s3.amazonaws.com/blobs/try/sha512/
> 5499bb151446ef07eeb59178a4334c99c039704e599d60c3be83f91d13327f5dc1b6c274e3b42
> 578457bfc32489f43f78fcdab18d2391215d6267bf91e406424
> 
> Christopher, did you see something similar when writing this test? The test
> has been changed a lot since the original implementation, so this may be a
> new thing after all.
> 
> Maybe I can also try if the CSS checks work without opening the subview.
> Will try and set up a new Linux build later today.

Long time ago since I wrote those tests :-) I can't recall to see any errors like this. But I agree with Tanvi, we shouldn't disable those tests. It's unfortunate that they do not use tasks, but since those tests are important maybe it's worth trying to find someone to rewrite.

Comment 39

[:Paolo Amadini]

(In reply to Tanvi Vyas out until 10/8 [:tanvi] from comment #37)
> Please don't disable this test.  Do you know why this failure is existing
> with your patches but doesn't show up without it?  The ordering of the tests
> in the mochitest suite?

This would only be disabled on Linux, and still running on Windows and Mac OS X. The reason the failure happens is related to the additional click we make in the panel to open the subview.

I've filed bug 1212520 to re-enable the test once it's made asynchronous.

Comment 40

[:Paolo Amadini]

(In reply to Brian Grinstead [:bgrins] from comment #36)
> > +        this._identityBox.classList.add("insecureLoginForms");
> 
> Looks like this class never gets removed.  Probably due to this, if I set a
> type="password" back to type="text" on a page icon doesn't go away.
> 
> For HTTP pages can you check to make sure that the icon goes away when the
> password field is removed on the page and re-added when it comes back?

We reset the className before adding the new classes, so we should be fine. The reason the icon remains is that we decided not to handle password removals in the platform for now, the work to get there is tracked in bug 1193343.

> and-when-loginforms="insecure" seems to never be targeted by CSS.  Is this
> meant to be when-loginforms="insecure"?

Good catch! It was a regression in the normal mixed active content loaded messages after adding back the Learn More link. I've updated the test to check that the Learn More link doesn't show more than once.

Comment 41

[:Paolo Amadini]

Comment on attachment 8662455 [details]
MozReview Request: Bug 1179961 - Use a lock with a strikethrough for HTTP pages that have password fields in the Control Center. r=ttaubert,bgrins

Bug 1179961 - Use a lock with a strikethrough for HTTP pages that have password fields in the Control Center. r=ttaubert,bgrins

Comment 42

[:Paolo Amadini]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=eecd6fbd0d58

Comment 43

[Brian Grinstead [:bgrins]]

Comment on attachment 8662455 [details]
MozReview Request: Bug 1179961 - Use a lock with a strikethrough for HTTP pages that have password fields in the Control Center. r=ttaubert,bgrins

https://reviewboard.mozilla.org/r/19615/#review19349

::: browser/components/controlcenter/content/panel.inc.xul:146
(Diff revision 4)
> -        <description when-mixedcontent="active-loaded">&identity.description.activeLoaded;</description>
> +        <description when-mixedcontent="active-loaded"

Nit: I think this would be easier to follow if these were wrapped into a box so there was less total attributes to read.. so something like:

<vbox when-mixedcontent="active-loaded"
      and-when-loginforms="secure">
  <description>&identity.description.activeLoaded;</description>
  <description>&identity.description.activeLoaded2; <label observes="identity-popup-mcb-learn-more"/></description>
</vbox>

<vbox when-mixedcontent="active-loaded"
      and-when-loginforms="insecure">
  <!-- Show only the first message when there are insecure login forms,
       and make sure the Learn More link is included. -->
  <description>&identity.description.activeLoaded; <label observes="identity-popup-mcb-learn-more"/></description>
</vbox>

I'm assuming this wouldn't cause any layout issues (since they are already in a vbox), but if it does cause any complications I'm fine with how it is.

Comment 44

[:Paolo Amadini]

(In reply to Brian Grinstead [:bgrins] from comment #43)
> I'm assuming this wouldn't cause any layout issues (since they are already
> in a vbox), but if it does cause any complications I'm fine with how it is.

I tried that and there are probably other styles that depend on the XUL structure so the layout breaks. I'll land this as is then.

Comment 45

[Pulsebot]

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

Comment 46

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/578fbd8d8b3c

Comment 47

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7093f4b262e3

Comment 48

[Tanvi Vyas[:tanvi]]

Since this was backed out, can we complete https://bugzilla.mozilla.org/show_bug.cgi?id=1212520 and re-enable browser_bug906190.js before re-landing?

Comment 49

[:Paolo Amadini]

We discussed this with Tanvi and concluded to land the change and temporarily disable the test on Linux, while prioritizing the test refactoring to re-enable it as soon as possible.

Today at the daily team meeting we looked at the remaining scope for the 44 release, and we've been able to include bug 1212520 as part of the release, so we might be able to complete it within the next two weeks.

Comment 50

[Pulsebot]

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

Comment 51

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/672eab895605

Comment 52

[Paul Silaghi, QA [:pauly]]

(In reply to :Paolo Amadini from comment #18)
> Created attachment 8662398 [details]
> Login form present on HTTP subframe of HTTPS page, details view
Shouldn't be displayed the shield icon too?
I also don't have the "enable/disable protection" buttons in 44.0a1 (2015-10-21)/Win 7.

Comment 53

[Sylvestre Ledru [:Sylvestre]]

Release Note Request (optional, but appreciated)
[Why is this notable]: Improve the security of our users
[Suggested wording]: Usage of the password field on HTTP marks the website as insecure
[Links (documentation, blog post, etc)]: Not to link against a third party website but FYI: http://www.ghacks.net/2015/10/21/firefox-44-special-notification-if-logins-are-not-secure/

Comment 54

[:Paolo Amadini]

(In reply to Paul Silaghi, QA [:pauly] from comment #52)
> Shouldn't be displayed the shield icon too?

We don't display the shield for Mixed Content, in this case an HTTP frame inside an HTTP page.

If there are, additionally, tracking elements that have been blocked, the shield icon should be displayed.

> I also don't have the "enable/disable protection" buttons in 44.0a1
> (2015-10-21)/Win 7.

This should happen if you have an HTTP page inside an HTTPS one. The URL in the screenshot is a synthetic test case for that, not sure which other sites you can test with.

Comment 55

[Kohei Yoshino]

I have already posted the site compatibility doc: https://www.fxsitecompat.com/en-US/docs/2015/non-https-sites-containing-login-form-will-be-marked-insecure/

Comment 56

[Paul Silaghi, QA [:pauly]]

44.0a1 (2015-10-26) Win 7
1. Open http://www.imdb.com/ -> no lock with a strikethrough displayed
2. Click on the Login/Login -> no <input type="password">, lock with a strikethrough displayed
3. Close the pop-up -> lock with a strikethrough still displayed in home
Thoughts?

Comment 57

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Paul Silaghi, QA [:pauly] from comment #56)
> 44.0a1 (2015-10-26) Win 7
> 1. Open http://www.imdb.com/ -> no lock with a strikethrough displayed

That's fine, no password field is present yet.

> 2. Click on the Login/Login -> no <input type="password">, lock with a
> strikethrough displayed

Looking with the devtools inspector I see this:

<input id="passwordprompt" size="20" name="password" type="password">

It may be hidden from view, but it is present in the DOM.

> 3. Close the pop-up -> lock with a strikethrough still displayed in home
> Thoughts?

Presumably the login form doesn't get removed once the popup is gone, since it's not even visible anyway.

Comment 58

[:Paolo Amadini]

(In reply to Panos Astithas [:past] from comment #57)
> Presumably the login form doesn't get removed once the popup is gone, since
> it's not even visible anyway.

There's also bug 1193343 filed for removals of password fields from the DOM.

Comment 59

[Tanvi Vyas[:tanvi]]

Panos and past have done a fine job explaining the behavior on the site and the browser, so clearing my needinfo.

Comment 60

[Paul Silaghi, QA [:pauly]]

The lock with a strikethrough is displayed fine on the test pages:
http://people.mozilla.org/~tvyas/password/password_insecure.html
http://people.mozilla.org/~tvyas/password/frame_password.html
Also, based on comments 56-58, marking this bug as verified on 44.0a1.

Comment 61

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Added to FF44 release notes.

Comment 62

[Paul Silaghi, QA [:pauly]]

I no longer see the lock with a strikethrough in Aurora 44.0a2 (2015-11-03) Win 7, but I see it in Nightly 45.0a1 (2015-11-02).
Thoughts?

Comment 63

[Panos Astithas (he/him) [:past] (please ni?)]

It was a deliberate decision in bug 1217156. It will be enabled again in Aurora once bug 1217133 is fixed.

Comment 64

[Tanvi Vyas[:tanvi]]

I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1221206 to take this to dev edition once we are ready.

Comment 65

[Tanvi Vyas[:tanvi]]

(In reply to Ritu Kothari (:ritu) from comment #61)
> Added to FF44 release notes.

Should we remove from release notes for FF 44, since it is currently disabled there?

Comment 66

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

(In reply to Tanvi Vyas - out 11/11 [:tanvi] from comment #65)
> (In reply to Ritu Kothari (:ritu) from comment #61)
> > Added to FF44 release notes.
> 
> Should we remove from release notes for FF 44, since it is currently
> disabled there?

I've removed it and resetting the relnote flag here so it get's added to DevEd45 notes if that is the plan (see https://bugzilla.mozilla.org/show_bug.cgi?id=1221206#c2)

Comment 67

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Based on comment 63, updating FF44 status to disabled.

Comment 68

[Sylvestre Ledru [:Sylvestre]]

Release notes flag moved to bug 1221206

Comment 69

[Ben Bucksch (:BenB)]

As mentioned before in bug 667233 comment 77, there's a dependency here on bug 667233, which must be fixed at least one Firefox release *before* this one here, to allow websites time to flip the switch and migrate users over, before being punished for something they can't fix.
A customer of mine, one of the biggest websites in Germany, is very concretely waiting for Firefox to fix bug 667233 before they switch to https. They feel they *cannot* switch to SSL, if that means to break all stores passwords. It would incur massive calls to the hotline - that's a fact, because they tested with a test group.

So, in essence: The goal of this bug is to encourage websites to switch to https for login forms. But it's Firefox itself, specifically bug 667233, which is holding them back from doing so. Please fix your own bugs before punishing websites.

Given that FF43 has already been released, please back this out of FF44.

Comment 70

[:Paolo Amadini]

(In reply to Ben Bucksch (:BenB) from comment #69)
> As mentioned before in bug 667233 comment 77, there's a dependency here on
> bug 667233, which must be fixed at least one Firefox release *before* this
> one here

I agree, and in fact this warning is only displayed in the Developer Edition as a more concrete indication that a website is affected, and raise awareness of the issue in general.

We have bug 1240829 on file to implement a better interface for insecure login forms on Release, but we will not work on it until bug 667233 is fixed.

Comment 71

[Ben Bucksch (:BenB)]

Ah, thanks Paolo, I'm relieved!
I misread the "Target Milestone: FF44", sorry for that.
Thank you.

Comment 72

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Added this to Fx50 Beta release notes.