From 4d68c2297731b91dddd0f53b9880f3c0bac689c0 Mon Sep 17 00:00:00 2001 From: Paul Schaub Date: Thu, 29 Sep 2022 12:50:32 +0200 Subject: [PATCH] Use BCs Arrays.constantTimeAreEqual(char[], char[]) --- .../main/java/org/pgpainless/util/BCUtil.java | 54 ------------------- .../java/org/pgpainless/util/Passphrase.java | 4 +- .../java/org/pgpainless/util/BCUtilTest.java | 17 +++--- 3 files changed, 10 insertions(+), 65 deletions(-) delete mode 100644 pgpainless-core/src/main/java/org/pgpainless/util/BCUtil.java diff --git a/pgpainless-core/src/main/java/org/pgpainless/util/BCUtil.java b/pgpainless-core/src/main/java/org/pgpainless/util/BCUtil.java deleted file mode 100644 index bb811cea..00000000 --- a/pgpainless-core/src/main/java/org/pgpainless/util/BCUtil.java +++ /dev/null @@ -1,54 +0,0 @@ -// SPDX-FileCopyrightText: 2018 Paul Schaub -// -// SPDX-License-Identifier: Apache-2.0 - -package org.pgpainless.util; - -public final class BCUtil { - - private BCUtil() { - - } - - /** - * 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. - * - * TODO: This method was proposed as a patch to BC: - * https://github.com/bcgit/bc-java/pull/1141 - * Replace usage of this method with upstream eventually. - * Remove once BC 172 gets released, given it contains the patch. - * - * @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 = Math.min(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 4cef145a..9576fb3e 100644 --- a/pgpainless-core/src/main/java/org/pgpainless/util/Passphrase.java +++ b/pgpainless-core/src/main/java/org/pgpainless/util/Passphrase.java @@ -8,8 +8,6 @@ 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(); @@ -165,6 +163,6 @@ public class Passphrase { } Passphrase other = (Passphrase) obj; return (getChars() == null && other.getChars() == null) || - constantTimeAreEqual(getChars(), other.getChars()); + org.bouncycastle.util.Arrays.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 6fb90e63..82368762 100644 --- a/pgpainless-core/src/test/java/org/pgpainless/util/BCUtilTest.java +++ b/pgpainless-core/src/test/java/org/pgpainless/util/BCUtilTest.java @@ -19,6 +19,7 @@ import org.bouncycastle.openpgp.PGPPublicKeyRing; import org.bouncycastle.openpgp.PGPPublicKeyRingCollection; import org.bouncycastle.openpgp.PGPSecretKeyRing; import org.bouncycastle.openpgp.PGPSecretKeyRingCollection; +import org.bouncycastle.util.Arrays; import org.junit.jupiter.api.Test; import org.pgpainless.PGPainless; import org.pgpainless.algorithm.KeyFlag; @@ -94,14 +95,14 @@ public class BCUtilTest { @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())); + assertTrue(Arrays.constantTimeAreEqual(b, b)); + assertTrue(Arrays.constantTimeAreEqual("Hello".toCharArray(), "Hello".toCharArray())); + assertTrue(Arrays.constantTimeAreEqual(new char[0], new char[0])); + assertTrue(Arrays.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)); + assertFalse(Arrays.constantTimeAreEqual("Hello".toCharArray(), "Hello World".toCharArray())); + assertFalse(Arrays.constantTimeAreEqual(null, "Hello".toCharArray())); + assertFalse(Arrays.constantTimeAreEqual("Hello".toCharArray(), null)); + assertFalse(Arrays.constantTimeAreEqual((char[]) null, null)); } }