keybase / keybase-issues

A single repo for managing publicly recognized issues with the keybase client, installer, and website.
899 stars 37 forks source link

Fix login password remembering in Firefox/Chrome/password managers #1591

Open pkirkovsky opened 9 years ago

pkirkovsky commented 9 years ago

Keybase currently does not work with the password management functionality in either Firefox (#21) or Chrome. This issue also extends to external password managers such as LastPass, which may use similar login form detection methods (#1172).

Password saving doesn't work due to multiple reasons:

  1. The password field does not have type="password" set
  2. The username field does not have type="username" set (required by Chrome apparently)
  3. No actual submit button in the <form>
  4. No form action
  5. Disabling/changing default form event handler for form submission due to preventDefault() (I didn't dig too deeply into Keybase's JS but it looks like this may also apply)

Compatibility with password managers is an important security concern, as a lack of it encourages weak memorable passwords and password re-use.

References: AJAX login form tests (source) Firefox's nsLoginManager.js (_getPasswordFields() and _getFormFields()) Chromium: Prompt to save password after history.pushState() Chromium: Password saving fails if default submit handler is disabled Getting Chrome to prompt to save password when using AJAX to login

malgorithms commented 9 years ago

this is a great list. The passphrase field is set to type=password. I'm changing the username field to type="username" for Chrome support, although I'd never heard of that. I guess it can't hurt as long as other browers don't get pissed the type="text" is missing.

Buuut... numbers 3, 4, and 5 are going to be problematic. Keybase does not "submit" a form with your password to Keybase's servers. The protocol for authenticating yourself is described on these 2 pages:

https://keybase.io/docs/api/1.0/call/getsalt https://keybase.io/docs/api/1.0/call/login

pkirkovsky commented 9 years ago

Thanks @malgorithms!

After messing around in Firebug I realized that the final step of the fix is to simply replace <a id="btn-login" ... link with a regular button:

<button id="btn-login" class="btn btn-primary btn-lg" onclick="return false;">Login</button>

This retains login functionality, retains the "scrypt" status on the button title, and triggers Firefox's password manager function.

malgorithms commented 9 years ago

hah, well, @pkirkovsky the problem is I didn't push my changes live yet!

ok, they're live.....now! I also just changed the <a> to a <button> as you just requested. Interestingly, I'd already done that with the signup form at one point, but not the login form...

that's all live if you want to test more. Thanks again for your help.

If you want any extra invitation codes to generate some throwaway fake accounts for signing up, let me know. (email me.)

pkirkovsky commented 9 years ago

You forgot to add this to the button:

onclick="return false;"

Login is currently broken, and it's trying to submit the un-encrypted login passphrase as a GET parameter which is probably showing up in your access logs :scream:

screen shot 2015-05-27 at 1 36 32 pm

malgorithms commented 9 years ago

@pkirkovsky reverting while I check this out!

malgorithms commented 9 years ago

@pkirkovsky , you saved me a mega-embarrassing push there by only letting it live for a few mins.

What happened: I tested your proposed change and made 2 mistakes: (1) I accidentally deleted the return false, which I had in there before. And (2) I then tested against the wrong instance (not my dev machine) and saw it to work fine before pushing.

2 things:

  1. In the couple mins this was up, 6 people sent their passwords to us over TLS as GET requests, and we've nuked our logs. I'm communicating with them privately to let them know they can decide if they want to change them.
  2. I'm giving you 5 BTC (my own money, not Keybase's :-) ) as a bug bounty. Since you haven't posted a bitcoin address on your profile, email me one...make sure to sign the message!

Thanks so much!

malgorithms commented 9 years ago

(all-time least favorite moment since starting to work on Keybase.)

On a brighter note, can you confirm you're now getting the password saving behavior you were hoping for?

coyotebush commented 9 years ago

Still not working as desired for me (Firefox 38.0.1), but changing the submit <button> (again) to an <input type="submit"> makes it offer to save the password, and changing the username input back to type="text" makes it recognized as a username.

