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

Keyring Service should use different nonces for encrypting imported private keys #24742

Open
kdenhartog opened this issue Aug 17, 2022 · 0 comments
Assignees
Labels
feature/web3/wallet Integrating Ethereum+ wallet support OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. QA/Yes release-notes/include security

Comments

@kdenhartog
Copy link
Member

Currently we're reusing nonces to re-encrypt private keys and mnemonics in a way that's only safe due to the cipher chosen (AES-GCM-SIV) which is nonce reuse resistant.

However, while this isn't a crypto vulnerability it isn't recommended to be used in this way as it reduces the number of messages that can be re-encrypted by the same key/nonce combo.

Additionally, because it's reusing the same nonce/key combination this would output a deterministic ciphertext if the same plaintext is chosen making it not sufficient for IND-CPA security.

These were all acceptable tradeoffs when first analyzed, but in an attempt to further improve the security of the wallet it would be good for us to adhere with the best practices defined in RFC 8452 security considerations which states:

We stress that the nonce misuse-resistance property is not intended to be coupled with intentional nonce reuse; rather, such schemes provide the best possible security in the event of nonce reuse. Due to all of the above, it is RECOMMENDED that AES-GCM-SIV nonces be randomly generated.

To address this issue we can attach the nonce value used during encryption to the end of the base64 encoded ciphertext such that it can be parsed off during decryption without a prefs lookup for each ciphertext.

This would allow us to maintain best practices with the security of imported private keys while using this cipher.

@kdenhartog kdenhartog added OS/Desktop security priority/P3 The next thing for us to work on. It'll ride the trains. feature/web3/wallet Integrating Ethereum+ wallet support release-notes/include QA/Yes labels Aug 17, 2022
@supermassive supermassive self-assigned this Aug 18, 2022
@kdenhartog kdenhartog changed the title Keyring Service should use different nonces for encrypting private keys Keyring Service should use different nonces for encrypting imported private keys Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/web3/wallet Integrating Ethereum+ wallet support OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. QA/Yes release-notes/include security
Projects
None yet
Development

No branches or pull requests

2 participants