sodium-friends / sodium-javascript

Pure Javascript version of sodium-native
MIT License
92 stars 24 forks source link

Support react-native (it's working, it just needs to be merged in) #48

Open RangerMauve opened 4 years ago

RangerMauve commented 4 years ago

Hey friends,

We've been working on getting sodium-javascript working in React-Native at @consento-org and we've got some stuff together to make it happen.

The main piece that was breaking was the randombytes implementation only working in Node and the web.

To fix that @martinheidegger put together get-random-values-polypony which supports most JS environments and has tests passing across them.

We also needed to change up memory.js to use the old version which didn't use the worker_threads module since that isn't available in react-native. I'm not sure if this is the best approach, however, and I'd be happy to adjust code to account for this. For example, I'd love to fix #47 at the same time.

Right now all the work we've done is here: https://github.com/sodium-friends/sodium-javascript/compare/master...consento-org:rn-tape

The other useful bit that was added was running tests on Android/iOS with react-native and Expo via Browserstack. This is encapsulated in our rn-tape module. I think it'd be nice to add it to the CI to cover tests for these environments as well. Main thing that would need to be done is setting up environment variables for a BrowserStack account for the sodium-friends org. I'm pretty sure there's an open source tier we could use and I'd be happy to help set that up if needed.

All that to say, I'd like to do any of the following:

I wanted to put this out there and ask if there are any maintainers that have bandwidth to review and merge these changes as I submit them.

I think sodium-javascript is a super useful tool, and lots of people could benefit from it being available in React-Native out of the box (we're going to be using it for hypercore stuff).

mafintosh commented 4 years ago

I'm not sure I fully follow what is missing? It doesn't work if you polyfill in what you need here https://github.com/sodium-friends/sodium-javascript/compare/master...consento-org:rn-tape#diff-1dd241c4cd3fd1dd89c570cee98b79ddR8 ?

mafintosh commented 4 years ago

On the worker_threads thing, ya would be good if that's simple

spieglt commented 4 years ago

I've been struggling to use libsodium in a React Native project and would greatly benefit from this as well.

RangerMauve commented 4 years ago

@spieglt Here's the shimming we did to get it to run with Hypercore:

https://github.com/consento-org/hypercore-rn/blob/main/shim.js

Note that we're currently using a branch of sodium-javascript until #51 gets merged. 😁

emilbayes commented 4 years ago

One thing of note that can improve #51 if you choose to use it is to return after the sodium_memzero. Otherwise you have secrets floating around in memory for an undefined period of time. Sending off the buffer on a MessageChannel just ensures that you will not accidentally read from a buffer that has been zero'ed out (ie. encrypting with the zero key).

I'm still not keen on merging the PR since it will lead to "use after free" like vulnerabilities, if there's no way to transfer away the underlying ArrayBuffer

spieglt commented 4 years ago

Thank you @RangerMauve! I added shim.js to my project, added your branch to my package.json, and seeing the require('process') ran npm install process to match your package.json. But it's still missing buffer because of the lack of the Node standard library. Where does that come from in hypercore-rn?

RangerMauve commented 4 years ago

@emilbayes Thanks for the heads up, I'll follow up on that this week. 😁

@spieglt I think process and buffer are regular NPM packages that you can install.

spieglt commented 4 years ago

Oh duh, thanks it's working. I didn't see it in hypercore-rn's package.json however. Where does it come from in that project's React Native build, out of curiosity?

RangerMauve commented 4 years ago

@spieglt I think it might actually be getting imported as a dependency of some other dependency. 😂 I should fix that up to make it explicit.

RangerMauve commented 3 years ago

K, so #51 doesn't matter anymore. I've taken a slightly different approach since. Though #56 is still needed because it messes up the crypto shimming.

  1. Set up some globals by requiring a file before anything else loads
// Lots of hypercore code assumes a global `process`
global.process = require('process')

// Lots of the sodium code assumes a global Buffer
global.Buffer = require('buffer').Buffer

// iOS defines a WebAssembly global, but doesn't provide a way to create instances
// We shold delete the WebAssembly global in that case so that the tests pass
if (typeof WebAssembly !== 'undefined' && global.WebAssembly) {
  const canMakeInstance = global.WebAssembly.Instance || global.WebAssembly.instantiate
  if (canMakeInstance) {
    global.WebAssembly = undefined
  }
}

// We should make sure there is a randombytes implementation
require('get-random-values-polypony').polyfill()
  1. Alias some modules to make them all play nice
module.exports = function (api) {
  api.cache(true)
  return {
    plugins: [
      [
        'babel-plugin-rewrite-require',
        {
          aliases: {
            'sodium-native': 'sodium-javascript',
            'sodium-universal': 'sodium-javascript',
            stream: 'readable-stream',
            hyperswarm: 'hyperswarm-web',
            crypto: 'get-random-values-polypony',
            worker_threads: './worker-threads-shim.js'
          }
        }
      ]
    ],
    presets: ['babel-preset-expo']
  }
}
  1. Make an absolutely disgusting worker_threads implementation since a fake global MessageChannel can mess up a bunch of other RN related code. worker-threads-shim.js
module.exports = {
  MessageChannel: function () {
    return {
      port1: {
        postMessage: () => null
      }
    }
  }
}

With all this in place you can run sodium-universal based codebases like hypercore.

emilbayes commented 3 years ago

Just one word of warning reading this, the random number generator polyfill is something to be very wary of. I only skimmed the source for the one linked above, so I only have superficial impressions, so I won't comment specifically on that module. But! Randomness is the most sensitive element in a cryptographic stack. There are numerous examples of applications that have been compromised due to weak randomness, and it's something that can't be caught by unit tests. It can hardly be caught by statistical tests, but a shrewd adversary may be able to find a weakness that leads to predictability. SO just a fair word of warning that you want to be very sure your random number generator is based on a very strong source of entropy, unless you are absolutely certain that you are not doing anything that requires random input and that no one down the line will introduce such operations to your application :)

The example that springs to my mind is the android bitcoin wallet that was using a weak random number generator, leading to all funds stolen for their users, even though the keys were ever only held on the users' own devices.

martinheidegger commented 3 years ago

@emilbayes In the past year several other alternatives to get the random value has popped up for getRandomValues, enough to choose from, none thoroughly security audited. As for my personal efforts: I do not have the resources to put my effort through a thorough security review which is why I need to put a warning on it to not be used in serious production yet. That being sad, I still think that the approach is robust and should provide the a sturdy solution. Hopefully in future it may proof valuable.

emilbayes commented 3 years ago

Sorry to bop your head but just saw that murmurhash was in there as a mixing function. Maybe what they do in the linux kernel could serve as inspiration? They seed an chacha with true entropy and then sample random bytes from that, mixing in more entropy as is available. How one would go about implementing this in a safe way I wouldn't know

emilbayes commented 3 years ago

Here's the patchset that wikipedia links to for this: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=818e607b57c94ade9824dad63a96c2ea6b21baf3

martinheidegger commented 3 years ago

You just reminded me of putting up that message but I knew for a while now that this is an issue. no worries 😉 . I don't intend to work on it any more as I don't have th budget to do a proper audit no matter what I develop. At some point someone needs to probably do an audit for another random-bytes implementation as I havn't found any audited yet, only partially audited ones (where the seed function is audited but not the react-native specific parts).

As a side-node: get-random-values uses true (system) entropy as available on android/ios to start and has the ability to mix in more system entropy (even though I didn't add that). Switching the murmur to chacha seems like a possible way to change the implementation but I am not in a position to judge the de-merits.

spieglt commented 3 years ago

@emilbayes Do your comments above mean there is no secure crypto replacement available for React Native projects currently? I've implemented hchacha20, I almost have secretstream_xchacha20poly1305 working properly, and I was going to move onto the pwhash API after that, but that's all useless to my React Native project if I don't have a secure source of random bytes I suppose.

Edit: found https://docs.expo.io/versions/latest/sdk/random/