Update to new Scalar API

Cargo.toml

@@ -63,9 +63,13 @@ batch = ["alloc", "merlin", "rand_core"]
 fast = ["curve25519-dalek/precomputed-tables"]
 digest = ["signature/digest"]
 # This features turns off stricter checking for scalar malleability in signatures
-legacy_compatibility = []
+legacy_compatibility = ["curve25519-dalek/legacy_compatibility"]
 pkcs8 = ["ed25519/pkcs8"]
 pem = ["alloc", "ed25519/pem", "pkcs8"]
 rand_core = ["dep:rand_core"]
 serde = ["dep:serde", "ed25519/serde"]
 zeroize = ["dep:zeroize", "curve25519-dalek/zeroize"]
+
+[patch.crates-io.curve25519-dalek]
+git = "https://github.com/dalek-cryptography/curve25519-dalek.git"
+rev = "f460ae149b0000695205cc78f560d74a2d3918eb"

src/signature.rs

@@ -63,6 +63,10 @@ impl Debug for InternalSignature {
     }
 }
 
+#[deprecated(
+    since = "2.0.0",
+    note = "The legacy_compatibility feature permits signature malleability. See README."
+)]
 #[cfg(feature = "legacy_compatibility")]
 #[inline(always)]
 fn check_scalar(bytes: [u8; 32]) -> Result<Scalar, SignatureError> {
@@ -76,24 +80,15 @@ fn check_scalar(bytes: [u8; 32]) -> Result<Scalar, SignatureError> {
         return Err(InternalError::ScalarFormat.into());
     }
 
+    // You cannot do arithmetic with scalars construct with Scalar::from_bits. We only use this
+    // scalar for EdwardsPoint::vartime_double_scalar_mul_basepoint, which is an accepted usecase.
+    #[allow(deprecated)]
     Ok(Scalar::from_bits(bytes))
 }
 
 #[cfg(not(feature = "legacy_compatibility"))]
 #[inline(always)]
 fn check_scalar(bytes: [u8; 32]) -> Result<Scalar, SignatureError> {
-    // Since this is only used in signature deserialisation (i.e. upon
-    // verification), we can do a "succeed fast" trick by checking that the most
-    // significant 4 bits are unset.  If they are unset, we can succeed fast
-    // because we are guaranteed that the scalar is fully reduced.  However, if
-    // the 4th most significant bit is set, we must do the full reduction check,
-    // as the order of the basepoint is roughly a 2^(252.5) bit number.
-    //
-    // This succeed-fast trick should succeed for roughly half of all scalars.
-    if bytes[31] & 240 == 0 {
-        return Ok(Scalar::from_bits(bytes));
-    }
-
     match Scalar::from_canonical_bytes(bytes).into() {
         None => Err(InternalError::ScalarFormat.into()),
         Some(x) => Ok(x),

src/signing.rs

@@ -25,7 +25,7 @@ use curve25519_dalek::digest::generic_array::typenum::U64;
 use curve25519_dalek::digest::Digest;
 use curve25519_dalek::edwards::CompressedEdwardsY;
 use curve25519_dalek::edwards::EdwardsPoint;
-use curve25519_dalek::scalar::Scalar;
+use curve25519_dalek::scalar::{clamp_integer, Scalar};
 
 use ed25519::signature::{KeypairRef, Signer, Verifier};
 
@@ -462,12 +462,23 @@ impl SigningKey {
         self.verifying_key.verify_strict(message, signature)
     }
 
-    /// Convert this signing key into a Curve25519 scalar.
+    /// Convert this signing key into a byte representation of a(n) (unreduced) Curve25519 scalar.
     ///
-    /// This is useful for e.g. performing X25519 Diffie-Hellman using
-    /// Ed25519 keys.
-    pub fn to_scalar(&self) -> Scalar {
-        ExpandedSecretKey::from(&self.secret_key).scalar
+    /// This can be used for performing X25519 Diffie-Hellman using Ed25519 keys. The bytes output
+    /// by this function are a valid secret key for the X25519 public key given by
+    /// `self.verifying_key().to_montgomery()`.
+    ///
+    /// # Note
+    ///
+    /// We do NOT recommend this usage of a signing/verifying key. Signing keys are usually
+    /// long-term keys, while keys used for key exchange should rather be ephemeral. If you can
+    /// help it, use a separate key for encryption.
+    ///
+    /// 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
     }
 }
 
@@ -736,13 +747,18 @@ impl<'d> Deserialize<'d> for SigningKey {
 // better-designed, Schnorr-based signature scheme, see Trevor Perrin's work on
 // "generalised EdDSA" and "VXEdDSA".
 pub(crate) 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],
     pub(crate) scalar: Scalar,
     pub(crate) nonce: [u8; 32],
 }
 
 #[cfg(feature = "zeroize")]
 impl Drop for ExpandedSecretKey {
     fn drop(&mut self) {
+        self.scalar_bytes.zeroize();
         self.scalar.zeroize();
         self.nonce.zeroize()
     }
@@ -755,9 +771,15 @@ impl From<&SecretKey> for ExpandedSecretKey {
         // TODO: Use bytes.split_array_ref once it’s in MSRV.
         let (lower, upper) = hash.split_at(32);
 
-        // The try_into here converts to fixed-size array
+        // The lower bytes are the scalar. The try_into here converts to fixed-size array
+        let scalar_bytes = lower.try_into().unwrap();
+        // For signing, we'll need the integer, clamped, and converted to a Scalar. See
+        // PureEdDSA.keygen in RFC 8032 Appendix A.
+        let scalar = Scalar::from_bytes_mod_order(clamp_integer(scalar_bytes));
+
         ExpandedSecretKey {
-            scalar: Scalar::from_bits_clamped(lower.try_into().unwrap()),
+            scalar_bytes,
+            scalar,
             nonce: upper.try_into().unwrap(),
         }
     }

src/verifying.rs

@@ -63,7 +63,7 @@ pub struct VerifyingKey {
 }
 
 impl Debug for VerifyingKey {
-    fn fmt(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
         write!(f, "VerifyingKey({:?}), {:?})", self.compressed, self.point)
     }
 }
@@ -89,8 +89,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 {
-        let bits: [u8; 32] = expanded_secret_key.scalar.to_bytes();
-        VerifyingKey::clamp_and_mul_base(bits)
+        VerifyingKey::clamp_and_mul_base(expanded_secret_key.scalar_bytes)
     }
 }
 
