Added some checks for None algorithm (#402)

* Disallowed signature checking for algorithm None * Updated tests * Bumped version to 9.0.1 Co-authored-by: Alexander Batishchev <[email protected]>
jwt-dotnet · Jun 4, 2022 · c0e9d85 · c0e9d85
1 parent 518920f
commit c0e9d85
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 17 deletions.
diff --git a/src/JWT/Builder/JwtBuilder.cs b/src/JWT/Builder/JwtBuilder.cs
@@ -159,6 +159,12 @@ public JwtBuilder WithAlgorithmFactory(IAlgorithmFactory algFactory)
         public JwtBuilder WithAlgorithm(IJwtAlgorithm algorithm)
         {
             _algorithm = algorithm;
+
+            if (_algorithm is NoneAlgorithm)
+            {
+                _valParams.ValidateSignature = false;
+            }
+
             return this;
         }
 
@@ -206,11 +212,8 @@ public JwtBuilder DoNotVerifySignature() =>
         /// Instructs whether to verify the JWT signature.
         /// </summary>
         /// <returns>Current builder instance</returns>
-        public JwtBuilder WithVerifySignature(bool verify)
-        {
-            _valParams = _valParams.With(p => p.ValidateSignature = verify);
-            return this;
-        }
+        public JwtBuilder WithVerifySignature(bool verify) =>
+            WithValidationParameters(p => p.ValidateSignature = verify);
 
         /// <summary>
         /// Sets the JWT signature validation parameters.
@@ -220,7 +223,11 @@ public JwtBuilder WithVerifySignature(bool verify)
         /// <returns>Current builder instance</returns>
         public JwtBuilder WithValidationParameters(ValidationParameters valParams)
         {
+            if (valParams.ValidateSignature && _algorithm is NoneAlgorithm)
+                throw new InvalidOperationException("Verify signature is not allowed for algorithm None");
+
             _valParams = valParams;
+
             return this;
         }
 
@@ -230,11 +237,8 @@ public JwtBuilder WithValidationParameters(ValidationParameters valParams)
         /// <param name="valParams">Parameters to be used for validation</param>
         /// <exception cref="ArgumentNullException" />
         /// <returns>Current builder instance</returns>
-        public JwtBuilder WithValidationParameters(Action<ValidationParameters> action)
-        {
-            _valParams = _valParams.With(action);
-            return this;
-        }
+        public JwtBuilder WithValidationParameters(Action<ValidationParameters> action) =>
+            WithValidationParameters(_valParams.With(action));
 
         /// <summary>
         /// Encodes a token using the supplied dependencies.

diff --git a/src/JWT/JWT.csproj b/src/JWT/JWT.csproj
@@ -20,7 +20,7 @@
     <Authors>Alexander Batishchev, John Sheehan, Michael Lehenbauer</Authors>
     <PackageTags>jwt;json;authorization</PackageTags>
     <PackageLicense>CC0-1.0</PackageLicense>
-    <Version>9.0.2</Version>
+    <Version>9.0.3</Version>
     <FileVersion>9.0.0.0</FileVersion>
     <AssemblyVersion>9.0.0.0</AssemblyVersion>
     <RootNamespace>JWT</RootNamespace>

diff --git a/src/JWT/JwtDecoder.cs b/src/JWT/JwtDecoder.cs
@@ -124,6 +124,10 @@ public string Decode(JwtParts jwt, byte[] key, bool verify)
 
                 Validate(jwt, key);
             }
+            else
+            {
+                ValidateNoneAlgorithm(jwt);
+            }
             return Decode(jwt);
         }
 
@@ -146,6 +150,10 @@ public string Decode(JwtParts jwt, byte[][] keys, bool verify)
 
                 Validate(jwt, keys);
             }
+            else
+            {
+                ValidateNoneAlgorithm(jwt);
+            }
             return Decode(jwt);
         }
 
@@ -271,5 +279,15 @@ private static bool AllKeysHaveValues(byte[][] keys)
 
             return Array.TrueForAll(keys, key => key.Length > 0);
         }
