w3c-ccg / universal-wallet-interop-spec

A data model and abstract interfaces for digital wallets
http://w3id.org/wallet
Other
56 stars 13 forks source link

Locked wallet export #108

Open dirkcuys opened 2 years ago

dirkcuys commented 2 years ago

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 commented 1 year ago

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=="
}

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 commented 1 year ago

+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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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.