ed25519-dalek: remove ExpandedSecretKey::to_bytes
#545
Conversation
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, `ExpandedSecretKey`s 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.
ed25519-dalek/tests/x25519.rs
Outdated
| assert_eq!( | ||
| scalar_a_bytes, | ||
| hex!("357c83864f2833cb427a2ef1c00a013cfdff2768d980c0a3a520f006904de90f") | ||
| ); | ||
| assert_eq!( | ||
| scalar_b_bytes, | ||
| hex!("6ebd9ed75882d52815a97585caf4790a7f6c6b3b7f821c5e259a24b02e502e11") | ||
| ); |
There was a problem hiding this comment.
Note: the rest of the D-H workflow is tested, however I removed checking for a particular scalar serialization here since the Scalar has been pre-clamped at this point.
We could check for the clamped version and make a note of it in a comment.
|
Another advantage of this approach is it leaves fewer copies of the secret scalar in memory. Otherwise every |
|
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 dalek-cryptography/ed25519-dalek#137 |
|
@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 |
|
Copying reasoning from my comment in #544: The original reason I included But this detail doesn't matter! Yes it is true that sometimes |
ed25519-dalek/tests/x25519.rs
Outdated
| assert_eq!( | ||
| scalar_a_bytes, | ||
| hex!("357c83864f2833cb427a2ef1c00a013cfdff2768d980c0a3a520f006904de90f") | ||
| ); | ||
| assert_eq!( | ||
| scalar_b_bytes, | ||
| hex!("6ebd9ed75882d52815a97585caf4790a7f6c6b3b7f821c5e259a24b02e502e11") | ||
| ); |
| scalar_b_bytes, | ||
| hex!("6ebd9ed75882d52815a97585caf4790a7f6c6b3b7f821c5e259a24b02e502e11") | ||
| scalar_b.to_bytes(), | ||
| clamp_and_reduce(&Sha512::digest(ed25519_secret_key_b)[..32]), |
There was a problem hiding this comment.
Manually verified that Sha512::digest matches the hex here. I removed the hex because it didn't come from any test vector. This seems like a more principled way of testing things.
There was a problem hiding this comment.
$ echo "9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60" | xxd -r -p | shasum -a 512 | head -c 64
357c83864f2833cb427a2ef1c00a013cfdff2768d980c0a3a520f006904de90f⏎
$ echo "4ccd089b28ff96da9db6c346ec114e0f5b8a319f35aba624da8cf6ed4fb8a6fb" | xxd -r -p | shasum -a 512 | head -c 64
6ebd9ed75882d52815a97585caf4790a7f6c6b3b7f821c5e259a24b02e502e11⏎
tarcieri
deleted the
ed25519-dalek/remove-expanded-secret-key-to-bytes
branch
The reason
ExpandedSecretKeyneeds a privatescalar_bytesfield is to retain the canonical scalar bytes as output by SHA-512 during key expansion so they can be serialized by theto_bytesmethod.However,
ExpandedSecretKeys should not be serialized to the wire.Removing this method allows the private field to be removed, which allows
ExpandedSecretKeyto 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_byteswas changed toto_scalaras the canonical scalar bytes are no longer retained, however this has no impact on its main use case, X25519 Diffie-Hellman exchanges, where theScalarshould NOT be written to the wire anyway.