browserify / diffie-hellman

pure js diffie-hellman
MIT License
94 stars 26 forks source link

Compute secret should have blinds #22

Open bastien-roucaries opened 7 years ago

bastien-roucaries commented 7 years ago

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=860771#10From

Is this timing safe? From the github page it uses a pure-JS BigNum implementation (bn.js) for the complicated stuff, but the README of that code doesn't mention timing at all. And from perusing the source code of bn.js, it doesn't appear to be the case that their implementation of exponentiation in a prime field is geared towards constant-time execution (when the sizes are the same).

If you look at e.g. OpenSSL's source code (bn_exp.c), there's a specific function (bn_mod_exp_mont_consttime) in there that takes great care of making sure that the operation runs in constant time - down to how the memory layout is organized. I wouldn't know how you'd even do that in an interpreted language such as JavaScript, but even if that's possible, I'd suspect that a lot of brain power would need to go into designing that [1], while bn.js's implementation of the Red.pow function seems rather straight-forward. (Which is fine, bn.js appears to have the goal to be a generic bignum library, and not targeted at crypto.)

What I'm saying is: while not having tested that, I believe that this implementation of DH is going to be susceptible to timing attacks. (And if it isn't, the author should really provide some rationale why not, with some test results. The README is rather sparse, though.) Which would be fine if you just wanted to use this library to generate the DH prime itself (that is not timing critical), or just use it in an academic context (to let people play around with DH), but I'd not want to use this for real-world applications of the actual key exchange protocol.

Regards, Christian

[1] Especially if this is to be run in browsers, with different JITs etc. Designing algorithms in pure JS for these environments that are timing-safe looks rather daunting to me.

calvinmetcalf commented 7 years ago

I replied on the list, but basically for actual secure key exchange the performance of this is so terrible you or anyone is probably not going to use it for anything secure in the browser and it's mainly maintained for compatibility. That being said there isn't any reason we can't add a blinding step like we do for the RSA signature computations.

dcousens commented 7 years ago

this is so terrible you or anyone is probably not going to use it for anything secure in the browser and it's mainly maintained for compatibility

@calvinmetcalf if that is the standard its to be held to... why bother at all?

calvinmetcalf commented 7 years ago

well I didn't know the performance was going to be that bad until after I wrote it

bastien-roucaries commented 7 years ago

We do not know where the code will be used... So please be secure

calvinmetcalf commented 7 years ago

as I said in my reply to the list, you can probably be safe removing the package from debian and replacing it with a stub.

bastien-roucaries commented 7 years ago

browserify need it and we do not want to be too far upstream

calvinmetcalf commented 7 years ago

ok we can add a blind, do you have a link to the function you were talking about in openssl?

On Mon, Apr 24, 2017 at 9:59 AM bastien-roucaries notifications@github.com wrote:

browserify need it and we do not want to be too far upstream

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/crypto-browserify/diffie-hellman/issues/22#issuecomment-296677288, or mute the thread https://github.com/notifications/unsubscribe-auth/ABE4n6xKt2ZFqlRjlHJQ_v_8wzw37nNjks5rzKrKgaJpZM4NFD4v .

bastien-roucaries commented 7 years ago

Same than RSA

On Mon, Apr 24, 2017 at 4:42 PM, Calvin Metcalf notifications@github.com wrote:

ok we can add a blind, do you have a link to the function you were talking about in openssl?

On Mon, Apr 24, 2017 at 9:59 AM bastien-roucaries < notifications@github.com> wrote:

browserify need it and we do not want to be too far upstream

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/crypto-browserify/diffie-hellman/ issues/22#issuecomment-296677288, or mute the thread https://github.com/notifications/unsubscribe- auth/ABE4n6xKt2ZFqlRjlHJQ_v_8wzw37nNjks5rzKrKgaJpZM4NFD4v

.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/crypto-browserify/diffie-hellman/issues/22#issuecomment-296690325, or mute the thread https://github.com/notifications/unsubscribe-auth/AEnaHCe4Rt_WIvOyVr0cjyQwxzRnUgvWks5rzLTRgaJpZM4NFD4v .

calvinmetcalf commented 7 years ago

from what I can tell libressl at least does something different for dh vs rsa, rsa uses a blind (like we do in our rsa implementation) while dh uses a different constant time technique which may be interesting to convert to JS.

bastien-roucaries commented 7 years ago

Any progress ?

calvinmetcalf commented 7 years ago

honestly not really, though pull requests would be accepted

pravi commented 6 years ago

@calvinmetcalf if we can't implement it, can't we disable it in crypto-browserify module itself?

calvinmetcalf commented 6 years ago

yeah if this is such a worry for debian disabling this module in your release is probably what you want to do