Skip to content
This repository has been archived by the owner on Dec 6, 2022. It is now read-only.

Use TweetNaCL to lower bundle size to drastically #9

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pelle
Copy link
Contributor

@pelle pelle commented Jul 19, 2019

As we are developing tools that we want to reach a very wide audience it is important to think of bundle size.

I have changed the use of libsodium to tweetnacl. Todo this I had to change the symmetric encryption algorithm from chacha20poly1305_ietf to xsalsa20-poly1305 aka secretbox. This is equally well supported by libsodium, but allows DIDComm to be a realistic dependency in client libraries.

The other option that we could entertain is that someone implements a lightweight library of only the required libsodium functions or that a subset of libsodium is implemented in wasm and distributed in a cached CDN'd version.

Using libsodium.js

The bundle sizes are:

-rw-r--r--  1 pelleb  staff  890376 Jul 18 20:44 main.js
-rw-r--r--  1 pelleb  staff  300687 Jul 18 20:43 main.js.gz

Using tweetnacl.js

The bundle size goes down to:

-rw-r--r--  1 pelleb  staff  77581 Jul 18 20:48 main.js
-rw-r--r--  1 pelleb  staff  25286 Jul 18 20:47 main.js.gz

@kdenhartog
Copy link
Contributor

This all looks good, and I'm on board with it other than one exception. Currently the 'protected' field is used as authenticated additional data in order to integrity protect that data. This makes it possible to detect tampering, however with the new update this is no longer being done.

I did see that there's a few libraries that extend this tweetnacl library, and it may be worth exploring them to add them in. The detached part is not a big deal because the nacl_box is just appending it onto the ciphertext and we can manually detach and attach it.

Other than the small issue with the protected data section, I like the idea of going this route and if we can find an amicable I'm all for merging it.

Thanks for the PR, and I'll keep my eyes open for finding a solution. Also, if you want to jump on a call to discuss this further I'm open to that as well. Just message me on DIF and we can get something setup.

@OR13
Copy link

OR13 commented Aug 4, 2019

What is blocking this PR from getting merged? I suggest we use the CR tools on github and request changes or approve them.

@OR13
Copy link

OR13 commented Aug 4, 2019

There is also this library https://www.npmjs.com/package/chloride

Which may not help, just pointing it out here.

@kdenhartog
Copy link
Contributor

What is blocking this PR from getting merged? I suggest we use the CR tools on github and request changes or approve them.

I provided details of what needs to be changed in the comment above. After further investigation (provided in the issues section), we've concluded that tweetNaCl won't support our needs because it only uses Salsa/XSalsa which are only authenticated encryption algorithms not an AEAD algorithm which is required by the JWE spec.

@kdenhartog
Copy link
Contributor

kdenhartog commented Aug 5, 2019

There is also this library https://www.npmjs.com/package/chloride

Which may not help, just pointing it out here.

I've seen this library, but haven't been able to dig into it yet. From what I can tell it's very similar to the dependency we're currently using (libsodium-wrappers) which is too large. I believe they both rely on a wasm compilation of the libsodium library and that makes it rather large. After checking into this, Chloride appears to be larger than the current package we're using according to https://bundlephobia.com

The path we're looking at taking is to be dependent on stablelib which the author of TweetNaCl pointed us to in this issue. More discovery is being done still.

@kdenhartog
Copy link
Contributor

Update on moving to StableLib. I'm currently working on an upstream PR to make it possible to use Xchacha from Stablelib.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants