Fix validation when the JWE does not contains a JWS (#505)

uruk-project · Oct 21, 2020 · ffd710a · ffd710a
1 parent 2287c94
commit ffd710a
Show file tree

Hide file tree

Showing 5 changed files with 94 additions and 6 deletions.
diff --git a/src/JsonWebToken/JwtReader.cs b/src/JsonWebToken/JwtReader.cs
@@ -346,7 +346,7 @@ private TokenValidationResult TryReadJwe(
                     }
                     else
                     {
-                        if (decryptionResult.Status == TokenValidationStatus.MalformedToken)
+                        if (decryptionResult.Status == TokenValidationStatus.MalformedToken && !policy.HasValidation)
                         {
                             // The decrypted payload is not a nested JWT
                             jwe = new Jwt(header, compressed ? decompressedBytes.ToArray() : decryptedBytes.ToArray(), decryptionKey);

diff --git a/src/JsonWebToken/SignatureValidationPolicy.cs b/src/JsonWebToken/SignatureValidationPolicy.cs
@@ -22,6 +22,11 @@ public abstract class SignatureValidationPolicy
         /// Allows to ignore the signature, whatever ther is an algorithm defined or not.
         /// </summary>
         public static readonly SignatureValidationPolicy IgnoreSignature = new IgnoreSignatureValidationContext();
+
+        /// <summary>
+        /// Gets whether the signature validation is enabled.
+        /// </summary>
+        public abstract bool IsEnabled { get; }
 
         /// <summary>
         /// Try to validate the token signature.
@@ -58,6 +63,9 @@ public DefaultSignatureValidationPolicy(IKeyProvider keyProvider, SignatureAlgor
                 _algorithm = algorithm;
             }
 
+            /// <inheritdoc />
+            public override bool IsEnabled => true;
+
             public override SignatureValidationResult TryValidateSignature(JwtHeader header, ReadOnlySpan<byte> contentBytes, ReadOnlySpan<byte> signatureSegment)
             {
                 if (contentBytes.IsEmpty && signatureSegment.IsEmpty)
@@ -122,6 +130,9 @@ public override SignatureValidationResult TryValidateSignature(JwtHeader header,
 
         private sealed class NoSignatureValidationContext : SignatureValidationPolicy
         {
+            /// <inheritdoc />
+            public override bool IsEnabled => true;
+
             public override SignatureValidationResult TryValidateSignature(JwtHeader header, ReadOnlySpan<byte> contentBytes, ReadOnlySpan<byte> signatureSegment)
             {
                 return (contentBytes.Length == 0 && signatureSegment.Length == 0) || (signatureSegment.IsEmpty && header.SignatureAlgorithm == SignatureAlgorithm.None)
@@ -132,6 +143,9 @@ public override SignatureValidationResult TryValidateSignature(JwtHeader header,
 
         private sealed class IgnoreSignatureValidationContext : SignatureValidationPolicy
         {
+            /// <inheritdoc />
+            public override bool IsEnabled => false;
+
             public override SignatureValidationResult TryValidateSignature(JwtHeader header, ReadOnlySpan<byte> contentBytes, ReadOnlySpan<byte> signatureSegment)
             {
                 return SignatureValidationResult.Success();

diff --git a/src/JsonWebToken/TokenValidationPolicy.cs b/src/JsonWebToken/TokenValidationPolicy.cs
@@ -80,7 +80,7 @@ internal TokenValidationPolicy(
         /// <summary>
         /// Gets whether the <see cref="TokenValidationPolicy"/> has validation.
         /// </summary>
-        public bool HasValidation => _validators.Length != 0;
+        public bool HasValidation => _control != 0 || _validators.Length != 0 || SignatureValidationPolicy.IsEnabled;
 
         /// <summary>
         /// Gets whether the issuer 'iss' is required.

diff --git a/src/JsonWebToken/TokenValidationPolicyBuilder.cs b/src/JsonWebToken/TokenValidationPolicyBuilder.cs
@@ -171,7 +171,7 @@ public TokenValidationPolicyBuilder RequireSignature(Jwk key)
                 throw new InvalidOperationException($"The key does not define an 'alg' parameter. Use the method {nameof(RequireSignature)} with a {nameof(Jwk)} and a {nameof(SignatureAlgorithm)}.");
             }
 
-            return RequireSignature(key, null);
+            return RequireSignature(key, (SignatureAlgorithm?)null);
         }
 
         /// <summary>
@@ -180,7 +180,29 @@ public TokenValidationPolicyBuilder RequireSignature(Jwk key)
         /// <param name="key"></param>
         /// <param name="algorithm"></param>
         /// <returns></returns>
-        public TokenValidationPolicyBuilder RequireSignature(Jwk key, SignatureAlgorithm? algorithm) => RequireSignature(new Jwks(key), algorithm);
+        public TokenValidationPolicyBuilder RequireSignature(Jwk key, SignatureAlgorithm? algorithm) 
+            => RequireSignature(new Jwks(key), algorithm);
+
+        /// <summary>
+        /// Requires a valid signature.
+        /// </summary>
+        /// <param name="key"></param>
+        /// <param name="algorithm"></param>
+        /// <returns></returns>
+        public TokenValidationPolicyBuilder RequireSignature(Jwk key, string? algorithm)
+        {
+            if (algorithm is null)
+            {
+                throw new ArgumentNullException(nameof(algorithm));
+            }
+
+            if (!SignatureAlgorithm.TryParse(Utf8.GetBytes(algorithm), out var alg))
+            {
+                throw new NotSupportedException($"The algorithm '{alg}' is not supported.");
+            }
+
+            return RequireSignature(new Jwks(key), alg);
+        }
 
         /// <summary>
         /// Requires a valid signature.
@@ -210,7 +232,8 @@ public TokenValidationPolicyBuilder RequireSignature(Jwk key)
         /// <param name="keySet"></param>
         /// <param name="algorithm"></param>
         /// <returns></returns>
-        public TokenValidationPolicyBuilder RequireSignature(Jwks keySet, SignatureAlgorithm? algorithm) => RequireSignature(new StaticKeyProvider(keySet), algorithm);
+        public TokenValidationPolicyBuilder RequireSignature(Jwks keySet, SignatureAlgorithm? algorithm)
+            => RequireSignature(new StaticKeyProvider(keySet), algorithm);
 
         /// <summary>
         /// Requires a valid signature.

diff --git a/test/JsonWebToken.Tests/JsonWebTokenReaderTests.cs b/test/JsonWebToken.Tests/JsonWebTokenReaderTests.cs
@@ -191,6 +191,49 @@ public void ReadJwt_CriticalHeader(string jwt, TokenValidationStatus expected)
             Assert.Equal(expected, result.Status);
         }
 
+        [Theory]
+        [InlineData("eyJlbmMiOiJBMTI4Q0JDLUhTMjU2IiwiYWxnIjoiQTEyOEtXIn0.4VETXwjtEQIHzctz2FTAef8iHvk8ShfMJrRvDNVISdUh9Zju4tl75w.o0IVPs65CR8B0b6fxH3mow.p8DIesdqyemto-EKiHSA19jiobfS6sR4kfe4PGEyruI.VtIn9WFytiZNjP7wXBeNNg")]
+        public void Issue504_Valid(string jwt)
+        {
+            var reader = new JwtReader(new SymmetricJwk("R9MyWaEoyiMYViVWo8Fk4T"));
+            var policy = new TokenValidationPolicyBuilder()
+                .IgnoreSignature()
+                .Build();
+
+            var result = reader.TryReadToken(jwt, policy);
+            Assert.Equal(TokenValidationStatus.Success, result.Status);
+        }
+
+        [Theory]
+        [InlineData("eyJlbmMiOiJBMTI4Q0JDLUhTMjU2IiwiYWxnIjoiQTEyOEtXIn0.4VETXwjtEQIHzctz2FTAef8iHvk8ShfMJrRvDNVISdUh9Zju4tl75w.o0IVPs65CR8B0b6fxH3mow.p8DIesdqyemto-EKiHSA19jiobfS6sR4kfe4PGEyruI.VtIn9WFytiZNjP7wXBeNNg", true, false, false)]
+        [InlineData("eyJlbmMiOiJBMTI4Q0JDLUhTMjU2IiwiYWxnIjoiQTEyOEtXIn0.4VETXwjtEQIHzctz2FTAef8iHvk8ShfMJrRvDNVISdUh9Zju4tl75w.o0IVPs65CR8B0b6fxH3mow.p8DIesdqyemto-EKiHSA19jiobfS6sR4kfe4PGEyruI.VtIn9WFytiZNjP7wXBeNNg", false, true, false)]
+        [InlineData("eyJlbmMiOiJBMTI4Q0JDLUhTMjU2IiwiYWxnIjoiQTEyOEtXIn0.4VETXwjtEQIHzctz2FTAef8iHvk8ShfMJrRvDNVISdUh9Zju4tl75w.o0IVPs65CR8B0b6fxH3mow.p8DIesdqyemto-EKiHSA19jiobfS6sR4kfe4PGEyruI.VtIn9WFytiZNjP7wXBeNNg", false, false, true)]
+        public void Issue504_Invalid(string jwt, bool requireAudience, bool requireSignature, bool requireOther)
+        {
+            var reader = new JwtReader(new SymmetricJwk("R9MyWaEoyiMYViVWo8Fk4T"));
+            var builder = new TokenValidationPolicyBuilder();
+            if (requireAudience)
+            {
+                builder.RequireAudience("test");
+            }
+            if (requireSignature)
+            {
+                builder.RequireSignature(SymmetricJwk.FromBase64Url("R9MyWaEoyiMYViVWo8Fk4TUGWiSoaW6U1nOqXri8_XU"), "HS256");
+            }
+            else
+            {
+                builder.IgnoreSignature();
+            }
+            if (requireOther)
+            {
+                builder.AddValidator(new FakeValidator());
+            }
+
+            var policy = builder.Build();
+            var result = reader.TryReadToken(jwt, policy);
+            Assert.Equal(TokenValidationStatus.MalformedToken, result.Status);
+        }
+
         [Theory]
         [InlineData("eyJhbGciOiJub25lIn0.eyJleHAiOjk5MDAwMDAwMDAsIm5iZiI6MTUwMDAwMDAwMH0.")]
         [InlineData("eyJhbGciOiJub25lIn0.eyJleHAiOjk5MDAwMDAwMDB9.")]
@@ -238,7 +281,7 @@ public void Issue489_NoValidation_Valid(string jwt)
                 .Build();
 
             var result = reader.TryReadToken(jwt, policy);
-            Assert.Equal(TokenValidationStatus.Success , result.Status);
+            Assert.Equal(TokenValidationStatus.Success, result.Status);
         }
 
         private HttpResponseMessage BackchannelRequestToken(HttpRequestMessage req)
@@ -325,4 +368,12 @@ public bool TryHandle(JwtHeader heade, string headerName)
             return _value;
         }
     }
+
+    internal class FakeValidator : IValidator
+    {
+        public TokenValidationResult TryValidate(Jwt jwt)
+        {
+            return TokenValidationResult.MalformedToken();
+        }
+    }
 }