Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Tests use invalid Curve25519 private keys #24

Open
mmdriley opened this issue Jun 7, 2016 · 4 comments
Open

Tests use invalid Curve25519 private keys #24

mmdriley opened this issue Jun 7, 2016 · 4 comments

Comments

@mmdriley
Copy link

mmdriley commented Jun 7, 2016

See also: signalapp/libsignal-protocol-c#15.

RatchetingSessionTest and RootKeyTest each include an invalid Curve25519 private key where the first byte isn't divisible by 8 (here and here).

The Curve25519 functions have code to sanitize private keys before they're used, but that code has been commented out in curve25519-java.

@mmdriley
Copy link
Author

Perhaps it's more accurate to say: these tests have "golden values" that rely on the X25519 implementation in this codebase, which doesn't appropriately decode keys before using them. Other X25519 implementations won't produce the same output values.

The input values can stay the same, but the code to normalize those keys should be restored and the expected output value should be updated.

@Arthur111
Copy link

Thank you mmdriley to pointed out this issue.
I just learned Open Whisper Systems use a doctored, proprietary, obscure version of Curve25519.
Their keys are not compatible with Curve25519.

@mmdriley
Copy link
Author

That's probably a bit extreme. This codebase (and others by WhisperSystems) normalize keys on creation rather than on use. In practice, this only causes difficulty when importing keys created by other software.

These tests are annoying because they use nonnormal keys that curve25519-java would never actually produce, and which produce different outputs depending on whether the X25519 implementation decodes the keys. But it's still fine to swap in vanillla X25519 implementations when using the library.

@Arthur111
Copy link

They have to fork the project curve25519-donna and modify it.
What they do imply they use the original curve25519 which is not the case.
And they add confusion by referencing to their java version.
Its just about clearness.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

2 participants