diff --git a/Cargo.lock b/Cargo.lock index 5ef6955..fe13ccc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -249,8 +249,7 @@ dependencies = [ [[package]] name = "curve25519-dalek" version = "4.0.0-rc.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "03d928d978dbec61a1167414f5ec534f24bea0d7a0d24dd9b6233d3d8223e585" +source = "git+https://github.com/dalek-cryptography/curve25519-dalek.git?rev=f460ae149b0000695205cc78f560d74a2d3918eb#f460ae149b0000695205cc78f560d74a2d3918eb" dependencies = [ "cfg-if", "digest", diff --git a/Cargo.toml b/Cargo.toml index cdbe10a..ec28d59 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -67,9 +67,13 @@ digest = ["signature/digest"] # Exposes the hazmat module hazmat = [] # 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" diff --git a/src/batch.rs b/src/batch.rs index d94008d..d5d1746 100644 --- a/src/batch.rs +++ b/src/batch.rs @@ -177,7 +177,7 @@ pub fn verify_batch( h.update(signatures[i].r_bytes()); h.update(verifying_keys[i].as_bytes()); h.update(&messages[i]); - h.finalize().try_into().unwrap() + *h.finalize().as_ref() }) .collect(); diff --git a/src/hazmat.rs b/src/hazmat.rs index 5f16d3a..4414a84 100644 --- a/src/hazmat.rs +++ b/src/hazmat.rs @@ -15,7 +15,7 @@ use crate::{InternalError, SignatureError}; -use curve25519_dalek::Scalar; +use curve25519_dalek::scalar::{clamp_integer, Scalar}; #[cfg(feature = "zeroize")] use zeroize::{Zeroize, ZeroizeOnDrop}; @@ -35,6 +35,10 @@ 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 @@ -64,18 +68,24 @@ impl ExpandedSecretKey { bytes } - /// Construct an `ExpandedSecretKey` from an array of 64 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. pub fn from_bytes(bytes: &[u8; 64]) -> Self { // TODO: Use bytes.split_array_ref once it’s in MSRV. - let mut lower: [u8; 32] = [0u8; 32]; - let mut upper: [u8; 32] = [0u8; 32]; + let mut scalar_bytes: [u8; 32] = [0u8; 32]; + let mut hash_prefix: [u8; 32] = [0u8; 32]; + scalar_bytes.copy_from_slice(&bytes[00..32]); + hash_prefix.copy_from_slice(&bytes[32..64]); - lower.copy_from_slice(&bytes[00..32]); - upper.copy_from_slice(&bytes[32..64]); + // 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_bytes_mod_order(lower), - hash_prefix: upper, + scalar_bytes, + scalar, + hash_prefix, } } @@ -86,18 +96,15 @@ impl ExpandedSecretKey { /// A `Result` whose okay value is an EdDSA `ExpandedSecretKey` or whose error value is an /// `SignatureError` describing the error that occurred, namely that the given slice's length /// is not 64. - #[allow(clippy::unwrap_used)] pub fn from_slice(bytes: &[u8]) -> Result { - if bytes.len() != 64 { - Err(InternalError::BytesLength { + // Try to coerce bytes to a [u8; 64] + bytes.try_into().map(Self::from_bytes).map_err(|_| { + InternalError::BytesLength { name: "ExpandedSecretKey", length: 64, } - .into()) - } else { - // If the input is 64 bytes long, coerce it to a 64-byte array - Ok(Self::from_bytes(bytes.try_into().unwrap())) - } + .into() + }) } } @@ -213,7 +220,6 @@ where mod test { use super::*; - use curve25519_dalek::Scalar; use rand::{rngs::OsRng, CryptoRng, RngCore}; // Pick distinct, non-spec 512-bit hash functions for message and sig-context hashing @@ -224,17 +230,9 @@ mod test { // Make a random expanded secret key for testing purposes. This is NOT how you generate // expanded secret keys IRL. They're the hash of a seed. fn random(mut rng: R) -> Self { - // The usual signing algorithm clamps its scalars - let scalar_bytes = [0u8; 32]; - let scalar = Scalar::from_bits_clamped(scalar_bytes); - - let mut hash_prefix = [0u8; 32]; - rng.fill_bytes(&mut hash_prefix); - - ExpandedSecretKey { - scalar, - hash_prefix, - } + let mut bytes = [0u8; 64]; + rng.fill_bytes(&mut bytes); + ExpandedSecretKey::from_bytes(&bytes) } } diff --git a/src/signature.rs b/src/signature.rs index 72b7b0e..36174c8 100644 --- a/src/signature.rs +++ b/src/signature.rs @@ -63,6 +63,9 @@ impl Debug for InternalSignature { } } +/// Ensures that the scalar `s` of a signature is within the bounds [0, 2^253). +/// +/// **Unsafe**: This version of `check_scalar` permits signature malleability. See README. #[cfg(feature = "legacy_compatibility")] #[inline(always)] fn check_scalar(bytes: [u8; 32]) -> Result { @@ -76,24 +79,17 @@ fn check_scalar(bytes: [u8; 32]) -> Result { 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. + // The `from_bits` method is deprecated because it's unsafe. We know this. + #[allow(deprecated)] Ok(Scalar::from_bits(bytes)) } +/// Ensures that the scalar `s` of a signature is within the bounds [0, ℓ) #[cfg(not(feature = "legacy_compatibility"))] #[inline(always)] fn check_scalar(bytes: [u8; 32]) -> Result { - // 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), @@ -152,13 +148,17 @@ impl InternalSignature { /// only checking the most significant three bits. (See also the /// documentation for [`crate::VerifyingKey::verify_strict`].) #[inline] - #[allow(clippy::unwrap_used)] + #[allow(non_snake_case)] pub fn from_bytes(bytes: &[u8; SIGNATURE_LENGTH]) -> Result { // TODO: Use bytes.split_array_ref once it’s in MSRV. - let (lower, upper) = bytes.split_at(32); + let mut R_bytes: [u8; 32] = [0u8; 32]; + let mut s_bytes: [u8; 32] = [0u8; 32]; + R_bytes.copy_from_slice(&bytes[00..32]); + s_bytes.copy_from_slice(&bytes[32..64]); + Ok(InternalSignature { - R: CompressedEdwardsY(lower.try_into().unwrap()), - s: check_scalar(upper.try_into().unwrap())?, + R: CompressedEdwardsY(R_bytes), + s: check_scalar(s_bytes)?, }) } } diff --git a/src/signing.rs b/src/signing.rs index 500f8b5..b0f0b49 100644 --- a/src/signing.rs +++ b/src/signing.rs @@ -473,12 +473,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 } } @@ -715,14 +726,7 @@ impl From<&SecretKey> for ExpandedSecretKey { #[allow(clippy::unwrap_used)] fn from(secret_key: &SecretKey) -> ExpandedSecretKey { let hash = Sha512::default().chain_update(secret_key).finalize(); - // 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 - ExpandedSecretKey { - scalar: Scalar::from_bits_clamped(lower.try_into().unwrap()), - hash_prefix: upper.try_into().unwrap(), - } + ExpandedSecretKey::from_bytes(hash.as_ref()) } } diff --git a/src/verifying.rs b/src/verifying.rs index e97e203..1d25f38 100644 --- a/src/verifying.rs +++ b/src/verifying.rs @@ -65,7 +65,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) } } @@ -91,8 +91,7 @@ impl PartialEq 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) } } @@ -191,8 +190,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 @@ -501,15 +499,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 + /// + /// 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.pdf). + /// 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() } diff --git a/tests/x25519.rs b/tests/x25519.rs index bb588f7..18ae502 100644 --- a/tests/x25519.rs +++ b/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_bytes = ed25519_signing_key_a.to_scalar_bytes(); + let scalar_b_bytes = ed25519_signing_key_b.to_scalar_bytes(); assert_eq!( - scalar_a.to_bytes(), - hex!("307c83864f2833cb427a2ef1c00a013cfdff2768d980c0a3a520f006904de94f") + scalar_a_bytes, + hex!("357c83864f2833cb427a2ef1c00a013cfdff2768d980c0a3a520f006904de90f") ); assert_eq!( - scalar_b.to_bytes(), - hex!("68bd9ed75882d52815a97585caf4790a7f6c6b3b7f821c5e259a24b02e502e51") + scalar_b_bytes, + 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_bytes).to_bytes(), expected_shared_secret ); assert_eq!( - (x25519_public_key_b * scalar_a).to_bytes(), + x25519_public_key_b.mul_clamped(scalar_a_bytes).to_bytes(), expected_shared_secret ); }