signature: final comment period and intent to ship 1.0

[tarcieri]

This is an announcement of a 2-week final comment period (ending March 22nd) which will be followed by a 1.0 release of the signature crate some time thereafter.

Original 1.0 stabilization proposal here.

Current status

I've published signature v1.0.0-pre.3, which includes significantly more documentation and clearly calls out (using doc_cfg) the 1.0 stable versus unstable "preview" parts of the API:

Docs: https://docs.rs/signature/1.0.0-pre.3/signature/index.html

I'd love feedback on the docs before a final release, particularly what parts of the current design remain unclear.

Note that I do intend to include more usage examples, but I would like those examples to highlight notable real-world ecosystem examples rather than contrived ones, and there's a bit of a chicken-and-egg problem in doing that.

API to be stabilized

The goal of this stabilization proposal is to stabilize the following types in the signature crate:

• Signature
• Signer
• Verifier
• Error

The remaining types (e.g. Digest*) now require enabling off-by-default *-preview features which call out their instability, but with a pledge that breaking changes are accompanied by a minor version bump. I've opened separate tracking issues for each of these here:

• #92 digest-preview: DigestSignature, DigestSigner, DigestVerifier
• #93 derive-preview: custom derive for Signer and Verifier for use with DigestSignature
• #94: rand-preview: RandomizedSigner

Rationale for stabilization

The four types/traits called out earlier for stabilization are sufficient for stable use of Ed25519 signatures/signers/verifiers via the ed25519 crate, which is intended to see a 1.0 release as well after a 1.0 release of this crate is published. This is the main impetus for a 1.0 release of this crate.

Current usage

The signature traits have seen a lot of real world usage, both in libraries that natively use them, and in crates which provide thin wrappers (and therefore a unified object-safe API) across several other notable ecosystem crates:

• ed25519-dalek: via the signatory-dalek wrapper crate. There is also an open PR to use the signature traits and ed25519 crate directly
• ring: via the signatory-ring crate
• secp256k1-rs: via the signatory-secp256k1 crate
• sodiumoxide: via the signatory-sodiumoxide crate

Crates which natively leverage the signature traits include:

• yubihsm for ECDSA and Ed25519 signing
• ledger-tendermint for Ed25519 signing

Alternatives considered

NOTE: See also the "Alternatives considered" section of the signature rustdoc.

This stabilization proposal is for a trait/object-safe API, as enumerated above. The main audience of this API is consumers of digital signature systems, and thus its design is very much optimized with that audience in mind.

Several people have proposed, and we have previously investigated, an alternative design based on generic signature types with algorithm-specific marker traits, as well as leveraging use of associated types, rather than generic type parameters on traits, to allow the usage of more concrete types. An API like this would be more beneficial to implementers of signature provider libraries, and allow less invasive first-class support of the signature traits.

The good news is that these approaches aren't orthogonal, and several of the other crates in RustCrypto/traits provide both an object-safe high-level API as well as an associated type-leveraging lower-level API for similar reasons. Done correctly there can be a blanket impl of the object-safe API for the provider-oriented API.

That said, the design and implementation of such an API isn't considered a blocker on a 1.0 release specifically for the reasons that it can be implemented in the future in a purely additive manner.

[tarcieri]

One notable issue with the current Signature trait is that it isn't object safe, and not for a particularly good reason: it has an impl AsRef<[u8]> in its from_bytes method. The trait could be made object safe by switching that parameter to &[u8].

TryFrom<&[u8]> would probably be ideal as a bound, except the &[u8] would necessitate adding a lifetime to the trait or using HRTB. The latter may still be worth considering, but complicates the Signature bounds story for somewhat dubious reasons (and may also re-break object safety were it "fixed" as I mentioned above? I haven't investigated that specifically).

[tarcieri]

Removed BoxError from the public API: #81

[roblabla]

Hey, thanks for the new docs, they're really helpful.

IMO, there's still a bit of documentation missing on each trait. In particular, it'd be good to explain in Signer and Verifier that they take a Signature type argument to allow for a single concrete type to generate different "kinds" of signatures. Maybe give some concrete example (I believe ECDSA has multiple kinds of signatures, and some HSMs are able to use multiple different signatures).

There should be a section of documentation show-casing a simple implementation. As it currently stands, the crate doc has no guidelines on how to write your own impls, and the traits aren't so straightforward that it can be figured out right away. IMO there should be a simple example, maybe wrapping ring or something. The impl here seems very simple and straightforward, perfect for inclusion in docs.

Also, something that bothers me is that every single example uses signatory as a wrapper for signature. It gives this very weird impression that signature isn't usable without signatory (which I know is not the case, but it's just how it feels when literally every single example goes through signatory instead). In particular, I can't help but notice all implementations share their Signature type with it. What's the use-case for this (VS just having each crate define their own)? If I'm making an impl for a new kind of signature (say, RSA), should I make a separate crate with a simple RSA signature type, allowing it to get shared with other potential implementations? Should it get added to signatory (even though signatory seems to be mostly about eliptic curves)? Those are all questions left unanswered in the current signature docs.

