From 989c5e4c18d4d36c5ac849c462caa333934200c2 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Tue, 14 Jul 2020 00:25:40 +0000 Subject: [PATCH] Fix ed25519ph context length error handling in sign_prehashed(). RFC8032 specifies that the context cannot be greater than 255 octets, but in the previous implementation in ed25519-dalek, this error would only be caught by a debug_assert. This changes the sign_prehashed() function to return a Result so that the error can be handled at runtime and the library no longer allows misuse by creating signatures that other libraries cannot handle. --- src/errors.rs | 4 ++++ src/secret.rs | 12 ++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/errors.rs b/src/errors.rs index 08f04de..5a9182e 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -41,6 +41,8 @@ pub(crate) enum InternalError { ArrayLengthError{ name_a: &'static str, length_a: usize, name_b: &'static str, length_b: usize, name_c: &'static str, length_c: usize, }, + /// An ed25519ph signature can only take up to 255 octets of context. + PrehashedContextLengthError, } impl Display for InternalError { @@ -59,6 +61,8 @@ impl Display for InternalError { name_c: nc, length_c: lc, } => write!(f, "Arrays must be the same length: {} has length {}, {} has length {}, {} has length {}.", na, la, nb, lb, nc, lc), + InternalError::PrehashedContextError + => write!(f, "An ed25519ph signature can only take up to 255 octets of context"), } } } diff --git a/src/secret.rs b/src/secret.rs index 3579e2b..b305eed 100644 --- a/src/secret.rs +++ b/src/secret.rs @@ -441,7 +441,9 @@ impl ExpandedSecretKey { /// /// # Returns /// - /// An Ed25519ph [`Signature`] on the `prehashed_message`. + /// A `Result` whose `Ok` value is an Ed25519ph [`Signature`] on the + /// `prehashed_message` if the context was 255 bytes or less, otherwise + /// a `SignatureError`. /// /// [rfc8032]: https://tools.ietf.org/html/rfc8032#section-5.1 #[allow(non_snake_case)] @@ -450,7 +452,7 @@ impl ExpandedSecretKey { prehashed_message: D, public_key: &PublicKey, context: Option<&'a [u8]>, - ) -> ed25519::Signature + ) -> Result where D: Digest, { @@ -463,7 +465,9 @@ impl ExpandedSecretKey { let ctx: &[u8] = context.unwrap_or(b""); // By default, the context is an empty string. - debug_assert!(ctx.len() <= 255, "The context must not be longer than 255 octets."); + if ctx.len() > 255 { + return Err(SignatureError(InternalError::PrehashedContextError)); + } let ctx_len: u8 = ctx.len() as u8; @@ -505,7 +509,7 @@ impl ExpandedSecretKey { k = Scalar::from_hash(h); s = &(&k * &self.key) + &r; - InternalSignature { R, s }.into() + Ok(InternalSignature { R, s }.into()) } }