archimatetool / archi-modelrepository-plugin

coArchi - a plug-in to share and collaborate on Archi models.
152 stars 52 forks source link

Use encryption for storing credentials with primary password and key #152

Closed Phillipus closed 3 years ago

Phillipus commented 3 years ago

From @jbsarrodie:

When storing of username/password credentials is enabled (see #151), store these in an encrypted file.

Phillipus commented 3 years ago

Note - we should not use the word "master" now. Mozilla has chosen to use the word "primary" as in "primary password" and "primary key". See https://support.mozilla.org/en-US/kb/primary-password-replacing-master-password

Phillipus commented 3 years ago

I have created a new class EncryptedCredentialsStorage that:

  1. Creates a random SecretKey using javax.crypto.KeyGenerator
  2. Stores this key encrypted with a user provided primary password into a file primary_key
  3. Stores each username/password into a file for each repository (secure_credentials) using the key generated in (1) as the encryption key
Phillipus commented 3 years ago

In my tests, the main problem I am encountering is when to ask the user for the primary password (and create the primary_key file if not already created).

For example:

  1. User opens Authentication tab in Properties
  2. secure_credentials needs to be accessed for username/password
  3. This requires access to primary_key and user to provide primary password
  4. So a dialog asks user for primary password
  5. If user pressed cancel or enters wrong password the fields are blank
  6. When can user enter password again?

I'm wondering now whether to store the user name for each repo separately to the encrypted password file so at least we can display and edit that. And hide the password field and only allow to edit/set it in a dialog box.

Do we need to encrypt the user name?

Phillipus commented 3 years ago

In branch encryption:

I've now replaced all instances of the old SimpleCredentialsStorage with EncryptedCredentialsStorage and it works but some things need to be tidied up:

So, at this stage it's a little messy but the encryption/decryption and primary key secured by primary password is working.

Phillipus commented 3 years ago

Also, as mentioned, because the username for a repo (and username for proxy credentials in the preference page) is encrypted we can't display the user name until the primary password has been entered. Which is a bit of a nuisance.

We could not encypt the username and retrieve that anyway and only encrypt the password?

Phillipus commented 3 years ago

SmartGit uses a credentials.yml file that stores usernames for a repo in plain text with a reference to a password that is stored separately in an encrypted file.

jbsarrodie commented 3 years ago

We could not encypt the username and retrieve that anyway and only encrypt the password?

Yes, that makes sense

Note - we should not use the word "master" now. Mozilla has chosen to use the word "primary" as in "primary password" and "primary key". See https://support.mozilla.org/en-US/kb/primary-password-replacing-master-password

I never noticed this because in french master and principal both translate to 'principal' in this context. But of course I absolutely agree if master is offending for native speakers.

Phillipus commented 3 years ago

OK, I'll re-work things so we can retrieve the user name from plain text.

This means that wherever we allow the user to enter a password ("Authentication" tab and Preferences) we only need to access and decrypt the password when changing it.

Phillipus commented 3 years ago

BTW - the encrypted repo files are named secure-credentials so as not to overwrite the old encoded credentials files. This is useful while we test things.

Phillipus commented 3 years ago

This is all working now for me in branch encryption. I've refactored everything to account for non-Ascii characters in passwords and to use char arrays instead of Strings and a bunch of other stuff. The user is asked for primary password once when the session needs it. This can be changed in Preferences.

jbsarrodie commented 3 years ago

Ok, I've finally found some time to test this and... I can't even run it :-(

Eclipse raises an error in org.archicontribs.modelrepository.authentication.EncryptedCredentialsStorage#loadPrimaryKey, saying that The method readAllBytes() is undefined for the type FileInputStream:

            // Read in all bytes
            try(FileInputStream fis = new FileInputStream(file)) {
                bytes = fis.readAllBytes();
            }

Does this error rings a bell ? I don't see what's the issue on my eclipse setup... (so switching to other tests on branches)

Phillipus commented 3 years ago

You need to be using Java 11. The target for Archi is now Java 11 since some of Eclipse's plug-ins use that as a minimum.

Phillipus commented 3 years ago

You need to be using Java 11. The target for Archi is now Java 11 since some of Eclipse's plug-ins use that as a minimum.

Having said that, there is another way to do this that is Java 8 compatible:

Files.readAllBytes(file.toPath());

So I've just pushed this.

jbsarrodie commented 3 years ago

OK, here are my tests:

  • The first time Archi is run with coArchi, prompt user for a primary password, then generate a random encryption key (under « model-repository ») protected by the chosen password.

OK

  • Each time a password must be saved, save it at the usual place (model-folder/.git/secure_credentials), but instead of encoding the password, encrypt it using the encryption key.

OK

  • Each time a password must be saved or retrieved, check if the encryption key has already been loaded, if not, ask user for their primary password and load the encryption key.

OK, but if the entered primary password is not the right one, the error message is not user-friendly: image

Would it be possible to trap this and show something like "There was an error (a bad password was entered or the encryption key is invalid)" ?

  • (nice to have) there should be an option to change the primary password. This simply load the encryption key using the current primary password and then change the key password.

OK

Phillipus commented 3 years ago

Would it be possible to trap this and show something like "There was an error (a bad password was entered or the encryption key is invalid)" ?

Done

jbsarrodie commented 3 years ago

I've tested the latest version and this if perfect.

Closing this issue.

beno commented 4 months ago

I have an issue trying to use coArchi with an Azure git repo and their Personal Access Tokens. It works for a while and after I guess 24h every Publish action pops up the 'enter primary password' dialog. This PAT is not expired, and nothing else has changed. I don't have the password stored in cleartext (obviously) so all I can do is regenerate it, but so far I have not been able to get it working again an all I see is the 'Enter password' dialog. Any tips? Thanks for the great work BTW.