(I rambled a lot about the docs, but as far as the API goes, it looks fine to me).

[roblabla]

Oh one small misgiving about the Error type: As it stands it's possible in no_std to create an Error without going through the new method, since it has no fields. E.G. this works (when signature is built in no_std mode):

fn main() {
    let err = signature::Error {}
}

For the sake of forward-compatibility (should we eventually add the ability to define errors with sources in no_std), the Error type should be changed to the following to ensure that users have to go through the new constructor.

#[derive(Debug, Default)]
pub struct Error {
    /// Source of the error (if applicable).
    #[cfg(feature = "std")]
    source: Option<Box<dyn std::error::Error + Send + Sync + 'static>>,
    ensure_always_private: (),
}

[tarcieri]

Good point regarding the Error type. I'll do that. (Edit: done, see #83)

To answer some of your other questions:

What's the use-case for this (VS just having each crate define their own)? If I'm making an impl for a new kind of signature (say, RSA), should I make a separate crate with a simple RSA signature type, allowing it to get shared with other potential implementations? Should it get added to signatory (even though signatory seems to be mostly about eliptic curves)? Those are all questions left unanswered in the current signature docs.

The signature types used by Signatory come from the ecdsa and ed25519 crates in https://github.com/RustCrypto/signatures

Signatory provides a set of wrappers for notable Rust ecosystem signature provider crates which use the signature types from the ecdsa and ed25519 crates. This means if you want to write a crate that uses the Signature trait to produce an ed25519::Signature, you don't have to care about Signatory at all, but someone wanting to instantiate it with ed25519-dalek can use the signatory-dalek crate. Or if they wanted to use sodiumoxide, they could use signatory-sodiumoxide instead. Either way they get (or can verify) an ed25519::Signature.

Ideally though, provider crates would adopt these traits/types natively, so "wrapper crates" aren't needed. The yubihsm and tendermint-ledger crates are examples of this.

[burdges]

We could do that Error type much better, although I've still never gotten around to doing so.

[tarcieri]

Any concrete suggestions?

[burdges]

I'll try to make one, but maybe it'll need to wait until v2.

In essence, there are many rust crates that should be using an error type with the small box optimization, but doing that in rust is mildly tricky. It's not that the small box optimization matters so much for performance, although that's nice if you plan to error lots, but I think it can be made to work for small types even without alloc.

[roblabla]

I mean, for what it's worth the signature error cause should only be set in very rare situations from what we've talked about (basically, it should only be used to communicate transport errors such as HSM connection failures) and nothing else. Errors happening during the signature process should be "empty" to avoid leaking info. So the kind of errors that actually allocate are expected to be extremely rare and not require too much optimization work.

[tarcieri]

Just adding a quick note I took a look at HRTBs for adding a TryFrom bound to Signature:

pub trait Signature: AsRef<[u8]> + Debug + Sized
where
    for<'a> Self: TryFrom<&'a [u8], Error = Error>,
{
    /// Parse a signature from its byte representation
    fn from_bytes(bytes: &[u8]) -> Result<Self, Error> {
        bytes.try_into()
    }

    /// Borrow this signature as serialized bytes
    fn as_slice(&self) -> &[u8] {
        self.as_ref()
    }
}

This eliminates the ability to use Signature without the HRTB, which IMO is almost more annoying than having a lifetime on the trait. Just that change causes these compilation failures:

error[E0271]: type mismatch resolving `for<'a> <tests::DummySignature as std::convert::TryFrom<&'a [u8]>>::Error == signature::error::Error`
  --> signature/tests/signature_derive.rs:22:10
   |
