digitalbazaar / forge

A native implementation of TLS in Javascript and tools to write crypto-based and network-heavy webapps
https://digitalbazaar.com/
Other
5.03k stars 777 forks source link

Diffie-Hellman #160

Open jduncanator opened 10 years ago

jduncanator commented 10 years ago

I've noticed forge is missing a good chunk of TLS support by not having a DH implementation. I'd be willing to port in my Diffie-Hellman library (which hasn't seen much love for a while)! Thoughts?

I know it needs a lot of work, the actual implementation is sound but the key generation is a little crappy, I'd swap that out for Forges RNG and key generation algorithms anyway.

dlongley commented 10 years ago

There's also some code in #79 that could be refactored a bit (and abstracted).

We do want DH support; it was on my TODO list after fixing the buffer API issue. I also wanted to investigate making a higher-level DH API that works with EC or RSA ... and then add EC support (whilst avoiding the patent madness). Once ECDHE is available, then we can better abstract the hashing functions in TLS in order to support TLS 1.2. That will allow us to add ECDHE_RSA AES_GCM cipher support. So, yes, we want Diffie-Hellman, but it's actually more important to get ECDHE working than DHE-RSA as, I imagine, it would be awfully slow.

Of course, that also leads into my desire to explore doing RSA key generation with WebGL (to find out if that is too insane or if it would actually boost performance).

jduncanator commented 10 years ago

@dlongley Remember, handshake times on the client aren't AS important as on the server as handshakes are generally only performed once on the client. The slowest part of DH is the key generation and the actual handshake is actually pretty fast.

Ayms commented 10 years ago

There's also some code in #79 that could be refactored a bit (and abstracted).

Yes, DH, DHE, etc are not a big deal given that everything is already available in forge to do it, the code I gave was just to leave a trace/sample of it, probably it's not difficult to do something clean.

dlongley commented 10 years ago

@jduncanator,

The slowest part of DH is the key generation and the actual handshake is actually pretty fast.

Well, it's the key generation I'm concerned about; taking multiple seconds to start up a TLS connection is fairly unworkable for general use, IMO.

jduncanator commented 10 years ago

@dlongley Not that key generation. Handshake key generation depends on how fast you can generate a block of random data. There is no restriction on what the "random data" has to be, it just has to be random. The slow part is the bit you precompute, the shared DH keys...

jduncanator commented 10 years ago

Sorry I missunderstood the context in which we were speaking. Yes, in DHE the key is generated once per connection.

dlongley commented 10 years ago

Yes, in DHE the key is generated once per connection.

Yes, this is why using ECDHE is a better target for forge (or JS in general) -- key generation should be fast enough to be acceptable in general use.

jduncanator commented 10 years ago

@dlongley Do note that the WebCrypto API exposes DH Key generation functions so in newer browsers, key generation should be almost native!

jduncanator commented 10 years ago

The very nature of Ephemeral DH means you can generate a prime and use it across multiple handshakes, even the OpenSSL wiki suggests doing this: http://wiki.openssl.org/index.php/Diffie_Hellman

Because each DH handshake results in a new shared secret, reusing the same prime doesn't pose any security risk.

dlongley commented 10 years ago

The very nature of Ephemeral DH means you can generate a prime and use it across multiple handshakes, even the OpenSSL wiki suggests doing this.

Yeah, and actually, it's the server's responsibility in TLS to transmit the DHE parameters, so that alleviates the burden on the client for figuring out how to store that information (that storage is totally unnecessary). Handshakes will still be slow but it will be in terms of milliseconds, not seconds, so the problem isn't too bad.

jduncanator commented 10 years ago

@dlongley Yes, of course. When you said generating primes I thought you were talking server side. Calculating a 4096 shared secret takes around 15ms on my machine, which is extremely respectable for JavaScript. Assuming the server supports renegotiation, thats 15ms once, not per connection!

On the topic of general purpose crypto, if you were considering or even willing to go down that path then maybe now is a good time to repurpose? With all the rewrites going about with the TypedArray backed buffers and all these API changes, why not push it as Forge v2.0 and advertise it as a more generic crypto implementation? I think, with all these changes, now is a really good time to make a decision like this. It would also be nice to get rid of all the flash dependencies out of the source tree, but thats another discussion...

dlongley commented 10 years ago

On the topic of general purpose crypto, if you were considering or even willing to go down that path then maybe now is a good time to repurpose?

I think forge has already started down that path.

With all the rewrites going about with the TypedArray backed buffers and all these API changes, why not push it as Forge v2.0 and advertise it as a more generic crypto implementation?

There's not much advertising that goes on -- if you mean changing the main description of the library on the README, that's something that should have been done a while ago but we just never really got around to it. You'll note that the npm description is already more generalized and has been for quite some time. So what I'm saying is that forge is already seen by many in the community as a general crypto solution that also provides a TLS engine. That being said, there's more that could be done to better indicate that forge isn't just about TLS to others that might think it is. We don't want anyone to get that impression.

Btw, we can actually push Forge v1.0 as a general crypto solution -- as we've intentionally held off on hitting 1.0 until we fixed up the APIs.

It would also be nice to get rid of all the flash dependencies out of the source tree, but thats another discussion...

I would like to move that off to its own little corner at a minimum.

dlongley commented 10 years ago

To resolve this issue, the general plan should probably be along these lines:

  1. Create a "dh.js" module with an API that is something like:
    • dh.generateParameters(options): outputs RSA (logarithmic) or EC-based DH parameters, we will implement logarithmic first (we may want to be able to convert the output to PEM as well).
    • dh.generateKeyPair(options): outputs a public and private key pair, where the private key can be used in dh.generateSecretKey() (output should be convertable to forge buffers or PEM?)
    • dh.generateSecretKey(options): takes a shared-public key and the user's private key and outputs the shared secret key (as a forge buffer?).
  2. Add code to tls.js to handle both server/client side DHE in key exchange messages, using the dh.js module to generate the secret (which becomes the pre-master secret) and using dh params that come from the options used to create the tls connection.

This can be done incrementally. For instance, we can stub much of the TLS work out (eg: add the framework stuff to tls.js but throw an exception if it is used) and create tests that use some sample DH parameters instead of generating our own.

A side project that is related to this would be to better abstract the PRF used in tls.js (write now it is exposed directly in various calls as sha1+md5 digests) so that it's a generic interface with an update() call, etc. This will enable us to add TLS 1.2 support more easily -- and take advantage of ECDHE support once we have it implemented.

Thoughts? There are certainly more details that would be run into during implementation, but if this sounds like a decent high-level plan, PRs following it would be welcome.

We should also look here: http://nodejs.org/api/crypto.html#crypto_class_diffiehellman

jduncanator commented 10 years ago

The reason I opened this is I wrote a diffie Hellman library for browserify that completely mimicks the node API. It's in my repos, check it out.

jduncanator commented 10 years ago

It's old, and not the most amazing thing, but it works and it's a start. Would be much better in forge!

dlongley commented 10 years ago

@jduncanator, that's great. I'm not sure if we want to mirror node exactly (we may be able to improve/simplify), but if you want to take a crack at a forge-based implementation, feel free. We want to have to/from PEM in forge as well; we can write some tests to pull in some dhparams generated by openssl that have been exported to PEM. Please note that forge uses a dual BSD/GPL license and any contributions would need to fall under that same license.

We may also want to detect if the JS environment is using node and then fallback to the native implementation (with a switch to avoid that behavior). However, again, we shouldn't let this tempt us to match their API exactly if it doesn't make sense to do so.

jduncanator commented 10 years ago

@dlongley I'm pretty sure my code is released under GPL3, basically everything I do is! If it isn't, will happily relicense it!

I think the best course of action to go, especially when rewriting the buffer code is to make everything wrap around a "backend". The public facing API implementation would be maintained but internally they just wrap a "backend". You could then provide a WebCrypto backend, a JS backend, a node backend, any backend you wanted! Then adding different backends is trivial and you don't have massive amounts of feature detection in each block of code.

ghost commented 5 years ago

People be like, 5 years laters?! What happen? XD

DVSoftware commented 2 years ago

This would be really nice to have

Ayms commented 2 years ago

One implementation is here: https://github.com/Ayms/node-Tor/blob/master/lib/src/forge.js#L239 (I think, it's old, I don't remember exactly)

DVSoftware commented 2 years ago

Well, I tried adding it, but it still doesn't seem to be working with the server i'm working with. Server wants to use 'TLS_ECDHE_RSA_AES_256_GCM_SHA384' cipher, which isn't implemented.