From e6dc025e2a977a363f88345c32d02ab1ccccabb8 Mon Sep 17 00:00:00 2001 From: Daisuke Maki Date: Fri, 18 Nov 2022 18:41:42 +0900 Subject: [PATCH 1/2] Protect jws.Verify() from panic on go1.19+ --- jws/ecdsa.go | 4 ++++ jws/jws_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/jws/ecdsa.go b/jws/ecdsa.go index aadb05941..a2d644e43 100644 --- a/jws/ecdsa.go +++ b/jws/ecdsa.go @@ -172,6 +172,10 @@ func (v *ecdsaVerifier) Verify(payload []byte, signature []byte, key interface{} } } + if !pubkey.Curve.IsOnCurve(pubkey.X, pubkey.Y) { + return fmt.Errorf(`public key used does not contain a point (X,Y) on the curve`) + } + r := pool.GetBigInt() s := pool.GetBigInt() defer pool.ReleaseBigInt(r) diff --git a/jws/jws_test.go b/jws/jws_test.go index 96997d1cc..414df7019 100644 --- a/jws/jws_test.go +++ b/jws/jws_test.go @@ -28,6 +28,7 @@ import ( "github.com/lestrrat-go/jwx/v2/jwa" "github.com/lestrrat-go/jwx/v2/jwk" "github.com/lestrrat-go/jwx/v2/jws" + "github.com/lestrrat-go/jwx/v2/jwt" "github.com/lestrrat-go/jwx/v2/x25519" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -1919,3 +1920,34 @@ func TestGH681(t *testing.T) { return } } + +func TestGH840(t *testing.T) { + // Go 1.19+ panics if elliptic curve operations are called against + // a point that's _NOT_ on the curve + untrustedJWK := []byte(`{ + "kty": "EC", + "crv": "P-256", + "x": "MKBCTNIcKUSDii11ySs3526iDZ8AiTo7Tu6KPAqx7D4", + "y": "4Etl6SRW2YiLUrN5vfvVHuhp7x8PxltmWWlbbM4IFyM", + "d": "870MB6gfuTJ4HtUnUvYMyJpr5eUZNP4Bk43bVdj3eAE" + }`) + + // Parse, serialize, slice and dice JWKs! + privkey, err := jwk.ParseKey(untrustedJWK) + require.NoError(t, err, `jwk.ParseKey should succeed`) + + pubkey, err := jwk.PublicKeyOf(privkey) + require.NoError(t, err, `jwk.PublicKeyOf should succeed`) + + tok, err := jwt.NewBuilder(). + Issuer(`github.com/lestrrat-go/jwx`). + IssuedAt(time.Now()). + Build() + require.NoError(t, err, `jwt.NewBuilder should succeed`) + + signed, err := jwt.Sign(tok, jwt.WithKey(jwa.ES256, privkey)) + require.NoError(t, err, `jwt.Sign should succeed`) + + _, err = jwt.Parse(signed, jwt.WithKey(jwa.ES256, pubkey)) + require.Error(t, err, `jwt.Parse should FAIL`) // pubkey's X/Y is not on the curve +} From 9529ad975f56f393f39e2ed6f38d9b08cdc1925b Mon Sep 17 00:00:00 2001 From: Daisuke Maki Date: Fri, 18 Nov 2022 19:03:57 +0900 Subject: [PATCH 2/2] Same problem, but in jwe --- jwe/internal/keygen/keygen.go | 3 +++ jwe/jwe_test.go | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/jwe/internal/keygen/keygen.go b/jwe/internal/keygen/keygen.go index bab6041a9..0d9c7ece9 100644 --- a/jwe/internal/keygen/keygen.go +++ b/jwe/internal/keygen/keygen.go @@ -88,6 +88,9 @@ func (g Ecdhes) Generate() (ByteSource, error) { pubinfo := make([]byte, 4) binary.BigEndian.PutUint32(pubinfo, uint32(g.keysize)*8) + if !priv.PublicKey.Curve.IsOnCurve(g.pubkey.X, g.pubkey.Y) { + return nil, fmt.Errorf(`public key used does not contain a point (X,Y) on the curve`) + } z, _ := priv.PublicKey.Curve.ScalarMult(g.pubkey.X, g.pubkey.Y, priv.D.Bytes()) zBytes := ecutil.AllocECPointBuffer(z, priv.PublicKey.Curve) defer ecutil.ReleaseECPointBuffer(zBytes) diff --git a/jwe/jwe_test.go b/jwe/jwe_test.go index a729b0414..d3732ac82 100644 --- a/jwe/jwe_test.go +++ b/jwe/jwe_test.go @@ -819,3 +819,25 @@ func TestGH803(t *testing.T) { require.Equal(t, apu, msg.ProtectedHeaders().AgreementPartyUInfo()) require.Equal(t, apv, msg.ProtectedHeaders().AgreementPartyVInfo()) } + +func TestGH840(t *testing.T) { + // Go 1.19+ panics if elliptic curve operations are called against + // a point that's _NOT_ on the curve + untrustedJWK := []byte(`{ + "kty": "EC", + "crv": "P-256", + "x": "MKBCTNIcKUSDii11ySs3526iDZ8AiTo7Tu6KPAqx7D4", + "y": "4Etl6SRW2YiLUrN5vfvVHuhp7x8PxltmWWlbbM4IFyM", + "d": "870MB6gfuTJ4HtUnUvYMyJpr5eUZNP4Bk43bVdj3eAE" + }`) + + privkey, err := jwk.ParseKey(untrustedJWK) + require.NoError(t, err, `jwk.ParseKey should succeed`) + + pubkey, err := privkey.PublicKey() + require.NoError(t, err, `privkey.PublicKey should succeed`) + + const payload = `Lorem ipsum` + _, err = jwe.Encrypt([]byte(payload), jwe.WithKey(jwa.ECDH_ES_A128KW, pubkey)) + require.Error(t, err, `jwe.Encrypt should fail (instead of panic)`) +}