diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index ff6b4041b..cef93d84b 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -24,7 +24,7 @@ jobs: restore-keys: | ${{ runner.os }}-go- - name: Install Go stable version - uses: actions/setup-go@v4 + uses: actions/setup-go@v5 with: go-version: ${{ matrix.go }} - name: Install benchstat diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 21086b15e..c78d211d2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,7 +29,7 @@ jobs: run: curl --connect-timeout 1 https://azure.archive.ubuntu.com || (sudo sed -i 's/azure\.//' /etc/apt/sources.list && sudo apt-get update) - name: Install Go stable version if: matrix.go != 'tip' - uses: actions/setup-go@v4 + uses: actions/setup-go@v5 with: go-version: ${{ matrix.go }} - name: Install Go tip diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 7bd3f7f87..73fbb9002 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -6,7 +6,7 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - - uses: actions/setup-go@v4 + - uses: actions/setup-go@v5 with: go-version: 1.19 check-latest: true diff --git a/.github/workflows/smoke.yml b/.github/workflows/smoke.yml index 55cec567c..00edf0950 100644 --- a/.github/workflows/smoke.yml +++ b/.github/workflows/smoke.yml @@ -31,7 +31,7 @@ jobs: - name: Munge APT Repositories run: curl --connect-timeout 1 https://azure.archive.ubuntu.com || (sudo sed -i 's/azure\.//' /etc/apt/sources.list && sudo apt-get update) - name: Install Go stable version - uses: actions/setup-go@v4 + uses: actions/setup-go@v5 with: go-version: ${{ matrix.go }} - name: Install stringer diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml index 4277b638f..c69612c14 100644 --- a/.github/workflows/stale.yml +++ b/.github/workflows/stale.yml @@ -7,7 +7,7 @@ jobs: stale: runs-on: ubuntu-latest steps: - - uses: actions/stale@v8 + - uses: actions/stale@v9 with: stale-issue-message: 'This issue is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 7 days.' stale-pr-message: 'This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 14 days.' diff --git a/Changes b/Changes index b5ad31825..897ab3c66 100644 --- a/Changes +++ b/Changes @@ -1,6 +1,17 @@ Changes ======= +v1.2.28 09 Jan 2024 +[Security Fixes] + * [jws] JWS messages formated in full JSON format (i.e. not the compact format, which + consists of three base64 strings concatenated with a '.') with missing "protected" + headers could cause a panic, thereby introducing a possiblity of a DoS. + + This has been fixed so that the `jws.Parse` function succeeds in parsing a JWS message + lacking a protected header. Calling `jws.Verify` on this same JWS message will result + in a failed verification attempt. Note that this behavior will differ slightly when + parsing JWS messages in compact form, which result in an error. + v1.2.27 - 03 Dec 2023 [Security] * [jwe] A large number in p2c parameter for PBKDF2 based encryptions could cause a DoS attack, @@ -247,7 +258,7 @@ v1.2.6 24 Aug 2021 * Support `crypto.Signer` keys for RSA, ECDSA, and EdDSA family of signatures in `jws.Sign` [Miscellaneous] - * `jwx.GuessFormat()` now requires the presense of both `payload` and + * `jwx.GuessFormat()` now requires the presence of both `payload` and `signatures` keys for it to guess that a JSON object is a JWS message. * Slightly enhance `jwt.Parse()` performance. diff --git a/bench/performance/go.sum b/bench/performance/go.sum index 572121bc3..d7a8b7c8f 100644 --- a/bench/performance/go.sum +++ b/bench/performance/go.sum @@ -32,8 +32,8 @@ github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXl github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= -golang.org/x/crypto v0.16.0 h1:mMMrFzRSCF0GvB7Ne27XVtVAaXLrPmgPC7/v0tkwHaY= -golang.org/x/crypto v0.16.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4= +golang.org/x/crypto v0.17.0 h1:r8bRNjWL3GshPW3gkd+RpvzWrZAwPS49OmTGZ/uhM4k= +golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= diff --git a/examples/go.sum b/examples/go.sum index 309c3e924..1976b6d90 100644 --- a/examples/go.sum +++ b/examples/go.sum @@ -35,8 +35,8 @@ github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXl github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= -golang.org/x/crypto v0.16.0 h1:mMMrFzRSCF0GvB7Ne27XVtVAaXLrPmgPC7/v0tkwHaY= -golang.org/x/crypto v0.16.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4= +golang.org/x/crypto v0.17.0 h1:r8bRNjWL3GshPW3gkd+RpvzWrZAwPS49OmTGZ/uhM4k= +golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= diff --git a/go.mod b/go.mod index e474f42a4..cfd9f48e8 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/lestrrat-go/option v1.0.1 github.com/pkg/errors v0.9.1 github.com/stretchr/testify v1.8.4 - golang.org/x/crypto v0.16.0 + golang.org/x/crypto v0.17.0 ) retract v1.2.16 // Packaging problems. diff --git a/go.sum b/go.sum index 572121bc3..d7a8b7c8f 100644 --- a/go.sum +++ b/go.sum @@ -32,8 +32,8 @@ github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXl github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= -golang.org/x/crypto v0.16.0 h1:mMMrFzRSCF0GvB7Ne27XVtVAaXLrPmgPC7/v0tkwHaY= -golang.org/x/crypto v0.16.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4= +golang.org/x/crypto v0.17.0 h1:r8bRNjWL3GshPW3gkd+RpvzWrZAwPS49OmTGZ/uhM4k= +golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= diff --git a/jws/jws_test.go b/jws/jws_test.go index 374d499b9..c6cd45391 100644 --- a/jws/jws_test.go +++ b/jws/jws_test.go @@ -32,6 +32,7 @@ import ( "github.com/lestrrat-go/jwx/jwk" "github.com/lestrrat-go/jwx/jws" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const examplePayload = `{"iss":"joe",` + "\r\n" + ` "exp":1300819380,` + "\r\n" + ` "http://example.com/is_root":true}` @@ -1795,3 +1796,55 @@ func TestGH681(t *testing.T) { return } } + +func TestEmptyProtectedField(t *testing.T) { + // MEMO: this was the only test case from the original report + // This passes. It should produce an invalid JWS message, but + // that's not `jws.Parse`'s problem. + _, err := jws.Parse([]byte(`{"signature": ""}`)) + require.NoError(t, err, `jws.Parse should fail`) + + // Also test that non-flattened serialization passes. + _, err = jws.Parse([]byte(`{"signatures": [{}]}`)) + require.NoError(t, err, `jws.Parse should fail`) + + // MEMO: rest of the cases are present to be extra pedantic about it + + privKey, err := jwxtest.GenerateRsaJwk() + require.NoError(t, err, `jwxtest.GenerateRsaJwk should succeed`) + + // This fails. `jws.Parse` works, but the subsequent verification + // workflow fails to verify anything without the presence of a signature or + // a protected header. + _, err = jws.Verify([]byte(`{"signature": ""}`), jwa.RS256, privKey) + require.Error(t, err, `jws.Parse should fail`) + + // Create a valid signatre. + signed, err := jws.Sign([]byte("Lorem Ipsum"), jwa.RS256, privKey) + require.NoError(t, err, `jws.Sign should succeed`) + + _, payload, signature, err := jws.SplitCompact(signed) + require.NoError(t, err, `jws.SplitCompact should succeed`) + + // This fails as well. we have a valid signature and a valid + // key to verify it, but no protected headers + _, err = jws.Verify( + []byte(fmt.Sprintf(`{"signature": "%s"}`, signature)), + jwa.RS256, privKey, + ) + require.Error(t, err, `jws.Verify should fail`) + + // Test for cases when we have an incomplete compact form JWS + var buf bytes.Buffer + buf.WriteRune('.') + buf.Write(payload) + buf.WriteRune('.') + buf.Write(signature) + invalidMessage := buf.Bytes() + + // This is an error because the format is simply wrong. + // Whereas in the other JSON-based JWS's case the lack of protected field + // is not a SYNTAX error, this one is, and therefore we barf. + _, err = jws.Parse(invalidMessage) + require.Error(t, err, `jws.Parse should fail`) +} diff --git a/jws/message.go b/jws/message.go index 802b29771..13df17d72 100644 --- a/jws/message.go +++ b/jws/message.go @@ -91,11 +91,13 @@ func (s *Signature) UnmarshalJSON(data []byte) error { s.protected = prt } - decoded, err := base64.DecodeString(*sup.Signature) - if err != nil { - return errors.Wrap(err, `failed to base decode signature`) + if sup.Signature != nil { + decoded, err := base64.DecodeString(*sup.Signature) + if err != nil { + return errors.Wrap(err, `failed to base decode signature`) + } + s.signature = decoded } - s.signature = decoded return nil } @@ -282,6 +284,11 @@ func (m *Message) UnmarshalJSON(buf []byte) error { } sig.SetDecodeCtx(nil) + if sig.protected == nil { + // Instead of barfing on a nil protected header, use an empty header + sig.protected = NewHeaders() + } + if i == 0 { if !getB64Value(sig.protected) { b64 = false @@ -317,6 +324,11 @@ func (m *Message) UnmarshalJSON(buf []byte) error { sig.protected = prt } + if sig.protected == nil { + // Instead of barfing on a nil protected header, use an empty header + sig.protected = NewHeaders() + } + decoded, err := base64.DecodeString(*mup.Signature) if err != nil { return errors.Wrap(err, `failed to base64 decode flattened signature`)