Merge pull request from GHSA-7f9x-gw85-8grf

lestrrat-go · Dec 3, 2023 · 64f2a22 · 64f2a22
1 parent 55000b3
commit 64f2a22
Show file tree

Hide file tree

Showing 6 changed files with 113 additions and 0 deletions.
diff --git a/Changes b/Changes
@@ -4,6 +4,15 @@ Changes
 v2 has many incompatibilities with v1. To see the full list of differences between
 v1 and v2, please read the Changes-v2.md file (https://github.com/lestrrat-go/jwx/blob/develop/v2/Changes-v2.md)
 
+v2.0.18 UNRELEASED
+[Security Fixes]
+ * [jwe] A large number in p2c parameter for PBKDF2 based encryptions could cause a DoS attack,
+ similar to https://nvd.nist.gov/vuln/detail/CVE-2022-36083. All users who use JWE via this
+ package should upgrade. While the JOSE spec allows for encryption using JWE on JWTs, users of
+ the `jwt` package are not immediately susceptible unless they explicitly try to decrypt
+ JWTs -- by default the `jwt` package verifies signatures, but does not decrypt messages.
+ [GHSA-7f9x-gw85-8grf]
+
 v2.0.17 20 Nov 2023
 [Bug Fixes]
  * [jws] Previously, `jws.UnregisterSigner` did not remove the previous signer instance when

diff --git a/jwe/jwe.go b/jwe/jwe.go
@@ -10,6 +10,7 @@ import (
  "crypto/rsa"
  "fmt"
  "io"
+ "sync"
 
  "github.com/lestrrat-go/blackmagic"
  "github.com/lestrrat-go/jwx/v2/internal/base64"
@@ -24,6 +25,20 @@ import (
  "github.com/lestrrat-go/jwx/v2/x25519"
 )
 
+var muSettings sync.RWMutex
+var maxPBES2Count = 10000
+
+func Settings(options ...GlobalOption) {
+ muSettings.Lock()
+ defer muSettings.Unlock()
+ for _, option := range options {
+ switch option.Ident() {
+ case identMaxPBES2Count{}:
+ maxPBES2Count = option.Value().(int)
+ }
+ }
+}
+
 const (
  fmtInvalid = iota
  fmtCompact
@@ -702,6 +717,12 @@ func (dctx *decryptCtx) decryptContent(ctx context.Context, alg jwa.KeyEncryptio
  if !ok {
  return nil, fmt.Errorf("unexpected type for 'p2c': %T", count)
  }
+ muSettings.RLock()
+ maxCount := maxPBES2Count
+ muSettings.RUnlock()
+ if countFlt > float64(maxCount) {
+ return nil, fmt.Errorf("invalid 'p2c' value")
+ }
  salt, err := base64.DecodeString(saltB64Str)
  if err != nil {
  return nil, fmt.Errorf(`failed to b64-decode 'salt': %w`, err)

diff --git a/jwe/jwe_test.go b/jwe/jwe_test.go
@@ -911,3 +911,51 @@ func TestGH1001(t *testing.T) {
  require.Equal(t, "Lorem Ipsum", string(decrypted), `decrypted message should match`)
  require.NotNil(t, cek, `cek should not be nil`)
 }
+
+func TestGHSA_7f9x_gw85_8grf(t *testing.T) {
+ token := []byte("eyJhbGciOiJQQkVTMi1IUzI1NitBMTI4S1ciLCJlbmMiOiJBMjU2R0NNIiwicDJjIjoyMDAwMDAwMDAwLCJwMnMiOiJNNzczSnlmV2xlX2FsSXNrc0NOTU9BIn0=.S8B1kXdIR7BM6i_TaGsgqEOxU-1Sgdakp4mHq7UVhn-_REzOiGz2gg.gU_LfzhBXtQdwYjh.9QUIS-RWkLc.m9TudmzUoCzDhHsGGfzmCA")
+ key, err := jwk.FromRaw([]byte(`abcdefg`))
+ require.NoError(t, err, `jwk.FromRaw should succeed`)
+
+ {
+ ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+ defer cancel()
+
+ done := make(chan struct{})
+ go func(t *testing.T, done chan struct{}) {
+ _, err := jwe.Decrypt(token, jwe.WithKey(jwa.PBES2_HS256_A128KW, key))
+ require.Error(t, err, `jwe.Decrypt should fail`)
+ close(done)
+ }(t, done)
+
+ select {
+ case <-done:
+ case <-ctx.Done():
+ require.Fail(t, "jwe.Decrypt should not block")
+ }
+ }
+
+ // NOTE: HAS GLOBAL EFFECT
+ // Should allow for timeout to occur
+ jwe.Settings(jwe.WithMaxPBES2Count(100000000000000000))
+
+ // put it back to normal after the test
+ defer jwe.Settings(jwe.WithMaxPBES2Count(10000))
+ {
+ ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+ defer cancel()
+
+ done := make(chan struct{})
+ go func(t *testing.T, done chan struct{}) {
+ _, _ = jwe.Decrypt(token, jwe.WithKey(jwa.PBES2_HS256_A128KW, key))
+ close(done)
+ }(t, done)
+
+ select {
+ case <-done:
+ require.Fail(t, "jwe.Decrypt should block")
+ case <-ctx.Done():
+ // timeout occurred as it should
+ }
+ }
+}
diff --git a/jwe/options.yaml b/jwe/options.yaml
@@ -1,6 +1,9 @@
 package_name: jwe
 output: jwe/options_gen.go
 interfaces:
+ - name: GlobalOption
+ comment: |
+ GlobalOption describes options that changes global settings for this package
  - name: CompactOption
  comment: |
  CompactOption describes options that can be passed to `jwe.Compact`
@@ -129,3 +132,10 @@ options:
  
  This option is currently considered EXPERIMENTAL, and is subject to
  future changes across minor/micro versions.
+ - ident: MaxPBES2Count
+ interface: GlobalOption
+ argument_type: int
+ comment: |
+ WithMaxPBES2Count specifies the maximum number of PBES2 iterations
+ to use when decrypting a message. If not specified, the default
+ value of 10,000 is used.
diff --git a/jwe/options_gen.go b/jwe/options_gen.go
diff --git a/jwe/options_gen_test.go b/jwe/options_gen_test.go