+
+        private void ValidateNoneAlgorithm(JwtParts jwt)
+        {
+            var header = DecodeHeader<JwtHeader>(jwt);
+            if (String.Equals(header.Algorithm, nameof(JwtAlgorithmName.None), StringComparison.OrdinalIgnoreCase) &&
+                !String.IsNullOrEmpty(jwt.Signature))
+            {
+                throw new InvalidOperationException("Signature is not acceptable for algorithm None");
+            }
+        }
     }
 }
diff --git a/tests/JWT.Tests.Common/Builder/JwtBuilderDecodeTests.cs b/tests/JWT.Tests.Common/Builder/JwtBuilderDecodeTests.cs
@@ -83,17 +83,43 @@ public void Decode_Using_Asymmetric_Algorithm_Should_Return_Token()
         }
 
         [TestMethod]
-        public void Decode_Using_None_Algorithm_Should_Return_Token()
+        public void Decode_Using_Signature_Is_Not_Accepted_For_None_Algorithm()
+        {
+            Action action =
+                () => JwtBuilder.Create()
+                                .WithAlgorithm(new NoneAlgorithm())
+                                .DoNotVerifySignature()
+                                .Decode(TestData.TokenWithAlgNoneMissingSignature + "ANY");
+
+            action.Should()
+                  .Throw<InvalidOperationException>("Signature must be empty per https://datatracker.ietf.org/doc/html/rfc7518#section-3.6");
+        }
+
+        [TestMethod]
+        public void Decode_Using_Empty_Signature_Should_Work_For_None_Algorithm()
         {
             var token = JwtBuilder.Create()
                                   .WithAlgorithm(new NoneAlgorithm())
-                                  .DoNotVerifySignature()
-                                  .Decode(TestData.Token);
+                                  .WithSecret(TestData.Secret)
+                                  .Decode(TestData.TokenWithAlgNoneMissingSignature);
 
             token.Should()
-                 .NotBeNullOrEmpty("because the decoded token contains values and they should have been decoded");
+                 .NotBeNullOrEmpty("Using none algorithm should be valid without any signature");
         }
+
+        [TestMethod]
+        public void Decode_With_MustVerifySignature_Should_Not_Be_Allowed_For_For_None_Algorithm()
+        {
+            Action action =
+                () => JwtBuilder.Create()
+                                .WithAlgorithm(new NoneAlgorithm())
+                                .MustVerifySignature()
+                                .Decode(TestData.TokenWithAlgNoneMissingSignature);
 
+            action.Should()
+                  .Throw<InvalidOperationException>("verify signature is not supported for none algorithm");
+        }
+
         [TestMethod]
         public void Decode_With_VerifySignature_And_Without_Algorithm_Should_Throw_Exception()
         {
@@ -155,7 +181,7 @@ public void Decode_Without_Token_Should_Throw_Exception()
                                 .Decode(null);
 
             action.Should()
-                   .Throw<ArgumentException>("because null is not valid value for token");
+                  .Throw<ArgumentException>("because null is not valid value for token");
         }
 
         [TestMethod]
@@ -375,4 +401,4 @@ public void Decode_ToDictionary_Without_Serializer_Should_Throw_Exception()
                   .Throw<InvalidOperationException>("because token can't be decoded without valid serializer");
         }
     }
-}
+}
diff --git a/tests/JWT.Tests.Common/Models/TestData.cs b/tests/JWT.Tests.Common/Models/TestData.cs
@@ -29,6 +29,7 @@ public static class TestData
         public const long TokenTimestamp = 1605834255;
 
         public const string TokenWithIncorrectSignature = "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJGaXJzdE5hbWUiOiJCb2IiLCJBZ2UiOjM3fQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c";
+        public const string TokenWithAlgNoneMissingSignature = "eyJ0eXAiOiJKV1QiLCJhbGciOiJub25lIn0.eyJGaXJzdE5hbWUiOiJCb2IiLCJBZ2UiOjM3fQ.";
         public const string TokenWithoutHeader = "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9eyJGaXJzdE5hbWUiOiJCb2IiLCJBZ2UiOjM3fQ.oj1ROhq6SyGDG3C0WIPe8wDuMJjA47uKwXCHkxl6Zy0";
         public const string TokenWithoutAlgorithm = "eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0.eyJGaXJzdE5hbWUiOiJCb2IiLCJBZ2UiOjM3fQ.ANY";