1127579 - Address when a user captures a login for a HTTPS formOrigin and already has a saved login for the HTTP version of the origin

Status: VERIFIED FIXED

(Toolkit :: Password Manager, defect, P1)

Description

[Chris Karlof [:ckarlof]]

The related case "saves a login for an HTTP formOrigin and already has a saved login for the HTTPS version of the origin" is also relevant. 

I like the model of the saved logins as being a literal record of what and where the user saved login information. However, I feel like the situation described in the bug title should be addressed in some way. Bug 1127567 and bug 667233 would de-dup HTTP and HTTPS logins for the same username in some contexts, so I feel that those entries should not subsequently be naively updated independently.

- If the user has a saved HTTP login, and updates her password for the login from an HTTPS context, I propose that we should also provide this new password when she visits the page in a HTTP context. We could do this a few ways: 

1) also update the HTTP login
2) mark the HTTP login as "obsolete", which is a signal it should use the HTTPS credentials on the HTTP page
3) delete the HTTP login

1) + 3) lose information, and 2) introduces a new field, which causes problems for Sync.

There is also 4), do nothing with the HTTP login. The user only encounters a screw case if she updates her password on a site with both HTTP and HTTPS login pages. We could measure how often this happens. However, if

1) is probably the least bad option -- thoughts? I also like 1), because it mirrors the behavior we could also do to address "realms" (i.e., sites with the same login on multiple domains). 

- The opposite case is if the user has a saved HTTPS login, and updates her password for the login from an HTTP context. The right thing to do here depends on what we do to address the prior case, but if we go with 1), then it would mean updating the HTTPS version of the login as well.

Comment 1

[Chris Karlof [:ckarlof]]

