Conversation
|
@getvictor Here is a PR for supporting crypto.Signer. Let me know if this will work for your use cases or if it needs any changes |
| } | ||
| } else { | ||
| // crypto.Signer for ECDSA returns the signature in ASN.1 format | ||
| asn1Sig, err := pkSigner.Sign(rand.Reader, msgHash[:], crypto.SHA256) |
There was a problem hiding this comment.
In our case, TPM Sign does not return ASN.1 format, and we create the proper HTTP signature format ourselves: https://github.com/fleetdm/fleet/blob/34c45b256f54298aae0fc7bd70c68cd6a99faf7a/ee/orbit/pkg/securehw/securehw_tpm.go#L520
We don't want to asn1.Marshal the result just so it can be unmarshaled in this method.
Go’s encoding/asn1 works, but it’s reflective and allocation-heavy. Parsing/serializing with asn1.Unmarshal/asn1.Marshal is measurably slow.
We would like to return the HTTP signature directly from our Signer.
There was a problem hiding this comment.
That makes sense, which is why I kept the ecdsa.Sign method above.
However, the ecdsa.Sign method that implements crypto.Signer declares it returns ASN1 format - https://pkg.go.dev/crypto/ecdsa#PrivateKey.Sign
So if someone where to pass a crypto.Signer based on an ecdsa private key I have no way of knowing if the signing format is ASN1 or not.
Any suggestions for this? It feels like I have to respect the documentation here but I also don't want the ASN1 overhead. I could add member variable like isASN1 to the SigningKey struct but that feels ugly.
There was a problem hiding this comment.
Another option would be, if crypto.Signer is provided in the SigningKey, the code would have to check if the implementation of crypto.Signer was a *ecdsa.PrivateKey. If not *ecdsa.PrivateKey then assume the signature is not ASN.1 encoded.
I am not a big fan of that either. It feels better to be explicit.
Co-authored-by: Harshita Chaudhary <hchaudhary2511@gmail.com>
…her to expect an ASN1 encoded signature for ECDSA signatures.
|
@getvictor I redid the interface slightly to indicate whether the crypto.Signer returns ASN1 formatted signatures (default is no) and added test cases. Let me know what you think. If you have better ideas for the user interface (SigningKey, SigningKeyOpts) let me know. |
3 participants
This adds a
Signermember of typecrypto.Signerto theSigningKeystruct in addition to a private key to support TPM backed signing.SigningKeyretains thePrivateKeymember for usage ergonomics and to allow for type checking when signing algorithms.