@@ -180,8 +179,7 @@ impl VerifyingKey {
     /// 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 scalar = Scalar::from_bits_clamped(bits);
-        let point = EdwardsPoint::mul_base(&scalar);
+        let point = EdwardsPoint::mul_base_clamped(bits);
         let compressed = point.compress();
 
         // Invariant: VerifyingKey.1 is always the decompression of VerifyingKey.0
@@ -422,15 +420,19 @@ impl VerifyingKey {
 
     /// Convert this verifying key into Montgomery form.
     ///
-    /// This is useful for systems which perform X25519 Diffie-Hellman using
-    /// Ed25519 keys.
+    /// This can be used for performing X25519 Diffie-Hellman using Ed25519 keys. The output of
+    /// this function is a valid X25519 public key whose secret key is `sk.to_scalar_bytes()`,
+    /// where `sk` is a valid signing key for this `VerifyingKey`.
     ///
-    /// When possible, it's recommended to use separate keys for signing and
-    /// Diffie-Hellman.
+    /// # Note
     ///
-    /// 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.pdf).
+    /// We do NOT recommend this usage of a signing/verifying key. Signing keys are usually
+    /// long-term keys, while keys used for key exchange should rather be ephemeral. If you can
+    /// help it, use a separate key for encryption.
+    ///
+    /// 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_montgomery(&self) -> MontgomeryPoint {
         self.point.to_montgomery()
     }

tests/x25519.rs

@@ -16,16 +16,16 @@ 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 = ed25519_signing_key_a.to_scalar();
-    let scalar_b = ed25519_signing_key_b.to_scalar();
+    let scalar_a = ed25519_signing_key_a.to_scalar_bytes();
+    let scalar_b = ed25519_signing_key_b.to_scalar_bytes();
 
     assert_eq!(
-        scalar_a.to_bytes(),
-        hex!("307c83864f2833cb427a2ef1c00a013cfdff2768d980c0a3a520f006904de94f")
+        scalar_a,
+        hex!("357c83864f2833cb427a2ef1c00a013cfdff2768d980c0a3a520f006904de90f")
     );
     assert_eq!(
-        scalar_b.to_bytes(),
-        hex!("68bd9ed75882d52815a97585caf4790a7f6c6b3b7f821c5e259a24b02e502e51")
+        scalar_b,
+        hex!("6ebd9ed75882d52815a97585caf4790a7f6c6b3b7f821c5e259a24b02e502e11")
     );
 
     let x25519_public_key_a = ed25519_signing_key_a.verifying_key().to_montgomery();
@@ -44,11 +44,11 @@ fn ed25519_to_x25519_dh() {
         hex!("5166f24a6918368e2af831a4affadd97af0ac326bdf143596c045967cc00230e");
 
     assert_eq!(
-        (x25519_public_key_a * scalar_b).to_bytes(),
+        x25519_public_key_a.mul_clamped(scalar_b).to_bytes(),
         expected_shared_secret
     );
     assert_eq!(
-        (x25519_public_key_b * scalar_a).to_bytes(),
+        x25519_public_key_b.mul_clamped(scalar_a).to_bytes(),
         expected_shared_secret
     );
 }