Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
* Merge pull request from GHSA-rm8v-mxj3-5rmq ### Summary Decrypting AES-CBC encrypted JWE has Potential Padding Oracle Attack Vulnerability. ### Details On [v2.0.10](https://github.com/lestrrat-go/jwx/releases/tag/v2.0.10), decrypting AES-CBC encrypted JWE may return an error "failed to generate plaintext from decrypted blocks: invalid padding": https://github.com/lestrrat-go/jwx/blob/8840ffd4afc5839f591ff0e9ba9034af52b1643e/jwe/internal/aescbc/aescbc.go#L210-L213 ```go plaintext, err := unpad(buf, c.blockCipher.BlockSize()) if err != nil { return nil, fmt.Errorf(`failed to generate plaintext from decrypted blocks: %w`, err) } ``` Reporting padding error causes [Padding Oracle Attack](https://en.wikipedia.org/wiki/Padding_oracle_attack) Vulnerability. RFC 7516 JSON Web Encryption (JWE) says that we MUST NOT do this. > 11.5. Timing Attacks > To mitigate the attacks described in RFC 3218 [RFC3218], the > recipient MUST NOT distinguish between format, padding, and length > errors of encrypted keys. It is strongly recommended, in the event > of receiving an improperly formatted key, that the recipient > substitute a randomly generated CEK and proceed to the next step, to > mitigate timing attacks. In addition, the time to remove padding depends on the length of the padding. It may leak the length of the padding by Timing Attacks. https://github.com/lestrrat-go/jwx/blob/796b2a9101cf7e7cb66455e4d97f3c158ee10904/jwe/internal/aescbc/aescbc.go#L33-L66 ```go func unpad(buf []byte, n int) ([]byte, error) { lbuf := len(buf) rem := lbuf % n // First, `buf` must be a multiple of `n` if rem != 0 { return nil, fmt.Errorf("input buffer must be multiple of block size %d", n) } // Find the last byte, which is the encoded padding // i.e. 0x1 == 1 byte worth of padding last := buf[lbuf-1] // This is the number of padding bytes that we expect expected := int(last) if expected == 0 || /* we _have_ to have padding here. therefore, 0x0 is not an option */ expected > n || /* we also must make sure that we don't go over the block size (n) */ expected > lbuf /* finally, it can't be more than the buffer itself. unlikely, but could happen */ { return nil, fmt.Errorf(`invalid padding byte at the end of buffer`) } // start i = 1 because we have already established that expected == int(last) where // last = buf[lbuf-1]. // // we also don't check against lbuf-i in range, because we have established expected <= lbuf for i := 1; i < expected; i++ { if buf[lbuf-i] != last { return nil, fmt.Errorf(`invalid padding`) } } return buf[:lbuf-expected], nil } ``` To mitigate Timing Attacks, it MUST be done in constant time. ### Impact The authentication tag is verified, so it is not an immediate attack. Co-authored-by: ICHINOSE Shogo <[email protected]> * Update Changes * Bump golang.org/x/crypto from 0.9.0 to 0.10.0 (#938) * Bump golang.org/x/crypto from 0.9.0 to 0.10.0 Bumps [golang.org/x/crypto](https://github.com/golang/crypto) from 0.9.0 to 0.10.0. - [Commits](golang/crypto@v0.9.0...v0.10.0) --- updated-dependencies: - dependency-name: golang.org/x/crypto dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * run gazelle-update-repos --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Daisuke Maki <[email protected]> * Bump github.com/lestrrat-go/jwx/v2 from 2.0.8 to 2.0.11 in /cmd/jwx (#942) Bumps [github.com/lestrrat-go/jwx/v2](https://github.com/lestrrat-go/jwx) from 2.0.8 to 2.0.11. - [Release notes](https://github.com/lestrrat-go/jwx/releases) - [Changelog](https://github.com/lestrrat-go/jwx/blob/develop/v2/Changes) - [Commits](v2.0.8...v2.0.11) --- updated-dependencies: - dependency-name: github.com/lestrrat-go/jwx/v2 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump github.com/lestrrat-go/jwx/v2 from 2.0.8 to 2.0.11 in /examples (#943) Bumps [github.com/lestrrat-go/jwx/v2](https://github.com/lestrrat-go/jwx) from 2.0.8 to 2.0.11. - [Release notes](https://github.com/lestrrat-go/jwx/releases) - [Changelog](https://github.com/lestrrat-go/jwx/blob/develop/v2/Changes) - [Commits](v2.0.8...v2.0.11) --- updated-dependencies: - dependency-name: github.com/lestrrat-go/jwx/v2 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump github.com/lestrrat-go/jwx/v2 in /bench/performance (#944) Bumps [github.com/lestrrat-go/jwx/v2](https://github.com/lestrrat-go/jwx) from 2.0.8 to 2.0.11. - [Release notes](https://github.com/lestrrat-go/jwx/releases) - [Changelog](https://github.com/lestrrat-go/jwx/blob/develop/v2/Changes) - [Commits](v2.0.8...v2.0.11) --- updated-dependencies: - dependency-name: github.com/lestrrat-go/jwx/v2 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * remove unnecessarily err checks (#948) godoc says hash.Hash.Write never returns an error: https://pkg.go.dev/hash#Hash > Write (via the embedded io.Writer interface) adds more data to the running hash. > It never returns an error. So, we don't need to check errors of Write in Hmac.ComputeAuthTag. * Adam korcz fix 1 (#949) * fix panic from empty seed Signed-off-by: AdamKorcz <[email protected]> * Add test case --------- Signed-off-by: AdamKorcz <[email protected]> Co-authored-by: AdamKorcz <[email protected]> * add size check (#950) Signed-off-by: AdamKorcz <[email protected]> * Add test case to #952 (#953) * Do not ignore custom encrypt and sign options in jwt package Fixes #951 * Add test case * Update Changes --------- Co-authored-by: ItalyPaleAle <[email protected]> * Bump golang.org/x/crypto from 0.10.0 to 0.11.0 (#956) * Bump golang.org/x/crypto from 0.10.0 to 0.11.0 Bumps [golang.org/x/crypto](https://github.com/golang/crypto) from 0.10.0 to 0.11.0. - [Commits](golang/crypto@v0.10.0...v0.11.0) --- updated-dependencies: - dependency-name: golang.org/x/crypto dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * Update bazel repos --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Daisuke Maki <[email protected]> * Bump golang.org/x/crypto from 0.11.0 to 0.12.0 (#963) * Bump golang.org/x/crypto from 0.11.0 to 0.12.0 Bumps [golang.org/x/crypto](https://github.com/golang/crypto) from 0.11.0 to 0.12.0. - [Commits](golang/crypto@v0.11.0...v0.12.0) --- updated-dependencies: - dependency-name: golang.org/x/crypto dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * Run gazelle-update-repos --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Daisuke Maki <[email protected]> * Add documentation that explains #959 (#964) * Add documentation that explains #959 * run genoptions * Update Changes for v2.0.12 --------- Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: AdamKorcz <[email protected]> Co-authored-by: ICHINOSE Shogo <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: AdamKorcz <[email protected]> Co-authored-by: AdamKorcz <[email protected]> Co-authored-by: ItalyPaleAle <[email protected]>
- Loading branch information
6 people
authored
Aug 10, 2023
1 parent
6c41e38
commit 466c277