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

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

Open
RangerMauve opened this issue Sep 28, 2020 · 16 comments
Open

Comments

@RangerMauve
Copy link
Contributor

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: 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:

  • Submit a PR to replace the code in randombytes.js with get-random-values-polypony
  • Figure out what to do about the worker_threads dependency in memory.js
  • Get rn-tape integrated into the CI to make sure react-native doesn't break in the future

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
Copy link
Member

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

@mafintosh
Copy link
Member

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

@spieglt
Copy link
Contributor

spieglt commented Oct 26, 2020

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

@RangerMauve
Copy link
Contributor Author

@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
Copy link
Member

emilbayes commented Oct 27, 2020

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
Copy link
Contributor

spieglt commented Oct 28, 2020

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
Copy link
Contributor Author

@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
Copy link
Contributor

spieglt commented Oct 28, 2020

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
Copy link
Contributor Author

@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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link

martinheidegger commented Dec 1, 2020

@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
Copy link
Member

emilbayes commented Dec 1, 2020

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
Copy link
Member

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
Copy link

martinheidegger commented Dec 1, 2020

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
Copy link
Contributor

spieglt commented Dec 1, 2020

@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/

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

5 participants