merge develop/v2 (#748)

* Allow escape hatch for those who (incorrectly) use RFC3339 in Numeric Date fields (#735) * Add the ability to parse RFC3339 dates for NumericDate types * appease linter * Tweaks (#737) * Update go version * Tweak documentation * Update Changes * Tweak doc (#738) * show jwt.Sign using raw and jwk.Key (#739) * autodoc updates (#740) Co-authored-by: lestrrat <[email protected]> * Add example for using JWT fields (#741) * fix typo (#742) * [jwe/v2] Fix possible excessive unpadding for AESCBC (#745) * Fix possible excessive unpadding for AESCBC * Update Changes * Update Changes * Update Changes * Update Changes Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: lestrrat <[email protected]> Co-authored-by: Satoru Kitaguchi <[email protected]>
lestrrat-go · May 23, 2022 · dc603b6 · dc603b6
1 parent 0d365e4
commit dc603b6
Show file tree

Hide file tree

Showing 14 changed files with 258 additions and 27 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -48,7 +48,7 @@ jobs:
       - name: Test with coverage
         run: make cover-${{ matrix.go_tags }}
       - name: Upload code coverage to codecov
-        if: matrix.go == '1.17.x'
+        if: matrix.go == '1.18'
         uses: codecov/codecov-action@v1
         with:
           file: ./coverage.out

diff --git a/Changes b/Changes
@@ -4,6 +4,20 @@ 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.2 - 23 May 2022
+[Bug Fixes][Security]
+  * [jwe] An old bug from at least 7 years ago existed in handling AES-CBC unpadding,
+    where the unpad operation might remove more bytes than necessary (#744)
+    This affects all jwx code that is available before v2.0.2 and v1.2.25.
+
+[New Features]
+  * [jwt] RFC3339 timestamps are also accepted for Numeric Date types in JWT tokens.
+    This allows users to parse servers that errnously use RFC3339 timestamps in
+    some pre-defined fields. You can change this behavior by setting 
+    `jwt.WithNumericDateParsePedantic` to `false`
+  * [jwt] `jwt.WithNumericDateParsePedantic` has been added. This is a global
+    option that is set using `jwt.Settings`
+
 v2.0.1 - 06 May 2022
   * [jwk] `jwk.Set` had erronously been documented as not returning an error
     when the same key already exists in the set. This is a behavior change

diff --git a/docs/01-jwt.md b/docs/01-jwt.md
@@ -941,21 +941,34 @@ func ExampleJWT_SerializeJWS() {
     return
   }
 
-  key, err := jwk.FromRaw([]byte(`abracadavra`))
+  rawKey := []byte(`abracadavra`)
+  jwkKey, err := jwk.FromRaw(rawKey)
   if err != nil {
     fmt.Printf("failed to create symmetric key: %s\n", err)
     return
   }
 
-  serialized, err := jwt.Sign(tok, jwt.WithKey(jwa.HS256, key))
-  if err != nil {
-    fmt.Printf("failed to sign token: %s\n", err)
-    return
+  // This example shows you two ways to passing keys to
+  // jwt.Sign()
+  //
+  // * The first key is the "raw" key.
+  // * The second one is a jwk.Key that represents the raw key.
+  //
+  // If this were using RSA/ECDSA keys, you would be using
+  // *rsa.PrivateKey/*ecdsa.PrivateKey as the raw key.
+  for _, key := range []interface{}{rawKey, jwkKey} {
+    serialized, err := jwt.Sign(tok, jwt.WithKey(jwa.HS256, key))
+    if err != nil {
+      fmt.Printf("failed to sign token: %s\n", err)
+      return
+    }
+
+    fmt.Printf("%s\n", serialized)
   }
 
-  fmt.Printf("%s\n", serialized)
   // OUTPUT:
   // eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpYXQiOjIzMzQzMTIwMCwiaXNzIjoiZ2l0aHViLmNvbS9sZXN0cnJhdC1nby9qd3gifQ.rTlpyVnHFWosNud7seqlsvhM8UoXUIAKfdWHySFO5Ro
+  // eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpYXQiOjIzMzQzMTIwMCwiaXNzIjoiZ2l0aHViLmNvbS9sZXN0cnJhdC1nby9qd3gifQ.rTlpyVnHFWosNud7seqlsvhM8UoXUIAKfdWHySFO5Ro
 }
 ```
 source: [examples/jwt_serialize_jws_example_test.go](https://github.com/lestrrat-go/jwx/blob/v2/examples/jwt_serialize_jws_example_test.go)

diff --git a/docs/04-jwk.md b/docs/04-jwk.md
@@ -31,7 +31,7 @@ In this document we describe how to work with JWK using `github.com/lestrrat-go/
 
 ## JWK / Key
 
-Used to describe a JWK key, possibly of typeRSA, ECDSA, OKP, or Symmetric.
+Used to describe a JWK key, possibly of type RSA, ECDSA, OKP, or Symmetric.
 
 ## JWK Set / Set
 
@@ -47,6 +47,18 @@ Used to describe the underlying raw key that a JWK represents. For example, an R
 represent rsa.PrivateKey/rsa.PublicKey, ECDSA JWK can represent ecdsa.PrivateKey/ecdsa.PublicKey,
 and so forth
 
+---
+
+The table below shows the matrix of key types and their respective `jwk.Key` and "raw" types.
+If given anything else, `jwk.FromRaw` will return an error.
+
+|           | `jwk.Key` Type                               | Raw Key Type                              |
+|-----------|----------------------------------------------|-------------------------------------------|
+| RSA       | `jwk.RSAPublicKey` / `jwk.RSAPrivateKey`     | `*rsa.PublicKey` / `*rsa.PublicKey`       |
+| ECDSA     | `jwk.ECDSAPublicKey` / `jwk.ECDSAPrivateKey` | `*ecdsa.PublicKey` / `*ecdsa.PublicKey`   |
+| OKP       | `jwk.OKPPublicKey` / `jwk.OKPPrivateKey`     | `ed25519.PublicKey` / `ed25519.PublicKey` |
+| Symmetric | `jwk.SymmetricKey`                           | []byte                                    |
+
 # Parsing
 
 ## Parse a set
@@ -473,7 +485,7 @@ func ExampleJWK_FromRaw() {
   {
     raw, err := rsa.GenerateKey(rand.Reader, 2048)
     if err != nil {
-      fmt.Printf("failed to generate new RSA privatre key: %s\n", err)
+      fmt.Printf("failed to generate new RSA private key: %s\n", err)
       return
     }
 
@@ -494,7 +506,7 @@ func ExampleJWK_FromRaw() {
   {
     raw, err := ecdsa.GenerateKey(elliptic.P384(), rand.Reader)
     if err != nil {
-      fmt.Printf("failed to generate new ECDSA privatre key: %s\n", err)
+      fmt.Printf("failed to generate new ECDSA private key: %s\n", err)
       return
     }
 

diff --git a/examples/jwk_from_raw_example_test.go b/examples/jwk_from_raw_example_test.go
@@ -50,7 +50,7 @@ func ExampleJWK_FromRaw() {
 	{
 		raw, err := rsa.GenerateKey(rand.Reader, 2048)
 		if err != nil {
-			fmt.Printf("failed to generate new RSA privatre key: %s\n", err)
+			fmt.Printf("failed to generate new RSA private key: %s\n", err)
 			return
 		}
 
@@ -71,7 +71,7 @@ func ExampleJWK_FromRaw() {
 	{
 		raw, err := ecdsa.GenerateKey(elliptic.P384(), rand.Reader)
 		if err != nil {
-			fmt.Printf("failed to generate new ECDSA privatre key: %s\n", err)
+			fmt.Printf("failed to generate new ECDSA private key: %s\n", err)
 			return
 		}
 

diff --git a/examples/jwt_get_claims_example_test.go b/examples/jwt_get_claims_example_test.go
@@ -0,0 +1,67 @@
+package examples_test
+
+import (
+	"encoding/json"
+	"fmt"
+	"time"
+
+	"github.com/lestrrat-go/jwx/v2/jwt"
+)
+
+func ExampleJWT_GetClaims() {
+	tok, err := jwt.NewBuilder().
+		IssuedAt(time.Now()).
+		Issuer(`github.com/lestrrat-go/jwx`).
+		Subject(`example`).
+		Claim(`claim1`, `value1`).
+		Claim(`claim2`, `2022-05-16T07:35:56+00:00`).
+		Build()
+	if err != nil {
+		fmt.Printf("failed to build token: %s\n", err)
+		return
+	}
+
+	// Pre-defined fields have typed accessors.
+	var _ time.Time = tok.IssuedAt()
+	var _ string = tok.Issuer()
+	var _ string = tok.Subject()
+
+	var v interface{}
+	var ok bool
+
+	// But you can also get them via the generic `.Get()` method.
+	// However, v is of type interface{}, so you might need to
+	// use a type switch to properly use its value.
+	//
+	// For the key name you could also use jwt.IssuedAtKey constant
+	v, ok = tok.Get(`iat`)
+
+	// Private claims
+	v, ok = tok.Get(`claim1`)
+	v, ok = tok.Get(`claim2`)
+
+	// However, it is possible to globally specify that a private
+	// claim should be parsed into a custom type.
+	// In the sample below `claim2` is to be an instance of time.Time
+	jwt.RegisterCustomField(`claim2`, time.Time{})
+
+	tok = jwt.New()
+	if err := json.Unmarshal([]byte(`{"claim2":"2022-05-16T07:35:56+00:00"}`), tok); err != nil {
+		fmt.Printf(`failed to parse token: %s`, err)
+		return
+	}
+	v, ok = tok.Get(`claim2`)
+	if !ok {
+		fmt.Printf(`failed to get private claim "claim2"`)
+		return
+	}
+	if _, ok := v.(time.Time); !ok {
+		fmt.Printf(`claim2 expected to be time.Time, but got %T`, v)
+		return
+	}
+
+	_ = v
+	_ = ok
+
+	// OUTPUT:
+}
diff --git a/examples/jwt_serialize_jws_example_test.go b/examples/jwt_serialize_jws_example_test.go
@@ -19,19 +19,32 @@ func ExampleJWT_SerializeJWS() {
 		return
 	}
 
-	key, err := jwk.FromRaw([]byte(`abracadavra`))
+	rawKey := []byte(`abracadavra`)
+	jwkKey, err := jwk.FromRaw(rawKey)
 	if err != nil {
 		fmt.Printf("failed to create symmetric key: %s\n", err)
 		return
 	}
 
-	serialized, err := jwt.Sign(tok, jwt.WithKey(jwa.HS256, key))
-	if err != nil {
-		fmt.Printf("failed to sign token: %s\n", err)
-		return
+	// This example shows you two ways to passing keys to
+	// jwt.Sign()
+	//
+	// * The first key is the "raw" key.
+	// * The second one is a jwk.Key that represents the raw key.
+	//
+	// If this were using RSA/ECDSA keys, you would be using
+	// *rsa.PrivateKey/*ecdsa.PrivateKey as the raw key.
+	for _, key := range []interface{}{rawKey, jwkKey} {
+		serialized, err := jwt.Sign(tok, jwt.WithKey(jwa.HS256, key))
+		if err != nil {
+			fmt.Printf("failed to sign token: %s\n", err)
+			return
+		}
+
+		fmt.Printf("%s\n", serialized)
 	}
 
-	fmt.Printf("%s\n", serialized)
 	// OUTPUT:
 	// eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpYXQiOjIzMzQzMTIwMCwiaXNzIjoiZ2l0aHViLmNvbS9sZXN0cnJhdC1nby9qd3gifQ.rTlpyVnHFWosNud7seqlsvhM8UoXUIAKfdWHySFO5Ro
+	// eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpYXQiOjIzMzQzMTIwMCwiaXNzIjoiZ2l0aHViLmNvbS9sZXN0cnJhdC1nby9qd3gifQ.rTlpyVnHFWosNud7seqlsvhM8UoXUIAKfdWHySFO5Ro
 }
diff --git a/jwe/internal/aescbc/aescbc.go b/jwe/internal/aescbc/aescbc.go
@@ -33,23 +33,36 @@ func pad(buf []byte, n int) []byte {
 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)
 	}
 
-	count := 0
+	// Find the last byte, which is the encoded padding
+	// i.e. 0x1 == 1 byte worth of padding
 	last := buf[lbuf-1]
-	for i := lbuf - 1; i >= 0; i-- {
-		if buf[i] != last {
-			break
-		}
-		count++
+
+	// 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`)
 	}
-	if count != int(last) {
-		return nil, fmt.Errorf(`invalid padding`)
+
+	// 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-int(last)], nil
+	return buf[:lbuf-expected], nil
 }
 
 type Hmac struct {

diff --git a/jwt/internal/types/date.go b/jwt/internal/types/date.go
@@ -14,6 +14,7 @@ const (
 	MaxPrecision     uint32 = 9 // nanosecond level
 )
 
+var Pedantic uint32
 var ParsePrecision = DefaultPrecision
 var FormatPrecision = DefaultPrecision
 
@@ -52,6 +53,27 @@ func intToTime(v interface{}, t *time.Time) bool {
 
 func parseNumericString(x string) (time.Time, error) {
 	var t time.Time // empty time for empty return value
+
+	// Only check for the escape hatch if it's the pedantic
+	// flag is off
+	if Pedantic != 1 {
+		// This is an escape hatch for non-conformant providers
+		// that gives us RFC3339 instead of epoch time
+		for _, r := range x {
+			// 0x30 = '0', 0x39 = '9', 0x2E = '.'
+			if (r >= 0x30 && r <= 0x39) || r == 0x2E {
+				continue
+			}
+
+			// if it got here, then it probably isn't epoch time
+			tv, err := time.Parse(time.RFC3339, x)
+			if err != nil {
+				return t, fmt.Errorf(`value is not number of seconds since the epoch, and attempt to parse it as RFC3339 timestamp failed: %w`, err)
+			}
+			return tv, nil
+		}
+	}
+
 	var fractional string
 	whole := x
 	if i := strings.IndexRune(x, '.'); i > 0 {

diff --git a/jwt/jwt.go b/jwt/jwt.go
@@ -18,6 +18,7 @@ import (
 // Settings controls global settings that are specific to JWTs.
 func Settings(options ...GlobalOption) {
 	var flattenAudienceBool bool
+	var parsePedantic bool
 	var parsePrecision = types.MaxPrecision + 1  // illegal value, so we can detect nothing was set
 	var formatPrecision = types.MaxPrecision + 1 // illegal value, so we can detect nothing was set
 
@@ -26,6 +27,8 @@ func Settings(options ...GlobalOption) {
 		switch option.Ident() {
 		case identFlattenAudience{}:
 			flattenAudienceBool = option.Value().(bool)
+		case identNumericDateParsePedantic{}:
+			parsePedantic = option.Value().(bool)
 		case identNumericDateParsePrecision{}:
 			v := option.Value().(int)
 			// only accept this value if it's in our desired range
@@ -55,6 +58,17 @@ func Settings(options ...GlobalOption) {
 		}
 	}
 
+	{
+		v := atomic.LoadUint32(&types.Pedantic)
+		if (v == 1) != parsePedantic {
+			var newVal uint32
+			if parsePedantic {
+				newVal = 1
+			}
+			atomic.CompareAndSwapUint32(&types.Pedantic, v, newVal)
+		}
+	}
+
 	{
 		v := atomic.LoadUint32(&json.FlattenAudience)
 		if (v == 1) != flattenAudienceBool {

diff --git a/jwt/openid/openid_test.go b/jwt/openid/openid_test.go
@@ -15,6 +15,7 @@ import (
 	"github.com/lestrrat-go/jwx/v2/jwt/internal/types"
 	"github.com/lestrrat-go/jwx/v2/jwt/openid"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 const aLongLongTimeAgo = 233431200
@@ -676,3 +677,32 @@ func TestKeys(t *testing.T) {
 	at.Equal(`website`, openid.WebsiteKey)
 	at.Equal(`zoneinfo`, openid.ZoneinfoKey)
 }
+
+func TestGH734(t *testing.T) {
+	const src = `{
+    "nickname": "miniscruff",
+    "updated_at": "2022-05-06T04:57:24.367Z",
+    "email_verified": true
+  }`
+
+	expected, _ := time.Parse(time.RFC3339, "2022-05-06T04:57:24.367Z")
+	for _, pedantic := range []bool{true, false} {
+		t.Run(fmt.Sprintf("pedantic=%t", pedantic), func(t *testing.T) {
+			jwt.Settings(jwt.WithNumericDateParsePedantic(pedantic))
+			tok := openid.New()
+			_, err := jwt.Parse(
+				[]byte(src),
+				jwt.WithToken(tok),
+				jwt.WithVerify(false),
+				jwt.WithValidate(false),
+			)
+			if pedantic {
+				require.Error(t, err, `jwt.Parse should fail for pedantic parser`)
+			} else {
+				require.NoError(t, err, `jwt.Parse should succeed`)
+				require.Equal(t, expected, tok.UpdatedAt(), `updated_at should match`)
+			}
+		})
+	}
+	jwt.Settings(jwt.WithNumericDateParsePedantic(false))
+}