go 1.19 panic due to missing EC key validation

[thomas-fossati]

The following is ExampleJWX from the README slightly modified to read an EC key whose x-coord has been moved out of the P-256 curve.

package main

import (
	"fmt"
	"time"

	"github.com/lestrrat-go/jwx/v2/jwa"
	"github.com/lestrrat-go/jwx/v2/jwk"
	"github.com/lestrrat-go/jwx/v2/jwt"
)

func ExampleJWX() {
	untrustedJWK := []byte(`{
		"kty": "EC",
		"crv": "P-256",
		"x": "MKBCTNIcKUSDii11ySs3526iDZ8AiTo7Tu6KPAqx7D4",
		"y": "4Etl6SRW2YiLUrN5vfvVHuhp7x8PxltmWWlbbM4IFyM",
		"d": "870MB6gfuTJ4HtUnUvYMyJpr5eUZNP4Bk43bVdj3eAE"
	}`)

	// Parse, serialize, slice and dice JWKs!
	privkey, err := jwk.ParseKey(untrustedJWK)
	if err != nil {
		fmt.Printf("failed to parse JWK: %s\n", err)
		return
	}

	pubkey, err := jwk.PublicKeyOf(privkey)
	if err != nil {
		fmt.Printf("failed to get public key: %s\n", err)
		return
	}

	// Build a JWT!
	tok, err := jwt.NewBuilder().
		Issuer(`github.com/lestrrat-go/jwx`).
		IssuedAt(time.Now()).
		Build()
	if err != nil {
		fmt.Printf("failed to build token: %s\n", err)
		return
	}

	// Sign a JWT!
	signed, err := jwt.Sign(tok, jwt.WithKey(jwa.ES256, privkey))
	if err != nil {
		fmt.Printf("failed to sign token: %s\n", err)
		return
	}

	// Verify a JWT!
	{
		verifiedToken, err := jwt.Parse(signed, jwt.WithKey(jwa.ES256, pubkey))
		if err != nil {
			fmt.Printf("failed to verify JWS: %s\n", err)
			return
		}
		_ = verifiedToken
	}
}

func main() {
	ExampleJWX()
}

go1.19 crypto/elliptic reacts quite drastically when it finds out it's operating on invalid curve points (see release notes):

"Operating on invalid curve points (those for which the IsOnCurve method returns false, and which are never returned by Unmarshal or by a Curve method operating on a valid point) has always been undefined behavior and can lead to key recovery attacks. If an invalid point is supplied to Marshal, MarshalCompressed, Add, Double, or ScalarMult, they will now panic."

In fact, compiling and running the snippet above with go1.19 panics like this:

╰─○ go run main.go  
panic: crypto/elliptic: CombinedMult was called on an invalid point

goroutine 1 [running]:
crypto/elliptic.(*nistCurve[...]).CombinedMult(0x1054170a0, 0x140000d3598, 0x105038498?, {0x140000c8380?, 0x105038470?, 0x10523a0c0?}, {0x140000c83a0?, 0x20, 0x20})
        /opt/homebrew/Cellar/go/1.19.3/libexec/src/crypto/elliptic/nistec.go:242 +0x154
crypto/ecdsa.verifyGeneric(0x140000d4da0, {0x105280148, 0x1054170a0}, {0x140000c8340?, 0x646e617638325a74?, 0x80306e4934?}, 0x0?, 0x6003000000000000?)
        /opt/homebrew/Cellar/go/1.19.3/libexec/src/crypto/ecdsa/ecdsa.go:385 +0x178
crypto/ecdsa.verify(...)
        /opt/homebrew/Cellar/go/1.19.3/libexec/src/crypto/ecdsa/ecdsa_noasm.go:20
crypto/ecdsa.Verify(0x140000d4da0, {0x140000c8340, 0x20, 0x20}, 0x140000d4e40, 0x140000d4e60)
        /opt/homebrew/Cellar/go/1.19.3/libexec/src/crypto/ecdsa/ecdsa.go:363 +0x110
github.com/lestrrat-go/jwx/v2/jws.(*ecdsaVerifier).Verify(0x140000a2258, {0x1400008c300, 0x6c, 0x100}, {0x140000b2580, 0x40, 0x40}, {0x105271a80, 0x140000baaa0})
        /Users/thofos01/go/pkg/mod/github.com/lestrrat-go/jwx/[email protected]/jws/ecdsa.go:189 +0x4ac
github.com/lestrrat-go/jwx/v2/jws.Verify({0x1400018a1a0?, 0xc3?, 0xc3?}, {0x14000099270?, 0x1?, 0x104f3dea0?})
        /Users/thofos01/go/pkg/mod/github.com/lestrrat-go/jwx/[email protected]/jws/jws.go:360 +0x7a0
github.com/lestrrat-go/jwx/v2/jwt.verifyJWS(0x1400018a1a0?, {0x1400018a1a0?, 0x6?, 0x5?})
        /Users/thofos01/go/pkg/mod/github.com/lestrrat-go/jwx/[email protected]/jwt/jwt.go:233 +0x64
github.com/lestrrat-go/jwx/v2/jwt.parse(0x140000d3d78, {0x1400018a1a0, 0xc3, 0xc3})
        /Users/thofos01/go/pkg/mod/github.com/lestrrat-go/jwx/[email protected]/jwt/jwt.go:293 +0xd4
github.com/lestrrat-go/jwx/v2/jwt.parseBytes({0x1400018a1a0, 0xc3, 0xc3}, {0x140000d3ef8?, 0x1?, 0x140000824e0?})
        /Users/thofos01/go/pkg/mod/github.com/lestrrat-go/jwx/[email protected]/jwt/jwt.go:216 +0x154
github.com/lestrrat-go/jwx/v2/jwt.Parse(...)
        /Users/thofos01/go/pkg/mod/github.com/lestrrat-go/jwx/[email protected]/jwt/jwt.go:117
main.ExampleJWX()
[...]
exit status 2

Note that this same code compiled against 1.18 or earlier versions would just fail verification.

The trouble is this new behaviour makes it trivial to crash/DoS a verification service built on top of jwx.

BCP225 says:

"Some cryptographic operations [...] take inputs that may contain invalid values. This includes points not on the specified elliptic curve or other invalid points (e.g., [Valenta], Section 7.1). The JWS/JWE library itself must validate these inputs before using them, or it must use underlying cryptographic libraries that do so (or both!)."

so it'd seem right for jwx to do the input validation, at jwk.ParseKey, jwk.PublicKeyOf or jwt.Parse.

[lestrrat]

I made #841 which fixes jws.Verify, but I think jwe.Encrypt may be susceptible to the same problem...

[lestrrat]

Yeah, I patched jwe.Encrypt as well. Please let me know if this fixes your problems

[thomas-fossati]

#841 fixes my repro:

╰─± go run main.go 
failed to verify JWS: could not verify message using any of the signatures or keys

Thanks @lestrrat for the super-quick patch!

[lestrrat]

Cool. I think you know the drill: I'll merge, but I'm putting off a release until I either get enough stuff or I feel comfortable that nothing else broke or is missing.

Thanks for the headsup!