Make signal-protocol compatible with JS module system #11
Conversation
Added instructions for setting up tests with Sauce Labs credentials. Removed unnecessary dep on grunt-sass
This PR makes libsignal-protocol-javascript compatible with JS's module system.
The test suite now requires the library, bundling it into a single dependency.
We also add an integration test.
For backward compatibility, we still build dist/libsignal.js, and attach
the library to window.libsignal.
A few points remain, for the contributor's consideration:
- This package will not work in node due to a dependency on WebCrypto API. We could look into polyfills, or allow the caller pass in their own primatives (though I do NOT think this is a good idea).
- A few things are listed as dependencies in package.json. However, we are bundling those into a file anyway, so they are really dev dependencies (npm will not need to install them).
This is a combination of 21 commits. Each commit's comments listed below.
browserifying some test bundle
Starting to bundle the tests.
There is now a file test/main.js that requires and executes all the tests.
Each test file is now responsible for requiring its own dependencies.
test/index.html will eventually do nothing more than require chai/mocha/etc test lib bundle (test/test.js, which is built in the gruntfile concat task), and the test suite bundle (build/test_main.js). Perhaps we should move this bundle to test to be like build.
Crypto.js tested + module-ified
Curve.js still relies on some stuff in Internal that we will have to track down eventually.
Continuing bundle-ifiabilty
Stopping me now is the way protos/WhsiperTextProtocol.proto is built
I assume this is an emscripten setting in grunt.
What I intend to do is remove all Internal = Internal | {} garbage
and replace with a single module.exports at the end.
This way, the native stuff with expose itself as a function/object.
SessionCipherTest passing
This one required fiddling with the way protos are built
I simplified stuff I think without breaking anything - just a one line change after some thinking. It seems that Whisper devs intended to add more protobufs eventually. I'm not sure if that's still in the plans, but they should be able to with some hacks. However, a more module-friendly dynamic loading solution may be easier to reason aout. Not bothering to write that for now.
KeyHelper and NumericFingerprint pass
Nothing fancy here, just requiring.
SessionBuilderTest passing
Also added a few untracked files.
After all tests pass, I'm realizing I'll need to go back and make the build workflow sane again for testing. The browserify step is really *only* for the tests.
Passing store tests
The store tests come in a set of 5.
Each gets initialized as a function,
and SignalProtocolStore_test.js ties them all together.
Where before the methods were being called from out of scope,
here we require each store test in test/main.js, then pass all of these functions into test/SiganlProtocolStore_test.js, which now exposes a higher-order function.
Passing all tests!
Woohoo!
Now it's time to deal with those final places where I'm "cheating." In test/index.html, there are three things from ../build/ (and one from ../src/, a worker) that are still being added to the HTML the old fashioned way, through script tags.
The first order of business is that stuff in build/. Figuring out how it's written to JS, and how to module-ify it (and where to include it later) will surely take some time.
Moving toward modular emscripten + worker
I finally figured out how to require emscripten code from browserify!
http://insertafter.com/en/blog/native-node-module.html?
Now that I can require curve stuff instead of appending,
i'ts time to get the webworkers into browserify too.
Of course, substack has written something for this:
https://github.com/substack/webworkify
and it looks like that'll do what I need
....but, I'll need to get it working with the existing manager, first
That's a mindfuck in itself because of all the zig-zagging references, even within the small file.
What I'll need to do first is make sure 1 worker can get launched and do its job.
After that, we can tidy up the references in that file,
then make sure all references to startWorker fall in line.
Fixed typo
The crypto all seems to work now.
Not sure what the webworker is for - will return to it once I figure that out.
Amazingly, the only thing left is a regular old npm module.
Why the unexpected behavior? Is Long the issue?
How can I force Long in browserify, if so?
Fixed webworkers
WebWorkers now seem to work with browserify.
Bundled tests lib in build/
They were getting written to test/ before - confusing, no computer-generated stuff in test/
That should be human-written source only.
Now everything in test/index.html points to something in build/
Working!
All bundled up!
The trick was just requiring the bundled file apparently (?)
Only things left to do are:
- Make sure blanket runs at end-of-tests
- Turn linting back on
- Write up changes in README
Added blanket; tests fail
After adding blanket back, tests are suddenly failing!
Not sure why.
Removing blanket again
Indeed, removing blanket fixes the failures.
Oh well, will leave it out for now.
All tests running
Preserving legacy builds
We still have a prebundled JS file for people who want to include in HTML.
Passing grunt jscs
Updated jshint
Linting back on
elsehow
changed the title
elsehow
changed the title
I also tried to polyfill WebCrypto using Node, so that this lib accepts it. Node's core there's also a WebCrypto polyfill, but i'm not sure if is being kept up to date (& audited?). Edit: There's another one which seems to be better maintained, but a few tests are missing. |
|
I'm really a crypto beginner, but i got started to poke into building a Node shim like this: npm i webcrypto@anvilresearch/webcrypto
npm i bytebuffer@dcodeIO/bytebuffer.js
npm i protobufjs@dcodeIO/protobuf.js// pre bundle
// 'use strict';
const window = Object.assign(Object.create(global), {
crypto: require('webcrypto')
})
const libsignal = {}
const dcodeIO = {
ByteBuffer: require('bytebuffer'),
ProtoBuf: require('protobufjs')
}
;(function (window, libsignal, dcodeIO) {// post bundle
})(global, libsignal, dcodeIO); |
|
@derhuerst Thanks for your reply. Since the Node shims are out of scope for this PR, I'm working on them here: http://github.com/elsehow/signal-protocol/ For Crypto, I'm using node-webcrypto-ossl, which wraps OpenSSL. WebWorkers requires a shim as well. |
|
FYI : We maintain a few WebCrypto polyfills: Our focus is test coverage and compat, you can see some of the tests here: https://peculiarventures.github.io/pv-webcrypto-tests/ WebCrypto-liner would be useful for your WebWorkers case. We also have been thinking about extending these with ed25519 support and would consider prioritizing it if this project needed it. |
|
Stumbled upon this today and have been trying to get a working prototype up and running. Looks good so far, using mongoose-encrypt as a backing store for the long-term storage side of the app. Would love to see this merged into the core repo as building my own shim for Java has been fruitless, so far. |
|
@d3sandoval in the meantime, check https://github.com/elsehow/signal-protocol |
This PR makes libsignal-protocol-javascript compatible with JS's module system. The test suite now requires the library, bundling it into a single dependency. We also add an integration test.
For backward compatibility, we still build dist/libsignal.js, which callers can include in their HTML. They can then refer to
window.libsignalas before.This package will not work in node due to a dependency on WebCrypto API. We could look into polyfills, or allow the caller pass in their own primatives (though I do NOT think this is a good idea). I think the node community would be very curious to hear WhisperSystem's recommendations on good native polyfills (NaCl perhaps?)
P.S. The scope of this PR increased dramatically from the initial PR. Sorry! Since the PR auto-updated on my big commit, I decided to just roll this whole PR into one.
Thank you for all of the amazing work on this library.