22 |     impl Signature for DummySignature {
   |          ^^^^^^^^^ expected enum `std::convert::Infallible`, found struct `signature::error::Error`

error[E0277]: the trait bound `tests::DummySignature: std::convert::From<&'a [u8]>` is not satisfied
  --> signature/tests/signature_derive.rs:22:10
   |
22 |     impl Signature for DummySignature {
   |          ^^^^^^^^^ the trait `std::convert::From<&'a [u8]>` is not implemented for `tests::DummySignature`
   |
   = note: required because of the requirements on the impl of `std::convert::Into<tests::DummySignature>` for `&'a [u8]`
   = note: required because of the requirements on the impl of `for<'a> std::convert::TryFrom<&'a [u8]>` for `tests::DummySignature`

error[E0271]: type mismatch resolving `for<'a> <tests::DummySignature as std::convert::TryFrom<&'a [u8]>>::Error == signature::error::Error`
  --> signature/tests/signature_derive.rs:36:10
   |
36 |     impl DigestSignature for DummySignature {
   |          ^^^^^^^^^^^^^^^ expected enum `std::convert::Infallible`, found struct `signature::error::Error`

error[E0277]: the trait bound `tests::DummySignature: std::convert::From<&'a [u8]>` is not satisfied
  --> signature/tests/signature_derive.rs:36:10
   |
36 |     impl DigestSignature for DummySignature {
   |          ^^^^^^^^^^^^^^^ the trait `std::convert::From<&'a [u8]>` is not implemented for `tests::DummySignature`
   |
   = note: required because of the requirements on the impl of `std::convert::Into<tests::DummySignature>` for `&'a [u8]`
   = note: required because of the requirements on the impl of `for<'a> std::convert::TryFrom<&'a [u8]>` for `tests::DummySignature`

error[E0271]: type mismatch resolving `for<'a> <tests::DummySignature as std::convert::TryFrom<&'a [u8]>>::Error == signature::error::Error`
  --> signature/tests/signature_derive.rs:44:10
   |
44 |     impl DigestSigner<Sha256, DummySignature> for DummySigner {
   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `std::convert::Infallible`, found struct `signature::error::Error`

error[E0277]: the trait bound `tests::DummySignature: std::convert::From<&'a [u8]>` is not satisfied
  --> signature/tests/signature_derive.rs:44:10
   |
44 |     impl DigestSigner<Sha256, DummySignature> for DummySigner {
   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::convert::From<&'a [u8]>` is not implemented for `tests::DummySignature`
   |
   = note: required because of the requirements on the impl of `std::convert::Into<tests::DummySignature>` for `&'a [u8]`
   = note: required because of the requirements on the impl of `for<'a> std::convert::TryFrom<&'a [u8]>` for `tests::DummySignature`

error[E0271]: type mismatch resolving `for<'a> <tests::DummySignature as std::convert::TryFrom<&'a [u8]>>::Error == signature::error::Error`
  --> signature/tests/signature_derive.rs:57:10
   |
57 |     impl DigestVerifier<Sha256, DummySignature> for DummyVerifier {
   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `std::convert::Infallible`, found struct `signature::error::Error`

error[E0277]: the trait bound `tests::DummySignature: std::convert::From<&'a [u8]>` is not satisfied
  --> signature/tests/signature_derive.rs:57:10
   |
57 |     impl DigestVerifier<Sha256, DummySignature> for DummyVerifier {
   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::convert::From<&'a [u8]>` is not implemented for `tests::DummySignature`
   |
   = note: required because of the requirements on the impl of `std::convert::Into<tests::DummySignature>` for `&'a [u8]`
   = note: required because of the requirements on the impl of `for<'a> std::convert::TryFrom<&'a [u8]>` for `tests::DummySignature`

error: aborting due to 8 previous errors

[tarcieri]

One other note: the Signature::from_bytes and ::as_slice names are, at best, inconsistent. (Edit: done, see #87)

Suggestion: rename Signature::as_slice to ::as_bytes

[daira]

Trait signature::DigestSignature
Marker trait for Signature types computable as S(H(m))

S: signature algorithm
H: hash (a.k.a. digest) function
m: message
i.e. a traditional application of the Fiat-Shamir heuristic.

The Fiat-Shamir heuristic is about hashing the transcript of an interactive protocol, not about hashing the message. You can have a non-Fiat-Shamir signature algorithm that hashes the message in this way, and you can have a Fiat-Shamir-derived algorithm that does not. These aren't obscure cases, either; an example of the latter is Ed25519. The important thing is whether you can compute the digest separately from the signature (and without the private key), not how the signature algorithm was derived.

[tarcieri]

@daira yeah, these have been tricky things to document, that one in particular. See here for the original PR on it (which mentioned nothing about Fiat-Shamir):

RustCrypto/signatures#10

Notably as you mentioned DigestSignature does not work in cases like Ed25519 because it hashes twice.

I'm not sure if there's a better name for such a signature algorithm (some of the previous discussion called it "raw digest" signer. I think "prehash" is another common term ala Ed25519ph), but notably the signature crate and signature_derive can derive the Signature implementation in this case (as opposed to e.g. Ed25519, which has a more complex protocol that hashes the message twice)

[roblabla]

FWIW I think DigestSignature conversation should go in a dedicated tracking issue since it's not part of the API getting currently stabilized, and it'd suck for those discussion points to get lost in an unrelated ticket ^^.

Which brings me to something else: It'd be nice to have a tracking issue per preview feature to funnel API and stabilization discussion of those APIs there.

[tarcieri]

@roblabla good idea, will open an issue for each of the *-preview features (Edit: opened #92, #93, and #94 and added descriptions of them to the OP)

[tarcieri]

Quick update on this:

I got some good feedback it seems. Here are the remaining issues, neither of which are a 1.0 blocker, but both of which could probably use work before a 1.0 release:

• Further documentation improvements (always a nice to have)
• Rename DigestSignature -> PrehashSignature (technically not a part of the 1.0 API, but I think it'd still be good to change sooner than later)

Would still be nice to have some clarity / feedback around dalek-cryptography/ed25519-dalek#80 before shipping this as well.

All that said, I'm not in a huge rush here, so if you'd still like to give any final feedback, please do.

[tarcieri]

DigestSignature was renamed to PrehashSignature in #96

[tarcieri]

I've opened a PR for the final 1.0 release of the signature crate in #97

I'm going to do some preflight checks on it and then cut the release

[tarcieri]

It's out! 🎉

https://crates.io/crates/signature/1.0.0