Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Password profiles should be encrypted #185

Closed
vpfautz opened this issue Mar 30, 2017 · 24 comments
Closed

Password profiles should be encrypted #185

vpfautz opened this issue Mar 30, 2017 · 24 comments

Comments

@vpfautz
Copy link

vpfautz commented Mar 30, 2017

I don't see any encryption of settings, that are stored on the server. I think this is a privacy issue. Is it possible to encrypt the settings with the masterpassword?

@guillaumevincent
Copy link
Member

guillaumevincent commented Mar 31, 2017

Yes AES encryption with the master password

edit: LessPass does not (yet) encrypt password profiles on LessPass Database.
We will describe here the technical implementation when we will start to do the job.

@guillaumevincent guillaumevincent changed the title Settings are not encrypted (webextension) Password profiles should be encrypted Apr 2, 2017
@aurimasmb
Copy link

Could you clarify which AES algorithm is being used? thanks !

@aurimasmb
Copy link

Also, could you please clarify that if the master password is being used to encrypt the profile, how is it being stored in the LessPass app? Is a Hash of the master password used as the AES encryption key, or is the password itself being used?

@guillaumevincent
Copy link
Member

My first idea was to encrypt password profile with the master password after pbkdf2.
I will add the implementation description here before.

@betsythefc
Copy link

betsythefc commented Nov 9, 2018

I'm by no means an expert on security, would it be better to encrypt each string with the password after pbkdf2 and upload/download the json block with the encrypted strings instead of the plain text strings, or to encrypt the whole json block as a unit and upload that?

Here is what I mean (assuming I am not making sense):

{
    'site': '=Z# BT^..Oi.ae{e]',
    'username': 'o8gnW7&eik.gb92'
}

vs

aw;rjgoirnw4w98usvjnlkseIUYH#UIOnw90i:Lm3/r;j(*)h:KNWr9i3wjWm;lsmv)(Ur /wt9Urf:AJW:?gj

note, these strings mean nothing other than how I smashed my hands on the keyboard

@vpfautz
Copy link
Author

vpfautz commented Nov 13, 2018

If you encrypt the hole json block. The only Information you can get out of it is the length of the encrypted data. Also changes give you just a information about the length. If the values are encrypted, than you can easily find out if there was a password added, removed or probably even linked, because the other passwords are not reencrypted with another IV.
Another issue is, that if some attacker can modify the encrypted data. In the case of hole encryption the hole json would be corrupted. In the separated case an attacker could edit a specific password or site and and so corrupt it. With combination of other methods he could get more information.

On the other hand if you have decrypted data lying around, in the case of hole json encryption the hole content will be decrypted. In case of separate encryption there could be a password inserted without interfering with the other encrypted data.

I would prefer the hole json file to be encrypted.

@betsythefc
Copy link

@guillaumevincent, you mentioned encrypting the profiles with the "master password" after a pbkdf2. By "master password" do you mean the account password or the password that is added to the other attributes to create the hash?

I've worked a lot with python, some with encryption. I'd love to help build this out.

@guillaumevincent
Copy link
Member

This is not an easy one because:

  • we need to change the backend implementation to save a list of strings, one string corresponding to a password profile encrypted (AES-encrypted blob).
  • we need to store a derivation key in the database for a user
  • we need to migrate the actual passwords profiles, we need to wait for the users to log in, ask him the master password and encrypt the profiles with it.
  • we need to update the ui workflow and decrypt the password profiles on log in

The password profile should be encrypted on client side, using a local encryption key. This key should be derived from your master password using pbkdf2 and at least 100k iterations.

See https://security.stackexchange.com/questions/157422/store-encrypted-user-data-in-database for the workflow

@FaustXVI
Copy link
Contributor

That's an issue I'd be happy to work with.
Here are my thought :

  • the encryption should be made user side so that the database can't leak any information even with the server control (better privacy)
  • the main purpose of encryption should be avoiding brute forcing the master password. Here how it can go :
    • a database leeks passwords (happens every day) and I have the password of my target, let's say Xavier
    • a lesspass database is leaked too (will happen, either the main one or a self-hosted one) and I have all the profiles of Xavier but not his master password
    • no I can brute force the master password of a user by generating passwords until I have the same as the leaked database. Since I found the master password, I can now generate any password of Xavier :(
    • since the master password has to be remember, a lot of people are vulnerable to dictionary based brute-forcing, making it very fast
    • even worse, I can do that for any user in common between the two databases

I like @guillaumevincent proposition. Nevertheless, if the whole encryption is done client side, i don't thing there is much to do server-side. I also think that the encryption can be automatically done during the next user's connection and be totally transparent to the user (so no ui changes).
For exemple, after the connection, if we get a derivation key, we assume the data are encrypted already, if not, then we generate it from the connection password we just get, encrypt every data en save it. Then we are back in case 1.
Finally, when the user logs-in, we decrypt the profile thanks to the derivation key and keep it in memory.

@guillaumevincent
Copy link
Member

@FaustXVI on the backend we need to create an encrypted user key field. We also need to update the password update mechanism. So no there is a little work to do :)

