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]>
lestrrat-go · Jul 3, 2023 · 2d138a3 · 2d138a3
1 parent 8149455
commit 2d138a3
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 3 deletions.
diff --git 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/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`)
+}
diff --git 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() {