From 24cd9421d5746b34f824422a6ff9ea2c2e784c5f Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Fri, 2 Dec 2022 05:55:16 +0100 Subject: [PATCH] Change from_bytes methods to take fixed-size array argument Change from_bytes methods to take `&[u8; N]` argument (with `N` appropriate for given type) rather than `&[u8]`. This harmonises the convention with SigningKey and ed25519::Signature; helps type inference; and allows users to assert bytes size to be asserted at compile time. Creating from a slice is still possible via `TryFrom<&[u8]>` trait. This is an API breaking change. The simplest way to update existing code is to replace Foo::from_bytes with Foo::try_from. This should cover majority of uses. --- src/signature.rs | 28 +++++----------------------- src/signing.rs | 15 ++++----------- src/verifying.rs | 28 +++++++++++++--------------- tests/ed25519.rs | 37 +++++++++++++++++-------------------- 4 files changed, 39 insertions(+), 69 deletions(-) diff --git a/src/signature.rs b/src/signature.rs index 795bfad..9026cbb 100644 --- a/src/signature.rs +++ b/src/signature.rs @@ -162,30 +162,12 @@ impl InternalSignature { /// only checking the most significant three bits. (See also the /// documentation for `PublicKey.verify_strict`.) #[inline] - pub fn from_bytes(bytes: &[u8]) -> Result { - if bytes.len() != SIGNATURE_LENGTH { - return Err(InternalError::BytesLengthError { - name: "Signature", - length: SIGNATURE_LENGTH, - } - .into()); - } - let mut lower: [u8; 32] = [0u8; 32]; - let mut upper: [u8; 32] = [0u8; 32]; - - lower.copy_from_slice(&bytes[..32]); - upper.copy_from_slice(&bytes[32..]); - - let s: Scalar; - - match check_scalar(upper) { - Ok(x) => s = x, - Err(x) => return Err(x), - } - + 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); Ok(InternalSignature { - R: CompressedEdwardsY(lower), - s: s, + R: CompressedEdwardsY(lower.try_into().unwrap()), + s: check_scalar(upper.try_into().unwrap())?, }) } } diff --git a/src/signing.rs b/src/signing.rs index 719c18f..3e38b0c 100644 --- a/src/signing.rs +++ b/src/signing.rs @@ -122,17 +122,10 @@ impl SigningKey { /// is an `SignatureError` describing the error that occurred. #[inline] pub fn from_keypair_bytes(bytes: &[u8; 64]) -> Result { - if bytes.len() != KEYPAIR_LENGTH { - return Err(InternalError::BytesLengthError { - name: "SigningKey", - length: KEYPAIR_LENGTH, - } - .into()); - } - - let secret_key = - SecretKey::try_from(&bytes[..SECRET_KEY_LENGTH]).map_err(|_| SignatureError::new())?; - let verifying_key = VerifyingKey::from_bytes(&bytes[SECRET_KEY_LENGTH..])?; + // TODO: Use bytes.split_array_ref once it’s in MSRV. + let (secret_key, verifying_key) = bytes.split_at(SECRET_KEY_LENGTH); + let secret_key = secret_key.try_into().unwrap(); + let verifying_key = VerifyingKey::from_bytes(verifying_key.try_into().unwrap())?; if verifying_key != VerifyingKey::from(&secret_key) { return Err(InternalError::MismatchedKeypairError.into()); diff --git a/src/verifying.rs b/src/verifying.rs index f699798..51bec1a 100644 --- a/src/verifying.rs +++ b/src/verifying.rs @@ -129,18 +129,8 @@ impl VerifyingKey { /// A `Result` whose okay value is an EdDSA `VerifyingKey` or whose error value /// is an `SignatureError` describing the error that occurred. #[inline] - pub fn from_bytes(bytes: &[u8]) -> Result { - if bytes.len() != PUBLIC_KEY_LENGTH { - return Err(InternalError::BytesLengthError { - name: "VerifyingKey", - length: PUBLIC_KEY_LENGTH, - } - .into()); - } - let mut bits: [u8; 32] = [0u8; 32]; - bits.copy_from_slice(&bytes[..32]); - - let compressed = CompressedEdwardsY(bits); + pub fn from_bytes(bytes: &[u8; PUBLIC_KEY_LENGTH]) -> Result { + let compressed = CompressedEdwardsY(*bytes); let point = compressed .decompress() .ok_or(InternalError::PointDecompressionError)?; @@ -358,11 +348,19 @@ impl Verifier for VerifyingKey { impl TryFrom<&[u8]> for VerifyingKey { type Error = SignatureError; - fn try_from(bytes: &[u8]) -> Result { - VerifyingKey::from_bytes(bytes) + #[inline] + fn try_from(bytes: &[u8]) -> Result { + let bytes = bytes.try_into().map_err(|_| { + InternalError::BytesLengthError { + name: "VerifyingKey", + length: PUBLIC_KEY_LENGTH, + } + })?; + Self::from_bytes(bytes) } } + #[cfg(feature = "pkcs8")] impl DecodePublicKey for VerifyingKey {} @@ -431,6 +429,6 @@ impl<'d> Deserialize<'d> for VerifyingKey { D: Deserializer<'d>, { let bytes = ::deserialize(deserializer)?; - VerifyingKey::from_bytes(bytes.as_ref()).map_err(SerdeError::custom) + VerifyingKey::try_from(bytes.as_ref()).map_err(SerdeError::custom) } } diff --git a/tests/ed25519.rs b/tests/ed25519.rs index 87b8164..10752a7 100644 --- a/tests/ed25519.rs +++ b/tests/ed25519.rs @@ -14,6 +14,7 @@ use curve25519_dalek; use ed25519_dalek::*; use hex::FromHex; +use hex_literal::hex; use sha2::Sha512; @@ -61,9 +62,12 @@ mod vectors { let msg_bytes: Vec = FromHex::from_hex(&parts[2]).unwrap(); let sig_bytes: Vec = FromHex::from_hex(&parts[3]).unwrap(); - let signing_key = SigningKey::try_from(&sec_bytes[..SECRET_KEY_LENGTH]).unwrap(); + let sec_bytes = &sec_bytes[..SECRET_KEY_LENGTH].try_into().unwrap(); + let pub_bytes = &pub_bytes[..PUBLIC_KEY_LENGTH].try_into().unwrap(); + + let signing_key = SigningKey::from_bytes(sec_bytes); let expected_verifying_key = - VerifyingKey::from_bytes(&pub_bytes[..PUBLIC_KEY_LENGTH]).unwrap(); + VerifyingKey::from_bytes(pub_bytes).unwrap(); assert_eq!(expected_verifying_key, signing_key.verifying_key()); // The signatures in the test vectors also include the message @@ -83,26 +87,19 @@ mod vectors { // From https://tools.ietf.org/html/rfc8032#section-7.3 #[test] fn ed25519ph_rf8032_test_vector() { - let secret_key: &[u8] = b"833fe62409237b9d62ec77587520911e9a759cec1d19755b7da901b96dca3d42"; - let verifying_key: &[u8] = - b"ec172b93ad5e563bf4932c70e1245034c35467ef2efd4d64ebf819683467e2bf"; - let message: &[u8] = b"616263"; - let signature: &[u8] = b"98a70222f0b8121aa9d30f813d683f809e462b469c7ff87639499bb94e6dae4131f85042463c2a355a2003d062adf5aaa10b8c61e636062aaad11c2a26083406"; + let sec_bytes = hex!("833fe62409237b9d62ec77587520911e9a759cec1d19755b7da901b96dca3d42"); + let pub_bytes = hex!("ec172b93ad5e563bf4932c70e1245034c35467ef2efd4d64ebf819683467e2bf"); + let msg_bytes = hex!("616263"); + let sig_bytes = hex!("98a70222f0b8121aa9d30f813d683f809e462b469c7ff87639499bb94e6dae4131f85042463c2a355a2003d062adf5aaa10b8c61e636062aaad11c2a26083406"); - let sec_bytes: Vec = FromHex::from_hex(secret_key).unwrap(); - let pub_bytes: Vec = FromHex::from_hex(verifying_key).unwrap(); - let msg_bytes: Vec = FromHex::from_hex(message).unwrap(); - let sig_bytes: Vec = FromHex::from_hex(signature).unwrap(); - - let signing_key: SigningKey = - SigningKey::try_from(&sec_bytes[..SECRET_KEY_LENGTH]).unwrap(); - let expected_verifying_key: VerifyingKey = - VerifyingKey::from_bytes(&pub_bytes[..PUBLIC_KEY_LENGTH]).unwrap(); + let signing_key = SigningKey::from_bytes(&sec_bytes); + let expected_verifying_key = + VerifyingKey::from_bytes(&pub_bytes).unwrap(); assert_eq!(expected_verifying_key, signing_key.verifying_key()); - let sig1: Signature = Signature::try_from(&sig_bytes[..]).unwrap(); + let sig1 = Signature::try_from(&sig_bytes[..]).unwrap(); - let mut prehash_for_signing: Sha512 = Sha512::default(); - let mut prehash_for_verifying: Sha512 = Sha512::default(); + let mut prehash_for_signing = Sha512::default(); + let mut prehash_for_verifying = Sha512::default(); prehash_for_signing.update(&msg_bytes[..]); prehash_for_verifying.update(&msg_bytes[..]); @@ -187,7 +184,7 @@ mod vectors { } let signature = serialize_signature(&r, &s); - let pk = VerifyingKey::from_bytes(&pub_key.compress().as_bytes()[..]).unwrap(); + let pk = VerifyingKey::from_bytes(&pub_key.compress().as_bytes()).unwrap(); let sig = Signature::try_from(&signature[..]).unwrap(); // The same signature verifies for both messages assert!(pk.verify(message1, &sig).is_ok() && pk.verify(message2, &sig).is_ok());