diff --git a/CHANGELOG.md b/CHANGELOG.md index 21226825..35566d4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ method. ### Improvements * Stricter guarantees about which curves are used for EC key generation. [PR #127](https://github.com/corretto/amazon-corretto-crypto-provider/pull/127) +* Reduce timing signal from trimming zeros of TLSPremasterSecrets from DH KeyAgreement. [PR #129](https://github.com/corretto/amazon-corretto-crypto-provider/pull/129) ### Patches * Add version gating to some tests introduced in 1.5.0 [PR #128](https://github.com/corretto/amazon-corretto-crypto-provider/pull/128) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1ab27ae8..c1096c19 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -107,6 +107,7 @@ set(ACCP_SRC src/com/amazon/corretto/crypto/provider/AccessibleByteArrayOutputStream.java src/com/amazon/corretto/crypto/provider/AesCtrDrbg.java src/com/amazon/corretto/crypto/provider/AesGcmSpi.java + src/com/amazon/corretto/crypto/provider/ConstantTime.java src/com/amazon/corretto/crypto/provider/EcGen.java src/com/amazon/corretto/crypto/provider/EvpKeyAgreement.java src/com/amazon/corretto/crypto/provider/EvpKeyType.java @@ -528,6 +529,7 @@ add_jar( tst/com/amazon/corretto/crypto/provider/test/AESGenerativeTest.java tst/com/amazon/corretto/crypto/provider/test/AesCtrDrbgTest.java tst/com/amazon/corretto/crypto/provider/test/AesTest.java + tst/com/amazon/corretto/crypto/provider/test/ConstantTimeTests.java tst/com/amazon/corretto/crypto/provider/test/EcGenTest.java tst/com/amazon/corretto/crypto/provider/test/EvpKeyAgreementTest.java tst/com/amazon/corretto/crypto/provider/test/EvpKeyAgreementSpecificTest.java diff --git a/src/com/amazon/corretto/crypto/provider/ConstantTime.java b/src/com/amazon/corretto/crypto/provider/ConstantTime.java new file mode 100644 index 00000000..eed8af92 --- /dev/null +++ b/src/com/amazon/corretto/crypto/provider/ConstantTime.java @@ -0,0 +1,69 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package com.amazon.corretto.crypto.provider; + +/** + * Contains several constant time utilities + */ +final class ConstantTime { + private ConstantTime() { + // Prevent instantiation + } + + /** + * Equivalent to {@code val != 0 ? 1 : 0} + */ + static final int isNonZero(int val) { + return ((val | -val) >>> 31) & 0x01; // Unsigned bitshift + } + + /** + * Equivalent to {@code val == 0 ? 1 : 0} + */ + static final int isZero(int val) { + return 1 - isNonZero(val); + } + + /** + * Equivalent to {@code val < 0 ? 1 : 0} + */ + static final int isNegative(int val) { + return (val >>> 31) & 0x01; + } + + /** + * Equivalent to {@code x == y ? 1 : 0} + */ + static final int equal(int x, int y) { + final int difference = x - y; + // Difference is 0 iff x == y + return isZero(difference); + } + + /** + * Equivalent to {@code x > y ? 1 : 0} + */ + static final int gt(int x, int y) { + // Convert to long to avoid underflow + final long xl = x; + final long yl = y; + final long difference = yl - xl; + // If xl > yl, then difference is negative. + // Thus, we can just return the sign-bit + return (int) ((difference >>> 63) & 0x01); // Unsigned bitshift + } + + /** + * Equivalent to {@code selector != 0 ? a : b} + */ + static final int select(int selector, int a, int b) { + final int mask = isZero(selector) - 1; + // Mask == -1 (all bits 1) iff selector != 0 + // Mask == 0 (all bits 0) iff selector == 0 + + final int combined = a ^ b; + + return b ^ (combined & mask); + } +} diff --git a/src/com/amazon/corretto/crypto/provider/EvpKeyAgreement.java b/src/com/amazon/corretto/crypto/provider/EvpKeyAgreement.java index 0d89e2cd..c852cab4 100644 --- a/src/com/amazon/corretto/crypto/provider/EvpKeyAgreement.java +++ b/src/com/amazon/corretto/crypto/provider/EvpKeyAgreement.java @@ -196,17 +196,35 @@ protected void reset() { } private static byte[] trimZeros(final byte[] secret) { - // According to other implementations, we don't appear - // to need to worry about timing leaks of this data. int bytesToTrim = 0; - while (bytesToTrim < secret.length && secret[bytesToTrim] == 0) { - bytesToTrim++; + int foundNonZero = 0; + for (int x = 0; x < secret.length; x++) { + final int currByte = secret[x]; + // Have we found something that isn't a zero? + foundNonZero |= currByte; + + // foundNonZero == 0 iff we have not see any non-zero bytes + // Thus, we should update bytesToTrim iff foundNonZero == 0 + final int shouldUpdateTrim = ConstantTime.isZero(foundNonZero); + bytesToTrim = ConstantTime.select(shouldUpdateTrim, x + 1, bytesToTrim); } - if (bytesToTrim == 0) { - return secret; + // Allocating arrays of different lengths always risks non-constant time operation. + // There is no way to avoid this. + final byte[] result = new byte[secret.length - bytesToTrim]; + + // We'll always do the same number of byte copies, but the leading zeros will be overwritten by valid ones. + // While the memory access pattern won't be identical, there is no way to completely avoid this. + for (int x = 0; x < secret.length; x++) { + final int realIndex = x - bytesToTrim; + + final int notYetValid = ConstantTime.isNegative(realIndex); + + final int indexToUpdate = ConstantTime.select(notYetValid, 0, realIndex); + result[indexToUpdate] = secret[x]; } - return Arrays.copyOfRange(secret, bytesToTrim, secret.length); + + return result; } static class ECDH extends EvpKeyAgreement { diff --git a/tst/com/amazon/corretto/crypto/provider/test/ConstantTimeTests.java b/tst/com/amazon/corretto/crypto/provider/test/ConstantTimeTests.java new file mode 100644 index 00000000..f71ebe4d --- /dev/null +++ b/tst/com/amazon/corretto/crypto/provider/test/ConstantTimeTests.java @@ -0,0 +1,117 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package com.amazon.corretto.crypto.provider.test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.params.provider.Arguments.arguments; + +import java.util.ArrayList; +import java.util.List; + +import org.junit.jupiter.api.parallel.Execution; +import org.junit.jupiter.api.parallel.ExecutionMode; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + + +// Note: We don't actually test that these methods are constant time, just that they give the correct answers. +@ExtendWith(TestResultLogger.class) +@Execution(ExecutionMode.CONCURRENT) +public class ConstantTimeTests { + // A few common values which when combined can trigger edge cases + private static final int[] TEST_VALUES = {Integer.MIN_VALUE, Integer.MIN_VALUE + 1, -2, -1, 0, 1, 2, Integer.MAX_VALUE - 1, Integer.MAX_VALUE}; + private static final Class CONSTANT_TIME_CLASS; + static { + try { + CONSTANT_TIME_CLASS = Class.forName("com.amazon.corretto.crypto.provider.ConstantTime"); + } catch (Exception ex) { + throw new RuntimeException(ex); + } + } + + + public static List testPairs() { + final List result = new ArrayList<>(); + for (int a : TEST_VALUES) { + for (int b : TEST_VALUES) { + result.add(arguments(a, b)); + } + } + return result; + } + + public static int[] testSingles() { + return TEST_VALUES; + } + + @ParameterizedTest + @MethodSource("testSingles") + public void testIsNonZero(int val) { + final int expected = val != 0 ? 1 : 0; + assertEquals(expected, sneaky("isNonZero", val)); + } + + @ParameterizedTest + @MethodSource("testSingles") + public void testIsZero(int val) { + final int expected = val == 0 ? 1 : 0; + assertEquals(expected, sneaky("isZero", val)); + } + + @ParameterizedTest + @MethodSource("testSingles") + public void testIsNegative(int val) { + final int expected = val < 0 ? 1 : 0; + assertEquals(expected, sneaky("isNegative", val)); + } + + @ParameterizedTest + @MethodSource("testPairs") + public void testEqual(int x, int y) { + final int expected = x == y ? 1 : 0; + assertEquals(expected, sneaky("equal", x, y)); + } + + @ParameterizedTest + @MethodSource("testPairs") + public void testGt(int x, int y) { + final int expected = x > y ? 1 : 0; + assertEquals(expected, sneaky("gt", x, y)); + } + + @ParameterizedTest + @MethodSource("testSingles") + public void testSelect(int selector) { + final int a = 10; + final int b = 11; + final int expected = selector != 0 ? a : b; + assertEquals(expected, sneaky("select", selector, a, b)); + } + + private static int sneaky(String name, int a) { + try { + return TestUtil.sneakyInvoke_int(CONSTANT_TIME_CLASS, name, a); + } catch (final Throwable t) { + throw new AssertionError(t); + } + } + + private static int sneaky(String name, int a, int b) { + try { + return TestUtil.sneakyInvoke_int(CONSTANT_TIME_CLASS, name, a, b); + } catch (final Throwable t) { + throw new AssertionError(t); + } + } + + private static int sneaky(String name, int a, int b, int c) { + try { + return TestUtil.sneakyInvoke_int(CONSTANT_TIME_CLASS, name, a, b, c); + } catch (final Throwable t) { + throw new AssertionError(t); + } + } +} diff --git a/tst/com/amazon/corretto/crypto/provider/test/TestResultLogger.java b/tst/com/amazon/corretto/crypto/provider/test/TestResultLogger.java index 7c2ccef9..691a7748 100644 --- a/tst/com/amazon/corretto/crypto/provider/test/TestResultLogger.java +++ b/tst/com/amazon/corretto/crypto/provider/test/TestResultLogger.java @@ -74,7 +74,7 @@ private void printNotice(final String notice, ExtensionContext context, String d StringWriter causeText = new StringWriter(); if (cause != null) { try (PrintWriter printWriter = new PrintWriter(causeText)) { - printWriter.append(" @ ").append(getFailureLocation(cause).toString()); + printWriter.append(" @ ").append(String.valueOf(getFailureLocation(cause))); // Don't print out traces for Assert.* failures which throw subclasses of AssertionError. // Just for thrown exceptions if (AssertionError.class.equals(cause.getClass()) ||