From b628c3b2e5b77bb475e05fa7d0a526915ff7960e Mon Sep 17 00:00:00 2001 From: Jan Bujak Date: Wed, 6 Sep 2023 06:18:31 +0000 Subject: [PATCH 1/3] Simplify `serde_error_from_signature_error` --- src/errors.rs | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/errors.rs b/src/errors.rs index cdb37db..5569672 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -147,16 +147,5 @@ impl failure::Fail for SignatureError {} pub fn serde_error_from_signature_error(err: SignatureError) -> E where E: serde_crate::de::Error { - use self::SignatureError::*; - match err { - PointDecompressionError - => E::custom("Ristretto point decompression failed"), - ScalarFormatError - => E::custom("improper scalar has high-bit set"), // TODO ed25519 v high 3 bits? - BytesLengthError{ description, length, .. } - => E::invalid_length(length, &description), - NotMarkedSchnorrkel - => E::custom("Signature bytes not marked as a schnorrkel signature"), - _ => panic!("Non-serialisation error encountered by serde!"), - } + E::custom(err) } From 1cc9c7d4f828e204a89152406086317b3f670503 Mon Sep 17 00:00:00 2001 From: Jan Bujak Date: Wed, 6 Sep 2023 06:19:23 +0000 Subject: [PATCH 2/3] Update to `curve25519-dalek` 4.1.0 --- Cargo.toml | 5 ++--- src/errors.rs | 4 ++++ src/keys.rs | 30 ++++++++++++++++++++++++++---- src/sign.rs | 3 ++- 4 files changed, 34 insertions(+), 8 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index c730127..c26a5f5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,7 +17,7 @@ aead = { version = "0.5.2", default-features = false, optional = true } arrayref = { version = "0.3.6", default-features = false } # needs to match parity-scale-code which is "=0.7.0" arrayvec = { version = "0.7.0", default-features = false } -curve25519-dalek = { version = "4.0.0-rc.2", default-features = false, features = ["digest", "zeroize"] } +curve25519-dalek = { version = "4.1.0", default-features = false, features = ["digest", "zeroize", "precomputed-tables", "legacy_compatibility"] } subtle = { version = "2.5.0", default-features = false } merlin = { version = "3.0.0", default-features = false } rand_core = { version = "0.6.2", default-features = false } @@ -42,13 +42,12 @@ name = "schnorr_benchmarks" harness = false [features] -default = ["std", "getrandom", "precomputed-tables"] +default = ["std", "getrandom"] preaudit_deprecated = [] nightly = [] alloc = ["curve25519-dalek/alloc", "rand_core/alloc", "serde_bytes/alloc"] std = ["alloc", "getrandom", "serde_bytes/std"] asm = ["sha2/asm"] -precomputed-tables = ["curve25519-dalek/precomputed-tables"] serde = ["serde_crate", "serde_bytes", "cfg-if"] # We cannot make getrandom a direct dependency because rand_core makes # getrandom a feature name, which requires forwarding. diff --git a/src/errors.rs b/src/errors.rs index 5569672..e8c73c0 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -75,6 +75,8 @@ pub enum SignatureError { PointDecompressionError, /// Invalid scalar provided, usually to `Signature::from_bytes`. ScalarFormatError, + /// The provided key is not valid. + InvalidKey, /// An error in the length of bytes handed to a constructor. /// /// To use this, pass a string specifying the `name` of the type @@ -120,6 +122,8 @@ impl Display for SignatureError { write!(f, "Cannot decompress Ristretto point"), ScalarFormatError => write!(f, "Cannot use scalar with high-bit set"), + InvalidKey => + write!(f, "The provided key is not valid"), BytesLengthError { name, length, .. } => write!(f, "{name} must be {length} bytes in length"), NotMarkedSchnorrkel => diff --git a/src/keys.rs b/src/keys.rs index ddfd6c6..41f6c8e 100644 --- a/src/keys.rs +++ b/src/keys.rs @@ -196,6 +196,8 @@ impl MiniSecretKey { // We then divide by the cofactor to internally keep a clean // representation mod l. scalars::divide_scalar_bytes_by_cofactor(&mut key); + + #[allow(deprecated)] // Scalar's always reduced here, so this is OK. let key = Scalar::from_bits(key); let mut nonce = [0u8; 32]; @@ -518,14 +520,17 @@ impl SecretKey { let mut key: [u8; 32] = [0u8; 32]; key.copy_from_slice(&bytes[00..32]); - // TODO: We should consider making sure the scalar is valid, - // maybe by zeroing the high bit, or preferably by checking < l. - // key[31] &= 0b0111_1111; // We divide by the cofactor to internally keep a clean // representation mod l. scalars::divide_scalar_bytes_by_cofactor(&mut key); - let key = Scalar::from_bits(key); + let key = Scalar::from_canonical_bytes(key); + if bool::from(key.is_none()) { + // This should never trigger for keys which come from `to_ed25519_bytes`. + return Err(SignatureError::InvalidKey); + } + + let key = key.unwrap(); let mut nonce: [u8; 32] = [0u8; 32]; nonce.copy_from_slice(&bytes[32..64]); @@ -974,4 +979,21 @@ mod test { let public_from_secret: PublicKey = secret.to_public(); assert!(public_from_mini_secret == public_from_secret); } + + #[cfg(feature = "getrandom")] + #[test] + fn secret_key_can_be_converted_to_ed25519_bytes_and_back() { + let count = if cfg!(debug_assertions) { + 200000 + } else { + 2000000 + }; + + for _ in 0..count { + let key = SecretKey::generate(); + let bytes = key.to_ed25519_bytes(); + let key_deserialized = SecretKey::from_ed25519_bytes(&bytes).unwrap(); + assert_eq!(key_deserialized, key); + } + } } diff --git a/src/sign.rs b/src/sign.rs index 03e7678..4616027 100644 --- a/src/sign.rs +++ b/src/sign.rs @@ -72,7 +72,8 @@ pub(crate) fn check_scalar(bytes: [u8; 32]) -> SignatureResult { // 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 { + if bytes[31] & 0b11110000 == 0 { + #[allow(deprecated)] // Scalar's always reduced here, so this is OK. return Ok(Scalar::from_bits(bytes)) } From 8dc6b993ed3dd9f75a250700bf96e541d85cef2a Mon Sep 17 00:00:00 2001 From: Jan Bujak Date: Wed, 6 Sep 2023 06:30:51 +0000 Subject: [PATCH 3/3] Simplify `#[cfg]` to appease clippy --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index e4938bc..a0da3bf 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -214,7 +214,7 @@ #![deny(missing_docs)] // refuse to compile if documentation is missing #![allow(clippy::needless_lifetimes)] -#[cfg(any(feature = "std"))] +#[cfg(feature = "std")] #[macro_use] extern crate std;