From 6c0c9e2c0f3c86c3c9762509e81d03cf96d27552 Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Wed, 5 Jun 2019 15:14:28 -0700 Subject: [PATCH] Replace Digest-related blanket impls with methods The current "clever" blanket impl (for which I'm entirely responsible!) seems to have bounds which prevent anything from implementing `Signer` or `Verifier` when the `digest` feature is enabled: ``` = note: conflicting implementation in crate `signature`: - impl signature::signer::Signer for T where S: signature::signature::DigestSignature, T: signature::signer::DigestSigner<::Digest, S>; = note: upstream crates may add new impl of trait `signature::signature::DigestSignature` for type `signatory::ed25519::signature::Signature` in future versions = note: downstream crates may implement trait `signature::signer::DigestSigner<_, signatory::ed25519::signature::Signature>` for type `ed25519::Ed25519Signer` ``` This commit, while a bit ugly, at least eliminates all of the complexity around bounds on the blanket impl by entirely removing the blanket impl, and adding an additional method to precompute the digest. I'm not entirely happy with this and think the method names are a bit confusing and not descriptive enough, but it does have the following rather nice properties: - Easy to reason about: no blanket impls - Allows the same type to impl both the `Digest` and non-`Digest` forms of `Signer` and `Verifier` simultaneously. There are a lot of potential directions this could go in order to improve the API, potentially splitting it into two traits rather than one (which gets us back to the `Sha*Signer`/`Sha*Verifier` traits which Signatory used originally), however I think this is the MVP for actually using the `signature` crate in Signatory, so I'd like to start with this and then iterate towards a better API. --- signature-crate/src/signature.rs | 16 ---------------- signature-crate/src/signer.rs | 31 ++++++++++++++++--------------- signature-crate/src/verifier.rs | 23 +++++++---------------- 3 files changed, 23 insertions(+), 47 deletions(-) diff --git a/signature-crate/src/signature.rs b/signature-crate/src/signature.rs index 8d021467..8376a5a4 100644 --- a/signature-crate/src/signature.rs +++ b/signature-crate/src/signature.rs @@ -22,19 +22,3 @@ pub trait Signature: AsRef<[u8]> + Debug + Sized { self.as_slice().into() } } - -/// Marker trait for `Signature` types computable as `S(H(m))` -/// -/// - `S`: signature algorithm -/// - `H`: hash (a.k.a. digest) function -/// - `m`: message -/// -/// For signature types that implement this trait, a blanket impl of -/// `Signer` will be provided for all types that `impl DigestSigner` -/// along with a corresponding impl of `Verifier` for all types that -/// `impl DigestVerifier`. -#[cfg(feature = "digest")] -pub trait DigestSignature: Signature { - /// Preferred `Digest` algorithm to use when computing this signature type. - type Digest: digest::Digest; -} diff --git a/signature-crate/src/signer.rs b/signature-crate/src/signer.rs index 537c6c93..2348c48e 100644 --- a/signature-crate/src/signer.rs +++ b/signature-crate/src/signer.rs @@ -1,10 +1,7 @@ //! Traits for generating digital signatures #[cfg(feature = "digest")] -use crate::{ - digest::{generic_array::GenericArray, Digest}, - signature::DigestSignature, -}; +use crate::digest::{generic_array::GenericArray, Digest}; use crate::{error::Error, Signature}; /// Sign the provided message bytestring using `Self` (e.g. a cryptographic key @@ -30,7 +27,22 @@ where D: Digest, S: Signature, { + /// Sign the computed digest of the given message. + /// + /// Panics in the event of a signing error. + fn sign_msg_digest(&self, msg: &[u8]) -> S { + self.try_sign_msg_digest(msg) + .expect("signature operation failed") + } + + /// Attempt to sign the computed digest of the given message. + fn try_sign_msg_digest(&self, msg: &[u8]) -> Result { + self.try_sign_digest(D::digest(msg)) + } + /// Sign the given prehashed message `Digest`, returning a signature. + /// + /// Panics in the event of a signing error. fn sign_digest(&self, digest: GenericArray) -> S { self.try_sign_digest(digest) .expect("signature operation failed") @@ -40,14 +52,3 @@ where /// digital signature on success, or an error if something went wrong. fn try_sign_digest(&self, digest: GenericArray) -> Result; } - -#[cfg(feature = "digest")] -impl Signer for T -where - S: DigestSignature, - T: DigestSigner, -{ - fn try_sign(&self, msg: &[u8]) -> Result { - self.try_sign_digest(S::Digest::digest(msg)) - } -} diff --git a/signature-crate/src/verifier.rs b/signature-crate/src/verifier.rs index 0bd9154f..1612cbf1 100644 --- a/signature-crate/src/verifier.rs +++ b/signature-crate/src/verifier.rs @@ -1,10 +1,7 @@ //! Trait for verifying digital signatures #[cfg(feature = "digest")] -use crate::{ - digest::{generic_array::GenericArray, Digest}, - signature::DigestSignature, -}; +use crate::digest::{generic_array::GenericArray, Digest}; use crate::{error::Error, Signature}; /// Verify the provided message bytestring using `Self` (e.g. a public key) @@ -24,21 +21,15 @@ where D: Digest, S: Signature, { - /// Verify the signature against the given `Digest` + /// Verify the signature against the computed `Digest` output. + fn verify_msg_digest(&self, msg: &[u8], signature: &S) -> Result<(), Error> { + self.verify_digest(D::digest(msg), signature) + } + + /// Verify the signature against the given `Digest` output. fn verify_digest( &self, digest: GenericArray, signature: &S, ) -> Result<(), Error>; } - -#[cfg(feature = "digest")] -impl Verifier for T -where - S: DigestSignature, - T: DigestVerifier, -{ - fn verify(&self, msg: &[u8], signature: &S) -> Result<(), Error> { - self.verify_digest(S::Digest::digest(msg), signature) - } -}