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

Add encryptMessage to WalletAdapter interface #323

Closed
mschneider opened this issue Feb 21, 2022 · 10 comments
Closed

Add encryptMessage to WalletAdapter interface #323

mschneider opened this issue Feb 21, 2022 · 10 comments

Comments

@mschneider
Copy link

mschneider commented Feb 21, 2022

Is your feature request related to a problem? Please describe.
I'm trying to use nacl crypto_box style authenticated encryption to store encrypted messages on-chain.

Describe the solution you'd like
encryptMessage(message: Uint8Array, receiver: PublicKey): Promise;

Describe alternatives you've considered
Allow my users only to use CLI, where i can directly access the private key bytes.
Or ask them to input keypair recovery info in my dapp ui 😈

Additional context
Would be good to lobby all wallets to implement this quickly
Reference https://metamask.github.io/eth-sig-util/modules.html#encryptSafely

@amilich
Copy link

amilich commented Feb 27, 2022

This would be super, super helpful. More reference - https://docs.metamask.io/guide/rpc-api.html#restricted-methods

@fastestmannr
Copy link

I agree that this would be a great feature to support. A few thoughts.

  1. You should support encrypt and decrypt
  2. You should return the salt that we should use (random, sequential, or something else) as NaCl requires for security
  3. You should make each encrypt / decrypt require user to approve the action or have some other rate limiting to avoid attacks on the private key by nefarious sites

tweet-nacl's length is probably sufficient for most use cases, but supporting up to 1kb of message length would be better if possible. much longer is probably not needed because it will not fit in a single transaction packet

@AtlasPilotPuppy
Copy link

I would really like this feature. To be able to use my wallet to encrypt and decrypt byte arrays.

@jordaaash
Copy link
Collaborator

Update on this -- I've been working with the Dialect team on a proposed interface for encryption and decryption. A draft of that is here: https://github.com/dialectlabs/spec/blob/main/05_ENCRYPTION.md

I think we'll try to work this into an upcoming standard specification.

@fastestmannr
Copy link

The proposed method seems reasonable to me. Nonce re-use could be a security flaw, so allowing it may not be as optimal as the wallet controlling the nonce entirely.

@cbosborn
Copy link

cbosborn commented Jun 11, 2022

Chris from Dialect here, agree on a lot of the above. We're working with a bunch of wallets on Solana now to get a standard like this adopted. Glad to see this is moving forward.

Our reasoning behind the optional nonce is if the developer has data on hand that could serve as the nonce. For example, a protocol that wishes to encrypt account data could use the account address as the nonce, which will be unique, and saves a bit of storage.

Seems like the tradeoff then for this flexibility is the potential for misuse. Thoughts?

@samkim-crypto
Copy link

Nonce-reuse in symmetric encryption is a pretty strong security vulnerability, so generally, letting the wallet control the nonce entirely is a better option in my opinion. If a protocol that wishes to encrypt account data and use the account address as the nonce, then the account's data must be immutable. For most popular modes of encryption like AES-GCM, if the same nonce is used to encrypt two different messages, then each of the two messages can essentially be revealed.

That being said, there are modes of encryptions like AES-GCM-SIV that are nonce-misuse resistant, meaning that even if the same nonce is used more than once to encrypt different messages, the messages are secure. AES-GCM-SIV will, for example, work perfectly fine when account addresses are used as nonces to save space.

So if this were a standard, then we can consider:

  • Not expose the nonce, but let wallets choose the specific encryption scheme to use
  • Expose the nonce, but also standardize a specific mode of encryption that is misuse-resistant for all wallets to use

@fastestmannr
Copy link

Interface looks good to me.

@jordaaash
Copy link
Collaborator

Closing here since this is being implemented in https://github.com/wallet-standard/wallet-standard

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

8 participants
@mschneider @AtlasPilotPuppy @jordaaash @amilich @cbosborn @samkim-crypto @fastestmannr and others