search

dalek-cryptography / curve25519-dalek

A pure-Rust implementation of group operations on Ristretto and Curve25519
Other
867 stars 439 forks source link

ed25519-dalek: remove `ExpandedSecretKey::to_bytes` #545

Closed tarcieri closed 1 year ago

tarcieri commented 1 year ago

The reason ExpandedSecretKey needs a private scalar_bytes field is to retain the canonical scalar bytes as output by SHA-512 during key expansion so they can be serialized by the to_bytes method.

However, ExpandedSecretKeys should not be serialized to the wire.

Removing this method allows the private field to be removed, which allows ExpandedSecretKey to be constructed entirely from public fields. This provides an alternative to #544 for use cases like Ed25519-BIP32 where the private scalar is derived rather than clamped from bytes.

One other change is needed: to_scalar_bytes was changed to to_scalar as the canonical scalar bytes are no longer retained, however this has no impact on its main use case, X25519 Diffie-Hellman exchanges, where the Scalar should NOT be written to the wire anyway.

tarcieri commented 1 year ago

Note: this is an alternative to both #541 and #544

tarcieri commented 1 year ago

Another advantage of this approach is it leaves fewer copies of the secret scalar in memory.

Otherwise every ExpandedSecretKey contains two copies of the secret scalar: one unclamped, one clamped.

burdges commented 1 year ago

As an aside, Ed25519-BIP32 has some serious issues, so really it should not be encouraged. Tor has a similar derivation scheme winds up safer. cc https://github.com/dalek-cryptography/ed25519-dalek/issues/137

tarcieri commented 1 year ago

@burdges AFAICT this would also work with Tor's method which also uses a computed scalar.

(The point of having this "hazmat" API is so ed25519-dalek can remain agnostic to specific derivation methods while still allowing signatures to be computed with derived keys)

rozbb commented 1 year ago

Copying reasoning from my comment in #544:

The original reason I included scalar_bytes was because section 5.1.5 didn't say you do a modular reduction of the clamped integer when computing the pubkey. So I kept this by the letter of the law.

But this detail doesn't matter! Yes it is true that sometimes s.reduce() * H != s * H, but this is only for H that are not in the prime-order subgroup. The only place scalar_bytes is used (outside of defining scalar) is in the pubkey generation step EdwardsPoint::mul_base_clamped(scalar_bytes). Since the generator is in the prime order subgroup, I should be able to just do scalar * ED25519_BASEPOINT.