schollz / pake

PAKE library for generating a strong secret between parties over an insecure channel
MIT License
199 stars 14 forks source link

Recommendation: limit curves to only Curve25519 #4

Closed makew0rld closed 3 years ago

makew0rld commented 3 years ago

To start: I know nothing about the mathematics behind elliptic curves. I've just learned a bit about what's out there and have some questions

I'm curious as to why this library supports multiple curves, and why the current ones were chosen. I have heard that P-256 and other NIST curves are not fully trusted due to the Dual_EC_DRBG scandal. As cryptographer Bruce Schneier famously said: "I no longer trust the constants. I believe the NSA has manipulated them through their relationships with industry." Now I have no idea how much this applies to NIST curves. And they are used in modern TLS, so if there were problems with them the implications would be a lot bigger than just this library.

However, famous cryptographer Daniel J. Bernstein wrote this about NIST curves, specifically P-256:

Subsequent research (and to some extent previous research) showed that essentially all of these efficiency-related decisions were suboptimal, that many of them actively damaged efficiency, and that some of them were bad for security.

This is from his SafeCurves website, which to be honest, is very much beyond me. But it seems to indicate that all of the NIST curves tested are considered "unsafe". Unfortunately, it does not mention SIEC.

Moving on to SIEC, I cannot critique it as I don't have the mathematical background. But it seems to not be used in anywhere really but here, and I can find no information on it other than the original paper. It would likely be safer to use a more "battle-tested" curve.

My understanding is that Curve25519, developed by Daniel J. Bernstein is one of the best and fastest curves in elliptic curve cryptography. Why not use that one, which already has Go stdlib support and is recommended in TLS? And by restricting the curves available to only that one, you don't put the decision on the library user (as there is now), and only offer them a safe, reliable curve.

I think this Wikipedia section sums it up better than I can -- I think I've been copying it here from memory by accident, whoops. Here's a screenshot of the section:

image

By limiting the curves to only Curve25519, I believe you would be making this library (and applications that rely on it, like croc) more secure and easier to use. Thoughts?

schollz commented 3 years ago

By limiting the curves to only curve25519, I believe you would be making this library (and applications that rely on it, like croc) more secure and easier to use.

I think this is hard to ascertain. I don't think curve25519 is "easier to use" as all the curves are pretty easy to use (and all are hard to use correctly, because crypto is hard) and have already been coded up. As to being more secure - I'm sure that is arguable. In fact, this is anecdotal, but the use of siec in croc actually caused a bit of a burden to the attackers for the the vulnerability found recently because siec is uncommon doesn't have lots of super optimized implementations for the attacker's brute-forcing... Actually the security vulnerability found recently used a backdoor because I left open the constants to be chosen by the parties, and an attacker could then choose constants that they know the discrete log of (a backdoor). In this new v3 implementation of PAKE, the constants are chosen for you, and they are chosen based on a hash chosen out of thin air (croc1 and croc2) so its pretty well assured no one knows the discrete log.

makew0rld commented 3 years ago

I don't think curve25519 is "easier to use" as all the curves are pretty easy to use

Sorry for being unclear. My point about ease of use was that the library would be easier to use if there was less choice available, not that Curve25519 is easier specifically. Limiting choice is especially important for crypto libraries, where the wrong choice could mean an insecure application, and the people making the choices are usually not cryptographers. This is part of the philosophy behind the excellent NaCL library, which says:

Most programmers using cryptographic libraries are not expert cryptographic security evaluators. Often programmers pass the choice along to users—who usually have even less information about the security of cryptographic primitives. There is a long history of these programmers and users making poor choices of cryptographic primitives, such as MD5 and 512-bit RSA, years after cryptographers began issuing warnings about the security of those primitives.

This is further strengthened by what you say after, which is that by providing choice of constants to an attacker, croc was made less secure.


the use of siec in croc actually caused a bit of a burden to the attackers [...] because siec is uncommon

This is security through obscurity, and is in my opinion irrelevant. As a user, I want good crypto to be used, not unknown crypto that will take a bit longer to break because it's unknown.

Can you address the points I originally made:

Thanks for taking the time to reply.

schollz commented 3 years ago

This is security through obscurity, and is in my opinion irrelevant. As a user, I want good crypto to be used, not unknown crypto that will take a bit longer to break because it's unknown.

I think you missed some context. I relayed that scenario only as an anecdote (see above) and did not mean to imply that it was secure because it was obscure. My only implication was to say that a byproduct of its obscurity is that, in a real practical attack, it cost more resources to the attacker because it was not a super-optimized well known curve. You are definitely right that this anecdote is irrelevant to the security, but you are wrong that this is security through obscurity. The siec encryption curve provides real security independent of its popularity. You can read more about it here. It has a beneficial property too - it is "super isolated" so that it has properties that make it more resistant to attacks that are found for other elliptic curves in the same class.

Can you address the points I originally made

Sure. I didn't realize you wanted all the different points addressed since you asked for thoughts..

SIEC being seemingly untested - and therefore a strange choice to offer.

It is not untested. It is published and peer-reviewed with proofs.

NIST curves being considered suspicious or manipulated - and therefore a bad choice to offer.

I don't want to speak to whether NIST curves are suspicious or manipulated. But if you think they are, you are free to choose a different curve. I don't think its bad to have options.

Curve25519 being an industry-standard solution to all of this, with additional advantages like speed

I have nothing against adding curve25519 into PAKE. The only reason it hasn't been added is being AFAIK there is not a Go port for curve25519 that encapsulates the Curve type used for all the other curves here. There is an implementation by keybase but it is missing the Add function and the Go std lib implementation is missing Add and IsOnCurve. I'll happily accept a PR for this though!

makew0rld commented 3 years ago

Thanks for your response.

I don't think its bad to have options.

My problem with options is just that as a non-cryptographer I don't really know what to choose. I guess I'd go with P-256, because I've heard of it before, and it's used in TLS? But I wouldn't feel confident in my choice, and the fact it's widely used isn't even mentioned on the README.

Maybe I'm wrong, and as a library this should be providing options. I just think it's easy to make mistakes with that. In any case I think it'd be helpful to have suggestions on the README.


Glad to hear you wouldn't mind adding Curve25519. Unfortunately I don't think I have the knowledge right now to contribute a PR for it, but maybe someone else will eventually.

schollz commented 3 years ago

I think it'd be helpful to have suggestions on the README.

I feel uneasy about adding any suggestions in the README of what curve to use. It seems this is all a trust issue at its core, and there's no reason someone should trust a suggestion I put in a README more than anything else. I think it would be fine to add some facts into the README (like links to RFCs about which curves are used for TLS for example) but in a way that doesn't imply a recommendation.

I could probably put together PR for Curve25519. It would really help if you could support me with this effort by becoming a sponsor: https://github.com/sponsors/schollz

makew0rld commented 3 years ago

Thanks for the discussion. I don't think I can sponsor you right now unfortunately, but I wish you luck with your projects. You're doing great stuff :)