WIP: Parameterize RNG

[j0sh]

Here is a patch set to allow flexibility in the choice of RNG that Websocket uses. This is done by replacing the ?g parameter with a ?random_string parameter of type Rng.t = ?base64:bool -> int -> string.

The reason for doing this is that Nocrypto is quite large (typically adds ~1MB to a native build) and pulls in a system dependency on libgmp, which is unnecessary when all that's needed is random number generation and SHA1. (Yes, this was annoying enough -- libgmp especially -- that it justified putting the effort into this.)

Here, the default random_string has been set to use stdlib's Random, with self_init called unless otherwise specified. We use the B64 module that cohttp uses, so base64 is not an extra dependency. The only thing missing is SHA1 -- so an implementation was extracted from Uuidm, hopefully that is alright.

In addition to the Nocrypto implementation, there is also one for Cryptokit, mostly because it was easy and maybe it comes in handy for someone.

Most of the real complexity in these patches comes from the build system.

RNG selection is a tiny feature, so I didn't think it made sense to have separate compilation units and findlib libraries for each websocket RNG (this would probably have been implemented via functors and/or first-class modules). Tooling would have been simpler for development, but usability would have been worse on the user side. (However, I might still have missed something really obvious since I'm not really familiar with anything outside of Makefiles.)

So I went the hard way implementation-wise, in the hopes it'd be easier usability-wise. Everything is in a rng.ml{i} and conditionally compiled, ifdef-style, based on the packages available at compile time. This introduces a dependency on cppo. Then we have incantations to set up the necessary ocamlbuild tags with cppo, and the META file needs to be modified as well. I wonder if oasis would have made this simpler, since things are quite coarse with topkg.

There is also a minor fix that adds an asyc_ssl check to OPAM since that library is required for one of the tests.

Feedback on the approach welcome. Also glad to scrap all this if there is a better way.

[vbmithr]

Whow huge patch! You seem to have put much effort into this. @copy what do you think?

What about getting rid of optionality and use only your new lightweight RNG thing? Is nocrypto/cryptokit useful at all if you replaced them? Personally I like really simple things, I'm not a fond of having options when one option is clearly good enough and simpler. You needed something lightweight than nocrypto, fair enough, why not using exclusively this?

[j0sh]

There are a few reasons one might want to use nocrypto/cryptokit.

1. To create a separate RNG instance for websockets without having the internal state mutated by other parts of the application calling rand. Not usually a good idea but maybe desirable for testing or traceability.
2. The stdlib RNG is not meant to be cryptographically secure as far as I can tell. This directly contravenes RFC6455 Section 5.3. The reason for this requirement is to guard against broken proxies that try to interpret a TCP stream containing a malicious HTTP request. These proxies, typically ones that do not understand the Upgrade header, accounted for ~0.4% of traffic in 2010 [1]. How much of that is still true in 2016? Probably even less, is my guess. But using nocrypto/cryptokit allows for strict spec compliance.

I'm okay with simplifying and not offering any RNG choices, but I assume that the above reasons are partially why the stdlib was not used in the first place?

[1] http://w2spconf.com/2011/papers/websocket.pdf (Section IV, B-3)

[vbmithr]

Thanks.

[copy]

Looks good to me, I added a couple of comments. I'd also vote for removing the extra dependencies (including cppo) altogether.

[j0sh]

Remove cppo and only use the stdlib Random, or offer other RNGs as separate findlib packages, eg websocket.cryptokit and so forth? The former breaks spec, the latter seems heavy for such a small feature. Although I agree that the way this is tied together right now is not so light, from the build-system perspective. Light for users though.

[copy]

I was thinking of breaking the spec, but now that I think about it again it doesn't sound like a great idea. How about using /dev/urandom (reading 4 bytes for every frame)?

[j0sh]

/dev/urandom isn't portable (Windows and JS) and hard coding that defeats the purpose of having pluggable RNGs.

We could break the spec, but do so loudly with ample documentation and examples of how to plug in a compliant RNG?

[copy]

We could break the spec, but do so loudly with ample documentation and examples of how to plug in a compliant RNG?

That sounds good to me.