Repository: lestrrat-go/jwx
Pull request #953 (2023-07-03 00:46:51)

Add test case to #952
Adds a test to #952. Did verify that the issue exists before the patch and not after the patch using this test.
supersedes and closes #952

ItalyPaleAle: Thanks for this! I wasn't sure how to best write a test for this, and this looks great
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/lestrrat-go/jwx/pull/953?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lestrrat-go) Report
> :exclamation: No coverage uploaded for pull request base (`develop/v2@8149455`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lestrrat-go#section-missing-base-commit).
> The diff coverage is `n/a`.

```diff
@@              Coverage Diff              @@
##             develop/v2     #953   +/-   ##
=============================================
  Coverage              ?   71.92%           
=============================================
  Files                 ?       92           
  Lines                 ?    13568           
  Branches              ?        0           
=============================================
  Hits                  ?     9759           
  Misses                ?     2992           
  Partials              ?      817           
```




lestrrat: @ItalyPaleAle was going to let you know after I checked the CI, but I guess you already caught it :)

Patch:
diff --git a/Changes b/Changes
index 17f73ddd..27027178 100644
--- a/Changes
+++ b/Changes
@@ -4,6 +4,13 @@ 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.12 - UNRELEASED
+[Bug fixes]
+  * [jwt] jwt.Serializer was ignoring JWE flags (#951)
+
+[Miscellaneous]
+  * [jwk] Check for seed length on OKP JWKs to avoid panics (#947)
+
 v2.0.11 - 14 Jun 2023
 [Security]
   * Potential Padding Oracle Attack Vulnerability and Timing Attack Vulnerability 
diff --git a/jwt/options.go b/jwt/options.go
index 19e30fc4..c7575882 100644
--- a/jwt/options.go
+++ b/jwt/options.go
@@ -18,7 +18,7 @@ type identTypedClaim struct{}
 type identVerifyAuto struct{}
 
 func toSignOptions(options ...Option) ([]jws.SignOption, error) {
-	var soptions []jws.SignOption
+	soptions := make([]jws.SignOption, 0, len(options))
 	for _, option := range options {
 		//nolint:forcetypeassert
 		switch option.Ident() {
@@ -36,13 +36,16 @@ func toSignOptions(options ...Option) ([]jws.SignOption, error) {
 			}
 
 			soptions = append(soptions, jws.WithKey(wk.alg, wk.key, wksoptions...))
+		case identSignOption{}:
+			sigOpt := option.Value().(jws.SignOption) // this always succeeds
+			soptions = append(soptions, sigOpt)
 		}
 	}
 	return soptions, nil
 }
 
 func toEncryptOptions(options ...Option) ([]jwe.EncryptOption, error) {
-	var soptions []jwe.EncryptOption
+	soptions := make([]jwe.EncryptOption, 0, len(options))
 	for _, option := range options {
 		//nolint:forcetypeassert
 		switch option.Ident() {
@@ -58,13 +61,16 @@ func toEncryptOptions(options ...Option) ([]jwe.EncryptOption, error) {
 			}
 
 			soptions = append(soptions, jwe.WithKey(wk.alg, wk.key, wksoptions...))
+		case identEncryptOption{}:
+			encOpt := option.Value().(jwe.EncryptOption) // this always succeeds
+			soptions = append(soptions, encOpt)
 		}
 	}
 	return soptions, nil
 }
 
 func toVerifyOptions(options ...Option) ([]jws.VerifyOption, error) {
-	var voptions []jws.VerifyOption
+	voptions := make([]jws.VerifyOption, 0, len(options))
 	for _, option := range options {
 		//nolint:forcetypeassert
 		switch option.Ident() {


Test patch:
diff --git a/jwt/jwt_test.go b/jwt/jwt_test.go
index 15bc136a..6c0f0424 100644
--- a/jwt/jwt_test.go
+++ b/jwt/jwt_test.go
@@ -1693,3 +1693,53 @@ func TestGH888(t *testing.T) {
 		require.Error(t, err, `jwt.Parse with alg=none should fail`)
 	})
 }
+
+func TestGH951(t *testing.T) {
+	signKey, err := jwxtest.GenerateRsaKey()
+	require.NoError(t, err, `jwxtest.GenerateRsaKey should succeed`)
+
+	sharedKey := []byte{
+		25, 172, 32, 130, 225, 114, 26, 181, 138, 106, 254, 192, 95, 133, 74, 82,
+	}
+
+	token, err := jwt.NewBuilder().
+		Subject(`test-951`).
+		Issuer(`jwt.Test951`).
+		Build()
+	require.NoError(t, err, `jwt.NewBuilder should succeed`)
+
+	// this whole workflow actually works even if the bug in #951 is present.
+	// so we shall compare the results with and without the encryption
+	// options to see if there is a difference in the length of the
+	// cipher text, which is the second from last component in the message
+	serialized, err := jwt.NewSerializer().
+		Sign(jwt.WithKey(jwa.RS256, signKey)).
+		Encrypt(
+			jwt.WithKey(jwa.A128KW, sharedKey),
+			jwt.WithEncryptOption(jwe.WithContentEncryption(jwa.A128GCM)),
+			jwt.WithEncryptOption(jwe.WithCompress(jwa.Deflate)),
+		).
+		Serialize(token)
+	require.NoError(t, err, `jwt.NewSerializer()....Serizlie() should succeed`)
+
+	serialized2, err := jwt.NewSerializer().
+		Sign(jwt.WithKey(jwa.RS256, signKey)).
+		Encrypt(
+			jwt.WithKey(jwa.A128KW, sharedKey),
+		).
+		Serialize(token)
+	require.NoError(t, err, `jwt.NewSerializer()....Serizlie() should succeed`)
+
+	require.NotEqual(t,
+		len(bytes.Split(serialized, []byte{'.'})[3]),
+		len(bytes.Split(serialized2, []byte{'.'})[3]),
+	)
+
+	decrypted, err := jwe.Decrypt(serialized, jwe.WithKey(jwa.A128KW, sharedKey))
+	require.NoError(t, err, `jwe.Decrypt should succeed`)
+
+	verified, err := jwt.Parse(decrypted, jwt.WithKey(jwa.RS256, signKey))
+	require.NoError(t, err, `jwt.Parse should succeed`)
+
+	require.True(t, jwt.Equal(verified, token), `tokens should be equal`)
+}