Constant time secret check (#257)

* Constant time secret check

Switch all secret comparisons to be constant time to reduce the risk of creating an oracle through side channel timing.

* Use inbuilt function

* Removed unused function. Added comments.

---------

Co-authored-by: Badrish Chandramouli <badrishc@microsoft.com>
Co-authored-by: Lukas Maas <lumaas@microsoft.com>

libs/server/ACL/ACLPassword.cs

@@ -92,20 +92,7 @@ public override string ToString()
         /// <returns>True if equal, otherwise false.</returns>
         public bool Equals(ACLPassword password)
         {
-            if (PasswordHash.Length != password.PasswordHash.Length)
-            {
-                return false;
-            }
-
-            for (int i = 0; i < PasswordHash.Length; i++)
-            {
-                if (PasswordHash[i] != password.PasswordHash[i])
-                {
-                    return false;
-                }
-            }
-
-            return true;
+            return SecretsUtility.ConstantEquals(password.PasswordHash.AsSpan(), PasswordHash.AsSpan());
         }
 
         /// <summary>

libs/server/ACL/SecretsUtility.cs

@@ -0,0 +1,35 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+using System;
+using System.Security.Cryptography;
+
+namespace Garnet.server.ACL
+{
+    /// <summary>
+    /// A collection of shared utility methods to operate against secret key material.
+    /// </summary>
+    internal static class SecretsUtility
+    {
+        /*
+         * Future implementation note: the ConstantEquals => FixedTimeEquals call can accept strings through
+         *
+         * string a = ...;
+         * MemoryMarshal.Cast<char, byte>(a.AsSpan())
+         *
+         * If both a and b strings are ASCII or UTF-8. UTF-16 or other encoding requires comparing
+         * against char directly instead of casting to do it safely.
+         */
+
+        /// <summary>
+        /// Compare two spans of bytes and evaluate if they are equal in constant O(n) time.
+        /// This is intended to reduce the likelihood of leaking the contents of the spans through side-channel timing attacks.
+        /// 
+        /// Note: This function is explicitly not optimized for performance and must remain O(n) where a.Length == b.Length.
+        /// </summary>
+        /// <param name="a">The first span of data to compare</param>
+        /// <param name="b">The second span of data to compare against</param>
+        /// <returns>Returns true if both spans are of equal length and each sequential value matches</returns>
+        public static bool ConstantEquals(ReadOnlySpan<byte> a, ReadOnlySpan<byte> b) => CryptographicOperations.FixedTimeEquals(a, b);
+    }
+}

libs/server/ACL/User.cs

@@ -143,18 +143,21 @@ public bool ValidatePassword(ACLPassword password)
             }
 
             // Any of the registered password hashes is allowed
+
+            bool matched = false;
+
             lock (_passwordHashes)
             {
                 foreach (ACLPassword hash in _passwordHashes)
                 {
                     if (password.Equals(hash))
                     {
-                        return true;
+                        matched = true;
                     }
                 }
             }
 
-            return false;
+            return matched;
         }
 
         /// <summary>

libs/server/Auth/GarnetPasswordAuthenticator.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT license.
 
 using System;
+using Garnet.server.ACL;
 
 namespace Garnet.server.Auth
 {
@@ -27,7 +28,7 @@ public GarnetPasswordAuthenticator(byte[] pwd)
 
         public bool Authenticate(ReadOnlySpan<byte> password, ReadOnlySpan<byte> username)
         {
-            _authenticated = password.SequenceEqual(_pwd);
+            _authenticated = SecretsUtility.ConstantEquals(_pwd, password);
             return _authenticated;
         }
     }