pkirkovsky commented 9 years ago

@malgorithms Nice! Thank you very much for the bug bounty.

pkirkovsky commented 9 years ago

@coyotebush @malgorithms I think the culprit is type="button". I just went to the Keybase site, clicked Login, edited the button to remove the type parameter, filled in the login form, and Firefox offered to save my password when I clicked it (pressing Enter also works).

If type is not specified, <button> elements by default are set to type="submit" and that's how Firefox determines that the button is for form submission. Setting type="button" make it clickable but does not allow it to be detected as a form submission button. Login still works because the site JS seems to monitor the click event...

coyotebush commented 9 years ago

Yes, <button> without type="button" works fine for me too.

(And Firefox does still need type="text" on the username to offer to save "the password for coyotebush" instead of just "the password", and to autofill the username.)

pkirkovsky commented 9 years ago

OK, so it looks like removing type="button" from the button and changing the username field from type="username" to type="text" makes Keybase play nicely with login info saving on major browsers. Thanks for the heads up @coyotebush!

Tested on OS X 10.9.5 with:

(I guess Chrome doesn't absolutely require type="username" to be set after all. Sorry I pointed you in the wrong direction!)

malgorithms commented 9 years ago

ok, I've followed these suggestions.

How's it looking? Obviously I can't follow the form submission desires (since our login works differently), but if we're good otherwise, I'll close this issue out.

thanks all.

pkirkovsky commented 9 years ago

Retested on the live site with the browsers above and password saving works as expected. Maybe someone else should test too?

malgorithms commented 9 years ago

great news! I wonder if we'll get full coverage, since it doesn't submit like a normal form...but here's hoping.

paulsena commented 9 years ago

Login Form detection and saving doesn't work for me with LastPass plugin and the latest version of Chrome (43.0.2357) on Windows 8.1.

ax3l commented 8 years ago

first of all: great thread :)

great news! I wonder if we'll get full coverage, since it doesn't submit like a normal form...but here's hoping.

@malgorithms I am running Debian 8.5 Jessie and Firefox 45.3.0 "ESR" and unfortunately saving the credentials does not work yet.

I fixed saving with the following (in-browser) HTML change: Extend your <form> to include the "Login" <button id="btn-login" ...> element.

After that, it offers the "save password" dialogue on first login (and even without the change it already fills up the form, but as said, "saving/storing" it in first place was not offered).

ax3l commented 8 years ago

@pkirkovsky @malgorithms anyone still working on this? :) (one post above is the fix)

malgorithms commented 8 years ago

sorry, no, it's not dead, I've just been pulled into other things for the last couple weeks. And I appreciate your sleuthing.

to clarify your fix, there are two forms we're talking about: signup and login. Are you referring to just a change in the login form?

Currently the login form does have this:

<button class="btn btn-primary" id="btn-login" onclick="return false;">Login</button>

so what would be the precise change to that button?

thanks - happy to get to this soon

ax3l commented 8 years ago

@malgorithms thanks for having a look!

Well, it's actually both the login and the signup.

Your login code is basically:

    <div class="modal-body">
      <form name="modal-dialog-session-form">
          <input ...>
          <input ...>
      </form>
    </div>

    <div class="modal-footer">
      <button class="btn btn-primary" id="btn-login" onclick="return false;">Login</button>
    </div>

just move the button into the <form>...</form> and this will trigger a save passwort dialog in Firefox.

In the signup there is not even a form used yet at all :)

malgorithms commented 8 years ago

ok, I've merged a number of changes that should make this more friendly.

malgorithms commented 8 years ago

@ax3l @paulsena - can you confirm this works better for you now?

@pkirkovsky - I assume it didn't break anything for you with LastPass...let me know if so.

ax3l commented 8 years ago

@malgorithms I tried the Login with Firefox ESR 45.4.0 on Debian 8.6 and both storing and retrieving the password via the in-browser dialogs works.

Thank you! :sparkles: