Merge pull request #27834 from vespa-engine/vekterli/defer-side-channel-safe-array-ops-to-bc

Defer side channel-safe array checks to existing BC utils

3 files changed, 13 insertions, 24 deletions

diff --git a/security-utils/src/main/java/com/yahoo/security/SideChannelSafe.java b/security-utils/src/main/java/com/yahoo/security/SideChannelSafe.java
index bd085f6f624..3a46891085f 100644
--- a/security-utils/src/main/java/com/yahoo/security/SideChannelSafe.java
+++ b/security-utils/src/main/java/com/yahoo/security/SideChannelSafe.java
@@ -1,6 +1,8 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.security;
 
+import org.bouncycastle.util.Arrays;
+
 /**
  * Utility functions for comparing the contents of arrays without leaking information about the
  * data contained within them via timing side-channels. This is done by avoiding any branches
@@ -13,18 +15,11 @@ package com.yahoo.security;
 public class SideChannelSafe {
 
     /**
-     * @return true iff all bytes in the array are zero. An empty array always returns false
-     *         since it technically can't contain any zeros at all.
+     * @return true iff all bytes in the array are zero. An empty array always returns true
+     *         to be in line with BouncyCastle semantics.
      */
     public static boolean allZeros(byte[] buf) {
-        if (buf.length == 0) {
-            return false;
-        }
-        byte accu = 0;
-        for (byte b : buf) {
-            accu |= b;
-        }
-        return (accu == 0);
+        return Arrays.areAllZeroes(buf, 0, buf.length);
     }
 
     /**
@@ -32,23 +27,14 @@ public class SideChannelSafe {
      * about the contents of either of the arrays.
      *
      * <strong>Important:</strong> the <em>length</em> of the arrays is not considered secret, and
-     * will be leaked if arrays of differing sizes are given.
+     * <em>may</em> be leaked if arrays of differing sizes are given.
      *
      * @param lhs first array of bytes to compare
      * @param rhs second array of bytes to compare
      * @return true iff both arrays have the same size and are element-wise identical
      */
     public static boolean arraysEqual(byte[] lhs, byte[] rhs) {
-        if (lhs.length != rhs.length) {
-            return false;
-        }
-        // Only use constant time bitwise ops. `accu` will be non-zero if at least one bit
-        // differed in any byte compared between the two arrays.
-        byte accu = 0;
-        for (int i = 0; i < lhs.length; ++i) {
-            accu |= (byte)(lhs[i] ^ rhs[i]);
-        }
-        return (accu == 0);
+        return Arrays.constantTimeAreEqual(lhs, rhs);
     }
 
 }
diff --git a/security-utils/src/main/java/com/yahoo/security/token/TokenCheckHash.java b/security-utils/src/main/java/com/yahoo/security/token/TokenCheckHash.java
index 2ff47081784..b67b120ba7b 100644
--- a/security-utils/src/main/java/com/yahoo/security/token/TokenCheckHash.java
+++ b/security-utils/src/main/java/com/yahoo/security/token/TokenCheckHash.java
@@ -1,6 +1,8 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.security.token;
 
+import com.yahoo.security.SideChannelSafe;
+
 import java.util.Arrays;
 
 import static com.yahoo.security.ArrayUtils.hex;
@@ -18,8 +20,9 @@ public record TokenCheckHash(byte[] hashBytes) {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
         TokenCheckHash tokenCheckHash = (TokenCheckHash) o;
-        // We don't consider token hashes secret data, so no harm in data-dependent equals()
-        return Arrays.equals(hashBytes, tokenCheckHash.hashBytes);
+        // Although not considered secret information, avoid leaking the contents of
+        // the check-hashes themselves via timing channels.
+        return SideChannelSafe.arraysEqual(hashBytes, tokenCheckHash.hashBytes);
     }
 
     @Override
diff --git a/security-utils/src/test/java/com/yahoo/security/SideChannelSafeTest.java b/security-utils/src/test/java/com/yahoo/security/SideChannelSafeTest.java
index 7a66ed6eb7f..9731bbffc38 100644
--- a/security-utils/src/test/java/com/yahoo/security/SideChannelSafeTest.java
+++ b/security-utils/src/test/java/com/yahoo/security/SideChannelSafeTest.java
@@ -14,7 +14,7 @@ public class SideChannelSafeTest {
 
     @Test
     void all_zeros_checks_length_and_array_contents() {
-        assertFalse(SideChannelSafe.allZeros(new byte[0]));
+        assertTrue(SideChannelSafe.allZeros(new byte[0]));
         assertFalse(SideChannelSafe.allZeros(new byte[]{ 1 }));
         assertTrue(SideChannelSafe.allZeros(new byte[]{ 0 }));
         assertFalse(SideChannelSafe.allZeros(new byte[]{ 0, 0, 127, 0 }));