ed25519-dalek: remove ExpandedSecretKey::to_bytes (#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 #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 <michael@mrosenberg.pub>

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
     );
 }