The solution to this bug would likely involve changes to LoginManagerParent.onFormSubmit  (http://mxr.mozilla.org/mozilla-central/source/toolkit/components/passwordmgr/LoginManagerParent.jsm#203).

Comment 2

[Chris Karlof [:ckarlof]]

Case:
User has saved creds for HTTP, and we autofill in HTTPS.

Action:
If the we capture the same username and password, do nothing.

Comment 3

[Chris Karlof [:ckarlof]]

> Case:
> User has saved creds for HTTP, and we autofill in HTTPS.

> Action:
> If the we capture the same username and password, do nothing.

Except update the usage information in the HTTP credentials.

Comment 4

[Chris Karlof [:ckarlof]]

Case:
User has saved creds for HTTP, we capture updated creds for HTTPS

Action:
Update the creds for HTTP

Comment 5

[Chris Karlof [:ckarlof]]

Case:
User has saved creds for HTTPS, we capture updated creds for HTTP

Action:
Update the creds for HTTPS

Comment 6

[Chris Karlof [:ckarlof]]

Case:
User has saved password1 for HTTPS, saved password2 for HTTP (same username), we capture updated creds for HTTP

Action:
Update the creds for HTTPS

---------------------------------------------------------------

Case:
User has saved password1 for HTTPS, saved password2 for HTTP (same username), we capture updated creds for HTTPS

Action:
Update the creds for HTTPS

Comment 7

[Chris Karlof [:ckarlof]]

Can you add a link to the etherpad you made on this these cases, Tanvi?

Comment 8

[Tanvi Vyas[:tanvi]]

(In reply to Chris Karlof [:ckarlof] from comment #7)
> Can you add a link to the etherpad you made on this these cases, Tanvi?


https://etherpad.mozilla.org/same-domain-http-https-passwords

Comment 9

[Tanvi Vyas[:tanvi]]

(In reply to Tanvi Vyas [:tanvi] from comment #8)
> (In reply to Chris Karlof [:ckarlof] from comment #7)
> > Can you add a link to the etherpad you made on this these cases, Tanvi?
> 
> 
> https://etherpad.mozilla.org/same-domain-http-https-passwords

I went through this etherpad with some other security folks and also with Chris.  After talking to Chris, I realized there were different ways we could handle duplicate HTTP and HTTPS records in the database.  I came up with two more possible policies that involve those different methods.  I need to collect more data and talk to Chris again to choose which policy to apply.

Listing the details here, but please don't feel the need to read through all of these etherpads - they are long and will likely change.

https://etherpad.mozilla.org/same-domain-http-https-passwords - Keep two separate records in the database, one for HTTP and one for HTTPS.  And assumes we do not autofill on HTTP pages.

https://etherpad.mozilla.org/same-domain-http-https-passwords2 - Have the ability to consolidate HTTP and HTTPS records into one record that specifies that it can be used with both schemes.  Has proposals for both autocomplete on HTTP and autofill on HTTP.

https://etherpad.mozilla.org/same-domain-http-https-passwords3 - When updating a password for a page that has both an HTTP and an HTTPS record, just update the HTTPS record and delete the HTTP record.  And assumes we autofill on HTTP.

The last one is the simplest to implement.  But we aren't sure if it is okay from a usability perspective.  Have pages moved away from trends of having login forms on every one of their pages?  Is the signal of one-time-HTTPS usage likely to mean that most of the time the user will be logging in through HTTPS and hence an autocomplete UI on HTTP would be sufficient?

Comment 10

[Chris Karlof [:ckarlof]]

> Have pages moved away from trends of having login forms on every one of their pages?

It would be great to gather some data on this in a telemetry experiment. Maybe something like "login forms seen per domain, de-duped by URL".

Comment 11

[Tanvi Vyas[:tanvi]]

Doing a manual survey[1] of the Alexa Top 50[2] and a handful of other sites I found that most websites that have HTTPS logins do not provide an HTTP login experience[3].  There isn't as much back and forth between HTTP/HTTPS login experience as I had previously observed a couple years back.

The following sites DO offer logins on both HTTP and HTTPS.  If we apply a policy where we autocomplete HTTP pages with HTTPS records and delete HTTP entries (the third option in comment 9), then users will always have to autocomplete when going to the HTTP version of the following sites.
reddit.com
imgur.com
yandex.ru
vk.com - european social network
imdb.com (HTTPS has mixed content on it but is still functional)
espn.com (HTTPS version has mixed content on it that prevents the page from being functional in firefox)
alibaba - (but the http logins are on different subdomains, https login is on login.alibaba.com, so the password manager is going to have difficulty anyway without a recipe)

Are any of these show stoppers?  Would we be okay requiring autocomplete on HTTP for these pages?  If the answer is yes, let's continue to gather more data.  If the answer is no, the third option in comment 9 is out of the question.


[1] Yes, very manual.  This is just some data to consider, but we still need telemetry data to give us more accurate data and tell us when trends are changing.

[2] I looked at all the english sites and some non-english sites.  There were some non-english where I could still find the login form, and some where I couldn't.

[3] https://etherpad.mozilla.org/wt6QyA3BNc

Comment 12

[Tanvi Vyas[:tanvi]]

(In reply to Tanvi Vyas [:tanvi] from comment #9)
> Have pages moved away from trends of having
> login forms on every one of their pages?
From my brief and manual survey, I'd say yes.  But that's not the real question we need answered here anyway.  The next one is...

> Is the signal of one-time-HTTPS
> usage likely to mean that most of the time the user will be logging in
> through HTTPS and hence an autocomplete UI on HTTP would be sufficient?

We need to figure out if there is a way to gather telemetry on this.

One thing we can do is search for a duplicate records in the database - how often do we see both http:host:username and https:host:username.  Do we want to report this blindly, or do we want to dedup on hostname (so that a host with multiple username records doesn't get overcounted)?

We would only get data from users currently using the password manager.  Trying to do this for all users in general is tricky - 
* observer all password fields and record the url locally
* dedup by hostname
* what percentage of hostnames use both an http and https scheme?
Telemetry is reported every 24 hours (I believe) so this isn't going to work out so well - most users aren't going to login twice, once over HTTP and once over HTTPS in a single day.

Comment 13

[Tanvi Vyas[:tanvi]]

Talked to Paolo about this yesterday.  We could collect some data from the password manager database that try's to answer the question: How many websites flip back and forth between HTTP logins and HTTPS logins?


* Start by discarding all records with no username.
* Group the remaining database entries into two categories
** Group 1 - collect all records for domains that have only one scheme.  Report the number of records.  Report the number of unique domains from the records in this group. 
** Group 2 - collect all records for domains that have two schemes. Report the number of records.  Report the number of unique domains from the records in this group. 

Then from within group 2, determine how many of the domains have a HTTP record that has been used in the last 6 months.  This number over the total count of unique domains from group 1 and group 2 will tell us the percentage of sites that user's of our password manager encounter that flip back and forth between HTTP and HTTPS logins.

Instead of just relying on "6 months" we could also graph the lastUsed times for the HTTP records in Group 2.

Comment 14

[Tanvi Vyas[:tanvi]]

After discussing with password manager engineering team, we've decided on the following policy:
https://etherpad.mozilla.org/same-domain-http-https-passwords4

Comment 15

[Tanvi Vyas[:tanvi]]

Filed bug https://bugzilla.mozilla.org/show_bug.cgi?id=1150605 for the telemetry described in comment 13.

Comment 16

[Tanvi Vyas[:tanvi]]

I think this bug boils down to:

If you capture a password over HTTPS and the user only has a saved HTTP password for that domain, then update the HTTP password and create a new HTTPS entry with the same username and password without prompting the user.

Comment 17

[Matthew N. [:MattN]]

I think we can flip signon.schemeUpgrades to true in the patch for this bug unless some other blocker pops up. Bug 1272507 for HTTP auth will be seen as an enhancement unless this bug makes us not ask to capture an upgraded HTTP auth login. (i.e. saved HTTP auth login for HTTP then we won't autofill without bug 1272507 so we should at least ask to save a dupe for HTTPS).

Comment 18

[Matthew N. [:MattN]]

(In reply to Tanvi Vyas - behind on reviews [:tanvi] from comment #11)
> Doing a manual survey[1] of the Alexa Top 50[2] and a handful of other sites
> I found that most websites that have HTTPS logins do not provide an HTTP
> login experience[3].  There isn't as much back and forth between HTTP/HTTPS
> login experience as I had previously observed a couple years back.
> 
> The following sites DO offer logins on both HTTP and HTTPS.

Thank you very much for doing this Tanvi!

> If we apply a
> policy where we autocomplete HTTP pages with HTTPS records and delete HTTP
> entries (the third option in comment 9), then users will always have to
> autocomplete when going to the HTTP version of the following sites.
> reddit.com
> imgur.com
> yandex.ru
> vk.com - european social network

This site still offers both HTTP and HTTPS

> imdb.com (HTTPS has mixed content on it but is still functional)
> espn.com (HTTPS version has mixed content on it that prevents the page from
> being functional in firefox)

espn.com seems to only host the form on HTTP, I can't find an HTTPS version at the moment.

> alibaba - (but the http logins are on different subdomains, https login is
> on login.alibaba.com, so the password manager is going to have difficulty
> anyway without a recipe)

All the others are HTTPS only now from my quick checks.

Comment 19

[Matthew N. [:MattN]]

Attachment: Bug 1127579 - Handle a user capturing a HTTPS login with an already saved login for the HTTP version.

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

Comment 20

[Justin Dolske [:Dolske]]

Comment on attachment 8764659 [details]
Bug 1127579 - Handle a user capturing a HTTPS login with an already saved login for the HTTP version.

https://reviewboard.mozilla.org/r/60388/#review57334

I'm a little confused how one part works, but r+ anyway since it seems like it's just extending an already-confusing API.

::: toolkit/components/passwordmgr/nsILoginInfo.idl:106
(Diff revision 1)
>     * @param ignorePassword
>     *        If true, ignore the password when checking for match.
> +   * @param [ignoreOrigins = false]
> +   *        If true, don't compare the `hostname` or `formSubmitURL` fields.
>     */
> -  boolean matches(in nsILoginInfo aLoginInfo, in boolean ignorePassword);
> +  boolean matches(in nsILoginInfo aLoginInfo, in boolean ignorePassword, [optional] in boolean ignoreOrigins);

I think this is leftover from an earlier patch? I vaguely recall us discussing not doing this, and it's not used elsewhere here. (IIRC, our preferred approach was the 3rd options argument to doLoginsMatch() instead of another boolean here).

::: toolkit/components/passwordmgr/nsLoginManagerPrompter.js:1236
(Diff revision 1)
>      if (aNotifyObj == this._getPopupNote()) {
> +      aOldLogin.hostname = aNewLogin.hostname;
> +      aOldLogin.formSubmitURL = aNewLogin.formSubmitURL;
>        aOldLogin.password = aNewLogin.password;
>        aOldLogin.username = aNewLogin.username;
>        this._showLoginCaptureDoorhanger(aOldLogin, "password-change");

O_o How does this code even work (before your change)? Passing in a single login that's a mishmash of old and new?

Comment 21

[Matthew N. [:MattN]]

https://reviewboard.mozilla.org/r/60388/#review57334

> I think this is leftover from an earlier patch? I vaguely recall us discussing not doing this, and it's not used elsewhere here. (IIRC, our preferred approach was the 3rd options argument to doLoginsMatch() instead of another boolean here).

Yep, forgot to revert the IDL after reverting the implementation.

> O_o How does this code even work (before your change)? Passing in a single login that's a mishmash of old and new?

Yeah, this method being modified is `_showChangeLoginNotification` so it's specific to a change (not a remember) and therefore I believe aNewLogin is always present. The password change code was only focused on changing the password (and more recently the username with a patch from rittme last year) and so it simply modified those fields on the old login from storage. Doing it that way preserves all the metadata about first use and such.

The doorhanger takes a single login regardless of whether it will be a change or save and then again compares to storage to decide at edit time whether to display the remember or change main action button label so that it handles editing of the fields in the doorhanger. e.g. if the user changes the username in the doorhanger, and update will change to a save if that new username doesn't exist.

Let me know if that doesn't answer your question.

Comment 22

[Matthew N. [:MattN]]

Comment on attachment 8764659 [details]
Bug 1127579 - Handle a user capturing a HTTPS login with an already saved login for the HTTP version.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/60388/diff/1-2/

Comment 23

[Matthew N. [:MattN]]

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d34ca52b301b0b480cb830cb9e3f6d74a68fbe90

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/6503e2470c5e
Handle a user capturing a HTTPS login with an already saved login for the HTTP version. r=dolske
https://hg.mozilla.org/integration/fx-team/rev/1d1b4e27b684
Cleanup jsdoc comments in nsLoginManagerPrompter.js

Comment 25

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/6503e2470c5e
https://hg.mozilla.org/mozilla-central/rev/1d1b4e27b684

Comment 26

[Paul Silaghi, QA [:pauly]]

(In reply to Tanvi Vyas[:tanvi] from comment #16)
> I think this bug boils down to:
> 
> If you capture a password over HTTPS and the user only has a saved HTTP
> password for that domain, then update the HTTP password and create a new
> HTTPS entry with the same username and password without prompting the user.
By "create a new HTTPS entry" did you mean to replace the HTTP one in the Saved Logins window?
This is what I see after login on http://imgur.com/ and then on https://imgur.com/.
Moreover, on a 3rd login on http://imgur.com/, the password is updated, but there is still only one https entry in the Saved Logins window.

Comment 27

[Tanvi Vyas[:tanvi]]

(In reply to Paul Silaghi, QA [:pauly] from comment #26)
> (In reply to Tanvi Vyas[:tanvi] from comment #16)
> > I think this bug boils down to:
> > 
> > If you capture a password over HTTPS and the user only has a saved HTTP
> > password for that domain, then update the HTTP password and create a new
> > HTTPS entry with the same username and password without prompting the user.
> By "create a new HTTPS entry" did you mean to replace the HTTP one in the
> Saved Logins window?
> This is what I see after login on http://imgur.com/ and then on
> https://imgur.com/.
> Moreover, on a 3rd login on http://imgur.com/, the password is updated, but
> there is still only one https entry in the Saved Logins window.

I don't recall what Matt ended up implementing.  The captured password over HTTPS shouldn't be used to update the HTTP entry.  So either the HTTP entry should be left as is with the old password, or be deleted.  From what you are reporting, Matt probably implemented the later.  Matt?

Comment 28

[Matthew N. [:MattN]]

(In reply to Tanvi Vyas - behind on reviews [:tanvi] from comment #27)
> (In reply to Paul Silaghi, QA [:pauly] from comment #26)
> > (In reply to Tanvi Vyas[:tanvi] from comment #16)
> > > I think this bug boils down to:
> > > 
> > > If you capture a password over HTTPS and the user only has a saved HTTP
> > > password for that domain, then update the HTTP password and create a new
> > > HTTPS entry with the same username and password without prompting the user.
> > By "create a new HTTPS entry" did you mean to replace the HTTP one in the
> > Saved Logins window?

Kinda, the HTTP login is upgraded to an HTTPS one so the date and times used metadata is preserved but the URL updates to HTTPS

> > This is what I see after login on http://imgur.com/ and then on
> > https://imgur.com/.
> > Moreover, on a 3rd login on http://imgur.com/, the password is updated, but
> > there is still only one https entry in the Saved Logins window.
> 
> I don't recall what Matt ended up implementing.  The captured password over
> HTTPS shouldn't be used to update the HTTP entry.  So either the HTTP entry
> should be left as is with the old password, or be deleted.  From what you
> are reporting, Matt probably implemented the later.  Matt?

See above. It's an upgrade of the URLs which is like a delete but keeps the old usage metadata as the user sees it as the same login.

Comment 29

[Paul Silaghi, QA [:pauly]]

Based on comment 28, verified fixed FX 50.0a1 (2016-07-07) Win 7.

Comment 30

[Matthew N. [:MattN]]

Release Note Request (optional, but appreciated) This is for the combo of bug 667233 + this bug which prefs the feature on.
[Why is this notable]: Saved HTTP logins will now be offered on HTTPS pages of the same domain which helps with the adoption of HTTPS on the web as some authors were waiting for this change so their users aren't locked out of their site due to not having their saved password on HTTPS
[Suggested wording]: Saved HTTP logins will now be offered on HTTPS pages
[Links (documentation, blog post, etc)]: I will blog about this shortly. Needinfo to myself as a reminder.

[Tracking Requested - why for this release]: Notable feature.

Comment 31

[Matthew N. [:MattN]]

Comment on attachment 8764659 [details]
Bug 1127579 - Handle a user capturing a HTTPS login with an already saved login for the HTTP version.

Approval Request Comment
[Feature/regressing bug #]: This is a follow-up from bug 667233 which landed in 49. This is a fix so HTTP logins are used for the same site over HTTPS.

[User impact if declined]: Users may not be able to login to sites which upgrade to HTTPS (as is the trend, especially with Let's Encrypt). Large sites are resisting the transition to HTTPS to avoid an influx of user support requests for users who are locked out of their account due to the Fx password manager not upgrading their login.

[Describe test coverage new/current, TreeHerder]: 3 weeks baking with new browser-chrome tests

[Risks and why]: Low-medium risk of logins availability changing in some edge cases. This is likely worth the reward of availability of the HTTP logins overall though. There is a pref (signon.schemeUpgrades) if we need to disable this via a hotfix.

[String/UUID change made/needed]: None.

Comment 32

[Gerry Chang [:gchang]]

Comment on attachment 8764659 [details]
Bug 1127579 - Handle a user capturing a HTTPS login with an already saved login for the HTTP version.

This patch fixes HTTP logins being used for the same site over HTTPS. Let's take it in 49 aurora.

Comment 33

[Gerry Chang [:gchang]]

Track 49 for better handling a user capturing a HTTPS login with an already saved login for the HTTP version.

Comment 34

[Matthew N. [:MattN]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/01e60d85a78c
https://hg.mozilla.org/releases/mozilla-aurora/rev/dc272cf9d820

Comment 35

[Sylvestre Ledru [:Sylvestre]]

Added to the 49 release notes using MattN wording.

Comment 36

[Matthew N. [:MattN]]

Thanks. The blog post is at https://matthew.noorenberghe.com/blog/2016/08/filling-saved-http-logins-over-https-same-domain

Comment 37

[Paul Silaghi, QA [:pauly]]

"If a password is changed on the HTTPS version of the site, the saved login will be upgraded and only used on the HTTPS origin in the future. This is to prevent a new password created over HTTPS from leaking to HTTP. HTTPS logins will continue to only to be filled for the same origin (over HTTPS) and won't be downgraded (to HTTP) for security reasons."
If I save credentials on https://imgur.com/, then they are auto-completed on http://imgur.com/.
Is this correct?

Comment 38

[Sylvestre Ledru [:Sylvestre]]

Link added. 
As said on IRC, if you could move the content to an official blog, this would be better.

Comment 39

[Matthew N. [:MattN]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #38)
> As said on IRC, if you could move the content to an official blog, this
> would be better.

The security team decided not to put it on their blog so I'm considering Hacks instead

Comment 40

[Tanvi Vyas[:tanvi]]

(In reply to Paul Silaghi, QA [:pauly] from comment #37)

> If I save credentials on https://imgur.com/, then they are auto-completed on
> http://imgur.com/.
> Is this correct?

That doesn't sound right.  Are you sure you don't have any Login Manager entries stored for http://imgur.com?

Comment 41

[Paul Silaghi, QA [:pauly]]

Yes, I'm sure. Tried again with a clean profile, FX 51.0a1 (2016-08-16) Win 7 x64.
Maybe it's something wrong with the imgur.com.
On vk.com I can't reproduce the problem. I couldn't find other pages that allow login on both http/https to test with.

Comment 42

[Matthew N. [:MattN]]

imgur.com isn't a good test case since they use an HTTPS iframe in all cases. What matters is the origin of the input textbox, not what's in the address bar.

Comment 43

[Paul Silaghi, QA [:pauly]]

Thanks.
Verified fixed FX 49b5, Win 7.