New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

scalarmult() supports degenerate public-keys (insecure) #154

Closed
veorq opened this Issue Jan 26, 2017 · 3 comments

Comments

Projects
None yet
3 participants
@veorq
Copy link

veorq commented Jan 26, 2017

Currently scalarmult() accepts all-zero public keys, for which the result (DH shared secret) will always be zero regardless of the private key used.

Against this, libsodium's crypto_scalarmult_curve25519() returns a non-zero value if it encounters such degenerate keys. You should therefore check its return value when calling ffi::crypto_scalarmult_curve25519(&mut q, n, p);.

This is a similar issue as just reported to rbnacl crypto-rb/rbnacl#152

@dnaq

This comment has been minimized.

Copy link
Collaborator

dnaq commented Jan 26, 2017

@CodesInChaos

This comment has been minimized.

Copy link

CodesInChaos commented Mar 6, 2017

@veorq Can you explain why you consider this an issue?

In typical protocols an attacker who can trick you into thinking a low order public key belongs to your communication partner could also trick you into using a public key for which the attacker knows the corresponding private key.

How do I validate Curve25519 public keys?

Don't. The Curve25519 function was carefully designed to allow all 32-byte strings as Diffie-Hellman public keys.
There are some unusual non-Diffie-Hellman elliptic-curve protocols that need to ensure ``contributory'' behavior. In those protocols, you should reject [...]. But these exclusions are unnecessary for Diffie-Hellman.

@veorq

This comment has been minimized.

Copy link
Author

veorq commented Mar 6, 2017

This check ensures joint key control, to prevent dishonest parties from choosing the shared secret. Whether this check is required depends on the protocol and on the threat model. It may be superfluous, but it can't hurt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment