Remove the internal FixedTimeEquals helper and use BouncyCastle's Arrays.ConstantTimeAreEqual()

src/OpenIddict.Core/Managers/OpenIddictApplicationManager.cs

@@ -27,6 +27,10 @@ using Org.BouncyCastle.Crypto.Generators;
 using Org.BouncyCastle.Crypto.Parameters;
 #endif
 
+#if !SUPPORTS_TIME_CONSTANT_COMPARISONS
+using Org.BouncyCastle.Utilities;
+#endif
+
 namespace OpenIddict.Core
 {
     /// <summary>
@@ -1318,9 +1322,15 @@ namespace OpenIddict.Core
                     return false;
                 }
 
-                return FixedTimeEquals(
+#if SUPPORTS_TIME_CONSTANT_COMPARISONS
+                return CryptographicOperations.FixedTimeEquals(
                     left: payload.Slice(13 + salt.Length, keyLength),
                     right: DeriveKey(secret, salt, algorithm, iterations, keyLength));
+#else
+                return Arrays.ConstantTimeAreEqual(
+                    a: payload.Slice(13 + salt.Length, keyLength).ToArray(),
+                    b: DeriveKey(secret, salt, algorithm, iterations, keyLength));
+#endif
             }
 
             static uint ReadNetworkByteOrder(ReadOnlySpan<byte> buffer, int offset) =>
@@ -1330,7 +1340,7 @@ namespace OpenIddict.Core
                 ((uint) buffer[offset + 3]);
         }
 
-        private static ReadOnlySpan<byte> DeriveKey(string secret, ReadOnlySpan<byte> salt,
+        private static byte[] DeriveKey(string secret, ReadOnlySpan<byte> salt,
             HashAlgorithmName algorithm, int iterations, int length)
         {
 #if SUPPORTS_KEY_DERIVATION_WITH_SPECIFIED_HASH_ALGORITHM
@@ -1352,33 +1362,6 @@ namespace OpenIddict.Core
 #endif
         }
 
-        [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)]
-        private static bool FixedTimeEquals(ReadOnlySpan<byte> left, ReadOnlySpan<byte> right)
-        {
-#if SUPPORTS_TIME_CONSTANT_COMPARISONS
-            return CryptographicOperations.FixedTimeEquals(left, right);
-#else
-            // Note: these null checks can be theoretically considered as early checks
-            // (which would defeat the purpose of a time-constant comparison method),
-            // but the expected string length is the only information an attacker
-            // could get at this stage, which is not critical where this method is used.
-
-            if (left.Length != right.Length)
-            {
-                return false;
-            }
-
-            var result = true;
-
-            for (var index = 0; index < left.Length; index++)
-            {
-                result &= left[index] == right[index];
-            }
-
-            return result;
-#endif
-        }
-
         ValueTask<long> IOpenIddictApplicationManager.CountAsync(CancellationToken cancellationToken)
             => CountAsync(cancellationToken);
 

src/OpenIddict.Core/OpenIddict.Core.csproj

@@ -21,7 +21,7 @@
     <PackageReference Include="System.Linq.Async" Version="$(LinqAsyncVersion)" />
   </ItemGroup>
 
-  <ItemGroup Condition=" '$(TargetFramework)' == 'netstandard2.0' ">
+  <ItemGroup Condition=" '$(TargetFramework)' == 'net472' Or '$(TargetFramework)' == 'netstandard2.0' ">
     <PackageReference Include="Portable.BouncyCastle" Version="$(BouncyCastleVersion)" />
   </ItemGroup>
 

src/OpenIddict.Server/OpenIddict.Server.csproj

@@ -19,6 +19,12 @@
     <PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" Version="$(IdentityModelVersion)" />
   </ItemGroup>
 
+  <ItemGroup Condition=" '$(TargetFramework)' == 'net461' Or
+                         '$(TargetFramework)' == 'net472' Or
+                         '$(TargetFramework)' == 'netstandard2.0' ">
+    <PackageReference Include="Portable.BouncyCastle" Version="$(BouncyCastleVersion)" />
+  </ItemGroup>
+
   <PropertyGroup Condition=" '$(TargetFramework)' != 'net461' ">
     <DefineConstants>$(DefineConstants);SUPPORTS_ECDSA</DefineConstants>
   </PropertyGroup>
