Asymmetric encryption/decryption feature #1331

LaurentTrk · 2021-12-20T23:36:05Z

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 to util-crypto\sr25519 following ECIES
Add encrypt/decrypt methods to util-crypto\ed25519 using existing encryption feature, but with an ephemeral key (similar as ECIES)
Add encrypt method to keyring (encrypt data without the need of a keypair)
Add encrypt/decrypt methods to keyringPair

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 from
mnemonicGenerate
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 🙏

Nota : Some folks were interested in this feature:

h4x3rotab · 2021-12-24T04:25:14Z

Generally good. However I have two suggestions:

We need better documentation. The interface exposed is encrypt and decrypt, 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');
```

LaurentTrk · 2021-12-24T08:03:38Z

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.

defliction · 2022-01-07T19:56:48Z

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.

gdethier · 2022-03-25T11:07:10Z

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. ;-)

LaurentTrk · 2022-03-25T15:34:55Z

Thanks for your comment @gdethier ;)
I was expecting more reviews, but it seems that we are ready now...

gdethier

I just left a small suggestion.

gdethier · 2022-03-28T09:22:14Z

packages/keyring/src/pair/index.ts

+
+      return type === 'ed25519'
+        ? ed25519Decrypt(u8aToU8a(encryptedMessage), { publicKey, secretKey })
+        : sr25519Decrypt(u8aToU8a(encryptedMessage), { publicKey, secretKey });


In above 2 lines, no need to pass-in the public key, only the secret key is needed to decrypt the message.

gdethier · 2022-04-27T12:49:01Z

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.

mgravitt · 2022-05-17T23:24:18Z

I am also anxiously awaiting this feature, into this library and the corresponding one on the extension. thank you!

jacogr · 2022-05-18T06:16:30Z

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.

jacogr

I think this is nice work. However I'm not at all convinced about the requirement to have to specify the key type for the recipient - this is the stickler in making it available to all users, techy and non-techy.

... this is exactly the bigger issue that we are facing with this functionality: multiple key types, without users knowing what they are and them having to magically co-operate.

jacogr · 2022-06-11T09:18:24Z

packages/keyring/src/keyring.ts

+   * The encrypted message can be decrypted by the corresponding keypair using keypair.decrypt() method
+   */
+  public encrypt (message: string | Uint8Array, recipientPublicKey: string | Uint8Array, recipientKeyType?: KeypairType): Uint8Array {
+    return cryptoEncrypt(message, recipientPublicKey, recipientKeyType || this.type);


Generally users have no idea even what their own keypair types are, let alone somebody else knowing what a specific user has.

This means that the addition of the recipientKeyType here is an issue - the sender won't know which type the actual recipient uses. It could be anything and for most users this is not known at all.

Hi @jacogr,
Thanks for your review.
I actually did not catch your point here, the users of this library are not end users, I guess that they are more techy (as developers) and aware of what key types and encryption are.
I agree that passing the wrong key type for a given public key will fail, but we need this information.

In case the user don't know which type to use, the keyring will use its default value (the one used when creating new key pairs)

jacogr · 2022-06-11T09:19:35Z

packages/keyring/src/pair/index.spec.ts

+  it('allows encrypt/decrypt with sr25519 keypair', (): void => {
+    const aliceSR25519KeyPair = new Keyring().createFromUri(mnemonicGenerate(), { name: 'sr25519 pair' }, 'sr25519');
+    const bobSR25519KeyPair = new Keyring().createFromUri(mnemonicGenerate(), { name: 'sr25519 pair' }, 'sr25519');
+    const message = new Uint8Array([0x61, 0x62, 0x63, 0x64, 0x65]);
+    const encryptedMessage = aliceSR25519KeyPair.encrypt(message, bobSR25519KeyPair.publicKey);
+
+    expect(bobSR25519KeyPair.decrypt(encryptedMessage)).toEqual(message);
+    expect((): Uint8Array | null => aliceSR25519KeyPair.decrypt(encryptedMessage)).toThrow("Mac values don't match");
+  });


We would need an additional test (on the back of the above above comment), where the one party uses ed25519 and the other party using sr25519 and they can exchange messages bi-directionally.

This test has not been done as you cannot exchange messages if the 2 key types differs.

gdethier · 2022-06-28T10:43:23Z

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: LaurentTrk#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.

LaurentTrk · 2022-07-04T15:57:24Z

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).

jacogr · 2023-03-14T05:47:14Z

Closing, removing this functionality in #1762

polkadot-js-bot · 2023-04-04T07:23:42Z

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.

LaurentTrk added 3 commits December 21, 2021 00:09

Add encrypt/decrypt functions for ed/sr25519

d599e31

Add encryption/decryption features for keyring and keypairs

aefa49d

FIX Refactor decryption function selection

f37386c

LaurentTrk mentioned this pull request Dec 20, 2021

Encryption/Decryption Feature polkadot-js/extension#928

Closed

Add mac step to sr25519 encrypt/decrypt

4bbc836

LaurentTrk added 3 commits December 24, 2021 11:39

FIX Remove circular dependency in ed25519.decrypt

7f2fff7

Add optional sender keypair (instead of ephemeral generated key)

c3959bb

Add implementation details

2d98c0d

LaurentTrk marked this pull request as ready for review March 25, 2022 15:35

LaurentTrk changed the title ~~[WIP] Asymmetric encryption/decryption feature~~ Asymmetric encryption/decryption feature Mar 25, 2022

gdethier approved these changes Mar 28, 2022

View reviewed changes

gdethier mentioned this pull request Apr 27, 2022

feat: complete decrypt feature. LaurentTrk/extension#1

Merged

jacogr reviewed Jun 11, 2022

View reviewed changes

gdethier mentioned this pull request Jun 24, 2022

Enable public key type guessing in encryption LaurentTrk/common#1

Open

RyanDinh8 mentioned this pull request Aug 2, 2022

Support public and private key encryption Koniverse/SubWallet-Extension#516

Open

8 tasks

jacogr mentioned this pull request Mar 14, 2023

Remove support for encrypt/decrypt on keypair #1762

Merged

jacogr closed this Mar 14, 2023

polkadot-js locked as resolved and limited conversation to collaborators Apr 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Asymmetric encryption/decryption feature #1331

Asymmetric encryption/decryption feature #1331

LaurentTrk commented Dec 20, 2021 •

edited

h4x3rotab commented Dec 24, 2021 •

edited

LaurentTrk commented Dec 24, 2021 •

edited

defliction commented Jan 7, 2022 •

edited

gdethier commented Mar 25, 2022 •

edited

LaurentTrk commented Mar 25, 2022

gdethier left a comment

gdethier Mar 28, 2022

gdethier commented Apr 27, 2022

mgravitt commented May 17, 2022

jacogr commented May 18, 2022

jacogr left a comment

jacogr Jun 11, 2022

LaurentTrk Jul 4, 2022 •

edited

jacogr Jun 11, 2022

LaurentTrk Jul 4, 2022

gdethier commented Jun 28, 2022

LaurentTrk commented Jul 4, 2022 •

edited

jacogr commented Mar 14, 2023

polkadot-js-bot commented Apr 4, 2023

Asymmetric encryption/decryption feature #1331

Asymmetric encryption/decryption feature #1331

Conversation

LaurentTrk commented Dec 20, 2021 • edited

What has been done ?

Implementation details

h4x3rotab commented Dec 24, 2021 • edited

LaurentTrk commented Dec 24, 2021 • edited

defliction commented Jan 7, 2022 • edited

gdethier commented Mar 25, 2022 • edited

LaurentTrk commented Mar 25, 2022

gdethier left a comment

Choose a reason for hiding this comment

gdethier Mar 28, 2022

Choose a reason for hiding this comment

gdethier commented Apr 27, 2022

mgravitt commented May 17, 2022

jacogr commented May 18, 2022

jacogr left a comment

Choose a reason for hiding this comment

jacogr Jun 11, 2022

Choose a reason for hiding this comment

LaurentTrk Jul 4, 2022 • edited

Choose a reason for hiding this comment

jacogr Jun 11, 2022

Choose a reason for hiding this comment

LaurentTrk Jul 4, 2022

Choose a reason for hiding this comment

gdethier commented Jun 28, 2022

LaurentTrk commented Jul 4, 2022 • edited

jacogr commented Mar 14, 2023

polkadot-js-bot commented Apr 4, 2023

LaurentTrk commented Dec 20, 2021 •

edited

h4x3rotab commented Dec 24, 2021 •

edited

LaurentTrk commented Dec 24, 2021 •

edited

defliction commented Jan 7, 2022 •

edited

gdethier commented Mar 25, 2022 •

edited

LaurentTrk Jul 4, 2022 •

edited

LaurentTrk commented Jul 4, 2022 •

edited