str4d / ed25519-java

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

Fixes #61 #68

Closed sappenin closed 4 years ago

sappenin commented 5 years ago

Fixes #61

Signed-off-by: sappenin sappenin@gmail.com

str4d commented 5 years ago

Thanks for the PR!

It's been a while since I last had my head in this code, so I'm going to take a bit of time to make sure that these changes make sense in the context of the overall API (and potential changes I am currently sketching out). So I will probably not get to reviewing this in depth this month.

str4d commented 5 years ago

Okay, I've had time to get my head back into the codebase. Short answer is, I won't merge this PR as-is, because it causes the internal representations of GroupElement to be used as a serialization format, which is not good: they are in no way canonical, and may change at any time. It was a mistake on my part (back when I first wrote this library four years ago) to allow java.io.Serializable to be blanket-applied to EdDSAPublicKey and EdDSAPrivateKey. Fortunately the bug in #61 meant that no one has used it up to this point (at least without forking the library, at which point they are on their own).

Instead, we should override the java.io.Serializable functions to internally serialize using the canonical encodings of the private key and public key. I'm not sure whether that should be the X.509 format, or the canonical encoding of the underlying GroupElement, but given that it is a serialization of EdDSAPublicKey etc. I think the former makes sense.

str4d commented 5 years ago

I've implemented the core of this in https://github.com/cryptography-cafe/curve25519-elisabeth/pull/21