The only thing the `signature` crate uses `alloc` for is the
`Signature::to_vec` method, which is trivially accomplished otherwise as
`sig.as_ref().to_vec()` or various other ways.
By getting rid of it, we can completely get rid of the `alloc` feature,
and with that reduce the MSRV back to 1.31.
It might be worth considering a 1.0 release with a higher MSRV so we can
leverage `TryFrom`, but for now, this provides wider compatibility by
removing a single (mis)feature.
This upgrades to the `std::error::Error` features for boxed,
downcastable error sources introduced in Rust 1.30, namely switching
from `Error::cause` to `Error::source`, which adds a `'static` bound and
therefore allows it to support downcasting.
Additionally, it defines a `BoxError` type incorporating those bounds
along with `Send + Sync`, ensuring that `signature::Error` itself is
`Send + Sync`, which should improve the ergonomics.
The motivation for 1.0 stabilization of the `signature` crate is the
upcoming 1.0 release of `ed25519-dalek`. In order to promote Ed25519
interoperability, it would be great if `ed25519-dalek` could use the
traits from this crate along with the `ed25519::Signature` type from the
`ed25519` crate.
To get there, I think we need to do a 1.0 release of this crate, as well
as the `ed25519` crate.
The main impediment towards doing so is the `digest` crate is presently
stuck at v0.8. It would be nice to be able to continue upgrading it,
especially to a 1.0 release, but that would otherwise be a semver
breaking change.
To allow agility around `digest`, and `signatory_derive` which depends
on it, this commit places access to both under the `digest-preview` and
`derive-preview` Cargo features respectively, and calls them out as
not covered under SemVer and subject to change, but breaking changes
will be done with a minor version bump.
This reverts commit 48e33d8758.
After updating my downstream consumers to try to use this, I encountered
an important case where this falls down: deriving `Signer` and
`Verifier` on generic types where the `Digest` *can only* be generically
specified as an associated type, as the `yubihsm` crate is doing here:
https://github.com/tendermint/yubihsm-rs/blob/develop/src/ecdsa/signer.rs#L22
The goal of switching to a derive attribute was to make this
functionality more flexible and eliminate the need for a marker trate,
but in this particular case (one I personally consider very important)
it had the opposite effect.
The `DigestSignature` trait was originally used as the marker trait used
by a blanket impl of `Signer` for `DigestSigner` and `Verifier` for
`DigestVerifier` (#12, #16).
Unfortunately, this approach turned out to be too inflexible to cover
real-world use cases encountered and was replaced with a proc macro (#18)
The `DigestSignature` trait is leftover from this, and is no longer
needed.
This commit replaces it with a `#[digest(...)]` custom derive attribute
which can be used to specify the digest which should be used when
computing a signature.
Replaces `use` directives with fully qualified paths. This prevents
potential name clashes in the event types have the same name as the
traits needed for the custom derive.
We've explored several different possible trait bounds for a blanket
impl of `Signer` for `DigestSigner` (and likewise for `Verifier`).
Unfortunately, the approach we netted out at in #12 does not permit
anything but the blanket impl, and was removed in #16.
It's definitely still worth exploring if there's a way to define the
bounds of the default impl that works, however there is one approach we
haven't explored yet: a procedural macro.
This approach feels like a bit of a hack, but gets us to where we
originally wanted to be with the blanket impl API-wise. The impl it
derives uses the same approach and bounds as the blanket impl did:
the `Digest` to use is sourced from an associated type of the
`DigestSignature` trait, which means the digest to use is both automatic
and ensures the type deriving the trait supports the expected `Digest`
for the given signature.
The implementation uses `synstructure` which simplifies both handling of
generics and testing the output of the proc macro.
Additionally, it includes a complete integration test of the derived
code which ensures it works as expected.
It's gated under a cargo feature and disabled-by-default. This should
make it unobtrusive for downstream crates which don't need its
functionality.
This is needed forcompatibility with ed25519-dalek's Ed25519ph:
https://docs.rs/ed25519-dalek/1.0.0-pre.1/ed25519_dalek/struct.Keypair.html#method.sign_prehashed
Until const generics land, this API feels a lot cleaner to me. It gets
all the `GenericArray` crap out of the way.
It's also misuse resistant in that it ensures the prehashing is done by
the relevant hash function, as opposed to the user being able to pass in
arbitrary values. There's a potential attack if a verifier accidentally
accepts a raw value which isn't the output of a hash function which
could allow an attacker to forge signatures:
https://twitter.com/pwuille/status/1063582706288586752
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<S, T> signature::signer::Signer<S> for T
where S: signature::signature::DigestSignature, T: signature::signer::DigestSigner<<S as signature::signature::DigestSignature>::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.
For many signature providers, it's not possible for an error to occur
when creating a signature. Support for handling errors which occur when
computing signatures is intended for use with cloud KMS, HSMs, or other
hardware tokens where things like I/O errors are possible.
To simplify usage in the event that only software signers are in use,
this commit splits the `sign` API into `Result`-based `try_sign` method
and another `sign` method that always assumes success and unwraps the
result of the former method.
Tony Arcieri