[v3] wrong curve when marshaling ecdh keypair

[gunawanwijaya]

Contribution Guidelines

Before filing an issue, please read the contents of CONTRIBUTING.md, and follow its instructions.

Describe the bug

json marshaling jwk.Key resulting in a different curve when using ecdh keypair,
e.g. using P384 but instead returning X25519

$ go version
go version go1.23.2 linux/amd64

To Reproduce / Expected behavior
https://go.dev/play/p/YHIuYo-T-Lv

gist of jwk usage

func main() {
	// given this information
	keyCli, _ := ecdh.P384().GenerateKey(rand.Reader)
	pubCli := keyCli.PublicKey() // server is able to retrieve the pub key part of client

	// server generate ephemeral key for this request
	keySrv, _ := ecdh.P384().GenerateKey(rand.Reader)
	jwkSrv, _ := jwk.Import(keySrv.PublicKey()) // here the `crv="x25519"`
	jwkBuf, _ := json.Marshal(jwkSrv) // we want to response with this jwkBuf attached
	secretSrv, _ := keySrv.ECDH(pubCli)

	_ = secretSrv // doing some non-standard encryption & response with encrypted data

	// client
	pubSrv := &ecdh.PublicKey{}
	jwkCli, _ := jwk.ParseKey(jwkBuf) // extract jwkBuf
	_ = jwk.Export(jwkCli, pubSrv) // this is where the error happened
	// bcs we send p384 with 48 bytes key length but `crv="x25519"` expecting 32 bytes key length

	secretCli, _ := keyCli.ECDH(pubSrv)

	_ = secretCli // doing some non-standard encryption
}

[lestrrat]

@gunawanwijaya thank you. Can you please check #1266 and see if it works for you? (you can ignore the lint errors on CI)

[gunawanwijaya]

just done some smoke testing, but kty field is pointing to EC instead of OKP when using ecdh keypair with P384, meanwhile X25519 & Ed25519 work as expected

[gunawanwijaya]

I think the issue is on ecdhPrivateKeyToECJWK returning ecdsaPublicKeyToJWK and thus changing kty to EC

[gunawanwijaya]

my suggestion is map https://github.com/lestrrat-go/jwx/blob/develop/v3/jwk/okp.go#L47 to the respective ecdh key.Curve()

[lestrrat]

Can you please put down the expected behavior in a test (not just print the values, please put checks to show me exactly what you are expecting). It's less ambiguous than doing back on forth on comments.

FWIW I think originally in jwx ECDH on P-384 was returning OKP (before you raised the issue) and I subsequently researched and thought ECDH on P-384 should return EC, not OKP.

[gunawanwijaya]

sorry for the confusion, I've updated the issue with a snippet from my code, the expectation is for jwk.Import and jwk.Export to work with ecdh keypair

[lestrrat]

@gunawanwijaya I've just updated #1266.

One minor nit from your updated code:

In the above code here the crv="x25519" comment is wrong (and my code was wrong as well)
Since the key was generated with a P384 NIST curve, the resulting key must be an EC key with kty=EC, crv=P-384.

I'd almost say that resulting JWK should only be exportable to ECDSA keys, but I do agree that because of how things are, being able to convert it back to an ecdh.(Public|Private)Key is desirable. So as of the latest change in #1266, you can explicitly ask for the key to be exported to a ECDH key.