diff --git a/src/lib.rs b/src/lib.rs index d6118a42a..ead3d29df 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -241,6 +241,7 @@ #![no_std] #![warn(future_incompatible, rust_2018_idioms)] #![deny(missing_docs)] // refuse to compile if documentation is missing +#![deny(clippy::unwrap_used)] // don't allow unwrap #![cfg_attr(not(test), forbid(unsafe_code))] #![cfg_attr(docsrs, feature(doc_auto_cfg, doc_cfg, doc_cfg_hide))] #![cfg_attr(docsrs, doc(cfg_hide(docsrs)))] diff --git a/src/signature.rs b/src/signature.rs index 99aa55324..c78779f6f 100644 --- a/src/signature.rs +++ b/src/signature.rs @@ -162,6 +162,7 @@ impl InternalSignature { /// only checking the most significant three bits. (See also the /// documentation for [`crate::VerifyingKey::verify_strict`].) #[inline] + #[allow(clippy::unwrap_used)] 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); @@ -181,7 +182,10 @@ impl TryFrom<&ed25519::Signature> for InternalSignature { } impl From for ed25519::Signature { + #[allow(clippy::unwrap_used)] fn from(sig: InternalSignature) -> ed25519::Signature { + // This function only fails if the s half of the parsed input exceeds the scalar modulus. + // Since the bytes are coming straight from a Scalar, this is impossible. ed25519::Signature::from_bytes(&sig.as_bytes()).unwrap() } } diff --git a/src/signing.rs b/src/signing.rs index 814ecdaf1..ad299c131 100644 --- a/src/signing.rs +++ b/src/signing.rs @@ -681,25 +681,16 @@ impl Drop for ExpandedSecretKey { } impl From<&SecretKey> for ExpandedSecretKey { + #[allow(clippy::unwrap_used)] fn from(secret_key: &SecretKey) -> ExpandedSecretKey { - let mut h: Sha512 = Sha512::default(); - let mut hash: [u8; 64] = [0u8; 64]; - let mut lower: [u8; 32] = [0u8; 32]; - let mut upper: [u8; 32] = [0u8; 32]; - - h.update(secret_key); - hash.copy_from_slice(h.finalize().as_slice()); - - lower.copy_from_slice(&hash[00..32]); - upper.copy_from_slice(&hash[32..64]); - - lower[0] &= 248; - lower[31] &= 63; - lower[31] |= 64; + 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 { - key: Scalar::from_bits(lower), - nonce: upper, + key: Scalar::from_bits_clamped(lower.try_into().unwrap()), + nonce: upper.try_into().unwrap(), } } } diff --git a/src/verifying.rs b/src/verifying.rs index 726d97115..48f87692e 100644 --- a/src/verifying.rs +++ b/src/verifying.rs @@ -51,9 +51,8 @@ use crate::signing::*; /// considered unequal to the other equivalent encoding, despite the two representing the same /// point. More encoding details can be found /// [here](https://hdevalence.ca/blog/2020-10-04-its-25519am). -/// -/// If you don't care and/or don't want to deal with this, just make sure to use the -/// [`VerifyingKey::verify_strict`] function. +/// If you want to make sure that signatures produced with respect to those sorts of public keys +/// are rejected, use [`VerifyingKey::verify_strict`]. // Invariant: VerifyingKey.1 is always the decompression of VerifyingKey.0 #[derive(Copy, Clone, Default, Eq)] pub struct VerifyingKey(pub(crate) CompressedEdwardsY, pub(crate) EdwardsPoint); @@ -85,8 +84,8 @@ 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 mut bits: [u8; 32] = expanded_secret_key.key.to_bytes(); - VerifyingKey::mangle_scalar_bits_and_multiply_by_basepoint_to_produce_public_key(&mut bits) + let bits: [u8; 32] = expanded_secret_key.key.to_bytes(); + VerifyingKey::clamp_and_mul_base(bits) } } @@ -154,17 +153,10 @@ impl VerifyingKey { Ok(VerifyingKey(compressed, point)) } - /// Internal utility function for mangling the bits of a (formerly - /// mathematically well-defined) "scalar" and multiplying it to produce a - /// public key. - fn mangle_scalar_bits_and_multiply_by_basepoint_to_produce_public_key( - bits: &mut [u8; 32], - ) -> VerifyingKey { - bits[0] &= 248; - bits[31] &= 127; - bits[31] |= 64; - - let scalar = Scalar::from_bits(*bits); + /// 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 compressed = point.compress(); @@ -198,17 +190,21 @@ impl VerifyingKey { // Helper function for verification. Computes the _expected_ R component of the signature. The // caller compares this to the real R component. If `context.is_some()`, this does the // prehashed variant of the computation using its contents. + // Note that this returns the compressed form of R and the caller does a byte comparison. This + // means that all our verification functions do not accept non-canonically encoded R values. + // See the validation criteria blog post for more details: + // https://hdevalence.ca/blog/2020-10-04-its-25519am #[allow(non_snake_case)] fn recompute_r( &self, context: Option<&[u8]>, signature: &InternalSignature, M: &[u8], - ) -> EdwardsPoint { + ) -> CompressedEdwardsY { let k = Self::compute_challenge(context, &signature.R, &self.0, M); let minus_A: EdwardsPoint = -self.1; // Recall the (non-batched) verification equation: -[k]A + [s]B = R - EdwardsPoint::vartime_double_scalar_mul_basepoint(&k, &(minus_A), &signature.s) + EdwardsPoint::vartime_double_scalar_mul_basepoint(&k, &(minus_A), &signature.s).compress() } /// Verify a `signature` on a `prehashed_message` using the Ed25519ph algorithm. @@ -249,7 +245,7 @@ impl VerifyingKey { let message = prehashed_message.finalize(); let expected_R = self.recompute_r(Some(ctx), &signature, &message); - if expected_R.compress() == signature.R { + if expected_R == signature.R { Ok(()) } else { Err(InternalError::Verify.into()) @@ -337,7 +333,7 @@ impl VerifyingKey { } let expected_R = self.recompute_r(None, &signature, message); - if expected_R == signature_R { + if expected_R == signature.R { Ok(()) } else { Err(InternalError::Verify.into()) @@ -393,7 +389,7 @@ impl VerifyingKey { let message = prehashed_message.finalize(); let expected_R = self.recompute_r(Some(ctx), &signature, &message); - if expected_R == signature_R { + if expected_R == signature.R { Ok(()) } else { Err(InternalError::Verify.into()) @@ -412,7 +408,7 @@ impl Verifier for VerifyingKey { let signature = InternalSignature::try_from(signature)?; let expected_R = self.recompute_r(None, &signature, message); - if expected_R.compress() == signature.R { + if expected_R == signature.R { Ok(()) } else { Err(InternalError::Verify.into())