lesspass icon indicating copy to clipboard operation
lesspass copied to clipboard

Password profiles should be encrypted

Open vpfautz opened this issue 9 years ago • 23 comments

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?

vpfautz avatar Mar 30 '17 19:03 vpfautz

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 avatar Mar 31 '17 08:03 guillaumevincent

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

aurimasmb avatar Aug 08 '17 22:08 aurimasmb

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?

aurimasmb avatar Aug 08 '17 23:08 aurimasmb

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

guillaumevincent avatar Aug 09 '17 12:08 guillaumevincent

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

betsythefc avatar Nov 09 '18 17:11 betsythefc

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.

vpfautz avatar Nov 13 '18 17:11 vpfautz

@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.

betsythefc avatar Apr 16 '19 04:04 betsythefc

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

guillaumevincent avatar Apr 16 '19 14:04 guillaumevincent

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.

FaustXVI avatar Jul 24 '19 10:07 FaustXVI

@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

guillaumevincent avatar Jul 24 '19 12:07 guillaumevincent

@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 :)

FaustXVI avatar Jul 24 '19 14:07 FaustXVI

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?

guillaumevincent avatar Jul 24 '19 20:07 guillaumevincent

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 ?

FaustXVI avatar Jul 29 '19 07:07 FaustXVI

sounds good to me

guillaumevincent avatar Jul 30 '19 12:07 guillaumevincent

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 ?

FaustXVI avatar Jul 31 '19 12:07 FaustXVI

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.

guillaumevincent avatar Jul 31 '19 12:07 guillaumevincent

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.

FaustXVI avatar Jul 31 '19 13:07 FaustXVI

#457 first yes :+1:

guillaumevincent avatar Jul 31 '19 13:07 guillaumevincent

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.

biancarosa avatar Jul 25 '20 01:07 biancarosa

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.

guillaumevincent avatar Jul 26 '20 06:07 guillaumevincent

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

FaustXVI avatar Jan 19 '21 11:01 FaustXVI

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?

lolo120916 avatar Jan 25 '22 15:01 lolo120916

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

guillaumevincent avatar Jan 25 '22 20:01 guillaumevincent

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

guillaumevincent avatar Jan 02 '23 11:01 guillaumevincent