Feature/streamed encryption

[hellais]

This implements the feature that was described inside of #/173. For the moment I have only implemented encryption support with integrity protection, but without signing.

The API is based on node.js' streams. In particular it implements a stream.Transform. This means that you can read and write to either the message or the crypto module.

I have also written a few unittests that are also useful to illustrate usage of the new API.

Let me know what you think and if there is something that I should cleanup/improve.

Note: I did not use the sha1 implementation of openpgp.js, because that also does not support streaming. Luckily node.js (and browserify in particular) has a sha1 implementation that is stream capable so I used that.

[tanx]

Please do not use node apis. This would introduce a runtime dependency of the browserify node shims of buffer, stream, crypto and others.

Not only is the code quality of these shims lacking, it would increase the size of the minified build quite a bit making it nearly impossible to do a security audit of the code.

[hellais]

@tanx what do you suggest using instead? I would like the API to be something that is standard and familiar to some people.

[tanx]

I would suggest using standard typed arrays and the cipher suite already included in OpenPGP.js. Keep in mind that using the node.js crypto apis would introduce a second redundant ciphersuite that has not yet been reviewed:

https://github.com/openpgpjs/openpgpjs/wiki/Cure53-security-audit

[hellais]

@tanx so what worries you is only the use of Buffer and crypto.createHash? Is it ok for me to keep using stream.Transform?

[tanx]

Yes. If the patch contained only the stream dependencies, that would be ok I guess. These are quite lean:

https://github.com/substack/stream-browserify/blob/master/index.js
https://github.com/isaacs/readable-stream/blob/master/lib/_stream_transform.js

[hellais]

@tanx I was looking at ways of replacing Buffer and found that there is this implementation that maps node Buffer to Uint8Array, but exposes the same API: https://github.com/feross/buffer. The code seems pretty clean: https://github.com/feross/buffer/blob/master/index.js, have lots of unittests and performs very well: https://github.com/feross/buffer#performance.

Can I use it? If so how should I go about's including it? Copying it to the repo directly (like is done with jsSHA) or adding it as a dependency in package.json?

[hellais]

@tanx I have been looking into adapting jsSHA to support calling it incrementally with something like update (I opened a ticket on their issue tracker here: Caligatio/jsSHA#22), though it seems like it's going to require some bigish changes to their codebase in order to support this.

I therefore believe that instead of having to reinvent the wheel we should pick a better SHA implementation that has support for incremental update calls. I found this implementation used inside of forge that seems pretty solid: https://github.com/digitalbazaar/forge/blob/master/js/sha1.js https://github.com/digitalbazaar/forge/blob/master/js/sha256.js https://github.com/digitalbazaar/forge/blob/master/js/sha512.js.

Either way the code for the sha implementation will change so it's a choice of going for something that at least somebody else has taken a look at or writing something from scratch. Let me know your thoughts.

[hellais]

I got a suggestion from @infinity0 to use https://github.com/vibornoff/asmcrypto.js

[tanx]

I like the idea of completely replacing jsSHA with the forge implementation. I've already begun that process a while ago by integrating sha-256, since the forge implementation was much faster.

https://github.com/openpgpjs/openpgpjs/tree/master/src/crypto/hash

I haven't replaced the others yet, but I totally support that strategy, as it would speed things up even more and we'd have consistent sha implementations.

[hellais]

@tanx ok awesome! I will work on that then. Thanks for the heads up.

I will then proceed by integrating SHA1 from forge into openpgp.js by adding it to the codebase as you did with sha256.

[hellais]

@tanx what do you think about using the buffer implementation of @feross?

[tanx]

No node buffers please. We've got arraybuffers in js. They are more lightweight.

[hellais]

@tanx ok. I will work on this some more starting from the 1st of November.

[feross]

Hey guys, first off, OpenPGP.js is awesome – my friend @dcposch uses it in Scramble. Just wanted to share my thoughts about browserify's shims.

The shims for buffer and stream are incredibly good. Since browserify 4, the stream shim is based directly off the code in node.js core, via readable-stream. It's maintained by @izs and @rvagg and is quite literally the same code that's in core.

I maintain buffer. It's very fast, well-tested, supports browsers down to IE6, and is just over 5KB minified and gzipped. I just updated the perf benchmarks for it today and the results are very good, and getting better.

I know less about the crypto shim, and I suspect it wouldn't make sense to use in this project since it's redundant. @dominictarr could comment on it's maturity.

If you care about every ounce of performance and file size, it's probably a good idea to use ArrayBuffer directly and/or make your own lightweight streams implementation for internal use. But I'd look into exposing a standard node.js streams interface for users to consume, if you can. It's familiar and can hook into a thriving ecosystem of streams modules on npm.

[tanx]

@feross thanks for your input. I don't have anything against node apis. We use node heavily at whiteout.io. I just want to make sure the codebase for OpenPGP.js stays lean and as platform independent as possible.

Please excuse my bias against browserify shims, but the quality has not been the best in the past. I'm happy to see the quality has improved though and applaud your work on them.

Like I said. The stream api looks fine to me. I don't see a need for node buffer though since most use cases of buffer e.g. base64 encoding is already present in OpenPGP.js's util module. And with native typed arrays and the TextEncoder api, we should have all the parts we need natively in the browser.

[feross]

Yep, quality has improved a lot since browserify version 2 (when I started using it). I'm really happy with where things are now.

Note, that if you're looking to exclude the buffer module, you're going to need to customize the stream code a bit, since all the stream modules on npm assume they can write buffers.

You can probably support buffer by immediately converting to/from ArrayBuffer, and avoid bundling it. For example, if I create a transform stream to hash some content, and I write a buffer to it, you can turn that into an ArrayBuffer by just doing buffer.toArrayBuffer().