dalek-cryptography / x25519-dalek

X25519 elliptic curve Diffie-Hellman key exchange in pure-Rust, using curve25519-dalek.
BSD 3-Clause "New" or "Revised" License
328 stars 133 forks source link

Implement optional contributory behaviour check #78

Closed isislovecruft closed 2 years ago

rozbb commented 2 years ago

I agree that this function needs to be available. The HPKE spec requires this check, and the way it's performed now could be better.

I think, given this, the docs don't have to do as much justification as they currently do. A simple paragraph explaining what the check does and a link to one of the included blog posts would be sufficient I think.

On naming, I think the function should be called is_all_zeros or something to that effect, since that is the wording that the specs use for validation ("contributory" doesn't appear anywhere in the HPKE spec, for example). The downside of is_all_zeros though is that it's the negation of the current function. Something like that though.

AnomalRoil commented 2 years ago

Totally supportive of adding this, given how people are using X25519 to do all kind of things now. How about also rejecting here the other points mentioned in https://cr.yp.to/ecdh.html#validate ?

And how about adding a ct check to see if the resulting shared secret is all zeroes or not?

isislovecruft commented 2 years ago

Totally supportive of adding this, given how people are using X25519 to do all kind of things now. How about also rejecting here the other points mentioned in https://cr.yp.to/ecdh.html#validate ?

That's not necessary, as those are checking that the other party's public keys are not points of small order. Any such public keys would result in the shared secret being the identity element. Thus, we only check afterwards that the shared secret is not the identity.

And how about adding a ct check to see if the resulting shared secret is all zeroes or not?

This is a constant time check for if the resulting shared secret is the identity (all zeroes).