ed25519-dalek: remove ExpandedSecretKey::to_bytes (dalek-cryptograp…

…hy#545)

* ed25519-dalek: remove `ExpandedSecretKey::to_bytes`

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 dalek-cryptography#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.

* Added scalar byte comparison back to ed25519-dalek x25519 test

---------

Co-authored-by: Michael Rosenberg <[email protected]>

ed25519-dalek/src/hazmat.rs

@@ -35,10 +35,6 @@ use curve25519_dalek::digest::{generic_array::typenum::U64, Digest};
 ///
 /// Instances of this secret are automatically overwritten with zeroes when they fall out of scope.
 pub struct ExpandedSecretKey {
- // `scalar_bytes` and `scalar` are separate, because the public key is computed as an unreduced
- // scalar multiplication (ie `mul_base_clamped`), whereas the signing operations are done
- // modulo l.
- pub(crate) scalar_bytes: [u8; 32],
  /// The secret scalar used for signing
  pub scalar: Scalar,
  /// The domain separator used when hashing the message to generate the pseudorandom `r` value
@@ -59,15 +55,6 @@ impl ZeroizeOnDrop for ExpandedSecretKey {}
 // Some conversion methods for `ExpandedSecretKey`. The signing methods are defined in
 // `signing.rs`, since we need them even when `not(feature = "hazmat")`
 impl ExpandedSecretKey {
- /// Convert this `ExpandedSecretKey` into an array of 64 bytes.
- pub fn to_bytes(&self) -> [u8; 64] {
- let mut bytes: [u8; 64] = [0u8; 64];
-
- bytes[..32].copy_from_slice(self.scalar.as_bytes());
- bytes[32..].copy_from_slice(&self.hash_prefix[..]);
- bytes
- }
-
  /// Construct an `ExpandedSecretKey` from an array of 64 bytes. In the spec, the bytes are the
  /// output of a SHA-512 hash. This clamps the first 32 bytes and uses it as a scalar, and uses
  /// the second 32 bytes as a domain separator for hashing.
@@ -83,7 +70,6 @@ impl ExpandedSecretKey {
  let scalar = Scalar::from_bytes_mod_order(clamp_integer(scalar_bytes));
 
  ExpandedSecretKey {
- scalar_bytes,
  scalar,
  hash_prefix,
  }

ed25519-dalek/src/signing.rs

@@ -488,8 +488,8 @@ impl SigningKey {
  /// For more information on the security of systems which use the same keys for both signing
  /// and Diffie-Hellman, see the paper
  /// [On using the same key pair for Ed25519 and an X25519 based KEM](https://eprint.iacr.org/2021/509).
- pub fn to_scalar_bytes(&self) -> [u8; 32] {
- ExpandedSecretKey::from(&self.secret_key).scalar_bytes
+ pub fn to_scalar(&self) -> Scalar {
+ ExpandedSecretKey::from(&self.secret_key).scalar
  }
 }
 

ed25519-dalek/src/verifying.rs

@@ -91,7 +91,7 @@ impl PartialEq<VerifyingKey> for VerifyingKey {
 impl From<&ExpandedSecretKey> for VerifyingKey {
  /// Derive this public key from its corresponding `ExpandedSecretKey`.
  fn from(expanded_secret_key: &ExpandedSecretKey) -> VerifyingKey {
- VerifyingKey::clamp_and_mul_base(expanded_secret_key.scalar_bytes)
+ VerifyingKey::from(EdwardsPoint::mul_base(&expanded_secret_key.scalar))
  }
 }
 
@@ -187,16 +187,6 @@ impl VerifyingKey {
  self.point.is_small_order()
  }
 
- /// Internal utility function for clamping a scalar representation and multiplying by the
- /// basepont to produce a public key.
- fn clamp_and_mul_base(bits: [u8; 32]) -> VerifyingKey {
- let point = EdwardsPoint::mul_base_clamped(bits);
- let compressed = point.compress();
-
- // Invariant: VerifyingKey.1 is always the decompression of VerifyingKey.0
- VerifyingKey { compressed, point }
- }
-
  // A helper function that computes `H(R || A || M)` where `H` is the 512-bit hash function
  // given by `CtxDigest` (this is SHA-512 in spec-compliant Ed25519). If `context.is_some()`,
  // this does the prehashed variant of the computation using its contents.

ed25519-dalek/tests/x25519.rs

@@ -1,7 +1,16 @@
 //! Tests for converting Ed25519 keys into X25519 (Montgomery form) keys.
 
+use curve25519_dalek::scalar::{clamp_integer, Scalar};
 use ed25519_dalek::SigningKey;
 use hex_literal::hex;
+use sha2::{Digest, Sha512};
+
+/// Helper function to return the bytes corresponding to the input bytes after being clamped and
+/// reduced mod 2^255 - 19
+fn clamp_and_reduce(bytes: &[u8]) -> [u8; 32] {
+ assert_eq!(bytes.len(), 32);
+ Scalar::from_bytes_mod_order(clamp_integer(bytes.try_into().unwrap())).to_bytes()
+}
 
 /// Tests that X25519 Diffie-Hellman works when using keys converted from Ed25519.
 // TODO: generate test vectors using another implementation of Ed25519->X25519
@@ -16,16 +25,19 @@ fn ed25519_to_x25519_dh() {
  let ed25519_signing_key_a = SigningKey::from_bytes(&ed25519_secret_key_a);
  let ed25519_signing_key_b = SigningKey::from_bytes(&ed25519_secret_key_b);
 
- let scalar_a_bytes = ed25519_signing_key_a.to_scalar_bytes();
- let scalar_b_bytes = ed25519_signing_key_b.to_scalar_bytes();
+ let scalar_a = ed25519_signing_key_a.to_scalar();
+ let scalar_b = ed25519_signing_key_b.to_scalar();
 
+ // Compare the scalar bytes to the first 32 bytes of SHA-512(secret_key). We have to clamp and
+ // reduce the SHA-512 output because that's what the spec does before using the scalars for
+ // anything.
  assert_eq!(
- scalar_a_bytes,
- hex!("357c83864f2833cb427a2ef1c00a013cfdff2768d980c0a3a520f006904de90f")
+ scalar_a.to_bytes(),
+ clamp_and_reduce(&Sha512::digest(ed25519_secret_key_a)[..32]),
  );
  assert_eq!(
- scalar_b_bytes,
- hex!("6ebd9ed75882d52815a97585caf4790a7f6c6b3b7f821c5e259a24b02e502e11")
+ scalar_b.to_bytes(),
+ clamp_and_reduce(&Sha512::digest(ed25519_secret_key_b)[..32]),
  );
 
  let x25519_public_key_a = ed25519_signing_key_a.verifying_key().to_montgomery();
@@ -44,11 +56,11 @@ fn ed25519_to_x25519_dh() {
  hex!("5166f24a6918368e2af831a4affadd97af0ac326bdf143596c045967cc00230e");
 
  assert_eq!(
- x25519_public_key_a.mul_clamped(scalar_b_bytes).to_bytes(),
+ (x25519_public_key_a * scalar_b).to_bytes(),
  expected_shared_secret
  );
  assert_eq!(
- x25519_public_key_b.mul_clamped(scalar_a_bytes).to_bytes(),
+ (x25519_public_key_b * scalar_a).to_bytes(),
  expected_shared_secret
  );
 }