chore: migrate to crypto/ecdh

[shizhMSFT]

Resolves #168 by migrating to crypto/ecdh.

Here are notable changes:

• Since crypto/ecdh is introduced in go 1.21, the minimum go version is thus bumped to 1.21 from 1.18.
• Since crypto/ecdh does not expose low-level elliptic curve operations, custom curves are no longer supported. Thus the related test case is removed. /cc @qmuntal

[shizhMSFT]

The CI tests are failing since go-cose is not ready for go 1.21 yet as its current support windows is go [1.18, 1.19].

[shizhMSFT]

I would suggest reviewing and merging PR after the go 1.22 release (ETA: Feb 2024) so that we can move the support window to [1.21, 1.22] for crypto/ecdh.

[OR13]

Seems like this PR is on a good track, once CI is passing, and our test coverage is recovered, this seems like a great improvement

[SteveLasker]

@shizhMSFT, with 1.22 released, are you ready to finalize this PR?

[qmuntal]

I'm not sold about this solution. x509.ParsePKCS8PrivateKey calls curve.ScalarBaseMult(d) under the hood, so all the additional work we do, namely mapping an ECDSA key to an ECDH key, then encoding a x509 and finally decoding the same key, boils down to avoiding a deprecated function call by using a higher level functions that uses the same deprecated function.

IMO, we should either remove support for ECDSA curves without public component, or assume that we have to call a deprecated function.

[shizhMSFT]

I'm not sold about this solution. x509.ParsePKCS8PrivateKey calls curve.ScalarBaseMult(d) under the hood, so all the additional work we do, namely mapping an ECDSA key to an ECDH key, then encoding a x509 and finally decoding the same key, boils down to avoiding a deprecated function call by using a higher level functions that uses the same deprecated function.

I agree with that.

assume that we have to call a deprecated function

I'm afraid of being scanned as a vulnerability by some scanners in the future although it is not.

remove support for ECDSA curves without public component

Can you elaborate more on this?

[qmuntal]

Can you elaborate more on this?

Do what @OR13 recommended here:

I'd recommend just throwing when compressed points are passed... and not doing the point compression, and that would eliminate the warning.

[OR13]

Unless you are parsing a key that is inside a signature or encryption, developers can translate keys with compressed points to ones with uncompressed points.

Any library that doesn't support compressed points will need to throw if it cannot translate, because operations will fail that required both points.

[shizhMSFT]

Thanks for the elaboration.

remove support for ECDSA curves without public component

Do we want to go that way?

[SteveLasker]

Based on the above, we're going to close this PR and open a new issue for guidance.