Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protect jws.Verify() and jwe.Encrypt() from panic on go1.19+ #841

Merged
merged 2 commits into from
Nov 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions jwe/internal/keygen/keygen.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
22 changes: 22 additions & 0 deletions jwe/jwe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)`)
}
4 changes: 4 additions & 0 deletions jws/ecdsa.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
32 changes: 32 additions & 0 deletions jws/jws_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}