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

Locked wallet export #108

Open
dirkcuys opened this issue Jul 15, 2022 · 5 comments
Open

Locked wallet export #108

dirkcuys opened this issue Jul 15, 2022 · 5 comments

Comments

@dirkcuys
Copy link

I'm working on adding support for exporting a locked wallet as per the spec to digitalcredentials/learner-credential-wallet

The desired UX is that a user would enter a password to be used to encrypt/decrypt the wallet.

This flow is implemented in the @transmute/universal-wallet package

  1. the password is used to derive a key
  2. the derived key is used as the seed for the random number generator to deterministically get a key pair
  3. the key pair is used as input for the ECDH-ES+A256KW algorithm to encrypt the content encryption key

Looking through the code for step 1, I see that the password is derived with a hard-coded salt value.
This is the function call for getting a key from the password:

const derivedKey = await passwordToKey(password);

and here is the method signature:

export const passwordToKey = async (
  password: string,
  salt: string = "salt",
  iterations: number = 100000,
  digest: string = "SHA-256"
): Promise<Uint8Array> => {

Using a hard-coded salt value creates the possibility of a parallel attack on locked wallets - ie. using a database of leaked passwords, running that through step 1 and 2 to build a database of key pairs to be tested against multiple wallets. This could make it easier for an adversary to discover locked wallets using weak or compromised passwords.

Unless I'm missing something, which I may very well be?

Reading up a little on the JWE related standards, I came across this proposal for using password based encryption in JWE. This is fairly close to the method describe above, but with the benefit of having a proposed method for sharing the header parameters used by PBKDF2.

Or alternatively, would it be possible to use the method used by @transmute/universal-wallet, but using a randomized salt value and sharing the salt value in a standard compliant way?

@dirkcuys
Copy link
Author

What I've ended up implementing is PBES2-HS512+A256KW for deriving a key from the password and using that to wrap an encryption AES-GCM encryption key.

Here is an example of a JWE document:

"jwe": {
  "recipients": [
    {
      "header": {
        "alg": "PBES2-HS512+A256KW",
        "p2s": "lS7S/7QzQMNBApQzjsfnYA==",
        "p2c": 120000,
        "enc": "A256GCM"
      },
      "encrypted_key": "H8COfQhinpDVqxx8Kl6EJ8U/295UZ7/v9xxxe2t24oGiTw5RfCX4gA=="
    }
  ],
  "iv": "wJ8Mol+iVntgZU6/",
  "ciphertext": "gxUYD0f/WORxXF1s6VTRn+3mF/5zips2qZtUUUjTuW/L5sO5dcCGtPwmRjuatDgk2A7f... ",
  "tag": "pxbfT0lDBU/RZjGlTmO+1w=="
}
  • p2s is the seed that was used during key deriviation
  • p2c is the iteration count used for the PBKDF2 algorithm
  • ciphertext is the encrypted version of the unlocked wallet representation described in the spec

Here is the code we're using for encryption and decryption: https://github.com/digitalcredentials/learner-credential-wallet/blob/main/app/lib/lockedWallet.ts

@dmitrizagidulin
Copy link

+1 to all this, great work Dirk.

(One minor note, we should consider recommending Argon2 instead of PBKDF2 (see https://medium.com/analytics-vidhya/password-hashing-pbkdf2-scrypt-bcrypt-and-argon2-e25aaf41598e and similar papers)).

@dlongley
Copy link

One minor note, we should consider recommending Argon2 instead of PBKDF2...

Something to consider is that PBKDF2 has native support in browsers whereas Argon2 does not. So it may be the case that you'll end up actually getting better security properties via PBKDF2 than you would with Argon2.

@dmitrizagidulin
Copy link

@dlongley - ah, yeah, that's a great point. (I think that's why @dirkcuys and I settled on PBKDF2 in the first place, for the initial implementation).

@dirkcuys
Copy link
Author

dirkcuys commented Dec 1, 2022

I took a stab at updating the docs.

What is the primary use case for the locked wallet specification? For backing up a wallet, password based encryption makes the most sense, but in other context it might make more sense using ECDH-ES+A256KW.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants