rooch-network / rooch

VApp Container with Move Language
https://rooch.network
Apache License 2.0
128 stars 54 forks source link

[gh-755] encrypt w/ ChaCha20Poly1305 and password algorithm Argon2id #856

Closed feliciss closed 7 months ago

feliciss commented 7 months ago

Summary

  1. Add encryption algorithm ChaCha20Poly1305 to encrypt the key pair.
  2. Add password encryption algorithm Argon2id to encrypt password in plaintext and use input password compared with cipher password.
  3. Decryption takes nonce, ciphertext, tag, password from key store to compare with input password and if successful, retrieve the private key and form a key pair to sign transactions.

ChaCha20Poly1305 is a standardized encryption algorithm (AEAD) widely used by network softwares:

  1. https://en.wikipedia.org/wiki/ChaCha20-Poly1305
  2. https://datatracker.ietf.org/doc/html/rfc7539

Argon2id is a password encryption algorithm recommended by OWASP in their recent document:

https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html

vercel[bot] commented 7 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment | Name | Status | Preview | Comments | Updated (UTC) | | :--- | :----- | :------ | :------- | :------ | | **rooch** | ⬜️ Ignored ([Inspect](https://vercel.com/rooch/rooch/8CEpDPp46K4Kj4y5y9vKHx1MpKDU)) | [Visit Preview](https://rooch-git-fork-feliciss-755-rooch.vercel.app) | | Oct 5, 2023 3:24am |
feliciss commented 7 months ago

When to unlock the private key?

When decrypting private key, the user would need to provide:

  1. Mnemonic phrase
  2. Optional password

To decrypt the private key and sign the transaction, by design.

feliciss commented 7 months ago

Discussion:

  1. Remove salt, default to "".
  2. Provide an optional password when initializing, and record it using argon2 hash to key store for password verification.
  3. Provide a clap to enter the password in asterisk.
feliciss commented 7 months ago

TODO: do some refactor to key store and retrieve decrypted private keys from key store params in signing functions.

It may take some days and if it isn't included in Q3 release, I will handle this issue at a stable pace.

feliciss commented 7 months ago

This design may need further refactor:

  1. nonce shouldn't be used twice for Argon2 and ChaCha20Poly1305 cipher in encrypt_private_key.
  2. In encrypt_password, considering generate a random salt or encrypt current private_key as salt.
jolestar commented 7 months ago

Merge this PR first or wait to implement the password option to decrypt private key for commands, like rooch move run, rooch move publish, etc.

feliciss commented 7 months ago

Merge this PR first or wait to implement the password option to decrypt private key for commands, like rooch move run, rooch move publish, etc.

You can merge this PR first and then I will refactor key store, yaml, etc. and test the signature verification feature with encrypted key pair.

The password option has already been added to those commands to decrypt the private key.

jolestar commented 7 months ago

Some tests need to be fixed.

feliciss commented 7 months ago

This command will fail:

rooch move run --function 0x3::empty::empty --sender-account 0xac00d67a15ae97af88aff69cd80befdfe430330a08a6092163de84753cb220c4 --session-key 975a3d052b5ee4bdcc7c8c8a16ae878ef427b4fe6ecb1aa9088b43645111ae20 --password false

The session key 975a3d052b5ee4bdcc7c8c8a16ae878ef427b4fe6ecb1aa9088b43645111ae20 seems not have been saved properly, and it results the error:

Sign message error: signature error: Cannot find SessionKey for address: [0xac00d67a15ae97af88aff69cd80befdfe430330a08a6092163de84753cb220c4]

The new structure of the key store:

keys:
      0xac00d67a15ae97af88aff69cd80befdfe430330a08a6092163de84753cb220c4:
        RoochKeyPairType:
          hashed_password: $argon2id$v=19$m=19456,t=2,p=1$7vLoelDnzdybYdaHLYZXYQrqYSyOix7z5jC6Imf175A$jvcGd8dyjCrG4tAhYTqyq9J1aI54Ugvr0bUYdB9ygSo
          ...

I don't know the session key's design since the structure of the key store has been changed to eliminate the key pair, but is it required to save the session key as key pair in key store or in session_keys: {}?

jolestar commented 7 months ago

We can merge this PR first and discuss some refactors later:

  1. Should every private key have a new password? Or the whole keystore uses one password.
  2. Is it possible to eliminate the --password false options?
feliciss commented 7 months ago

We can merge this PR first and discuss some refactors later:

  1. Should every private key have a new password? Or the whole keystore uses one password.
  2. Is it possible to eliminate the --password false options?
  1. Every key has a password, but the user can ignore entering it. There's an issue with account list when different keys have different passwords.
  2. Yes. We can eliminate the option and make every key encrypted with "" password string.
jolestar commented 7 months ago

2. Yes. We can eliminate the option and make every key encrypted with "" password string.

I didn't mean to remove the password prompt. But it is OK. I will merge this PR first, and we discuss how to refactor it.