search

str4d / ed25519-java

Pure Java implementation of EdDSA
Creative Commons Zero v1.0 Universal
217 stars 73 forks source link

No precompute on scalarMultiply? #46

Closed ineiti closed 6 years ago

ineiti commented 7 years ago

I'm using this library to do some non-standard cryptographic functions, and I need the scalarMultiplication. Now I wonder why the following line is commented out, as it would work with this line, but without I have to call .precompute(true) in my code.

https://github.com/str4d/ed25519-java/blob/eab5cf1d54326823678db6c91fd01047aea42d58/src/net/i2p/crypto/eddsa/math/GroupElement.java#L884

str4d commented 6 years ago

In the library as-is, scalarMultiply() is only used with the base point, not general curve points. I commented this out because at the time (being rather new to, and wary of, implementing my own crypto), I was uncertain of exactly how the single precomputation depended on the base point, something a later reviewer picked up on: https://github.com/str4d/ed25519-java/blob/eab5cf1d54326823678db6c91fd01047aea42d58/src/net/i2p/crypto/eddsa/math/GroupElement.java#L467

I agree that this would ideally be uncommented, so that library users can use any method on GroupElement (and it would be slow the first time, but fast subsequently). I will spend some time reviewing this. It may require some refactoring, as we wouldn't want users who don't use doubleScalarMultiplyVariableTime() to be burdened with its precomputation inside scalarMultiply() (or in your case, outside the call).

str4d commented 6 years ago

Putting into 0.2.1 in case I can get away without an API change; otherwise this will be bumped to 0.3.

str4d commented 6 years ago

I believe this was addressed by #58. Please re-open this issue if you still have issues.