zeruniverse / Password-Manager

An online keepass-like tool to manage password. client-side AES encryption!
Other
170 stars 44 forks source link

salts in variables to edit #225

Closed Pofilo closed 5 years ago

Pofilo commented 5 years ago

When we install the Password-Manager, we should change all salts. So I move them before the comment STOP EDITING IF YOU DON'T KNOW WHAT YOU ARE DOING.

BenjaminHae commented 5 years ago

Maybe we should also add an example command for generating these salts using openssl or something similar. (I need to figure out the constraints on these variables beforehand).

BenjaminHae commented 5 years ago

This PR also contains the changes from #224. Please start with this projects master-branch next time you create a PR, this prevents the reintroducing other stuff. (Look at the files changed in this PR ;) )

BenjaminHae commented 5 years ago

But otherwise: Thank you very much. These are very reasonable changes!

Pofilo commented 5 years ago

Ooops, a rebase and it is fixed ^^

In addition of a command for generating those salts, do you know if there is a way to change them after the setup ? I mean if my instance is up for 5 years, my salts would be the same as at the beginning and would remain the same forever (imagine a not crypted backup of the configuration or something like that). I'm not good enough in crypto to know if we can make a script to change those salts and update all passwords in database.

Of course, it would deny all backups (made by the manager) made before the change.

BenjaminHae commented 5 years ago

Ooops,` a rebase and it is fixed ^^

Also please don't do rebases on things you already pushed ;) This breaks the history. (In this case it doesn't matter, but it would lead to problems if someone else had already checked out this branch) Do merges instead.

BenjaminHae commented 5 years ago

As far as I remember changing the salts afterwards is not as easy. You'd need to save backups for all users and import them after the values are changed.

Backups do include the salts, so you should be able to restore from older backups.

It is indeed possible to write a script that creates a backup for every user. (I had one, but I lost it :(, it wasn't very hard to write)

BenjaminHae commented 5 years ago

It's good practice to change the salts on installation, but I don't think it's necessary to change them afterwards. I'd also not say an installation is really insecure when you don't change the salts at the beginning. It just makes guessing the login credentials a very little bit easier.

Pofilo commented 5 years ago

I'd also not say an installation is really insecure

With bans on account + IP, it is okay, but if someone forget to change their salts, the script could be useful is this case too I guess. But this is not a priority !

In some projects, people prefer to only rebase (even merges) to have the cleanest history.

BenjaminHae commented 5 years ago

As long as the rebase happens when it's on your machine I'm fine with it. I do it myself the same way. The moment it's something public rebases should not be done.

zeruniverse commented 5 years ago

Actually, just no rebase on ‘master’

On Sat, Oct 13, 2018 at 2:05 PM Benjamin Häublein notifications@github.com wrote:

As long as the rebase happens when it's on your machine I'm fine with it. I do it myself the same way. The moment it's something public rebases should not be done.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/zeruniverse/Password-Manager/pull/225#issuecomment-429562851, or mute the thread https://github.com/notifications/unsubscribe-auth/AEbvNCl4_bLeu0YnWbZYft30Z8ujmRs3ks5ukiuDgaJpZM4XauCT .

BenjaminHae commented 5 years ago

Maybe you can add that way to create random strings: https://unix.stackexchange.com/a/230676/105732

SALT3 can be changed anytime, because it's not used to decrypt or hash anything that must stay the same. So you could add that to a comment as well.

zeruniverse commented 5 years ago

I think this proposal is good. If no opposition, I'll merge.