From c0e9d857544fc94554ca37eafe82addfd8b2af6f Mon Sep 17 00:00:00 2001 From: Markus Hartung Date: Sun, 5 Jun 2022 01:15:54 +0200 Subject: [PATCH] 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 --- src/JWT/Builder/JwtBuilder.cs | 24 +++++++----- src/JWT/JWT.csproj | 2 +- src/JWT/JwtDecoder.cs | 18 +++++++++ .../Builder/JwtBuilderDecodeTests.cs | 38 ++++++++++++++++--- tests/JWT.Tests.Common/Models/TestData.cs | 1 + 5 files changed, 66 insertions(+), 17 deletions(-) diff --git a/src/JWT/Builder/JwtBuilder.cs b/src/JWT/Builder/JwtBuilder.cs index 3bcd906c2..b7a182638 100644 --- 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. /// /// Current builder instance - public JwtBuilder WithVerifySignature(bool verify) - { - _valParams = _valParams.With(p => p.ValidateSignature = verify); - return this; - } + public JwtBuilder WithVerifySignature(bool verify) => + WithValidationParameters(p => p.ValidateSignature = verify); /// /// Sets the JWT signature validation parameters. @@ -220,7 +223,11 @@ public JwtBuilder WithVerifySignature(bool verify) /// Current builder instance 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) /// Parameters to be used for validation /// /// Current builder instance - public JwtBuilder WithValidationParameters(Action action) - { - _valParams = _valParams.With(action); - return this; - } + public JwtBuilder WithValidationParameters(Action action) => + WithValidationParameters(_valParams.With(action)); /// /// Encodes a token using the supplied dependencies. diff --git a/src/JWT/JWT.csproj b/src/JWT/JWT.csproj index 5ab6896b9..151358355 100644 --- a/src/JWT/JWT.csproj +++ b/src/JWT/JWT.csproj @@ -20,7 +20,7 @@ Alexander Batishchev, John Sheehan, Michael Lehenbauer jwt;json;authorization CC0-1.0 - 9.0.2 + 9.0.3 9.0.0.0 9.0.0.0 JWT diff --git a/src/JWT/JwtDecoder.cs b/src/JWT/JwtDecoder.cs index dad17c920..ae8f834cb 100644 --- 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(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 index 376744870..4cdcbf446 100644 --- 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("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("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("because null is not valid value for token"); + .Throw("because null is not valid value for token"); } [TestMethod] @@ -375,4 +401,4 @@ public void Decode_ToDictionary_Without_Serializer_Should_Throw_Exception() .Throw("because token can't be decoded without valid serializer"); } } -} \ No newline at end of file +} diff --git a/tests/JWT.Tests.Common/Models/TestData.cs b/tests/JWT.Tests.Common/Models/TestData.cs index 8a4f645d0..d98cda0dd 100644 --- 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";