From 176ad09d19ce0c388d634fe7efb067d997f9b41e Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Mon, 29 Nov 2021 21:55:35 +0100 Subject: [PATCH] Make Passphrase comparison constant time --- .../main/java/org/pgpainless/util/BCUtil.java | 37 +++++++++++++++++++ .../java/org/pgpainless/util/Passphrase.java | 5 ++- .../java/org/pgpainless/util/BCUtilTest.java | 16 ++++++++ 3 files changed, 57 insertions(+), 1 deletion(-) diff --git a/pgpainless-core/src/main/java/org/pgpainless/util/BCUtil.java b/pgpainless-core/src/main/java/org/pgpainless/util/BCUtil.java index f548af98..f059a571 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/util/BCUtil.java +++ b/pgpainless-core/src/main/java/org/pgpainless/util/BCUtil.java @@ -47,4 +47,41 @@ public final class BCUtil { return bitStrength; } + + /** + * A constant time equals comparison - does not terminate early if + * test will fail. For best results always pass the expected value + * as the first parameter. + * + * @param expected first array + * @param supplied second array + * @return true if arrays equal, false otherwise. + */ + public static boolean constantTimeAreEqual( + char[] expected, + char[] supplied) { + if (expected == null || supplied == null) { + return false; + } + + if (expected == supplied) { + return true; + } + + int len = (expected.length < supplied.length) ? expected.length : supplied.length; + + int nonEqual = expected.length ^ supplied.length; + + // do the char-wise comparison + for (int i = 0; i != len; i++) { + nonEqual |= (expected[i] ^ supplied[i]); + } + // If supplied is longer than expected, iterate over rest of supplied with NOPs + for (int i = len; i < supplied.length; i++) { + nonEqual |= ((byte) supplied[i] ^ (byte) ~supplied[i]); + } + + return nonEqual == 0; + } + } diff --git a/pgpainless-core/src/main/java/org/pgpainless/util/Passphrase.java b/pgpainless-core/src/main/java/org/pgpainless/util/Passphrase.java index bd2e203f..4cef145a 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/util/Passphrase.java +++ b/pgpainless-core/src/main/java/org/pgpainless/util/Passphrase.java @@ -8,6 +8,8 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; import java.util.Arrays; +import static org.pgpainless.util.BCUtil.constantTimeAreEqual; + public class Passphrase { public final Object lock = new Object(); @@ -162,6 +164,7 @@ public class Passphrase { return false; } Passphrase other = (Passphrase) obj; - return Arrays.equals(getChars(), other.getChars()); + return (getChars() == null && other.getChars() == null) || + constantTimeAreEqual(getChars(), other.getChars()); } } diff --git a/pgpainless-core/src/test/java/org/pgpainless/util/BCUtilTest.java b/pgpainless-core/src/test/java/org/pgpainless/util/BCUtilTest.java index 3eda2b66..6fb90e63 100644 --- a/pgpainless-core/src/test/java/org/pgpainless/util/BCUtilTest.java +++ b/pgpainless-core/src/test/java/org/pgpainless/util/BCUtilTest.java @@ -5,6 +5,8 @@ package org.pgpainless.util; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; import java.security.InvalidAlgorithmParameterException; @@ -88,4 +90,18 @@ public class BCUtilTest { assertEquals(pubColSize, secColSize); } + + @Test + public void constantTimeAreEqualsTest() { + char[] b = "Hello".toCharArray(); + assertTrue(BCUtil.constantTimeAreEqual(b, b)); + assertTrue(BCUtil.constantTimeAreEqual("Hello".toCharArray(), "Hello".toCharArray())); + assertTrue(BCUtil.constantTimeAreEqual(new char[0], new char[0])); + assertTrue(BCUtil.constantTimeAreEqual(new char[] {'H', 'e', 'l', 'l', 'o'}, "Hello".toCharArray())); + + assertFalse(BCUtil.constantTimeAreEqual("Hello".toCharArray(), "Hello World".toCharArray())); + assertFalse(BCUtil.constantTimeAreEqual(null, "Hello".toCharArray())); + assertFalse(BCUtil.constantTimeAreEqual("Hello".toCharArray(), null)); + assertFalse(BCUtil.constantTimeAreEqual(null, null)); + } }