convert signature library to implement crypto.Signer interface

[bobcallaway]

This is the first step of several to try to have signature library that can be used across all of sigstore's golang projects.

1. Have all non-KMS implementations implement crypto.Signer interface
2. Support hash algorithms other than SHA256
3. Replaces calls of fmt.Print(f|ln) to log.Printf()
4. Adds public key caching support for KMS providers
5. Use correct hashing algorithm per KMS provider response

[dlorenc]

cc @dekkagaijin

[dekkagaijin]

The reason why I intentionally did not implement the crypto.Signer interface is because it does not support plumbing through Contexts for RPC-based implementations (e.g. KMS). Additionally, we also need to return the input to the underlying crypto.Signer in order to support things like cosign which want/need to know any transformations applied to the raw payload (e.g. digest). It's a higher-level interface to allow for the broadest possible implementation and plugability.

Support hash algorithms other than SHA256
Replaces calls of fmt.Print(f|ln) to log.Printf()
Adds public key caching support for KMS providers
Use correct hashing algorithm per KMS provider response

I like these. Hash algs other than SHA256 are already supported via constructors, but can't sanely be used as an input to Sign and Verify since not all signing algs allow for pre-hashed inputs.

add additional APIs to interface to support io.Reader (not just []byte)

This 100%, too, but unfortunately it's not compatible with crypto libs and wouldn't have a practical upside unless the signer impl itself forgoes said libraries.

What's the end goal?

[dekkagaijin]

tl;dr crypto.Signer isn't really an abstraction. Hopefully we can improve things by providing sigstore.Signer as a useful superset of that API and creating our libraries over it. To that end, we need hash alg agnosticism and context propagation.

Additionally, for cosign and rekor we specifically need to know: payload, preHash(payload), sig(preHash(payload)). I think the usefulness of providing preHash(payload) via the generic sigstore Sign interface is questionable, but we'd need to figure out what we want to do instead.

[bobcallaway]

The reason why I intentionally did not implement the crypto.Signer interface is because it does not support plumbing through Contexts for RPC-based implementations (e.g. KMS).

Understood... by shifting the public key into a cache (which refreshes on its own context) and leveraging the crypto.SignerOpts interface to pass the context into the Sign() implementation, I think we could achieve this and still implement the interface.

Additionally, we also need to return the input to the underlying crypto.Signer in order to support things like cosign which want/need to know any transformations applied to the raw payload (e.g. digest). It's a higher-level interface to allow for the broadest possible implementation and plugability.

Support hash algorithms other than SHA256
Replaces calls of fmt.Print(f|ln) to log.Printf()
Adds public key caching support for KMS providers
Use correct hashing algorithm per KMS provider response

I like these. Hash algs other than SHA256 are already supported via constructors, but can't sanely be used as an input to Sign and Verify since not all signing algs allow for pre-hashed inputs.

Yup, you're right - I should combine the first and fourth items above since I really meant that just for the KMS implementation.

add additional APIs to interface to support io.Reader (not just []byte)

This 100%, too, but unfortunately it's not compatible with crypto libs and wouldn't have a practical upside unless the signer impl itself forgoes said libraries.

PGP libs do take an io.Reader but agree it won't help in most situations.

What's the end goal?

I'd like to get to a single implementation within sigstore for signing and verification, which is what I think your original goal was here as well. There's a fair bit of duplicated code across sigstore where we're directly calling rsa/ecdsa instead of using the signature library, and having support for the crypto.Signer interface seems like it would help.

There were two recent situations that triggered me to start working on this:

• While adding RFC3161 support in Rekor, @asraa ended up writing a fair bit of code to manually crafting messages to sign (instead of leveraging existing PKCS libraries which require passing a crypto.Signer).
• If we wanted to embed SCTs in the Fulcio-generated CA certs, I need to sign the precert, and the x509.CreateCertificate call (godoc) requires being passed a crypto.Signer. I can of course re-implement this, but would like to avoid that if at all possible.

+1 to crypto.Signer leaving a fair bit to be desired.

[dekkagaijin]

What if we do both? Implement both (signature, crypto).(Signer, Verifier)? Should be straightforward since the existing interfaces are meant translate to their crypto cousins well, anyway.

[bobcallaway]

What if we do both? Implement both (signature, crypto).(Signer, Verifier)? Should be straightforward since the existing interfaces are meant translate to their crypto cousins well, anyway.

I think that makes sense.

Let me mark this PR as a draft for now, and we can agree to the combined interface separately and then come back to this one.

[dekkagaijin]

IMHO we'll want to split the high and low level interfaces. I'm not sure there's a good way to provide a complete interface that can translate down to crypto.Signer from these types

[bobcallaway]

@dekkagaijin After looking at what you put up in #70, and thinking it through a bit more, I've come to this (which I think achieves both of our goals):

• For the non-KMS signers (i.e. RSA/ECDSA/ED25519 operations are completed totally in-memory), we directly implement crypto.Signer but do not declare it as part of the signature.Signer interface.
• For the KMS signers (that would need to have a context passed), I implemented a CryptoSignerWrapper like you suggested above.
• I also re-vamped my options code to use the method you proposed in replace context with Options in Signer, Verifier, and PublicKeyProvider interfaces #70 since I do agree that its a bit more flexible in the long run than the SignRequest/VerifyRequest approach I started with.

I'd appreciate your comments on this latest update... thanks!

[dekkagaijin]

I like :)
Thanks for indulging me