Go: crypto/ecdsa: add SignASN1, VerifyASN1

[jyasskin]

The crypto.Signer implementation provided by crypto.ecdsa encodes its signature into bytes using the ASN.1 format that's used by TLS, but it doesn't provide a matching verification routine that takes a []byte instead of a pair of big.Ints. It's straightforward to implement one, and both crypto.tls and crypto.x509 have done so, but it seems silly to make every consumer independently implement a standardized signature format.

[rsc]

/cc @agl

[rsc]

I ran into this oddness during BoringCrypto work too. It does seem like there should be a

package ecdsa
func VerifyBytes(pub *PublicKey, hash, sig []byte) bool

to match ecdsa.PrivateKey.Sign. It would be tempting to add SignBytes that returns []byte instead of two big.Int, but that name doesn't seem right. So maybe instead:

package ecdsa
func SignASN1(io.Reader, priv *PrivateKey, hash []byte) (sig []byte, err error)
func VerifyASN1(pub *PublicKey, hash, sig []byte) bool

?

@agl, I'm happy to write these if you sign off (ha) on the names.

[FiloSottile]

This came up during @katiehockman's work on Wycheproof tests, and it seems like a good idea, it looks like we are forcing everyone to use encoding/asn1 when they just want to deal with ECDSA. I'm in favor of adding these APIs.

[rsc]

Based on the discussion above, this seems like a likely accept.

Leaving open for a week for final comments.

[agl]

Exposing a public signature interface with big-ints was dumb(*). The interface should always have been byte-based.

(* happy to use strong language here because it was my screw-up.)

[rsc]

No change in consensus (agl strongly agrees!), so accepting.

[katiehockman]

Great, thanks! I have a change ready for this, so I'll pass it along shortly (though it won't go in until 1.15)

[erincandescent]

I wonder if there should be a crypto.Verifier interface which is symmetric with crypto.Signer, which public key types should implement?

[gopherbot]

Change https://golang.org/cl/217940 mentions this issue: crypto/ecdsa: add SignASN1, VerifyASN1

[SaulEntersekt]

Glad to have found this. I'm trying to discover whether the SignASN1 returns DER or BER by default (translating work from a JS lib). Perhaps this could be part of the docs?

[FiloSottile]

It's DER. I'd be happy to take a PR to replace "ASN.1" with "ASN.1 DER" in
the docs of both functions!

On Mon, Sep 21, 2020 at 9:40 AM SaulEntersekt [e-mail]
wrote:

Glad to have found this. I'm trying to discover whether the SignASN1
returns DER or BER by default (translating work from a JS lib). Perhaps
this could be part of the docs?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/golang/go/issues/20544#issuecomment-695954793, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAJLETSVGIL6EDZWSRPJPN3SG37QPANCNFSM4DNRJO6Q
.

[SaulEntersekt]

@FiloSottile Honestly I'm not sure where there repo for the docs would live. Looked around but nothing leaps out to me.

[FiloSottile]

The docs are right in the code!

https://github.com/golang/go/blob/master/src/crypto/ecdsa/ecdsa.go#L285

[SaulEntersekt]

@FiloSottile Ah nice. Quite new to go community.

Here is the PR: https://github.com/golang/go/pull/41521

[FiloSottile]

Thank you, and welcome!

You'll need to make the CLA bot happy for us to merge it, but it's
something you only have to do once!

On Mon, Sep 21, 2020 at 2:26 PM SaulEntersekt [e-mail]
wrote:

@FiloSottile https://github.com/FiloSottile Ah nice. Quite new to go
community.

Here is the PR: #41521 https://github.com/golang/go/pull/41521

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/golang/go/issues/20544#issuecomment-696080994, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAJLETSZGIU55UNZZXYXAN3SG5BARANCNFSM4DNRJO6Q
.

[SaulEntersekt]

@FiloSottile Sadly I now have a round trip through legal. :P