@@ -29,7 +35,9 @@
     <DefineConstants>$(DefineConstants);SUPPORTS_EPHEMERAL_KEY_SETS</DefineConstants>
   </PropertyGroup>
 
-  <PropertyGroup Condition=" '$(TargetFramework)' == 'netcoreapp3.0' Or '$(TargetFramework)' == 'netstandard2.1' ">
+  <PropertyGroup Condition=" '$(TargetFramework)' == 'netcoreapp2.1' Or
+                             '$(TargetFramework)' == 'netcoreapp3.0' Or
+                             '$(TargetFramework)' == 'netstandard2.1' ">
     <DefineConstants>$(DefineConstants);SUPPORTS_CERTIFICATE_HASHING_WITH_SPECIFIED_ALGORITHM</DefineConstants>
     <DefineConstants>$(DefineConstants);SUPPORTS_STATIC_RANDOM_NUMBER_GENERATOR_METHODS</DefineConstants>
     <DefineConstants>$(DefineConstants);SUPPORTS_TIME_CONSTANT_COMPARISONS</DefineConstants>

src/OpenIddict.Server/OpenIddictServerHandlers.Exchange.cs

@@ -7,8 +7,6 @@
 using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
-using System.Linq;
-using System.Runtime.CompilerServices;
 using System.Security.Cryptography;
 using System.Text;
 using System.Threading.Tasks;
@@ -20,6 +18,10 @@ using static OpenIddict.Abstractions.OpenIddictConstants;
 using static OpenIddict.Server.OpenIddictServerEvents;
 using static OpenIddict.Server.OpenIddictServerHandlerFilters;
 
+#if !SUPPORTS_TIME_CONSTANT_COMPARISONS
+using Org.BouncyCastle.Utilities;
+#endif
+
 namespace OpenIddict.Server
 {
     public static partial class OpenIddictServerHandlers
@@ -1654,7 +1656,7 @@ namespace OpenIddict.Server
 
                     // Note: when using the "plain" code challenge method, no hashing is actually performed.
                     // In this case, the raw ASCII bytes of the verifier are directly compared to the challenge.
-                    ReadOnlySpan<byte> data;
+                    byte[] data;
                     if (string.Equals(method, CodeChallengeMethods.Plain, StringComparison.Ordinal))
                     {
                         data = Encoding.ASCII.GetBytes(context.Request.CodeVerifier);
@@ -1674,7 +1676,11 @@ namespace OpenIddict.Server
 
                     // Compare the verifier and the code challenge: if the two don't match, return an error.
                     // Note: to prevent timing attacks, a time-constant comparer is always used.
-                    if (!FixedTimeEquals(data, Encoding.ASCII.GetBytes(challenge)))
+#if SUPPORTS_TIME_CONSTANT_COMPARISONS
+                    if (!CryptographicOperations.FixedTimeEquals(data, Encoding.ASCII.GetBytes(challenge)))
+#else
+                    if (!Arrays.ConstantTimeAreEqual(data, Encoding.ASCII.GetBytes(challenge)))
+#endif
                     {
                         context.Logger.LogError("The token request was rejected because the 'code_verifier' was invalid.");
 
@@ -1687,33 +1693,6 @@ namespace OpenIddict.Server
 
                     return default;
                 }
-
-                [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)]
-                private static bool FixedTimeEquals(ReadOnlySpan<byte> left, ReadOnlySpan<byte> right)
-                {
-#if SUPPORTS_TIME_CONSTANT_COMPARISONS
-                    return CryptographicOperations.FixedTimeEquals(left, right);
-#else
-                    // Note: these null checks can be theoretically considered as early checks
-                    // (which would defeat the purpose of a time-constant comparison method),
-                    // but the expected string length is the only information an attacker
-                    // could get at this stage, which is not critical where this method is used.
-
-                    if (left.Length != right.Length)
-                    {
-                        return false;
-                    }
-
-                    var result = true;
-
-                    for (var index = 0; index < left.Length; index++)
-                    {
-                        result &= left[index] == right[index];
-                    }
-
-                    return result;
-#endif
-                }
             }
 
             /// <summary>