proposal: crypto/ecdh: add compressed points functions

[linouxis9]

Proposal Details

Currently, we have to pass compressed bytes through elliptic.UnmarshalCompressed and the deprecated elliptic.Marshal to make use of compressed (e.g. NIST P-256) keys with crypto/ecdh: example.

An implementation was proposed here: crypto/ecdh: implement compressed points but was not merged, while the source issue got closed.

Any updates on the matter?
I would be glad to rebase the changes if needed !

Thanks a lot!

CC: @FiloSottile

Valentin

[seankhliao]

I think it was skipped in #52221 because it was unclear what actually needed this. Is the requirement for some widely used protocol?

[linouxis9]

It's specified as part of 3GPP TS 33.501 (Annex C) which specifies how Subscribers Identifiers should be concealed in 5G mobile networks: not sure if that's enough of an use case though @seankhliao.

[ianlancetaylor]

CC @golang/security

[FiloSottile]

NIST point decompression comes up very very rarely (unfortunately, since it's a better encoding than uncompressed), it might not be worth carrying support for in the standard library.

You can do the equivalent of UnmarshalCompressed/Marshal with https://pkg.go.dev/filippo.io/nistec#P256Point.SetBytes and https://pkg.go.dev/filippo.io/nistec#P256Point.Bytes, and Unmarshal/MarshalCompressed with https://pkg.go.dev/filippo.io/nistec#P256Point.SetBytes and https://pkg.go.dev/filippo.io/nistec#P256Point.BytesCompressed. Does that work for your use case?

[linouxis9]

Thank you @FiloSottile for your feedback! This would do the behavior I need, but I don't think this is a suitable solution.

I still think this fits the standard library for a few reasons:

• the deprecated functions were originally added to the standard library because there was an identified real need to manipulate compressed NIST points. Several projects had to reimplement this feature themselves beforehand. I think it's interesting to re-read the original proposal for the deprecated functions: crypto/elliptic: handle compressed formats in new MarshalCompressed, UnmarshalCompressed #34105 (comment)
  I don't think this need has disappeared since then.
• as you know, rolling out yourself crypto implementation when you are not an expert is never a good idea. Actually, I'm opening this issue, because the project free5gc I'm contributing to, used to have an invalid curve attack, by trying to reimplement ECDH with compressed NIST keys. This might be a somewhat niche library function, but it could prevent severe security issues by developers trying to reimplement the feature, and it's not especially a maintenance burden to have in the standard library as the size of your original PR alludes to: https://go-review.googlesource.com/c/go/+/404276
  I would believe it's better to have a centralized function that can be audited !
• I don't think deprecating a function without a suitable replacement is a good idea
• I don't believe adding a dependency on an external library with minimal usage (even if that's by you, a trusted Go security member), re-exporting internals of the Go standard lib in a library, and with limited interoperability with the rest of crypto/ecdh package is a good security practice

I'm not a crypto expert, so I'm sure you'll know better than me the pro / cons!

In any cases, thank you much for your help on this matter!

Valentin