On ui side we need to update the code, to build this key after the first re connection and save it.

on ui side during the login:

  • get or create key2 from user.encrypted_key

if key2 exists

  • get key1 with user's password (decrypt it)
  • get all password profiles
  • decrypt all password profiles using key1

if no key2 in database create key2:

  • create random key (key1)
  • encrypt all passwords profiles using key1 and save it in database
  • use user's password to encrypt key1 and save this key (key2) in the user.encrypted_key field in the database

@FaustXVI
Copy link
Contributor

@guillaumevincent yep, I think we agree. When i was say "no work on the backend" i was talking about logic code (if this then that).
I don't see any password update mecanism thought. I was thinking it'd be a nice feature to have but I don't see any button anywhere to change my password :/ Is it the "Forgot password?" mecanism ? If so, then I understand better your comment.
If we are ok with all of that, then I'd be happy to work on it :)

@guillaumevincent
Copy link
Member

Is it the "Forgot password?" mecanism ?

yes exactly

Feel free to start this if you want, one step at a time
Do you want to start on the backend or the frontend?

@FaustXVI
Copy link
Contributor

I am thinking to start with the key generation, even if it's not used at first. The idea is that we can have it in production first, even if it doesn't change anything.
The second step would be the decryption of the key.
Finally, it's usage.
What do you think ?

@guillaumevincent
Copy link
Member

sounds good to me

@FaustXVI
Copy link
Contributor

I just realized that the "forgot your password ?" feature won't be usable anymore once we encrypted data.
We are going to encrypt the derivation key using the user's password. As a consequence, when the user change its password, the current password will be needed to decrypt the key, then we'll encrypt it with the new password.
Using the "forgot password" feature, the user doesn't know it's password so we can't decrypt the derivation key so the user won't be able to get its data back.
My point of view is that its okay. It's annoying for the user, but it's a password management system. The goal is to protect your password against people not having the master password.
I think we should remove the "forgot password" feature and add a "change password" one.
What do you think ?

@guillaumevincent
Copy link
Member

Yes this is the drawback of using encrypted password profiles, we lost the capability to get old password profiles when we lost our password. Because you are using your master password, we assume you always remember it.
So yes a password change instead of password refresh will be good.

@FaustXVI
Copy link
Contributor

Ok, so what should we start with ?

  • encryption with no way to change password then add the change password feature
  • change password feature first, then encryption

I created #457 for the password change.

@guillaumevincent
Copy link
Member

#457 first yes 👍

@biancarosa
Copy link
Contributor

I'm gonna try to take a look at this.

@guillaumevincent what do you think, instead of using two keys and a random key, we can just use another model like EncryptedPasswordProfile, so in the next time the user logs we...

  • Retrieve not encrypted file.
  • Encrypt in the frontend.
  • Save the encrypted file.

I don't think we need to save the encrypted_key in the database in that scenario, we can just keep a boolean flag to see if the user is using the encrypted profile. That'd probably allow us to encrypt password profiles slowly, and we can generate a random key later if we feel like migrating the old profiles.

@guillaumevincent
Copy link
Member

Hello,
nope we need to generate an encryption key per user in case he want to change his password. We don't want to reencrypt all passwords profiles everytime.

@FaustXVI
Copy link
Contributor

@biancarosa Thank you so much for your work ! I am so overdue ! You clearly removed a huge weight from my shoulders !
You're awesome !

@lolo120916
Copy link

lolo120916 commented Jan 25, 2022

Hello. Thanks for this project and really useful associated tools.
This feature seems to be required for a safe online profiles storing. I have seen the code commits from biancarosa, but no trace of the profile encryption parts seems to remain in the master branch. Am I missing something? When do you plan to integrate profiles encryption?

@guillaumevincent
Copy link
Member

Hello @lolo120916,
The pull request to encrypt password profiles on the frontend is closed. #586
It's a lot of work to do it correctly.
I can't give you an ETA right now.

@guillaumevincent
Copy link
Member

LessPass Database will be closed in March. See announcement. This issue is no longer relevant. Closing it.

@guillaumevincent guillaumevincent closed this as not planned Won't fix, can't repro, duplicate, stale Jan 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants