common
common copied to clipboard
Asymmetric encryption/decryption feature
Hi all,
This work is an alternative way to add the asymmetric encryption/decryption feature than the one already implemented, which does not work with sr25519 (see workaround proposal by @RoyTimes).
It works more like the eth_decrypt function from Metamask, where only one public key/private key pair is involved : you can encrypt data with the public key of the recipient, and only this recipient can decrypt the data with his private/secret key (vs the already implemented solution using nacl.box which requires 2 keypairs).
What has been done ?
- Add
encrypt/decrypt
methods toutil-crypto\sr25519
following ECIES - Add
encrypt/decrypt
methods toutil-crypto\ed25519
using existing encryption feature, but with an ephemeral key (similar as ECIES) - Add
encrypt
method tokeyring
(encrypt data without the need of a keypair) - Add
encrypt/decrypt
methods tokeyringPair
Implementation details
Following SR25519 Encryption/Decryption following Elliptic Curve Integrated Encryption Scheme (ECIES)
See https://cryptobook.nakov.com/asymmetric-key-ciphers/ecies-public-key-encryption
The algorithms used were chosen among those already used in this library.
-
Ephemeral Key generation
Generate new keypair using the wasm
sr25519KeypairFromSeed
function, with a random seed frommnemonicGenerate
-
Key Agreement
Use wasm
sr25519Agree
function between the generated ephemeral private key and the recipient public key -
Key Derivation
Use
pbkdf2
(random salt is generated, default 2048 rounds) to derive a new secret from the previous step output The derived secret is split into :- MAC key (first 32 bytes)
- encryption key (last 32 bytes)
-
Encryption
Use
nacl.secretbox
api symmetric encryption (xsalsa20-poly1305) to encrypt the message with the encryption key generated at step 3. A nonce (24 bytes) is randomly generated. - MAC Generation HMAC SHA256 (using the MAC key from step 3) of the concatenation of the encryption nonce, ephemeral public key and encrypted message
The encrypted message is the concatenation of the following elements :
- nonce (24 bytes) : random generated nonce used for the symmetric encryption (step 4)
- keyDerivationSalt (32 bytes) : random generated salt used for the key derivation (step 3)
- public key (32 bytes): public key of the ephemeral generated keypair (step 1)
- macValue (32 bytes): mac value computed at step 5
- encrypted (remaining bytes): encrypted message (step 4)
Feel free to share your comments and feedbacks :pray:
Nota : Some folks were interested in this feature:
Generally good. However I have two suggestions:
-
We need better documentation. The interface exposed is
encrypt
anddecrypt
, which hides all the details behind. We should at least include the basic algorithm and message format used in the comments, including:- How the ephemeral key pair is generated?
- How the key is derived? Does it follow any crypto standard?
- Which symmetric encryption algorithm and parameter is used?
This is important because once the ecosystem grows, polkadotjs will not be the only library to implement the cryptographic. It's important to have clear guide for other SDKs to implement the standard with common crypto libraries. The later requirement also implies we should try our best to stick to popular crypto standard and algorithm.
-
Expose the key agreement interface. Per ECIES, under the hood the symmetric key is generated by two parties. In this PR, it only addresses a special case where one party is represented by an ephemeral key. It assumes all the developer has to use an ephemeral key. However, I believe the key agreement is very useful as well. An idea is to leave the key agreement exposed, and then add a new function to generate the ephemeral key. So the two use cases can be fully covered. An example API design:
-
keypair.encrypt(otherPartyPubkey, plainMessage)
-
keypari.decrypt(otherPartyPubkey, encryptedMessage)
The
otherPartyPubkey
can be easily generated by:const mnemonic = mnemonicGenerate(); const pair = keyring.addFromUri(mnemonic, { name: 'first pair' }, 'sr25519');
-
Thanks @h4x3rotab ! I will add implementation details shortly.
Regarding your proposal on exposing the agreement, I would like to propose the 2 options (ephemeral key as ECIES and 2nd party key), depending on the way you call the encrypt
method:
-
keypair.encrypt(otherPartyPubkey, plainMessage)
: use the private key of the key pair (no need of an ephemeral keypair) -
keyring.encrypt(otherPartyPubkey, plainMessage)
: generate an ephemeral keypair (as specified in ECIES)
In both options, the decrypt()
method does not need the public key argument, as the public key used (either from an ephemeral key or a 2nd party key) is embedded in the encrypted message.
Edit: add implementation details to PR description.
Thanks for this work @LaurentTrk - I'm not able to add any coding or encryption contributions but from a usability point eth_decrypt
requires the MetaMask user to decrypt each message by approving the popup dialogue; it would simpler for a user to permit decryption once and allow the dApp to encrypt decrypt at will from thereon. Although this comment may be relevant to the UI implementation following your work here.
Hello @LaurentTrk, thanks a lot for this contribution! I reviewed the code and IMHO, everything is fine (documentation, style, tests). Is there something preventing you to change the status of this PR to pending? If not, I suggest that you do so so that maintainers can review it as well. Remove also the [WIP] is the PR's title. ;-)
Thanks for your comment @gdethier ;)
I was expecting more reviews, but it seems that we are ready now...
Hello @jacogr ! Did you already have a look at this PR?
We would really love to see this feature integrated as we rely on it in this fork of the Polkadot JS extension which brings encryption/decryption: https://github.com/LaurentTrk/extension/tree/DecryptionFeature (we are actually waiting for this PR to be merged before creating the one on the extension).
If there is anything we can do to make your life easier, please do not hesitate to tell.
I am also anxiously awaiting this feature, into this library and the corresponding one on the extension. thank you!
Will bump it up the overflowing task queue. Sorry for the long time getting here, it is on the list, but not a lot of progress seems to be made on my side getting through.
Hello @jacogr and thank you very much for your review. Sorry to come back to you so late but I had to test a few things. I tried to implement encryption/decryption without any prior knowledge about the key type but was not completely successful.
Here is what I came up with: https://github.com/LaurentTrk/common/pull/1 (in another PR to keep this one "clean" of my experiments). In the code, when no type is provided, the key type is guessed. However, that method does not work in all cases (this test passes but this one does not).
I am no cryptographer so I think that I reached the limits of what I can achieve. Suggestions are more than welcome.
Hi @gdethier !
Thanks for your work, that's could be great yes, but unfortunately, sometimes just checking that a key could fit in one or the other type is not enough, some keys fit in both, but that leads to incorrect encryption/decryption.
That was the problem in the very initial version of this feature: sr22519 were forced to be treated as ed25519, but that just not work (despite the unit tests proving so, that were using keys that fitted both key types).
Closing, removing this functionality in https://github.com/polkadot-js/common/pull/1762
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.