signature v3: type inference regression

[tarcieri]

It seems after #1827 type inference is not working as well:

RustCrypto/RSA#505 (comment)

error[E0282]: type annotations needed
  --> tests/pkcs1v15.rs:16:9
   |
16 |     let signature = signing_key.sign(bytes);
   |         ^^^^^^^^^
...
22 |     assert_eq!(signature.to_string(), expected);
   |                --------- type must be known at this point
   |
help: consider giving `signature` an explicit type
   |
16 |     let signature: /* Type */ = signing_key.sign(bytes);
   |                  ++++++++++++

It can no longer automatically infer the signature type in the case there is only a single non-overlapping impl, which seems pretty bad from an ergonomic perspective.

[tarcieri]

The specific case where this seems to occur is when there is an explicit Signer impl and the blanket impl isn't satisfied.

Based on this, I'd suggest reverting #1827.

We can potentially still get rid of custom derive by replacing it with macro_rules.

[newpavlov]

I am a bit out of context, but why can't we make the signature type S an associated type instead of a generic parameter on the trait? For example, in the RSA case the implementation looks like this: impl<D> DigestVerifier<D, Signature<D>> for VerifyingKey<D> { .. }. It does not use the ability to be generic over signature type at all and probably can be changed to impl<D> DigestVerifier<D> for VerifyingKey<D> { type Signature = Signature<D>; ... } without any issues.

If other crates rely on this feature, we could move this generality to the implementor side.

[tarcieri]

@newpavlov overlapping impls. Some key types support many, many resulting signature types, e.g. https://docs.rs/ecdsa/latest/ecdsa/struct.SigningKey.html

[newpavlov]

Why can't we change it to SigningKey<C, S>, as I suggested at the end of my comment? I think it makes sense to parametrize the signer type instead of trait which it implements.

[tarcieri]

The ability of a single key to support multiple signature algorithms is important, for example if you're implementing something like a KMS system which may need to dynamically support multiple signature formats depending on what the client requests.

It's the reason why the rsa crate maintains an entire duplicate/parallel API on RsaPrivateKey for such purposes.

[newpavlov]

Can you show a concrete code example for this? I would think it should not be different from supporting multiple hash functions or AEAD algorithms, i.e. your KMS could return a custom signture type which under the hood could be an enum over supported signature algorithms.

It's the reason why the rsa crate maintains an entire duplicate/parallel API on RsaPrivateKey for such purposes.

Your mean generality over SignatureScheme?

[tarcieri]

Can you show a concrete code example for this?

Unfortunately not because everything that would be a good example I'm currently working on as a downstream consumer only supports one signature algorithm per key type. That's for want of features I eventually do plan on integrating in these systems though.

The problem comes up pretty much anywhere there's dynamism around signature algorithm choice where multiple signing algorithms are possible for a single asymmetric key type.

I would think it should not be different from supporting multiple hash functions

Hash functions aren't keyed.

AEAD algorithms

I think it's probably less common to reuse symmetric keys with different AEAD algorithms. They don't have something analogous to a common unifying public key scheme. Where that reuse happens is probably upgrading legacy users who have a symmetric key from a legacy unauthenticated algorithm to an AEAD.

Public key schemes, particularly RSA, also have expensive setup. I guess you could first decode to an RsaPublicKey (and perform precomputation), then convert to an e.g. rsa::pss::SigningKey<Sha256>, which would at least clone the private key material for each use, but IMO it's nice to be able to dispatch to a particular signature algorithm without having to clone the key material (or creating per-algorithm proxy objects that borrow the key material).

Your mean generality over SignatureScheme?

Yes, and sidebar while that API should probably be revisited, this isn't the release to do it as there are major unreleased security fixes and we should make that upgrade process as smooth as possible.

Backing up, the inference problems go away if we revert the blanket impl of Signer for DigestSigner, i.e. #1827. I wound up less happy with adding that blanket impl after I discovered we couldn't have symmetric blanket impls for the Async* traits (which I had originally worried about in #1419)