From 23d692fa002b0fd0c25f1f4f99db57fe4cdbaa1f Mon Sep 17 00:00:00 2001 From: Greg Rubin Date: Wed, 24 Nov 2021 08:38:12 -0800 Subject: [PATCH 1/6] Add more debug logic (#167) * Add more debug logic * Apply suggestions from code review Co-authored-by: Alex Chew Co-authored-by: Alex Chew --- CHANGELOG.md | 13 ++++++ CMakeLists.txt | 2 + .../AmazonCorrettoCryptoProvider.java | 29 ++++--------- .../corretto/crypto/provider/DebugFlag.java | 41 +++++++++++++++++++ .../corretto/crypto/provider/Loader.java | 16 +++++--- .../crypto/provider/NativeResource.java | 31 ++++++++++---- .../crypto/provider/SelfTestSuite.java | 2 +- .../corretto/crypto/provider/Utils.java | 25 +++++++++++ .../crypto/provider/test/UtilsTest.java | 2 +- 9 files changed, 124 insertions(+), 37 deletions(-) create mode 100644 src/com/amazon/corretto/crypto/provider/DebugFlag.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 1334dc0f..f9e17b1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,18 @@ # Changelog +## 1.6.2 (Unreleased) + +### Improvements +* Add "help" value to two of our properties which outputs (to STDERR) valid values. + * `com.amazon.corretto.crypto.provider.extrachecks` + * `com.amazon.corretto.crypto.provider.debug` +* Add new `com.amazon.corretto.crypto.provider.debug` property to gate possibly expensive debug logic. +Current values are: + * `FreeTrace` - Enables tracking of allocation and freeing of native objects from java for more detailed exceptions. + * `VerboseLogging` - Enables more detailed logging. + * `ALL` - Enables all of the above +(May still require changes to your logging configuration to see the new logs.) + ## 1.6.1 ### Patches * Fix an issue where a race condition can cause ACCP's MessageDigest hashing algorithms to return the same value for different inputs [PR #157](https://github.com/corretto/amazon-corretto-crypto-provider/pull/157) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5b39a769..8a686682 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -109,6 +109,7 @@ set(ACCP_SRC 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/DebugFlag.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 @@ -656,6 +657,7 @@ add_custom_target(check-junit-SecurityManager add_custom_target(check-junit-extra-checks COMMAND ${TEST_JAVA_EXECUTABLE} -Dcom.amazon.corretto.crypto.provider.extrachecks=ALL + -Dcom.amazon.corretto.crypto.provider.debug=ALL ${TEST_RUNNER_ARGUMENTS} --select-package=com.amazon.corretto.crypto.provider.test --exclude-package=com.amazon.corretto.crypto.provider.test.integration diff --git a/src/com/amazon/corretto/crypto/provider/AmazonCorrettoCryptoProvider.java b/src/com/amazon/corretto/crypto/provider/AmazonCorrettoCryptoProvider.java index 87ae9c35..74ea6313 100644 --- a/src/com/amazon/corretto/crypto/provider/AmazonCorrettoCryptoProvider.java +++ b/src/com/amazon/corretto/crypto/provider/AmazonCorrettoCryptoProvider.java @@ -12,7 +12,6 @@ import static java.util.logging.Logger.getLogger; import java.io.IOException; -import java.io.ObjectStreamException; import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; @@ -44,7 +43,7 @@ public final class AmazonCorrettoCryptoProvider extends java.security.Provider { private transient SelfTestSuite selfTestSuite = new SelfTestSuite(); static { - if (!Loader.IS_AVAILABLE) { + if (!Loader.IS_AVAILABLE && DebugFlag.VERBOSELOGS.isEnabled()) { getLogger("AmazonCorrettoCryptoProvider").fine("Native JCE libraries are unavailable - disabling"); rdRandSupported_ = false; } else { @@ -268,23 +267,9 @@ private void resetAllSelfTests() { public AmazonCorrettoCryptoProvider() { super("AmazonCorrettoCryptoProvider", PROVIDER_VERSION, ""); - final String[] extraCheckOptions = Loader.getProperty("extrachecks", "").split(","); - for (final String check : extraCheckOptions) { - if (check.equalsIgnoreCase("all")) { - extraChecks.addAll(EnumSet.allOf(ExtraCheck.class)); - break; - } - try { - final ExtraCheck value = ExtraCheck.valueOf(check.toUpperCase()); - if (value != null) { - extraChecks.add(value); - } - } catch (Exception ex) { - // Ignore - } - } + Utils.optionsFromProperty(ExtraCheck.class, extraChecks, "extrachecks"); - if (!Loader.IS_AVAILABLE) { + if (!Loader.IS_AVAILABLE && DebugFlag.VERBOSELOGS.isEnabled()) { getLogger("AmazonCorrettoCryptoProvider").fine("Native JCE libraries are unavailable - disabling"); // Don't implement anything @@ -339,7 +324,7 @@ public static boolean isRdRandSupported() { * {@link SelfTestStatus#NOT_RUN} will be returned if any tests have not be run. * {@link SelfTestStatus#PASSED} will only be returned if all tests have been run and have * all passed. - * + * *

Algorithms currently run by this method: *

    *
  • NIST800-90A/AES-CTR-256 @@ -349,7 +334,7 @@ public static boolean isRdRandSupported() { *
  • HMacSHA1 *
  • HMacMD5 *
- * + * * @see #runSelfTests() */ public SelfTestStatus getSelfTestStatus() { @@ -361,7 +346,7 @@ public SelfTestStatus getSelfTestStatus() { * Please see {@link #getSelfTestStatus()} for the algorithms tested and * the possible return values. (though this method will never return * {@link SelfTestStatus#NOT_RUN}). - * + * * @see #getSelfTestStatus() */ public SelfTestStatus runSelfTests() { @@ -379,7 +364,7 @@ public Throwable getLoadingError() { /** *

Throws an instance of {@link RuntimeCryptoException} if this library is not currently * functional. Otherwise does nothing. - * + * *

This library is considered healthy if {@link #getLoadingError()} returns {@code null} * and {@link #runSelfTests()} returns {@link SelfTestStatus#PASSED}. */ diff --git a/src/com/amazon/corretto/crypto/provider/DebugFlag.java b/src/com/amazon/corretto/crypto/provider/DebugFlag.java new file mode 100644 index 00000000..773fee97 --- /dev/null +++ b/src/com/amazon/corretto/crypto/provider/DebugFlag.java @@ -0,0 +1,41 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package com.amazon.corretto.crypto.provider; + +import java.util.EnumSet; + +/** + * Indicates whether a given debug mode is enabled for ACCP. None of these modes + * may compromise the security of ACCP but they are permitted to have + * significant performance costs. + * + * These are used by passing them by name (case-insensitive) to the + * {@code com.amazon.corretto.crypto.provider.debug} system property. Example: + * {@code -Dcom.amazon.corretto.crypto.provider.debug=FreeTrace}. + * + * Alternatively you can enable all debug flags with the magic value of "ALL". + */ +enum DebugFlag { + /** Trace when native values are created and freed. */ + FREETRACE, + /** + * Increases the verbosity of logs. + * May still need to be combined with increasing the log level of your configured logger. + */ + VERBOSELOGS; + + private static final EnumSet ENABLED_FLAGS = EnumSet.noneOf(DebugFlag.class); + + static { + Utils.optionsFromProperty(DebugFlag.class, ENABLED_FLAGS, "debug"); + } + + static boolean isEnabled(final DebugFlag flag) { + return ENABLED_FLAGS.contains(flag); + } + + boolean isEnabled() { + return isEnabled(this); + } +} diff --git a/src/com/amazon/corretto/crypto/provider/Loader.java b/src/com/amazon/corretto/crypto/provider/Loader.java index ecbbac88..d9cc5165 100644 --- a/src/com/amazon/corretto/crypto/provider/Loader.java +++ b/src/com/amazon/corretto/crypto/provider/Loader.java @@ -44,7 +44,7 @@ * */ final class Loader { - private static final String PROPERTY_BASE = "com.amazon.corretto.crypto.provider."; + static final String PROPERTY_BASE = "com.amazon.corretto.crypto.provider."; private static final String LIBRARY_NAME = "amazonCorrettoCryptoProvider"; private static final Pattern TEST_FILENAME_PATTERN = Pattern.compile("[-a-zA-Z0-9]+(\\.[a-zA-Z0-9]+)*"); private static final Logger LOG = Logger.getLogger("AmazonCorrettoCryptoProvider"); @@ -176,10 +176,12 @@ static String getProperty(String propertyName, String def) { } IS_AVAILABLE = available; LOADING_ERROR = error; - if (available) { - LOG.log(Level.CONFIG, "Successfully loaded native library version " + PROVIDER_VERSION_STR); - } else { - LOG.log(Level.CONFIG, "Unable to load native library", error); + if (DebugFlag.VERBOSELOGS.isEnabled()) { + if (available) { + LOG.log(Level.CONFIG, "Successfully loaded native library version " + PROVIDER_VERSION_STR); + } else { + LOG.log(Level.CONFIG, "Unable to load native library", error); + } } // Finally start up a cleaning thread if necessary @@ -270,7 +272,9 @@ private synchronized static Path createTmpFile(final String prefix, final String try { final Path result = Files.createFile(tmpFile, permissions); - LOG.log(Level.FINE, "Created temporary library file after " + attempt + " attempts"); + if (DebugFlag.VERBOSELOGS.isEnabled()) { + LOG.log(Level.FINE, "Created temporary library file after " + attempt + " attempts"); + } return result; } catch (final FileAlreadyExistsException ex) { // We ignore and retry this exception diff --git a/src/com/amazon/corretto/crypto/provider/NativeResource.java b/src/com/amazon/corretto/crypto/provider/NativeResource.java index 2be5b50e..fd8c1a94 100644 --- a/src/com/amazon/corretto/crypto/provider/NativeResource.java +++ b/src/com/amazon/corretto/crypto/provider/NativeResource.java @@ -20,6 +20,12 @@ private static void wakeCleaner() { private static final class Cell extends ReentrantLock { private static final long serialVersionUID = 1L; + // Debug stuff + private static final boolean FREE_TRACE_DEBUG = DebugFlag.FREETRACE.isEnabled(); + private Throwable creationTrace; + private Throwable freeTrace; + // End debug stuff + // @GuardedBy("this") // Restore once replacement for JSR-305 available private final long ptr; private final LongConsumer releaser; @@ -33,6 +39,7 @@ private Cell(final long ptr, final LongConsumer releaser) { this.ptr = ptr; this.releaser = releaser; this.released = false; + this.creationTrace = buildFreeTrace("Created", null); } public void release() { @@ -41,6 +48,7 @@ public void release() { if (released) return; released = true; + freeTrace = buildFreeTrace("Freed", creationTrace); releaser.accept(ptr); } finally { unlock(); @@ -50,11 +58,9 @@ public void release() { public long take() { lock(); try { - if (released) { - throw new IllegalStateException("Use after free"); - } - + assertNotFreed(); released = true; + freeTrace = buildFreeTrace("Freed", creationTrace); return ptr; } finally { unlock(); @@ -78,14 +84,25 @@ public boolean isReleased() { public T use(LongFunction function) { lock(); try { - if (released) { - throw new IllegalStateException("Use after free"); - } + assertNotFreed(); return function.apply(ptr); } finally { unlock(); } } + + private void assertNotFreed() { + if (released) { + throw new IllegalStateException("Use after free", freeTrace); + } + } + + private static Throwable buildFreeTrace(final String message, final Throwable cause) { + if (!FREE_TRACE_DEBUG) { + return null; + } + return new RuntimeCryptoException(message + " in Thread " + Thread.currentThread().getName(), cause); + } } private final Cell cell; diff --git a/src/com/amazon/corretto/crypto/provider/SelfTestSuite.java b/src/com/amazon/corretto/crypto/provider/SelfTestSuite.java index 79e2852d..208e1c0e 100644 --- a/src/com/amazon/corretto/crypto/provider/SelfTestSuite.java +++ b/src/com/amazon/corretto/crypto/provider/SelfTestSuite.java @@ -67,7 +67,7 @@ public SelfTestResult runTest() { private SelfTestResult runTest0() { SelfTestResult localResult = selfTestRunner.get(); - if (localResult.getStatus() == SelfTestStatus.PASSED) { + if (localResult.getStatus() == SelfTestStatus.PASSED && DebugFlag.VERBOSELOGS.isEnabled()) { LOGGER.finer(() -> String.format("Self-test result for JCE algo %s: PASSED", getAlgorithmName())); } else { diff --git a/src/com/amazon/corretto/crypto/provider/Utils.java b/src/com/amazon/corretto/crypto/provider/Utils.java index 27fc41c9..ec3b5b7a 100644 --- a/src/com/amazon/corretto/crypto/provider/Utils.java +++ b/src/com/amazon/corretto/crypto/provider/Utils.java @@ -16,6 +16,7 @@ import java.security.spec.PKCS8EncodedKeySpec; import java.security.spec.X509EncodedKeySpec; import java.util.Arrays; +import java.util.EnumSet; import javax.crypto.Cipher; import javax.crypto.Mac; @@ -410,5 +411,29 @@ static void zeroByteBuffer(ByteBuffer buffer) { buffer.put(src); } } + + static > void optionsFromProperty( + final Class clazz, final EnumSet set, final String propertyName) { + final String propertyValue = Loader.getProperty(propertyName, ""); + if (propertyValue.equalsIgnoreCase("help")) { + System.err.format("Valid values for %s%s are: %s or ALL", + Loader.PROPERTY_BASE, propertyName, EnumSet.allOf(clazz)); + } + final String[] extraCheckOptions = propertyValue.split(","); + for (final String check : extraCheckOptions) { + if (check.equalsIgnoreCase("all")) { + set.addAll(EnumSet.allOf(clazz)); + break; + } + try { + final E value = Enum.valueOf(clazz, check.toUpperCase()); + if (value != null) { + set.add(value); + } + } catch (Exception ex) { + // Ignore + } + } + } } diff --git a/tst/com/amazon/corretto/crypto/provider/test/UtilsTest.java b/tst/com/amazon/corretto/crypto/provider/test/UtilsTest.java index 3de7b024..9dca5b81 100644 --- a/tst/com/amazon/corretto/crypto/provider/test/UtilsTest.java +++ b/tst/com/amazon/corretto/crypto/provider/test/UtilsTest.java @@ -31,7 +31,7 @@ public class UtilsTest { try { UTILS_CLASS = Class.forName("com.amazon.corretto.crypto.provider.Utils"); } catch (final ClassNotFoundException ex) { - throw new AssertionError(ex); + throw new AssertionError(ex); } } From 068787fcb90f764934a970825d42a1cd38e11194 Mon Sep 17 00:00:00 2001 From: Greg Rubin Date: Mon, 29 Nov 2021 12:31:15 -0800 Subject: [PATCH 2/6] Improve zeroization of DRBG output (#162) --- CHANGELOG.md | 4 +++- build.gradle | 2 +- csrc/rdrand.cpp | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9e17b1b..e6576545 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,6 @@ # Changelog ## 1.6.2 (Unreleased) - ### Improvements * Add "help" value to two of our properties which outputs (to STDERR) valid values. * `com.amazon.corretto.crypto.provider.extrachecks` @@ -13,6 +12,9 @@ Current values are: * `ALL` - Enables all of the above (May still require changes to your logging configuration to see the new logs.) +### Patches +* Improve zeroization of DRBG output. [PR #162](https://github.com/corretto/amazon-corretto-crypto-provider/pull/162) + ## 1.6.1 ### Patches * Fix an issue where a race condition can cause ACCP's MessageDigest hashing algorithms to return the same value for different inputs [PR #157](https://github.com/corretto/amazon-corretto-crypto-provider/pull/157) diff --git a/build.gradle b/build.gradle index 57e407e5..622d88e4 100644 --- a/build.gradle +++ b/build.gradle @@ -9,7 +9,7 @@ plugins { } group = 'software.amazon.cryptools' -version = '1.6.1' +version = '1.6.2' def openssl_version = '1.1.1j' def opensslSrcPath = "${buildDir}/openssl/openssl-${openssl_version}" diff --git a/csrc/rdrand.cpp b/csrc/rdrand.cpp index 760bb965..cb177ae2 100644 --- a/csrc/rdrand.cpp +++ b/csrc/rdrand.cpp @@ -353,7 +353,7 @@ bool rd_into_buf(bool (*rng)(uint64_t *), unsigned char *buf, int len) { } memcpy(buf, &remain, len); - secureZero(&remain, 0); + secureZero(&remain, sizeof(remain)); } #else goto fail; From b5310adf0d660dc694141e48b677e9c0db2ab44b Mon Sep 17 00:00:00 2001 From: Greg Rubin Date: Mon, 29 Nov 2021 12:51:05 -0800 Subject: [PATCH 3/6] Improve consistency checks in rsa key generation unit test (#163) * Improve consistency checks in rsa key generation unit test * Add primality checks to RSA key tests --- .../crypto/provider/test/RsaGenTest.java | 32 +++++++++++++++---- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/tst/com/amazon/corretto/crypto/provider/test/RsaGenTest.java b/tst/com/amazon/corretto/crypto/provider/test/RsaGenTest.java index 8acebe8d..d6049843 100644 --- a/tst/com/amazon/corretto/crypto/provider/test/RsaGenTest.java +++ b/tst/com/amazon/corretto/crypto/provider/test/RsaGenTest.java @@ -8,6 +8,7 @@ import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertNotNull; import java.math.BigInteger; @@ -189,19 +190,36 @@ private static void assertConsistency(final RSAPublicKey pub, final RSAPrivateCr assertNotNull(priv); assertEquals(pub.getPublicExponent(), priv.getPublicExponent()); assertNotNull(pub.getModulus()); - assertEquals(pub.getModulus(), priv.getModulus()); + BigInteger modulus = priv.getModulus(); + assertEquals(pub.getModulus(), modulus); assertNotNull(priv.getPrivateExponent()); - assertNotNull(priv.getPrimeExponentP()); - assertNotNull(priv.getPrimeExponentQ()); + assertNotNull(priv.getPrimeP()); + assertNotNull(priv.getPrimeQ()); assertNotNull(priv.getPrimeExponentP()); assertNotNull(priv.getPrimeExponentQ()); assertNotNull(priv.getCrtCoefficient()); // Do the underlying math - assertEquals(priv.getModulus(), priv.getPrimeP().multiply(priv.getPrimeQ())); - assertEquals(priv.getPrivateExponent().mod(priv.getPrimeP().subtract(BigInteger.ONE)), priv.getPrimeExponentP()); - assertEquals(priv.getPrivateExponent().mod(priv.getPrimeQ().subtract(BigInteger.ONE)), priv.getPrimeExponentQ()); - assertEquals(priv.getPrimeQ().modInverse(priv.getPrimeP()), priv.getCrtCoefficient()); + final BigInteger p = priv.getPrimeP(); + final BigInteger q = priv.getPrimeQ(); + assertTrue(p.isProbablePrime(128)); + assertTrue(p.isProbablePrime(128)); + final BigInteger d = priv.getPrivateExponent(); + final BigInteger e = priv.getPublicExponent(); + final BigInteger dp = priv.getPrimeExponentP(); + final BigInteger dq = priv.getPrimeExponentQ(); + final BigInteger qInv = priv.getCrtCoefficient(); + + final BigInteger p1 = p.subtract(BigInteger.ONE); + final BigInteger q1 = q.subtract(BigInteger.ONE); + + assertEquals(modulus, p.multiply(q)); + assertEquals(d.mod(p1), dp); + assertEquals(d.mod(q1), dq); + assertEquals(q.modInverse(p), qInv); + + final BigInteger totient = p1.multiply(q1).divide(p1.gcd(q1)); + assertEquals(BigInteger.ONE, e.multiply(d).mod(totient)); // Actually use the key final Cipher cipher = Cipher.getInstance("RSA"); From 38e0cb6a09c62a592bf4d9a774dce5fe063c1abc Mon Sep 17 00:00:00 2001 From: Greg Rubin Date: Tue, 30 Nov 2021 13:10:27 -0800 Subject: [PATCH 4/6] Add TLS1.3 to local integ tests. (#169) * Add TLS1.3 to local integ tests. Resolves #166 Also fixes "AACP" typo throughout. * Make TLS1.3 test work on JDK8 --- .../AmazonCorrettoCryptoProvider.java | 2 +- .../test/RecursiveInitializationTest.java | 4 +- .../test/ServiceSelfTestMetaTest.java | 2 +- .../test/integration/HTTPSTestParameters.java | 48 +++++++++++++++++++ .../LocalHTTPSIntegrationTest.java | 42 ++++++---------- .../test/integration/TestHTTPSServer.java | 19 ++++---- 6 files changed, 77 insertions(+), 40 deletions(-) diff --git a/src/com/amazon/corretto/crypto/provider/AmazonCorrettoCryptoProvider.java b/src/com/amazon/corretto/crypto/provider/AmazonCorrettoCryptoProvider.java index 74ea6313..22d114f3 100644 --- a/src/com/amazon/corretto/crypto/provider/AmazonCorrettoCryptoProvider.java +++ b/src/com/amazon/corretto/crypto/provider/AmazonCorrettoCryptoProvider.java @@ -211,7 +211,7 @@ private void checkTests() throws NoSuchAlgorithmException { * end up falling back. */ if (!getType().equals("SecureRandom")) { - throw new NoSuchAlgorithmException("Can't use AACP before JAR validation completes"); + throw new NoSuchAlgorithmException("Can't use ACCP before JAR validation completes"); } } diff --git a/tst/com/amazon/corretto/crypto/provider/test/RecursiveInitializationTest.java b/tst/com/amazon/corretto/crypto/provider/test/RecursiveInitializationTest.java index 2fc98436..3ffe63fe 100644 --- a/tst/com/amazon/corretto/crypto/provider/test/RecursiveInitializationTest.java +++ b/tst/com/amazon/corretto/crypto/provider/test/RecursiveInitializationTest.java @@ -21,7 +21,7 @@ /** * This test is a special case - it tests a recursive initialization path that looks like: * - * AACP.[ctor] -> Self tests -> Mac.getInstance -> JceSecurity.[static init] -> SecureRandom.[ctor] -> + * ACCP.[ctor] -> Self tests -> Mac.getInstance -> JceSecurity.[static init] -> SecureRandom.[ctor] -> * BouncyCastle.createBaseRandom -> AesCtrDrbg.[ctor] -> NJCE.[ctor] -> Self tests -> Mac.getInstance -> * JceSecurity (uninitialized) * @@ -58,7 +58,7 @@ private static void loadNonFips() throws ReflectiveOperationException { } if (!installProviderAtHighestPriority(AmazonCorrettoCryptoProvider.INSTANCE)) { - throw new RuntimeException("Failed to install AACP"); + throw new RuntimeException("Failed to install ACCP"); } } diff --git a/tst/com/amazon/corretto/crypto/provider/test/ServiceSelfTestMetaTest.java b/tst/com/amazon/corretto/crypto/provider/test/ServiceSelfTestMetaTest.java index 8620ec32..16693971 100644 --- a/tst/com/amazon/corretto/crypto/provider/test/ServiceSelfTestMetaTest.java +++ b/tst/com/amazon/corretto/crypto/provider/test/ServiceSelfTestMetaTest.java @@ -44,7 +44,7 @@ public void setUp() throws Throwable { // We need RDRAND support for a lot of these tests to work properly Assumptions.assumeTrue(AmazonCorrettoCryptoProvider.isRdRandSupported(), "RDRAND is supported"); - // AACP instances cache the self-test status within each Service, so create a new instance to clear that cache. + // ACCP instances cache the self-test status within each Service, so create a new instance to clear that cache. // This also makes sure the native library is loaded. accp = new AmazonCorrettoCryptoProvider(); } diff --git a/tst/com/amazon/corretto/crypto/provider/test/integration/HTTPSTestParameters.java b/tst/com/amazon/corretto/crypto/provider/test/integration/HTTPSTestParameters.java index f580eef6..fbd8f0c7 100644 --- a/tst/com/amazon/corretto/crypto/provider/test/integration/HTTPSTestParameters.java +++ b/tst/com/amazon/corretto/crypto/provider/test/integration/HTTPSTestParameters.java @@ -11,6 +11,9 @@ import java.util.Map; public class HTTPSTestParameters { + static final String PROTOCOL_TLS_1_2 = "TLSv1.2"; + static final String PROTOCOL_TLS_1_3 = "TLSv1.3"; + // Map of key algorithm ("RSA", "ECDSA", "DSA") to supported key sizes private static final Map> ALGO_TO_KEY_BITS; @@ -65,4 +68,49 @@ public static List keySizesForSignatureMethod(String signatureMethod) { static String getKeyType(String signatureMethod) { return signatureMethod.replaceAll(".*with", ""); } + + static String protocolFromSuite(final String cipherSuite) { + // We only test on 1.3 and 1.2 for now. + // Everything older is being deprecated and none of our crypto + // should do anything different for older versions anyway. + switch (cipherSuite) { + case "TLS_AES_128_GCM_SHA256": + case "TLS_AES_256_GCM_SHA384": + case "TLS_CHACHA20_POLY1305_SHA256": + case "TLS_AES_128_CCM_SHA256": + case "TLS_AES_128_CCM_8_SHA256": + return PROTOCOL_TLS_1_3; + default: + return PROTOCOL_TLS_1_2; + } + } + + /** + * Returns {@code true} iff the TLS ciphersuite {@code suite} can be used with + * certificates signed using + * {@code signature}. + * + * @param suite the TLS ciphersuite + * @param signatureMethod the algorithm used to sign the TLS certificate + * @returns true if certificates signed by {@code signature} can be used with + * {@code suite}} + * @see https://docs.oracle.com/en/java/javase/17/docs/specs/security/standard-names.html + */ + static boolean suiteMatchesSignature(final String suite, final String signatureMethod) { + final String keyType = getKeyType(signatureMethod); + + // TLS 1.3 only supports RSA and ECDSA certificates + if (protocolFromSuite(suite).equals(PROTOCOL_TLS_1_3) + && (keyType.equals("RSA") || keyType.equals("ECDSA"))) { + return true; + } + + // DSA is called DSS in ciphersuites + if (keyType.equals("DSA")) { + return suite.contains("DSS"); + } + + // Otherwise, everything matches up + return suite.contains(keyType); + } } diff --git a/tst/com/amazon/corretto/crypto/provider/test/integration/LocalHTTPSIntegrationTest.java b/tst/com/amazon/corretto/crypto/provider/test/integration/LocalHTTPSIntegrationTest.java index d1209851..21b86a76 100644 --- a/tst/com/amazon/corretto/crypto/provider/test/integration/LocalHTTPSIntegrationTest.java +++ b/tst/com/amazon/corretto/crypto/provider/test/integration/LocalHTTPSIntegrationTest.java @@ -89,21 +89,8 @@ public static Object[][] data() throws Exception { continue; } - String keyAlgorithm; - - if (cipherSuite.contains("ECDSA")) { - keyAlgorithm = "ECDSA"; - } else if (cipherSuite.contains("DSS")) { - keyAlgorithm = "DSA"; - } else if (cipherSuite.contains("RSA")) { - keyAlgorithm = "RSA"; - } else { - // unsupported - continue; - } - for (String method : SIGNATURE_METHODS_TO_TEST) { - if (!method.endsWith("with" + keyAlgorithm)) { + if (!HTTPSTestParameters.suiteMatchesSignature(cipherSuite, method)) { // We generate our server certificates in such a way that the key type in the server certificate // matches the key type used to sign the certificate. As such, this key type must _also_ match // the key required by the cipher suite in use. We can't use a server cert showing a DH public key @@ -114,7 +101,7 @@ public static Object[][] data() throws Exception { List keySizes = HTTPSTestParameters.keySizesForSignatureMethod(method); for (int size: keySizes) { - // boolean flags: AACP on server, BC on client + // boolean flags: ACCP on server, BC on client params.add(new Object[] { true, true, cipherSuite, method, size }); params.add(new Object[] { false, true, cipherSuite, method, size }); params.add(new Object[] { true, false, cipherSuite, method, size }); @@ -127,11 +114,11 @@ public static Object[][] data() throws Exception { } private static TrustManagerFactory trustManagerFactory; - private static TestHTTPSServer withAACP, withoutAACP; + private static TestHTTPSServer withACCP, withoutACCP; @BeforeAll public static void launchServer() throws Exception { - // Do this before setting up providers, as loading BC early in the provider chain (even without AACP) breaks + // Do this before setting up providers, as loading BC early in the provider chain (even without ACCP) breaks // KeyStore. KeyStore keyStore = KeyStore.getInstance("JKS"); try (InputStream is = TestHTTPSServer.class.getResourceAsStream("test_CA.jks")) { @@ -140,26 +127,26 @@ public static void launchServer() throws Exception { trustManagerFactory = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); trustManagerFactory.init(keyStore); - withoutAACP = TestHTTPSServer.launch(false); + withoutACCP = TestHTTPSServer.launch(false); try { - withAACP = TestHTTPSServer.launch(true); + withACCP = TestHTTPSServer.launch(true); } catch (Throwable t) { - withoutAACP.kill(); + withoutACCP.kill(); throw t; } } @AfterAll public static void shutdown() { - withoutAACP.kill(); - withAACP.kill(); + withoutACCP.kill(); + withACCP.kill(); } @BeforeEach public void setup() throws Exception { resetProviders(); - if (!withoutAACP.isAlive() || !withAACP.isAlive()) { + if (!withoutACCP.isAlive() || !withACCP.isAlive()) { fail("Server died"); } @@ -174,14 +161,14 @@ public void cleanup() throws Exception { resetProviders(); } - @ParameterizedTest(name = "ServerAACPEnabled({0}) BCEnabled({1}) Suite({2}) SignatureType({3}) KeyBits({4})") + @ParameterizedTest(name = "ServerACCPEnabled({0}) BCEnabled({1}) Suite({2}) SignatureType({3}) KeyBits({4})") @MethodSource("data") - public void test(boolean serverAACPEnabled, boolean bcEnabled, String suite, String signatureType, int keyBits) throws Exception { + public void test(boolean serverACCPEnabled, boolean bcEnabled, String suite, String signatureType, int keyBits) throws Exception { if (bcEnabled) { Security.insertProviderAt(new BouncyCastleProvider(), 2); } - int port = serverAACPEnabled ? withAACP.getPort() : withoutAACP.getPort(); + int port = serverACCPEnabled ? withACCP.getPort() : withoutACCP.getPort(); HttpsURLConnection conn = (HttpsURLConnection) new URL("https://127.0.0.1:" + port).openConnection(); // this has the side effect of disabling the default SNI logic @@ -190,7 +177,7 @@ public void test(boolean serverAACPEnabled, boolean bcEnabled, String suite, Str // actual URL hostname conn.setHostnameVerifier((hostname, session) -> true); - SSLContext context = SSLContext.getInstance("TLS"); + SSLContext context = SSLContext.getInstance(HTTPSTestParameters.protocolFromSuite(suite)); context.init(null, trustManagerFactory.getTrustManagers(), null); @@ -216,6 +203,7 @@ public void test(boolean serverAACPEnabled, boolean bcEnabled, String suite, Str SSLParameters parameters = socket.getSSLParameters(); parameters.setEndpointIdentificationAlgorithm("HTTPS"); parameters.setServerNames(singletonList(new SNIHostName(signatureType + "." + keyBits))); + parameters.setProtocols(new String[] { HTTPSTestParameters.protocolFromSuite(suite) }); socket.setSSLParameters(parameters); diff --git a/tst/com/amazon/corretto/crypto/provider/test/integration/TestHTTPSServer.java b/tst/com/amazon/corretto/crypto/provider/test/integration/TestHTTPSServer.java index 17d4f326..8e59a37c 100644 --- a/tst/com/amazon/corretto/crypto/provider/test/integration/TestHTTPSServer.java +++ b/tst/com/amazon/corretto/crypto/provider/test/integration/TestHTTPSServer.java @@ -166,7 +166,7 @@ private static void runServer() throws Exception { TrustManagerFactory tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); tmf.init(keyStore); - SSLContext sslContext = SSLContext.getInstance("TLS"); + SSLContext sslContext = SSLContext.getInstance("TLSv1.3"); sslContext.init(new KeyManager[] { new SNIKeyManager() }, tmf.getTrustManagers(), null); HttpsServer server = HttpsServer.create(); @@ -175,18 +175,19 @@ private static void runServer() throws Exception { SSLEngine sampleEngine = SSLContext.getDefault().createSSLEngine(); String[] cipherSuites = sampleEngine.getSupportedCipherSuites(); String[] protocols = sampleEngine.getSupportedProtocols(); - SSLParameters defaultParams = SSLContext.getDefault().getDefaultSSLParameters(); + SSLParameters sslParams = SSLContext.getDefault().getDefaultSSLParameters(); + sslParams.setNeedClientAuth(false); + sslParams.setWantClientAuth(false); + sslParams.setProtocols(protocols); + // We'll let the client decide which protocols and cipher suites to test, by enabling support + // for _everything_, including ones that are obviously a bad idea. + sslParams.setCipherSuites(cipherSuites); server.setHttpsConfigurator( new HttpsConfigurator(sslContext) { @Override public void configure(HttpsParameters params) { - params.setSSLParameters(defaultParams); - - // We'll let the client decide which protocols and cipher suites to test, by enabling support - // for _everything_, including ones that are obviously a bad idea. - params.setCipherSuites(cipherSuites); - params.setProtocols(protocols); - params.setNeedClientAuth(false); + // Setting the SSL parameters causes everything else to be ignored. + params.setSSLParameters(sslParams); } } ); From fcbf0089e2f192deb135aa6d37505f9695e7886e Mon Sep 17 00:00:00 2001 From: Will Childs-Klein Date: Thu, 6 Jan 2022 22:42:07 -0500 Subject: [PATCH 5/6] Update openssl to 1.1.1m --- CHANGELOG.md | 3 ++- README.md | 2 +- build.gradle | 4 ++-- openssl.sha256 | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6576545..44b1df0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,8 @@ # Changelog -## 1.6.2 (Unreleased) +## 1.7.0 (Unreleased) ### Improvements +* Now uses [OpenSSL 1.1.1m](https://www.openssl.org/source/openssl-1.1.1m.tar.gz). [PR #173](https://github.com/corretto/amazon-corretto-crypto-provider/pull/173) * Add "help" value to two of our properties which outputs (to STDERR) valid values. * `com.amazon.corretto.crypto.provider.extrachecks` * `com.amazon.corretto.crypto.provider.debug` diff --git a/README.md b/README.md index fcb399f1..78705021 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ The Amazon Corretto Crypto Provider (ACCP) is a collection of high-performance cryptographic implementations exposed via the standard [JCA/JCE](https://docs.oracle.com/en/java/javase/11/security/java-cryptography-architecture-jca-reference-guide.html) interfaces. This means that it can be used as a drop in replacement for many different Java applications. (Differences from the default OpenJDK implementations are [documented here](./DIFFERENCES.md).) -Currently algorithms are primarily backed by OpenSSL's implementations (1.1.1j as of ACCP 1.6.0) but this may change in the future. +Currently algorithms are primarily backed by OpenSSL's implementations (1.1.1m as of ACCP 1.7.0) but this may change in the future. [Security issue notifications](./CONTRIBUTING.md#security-issue-notifications) diff --git a/build.gradle b/build.gradle index 622d88e4..6a19ebcc 100644 --- a/build.gradle +++ b/build.gradle @@ -9,9 +9,9 @@ plugins { } group = 'software.amazon.cryptools' -version = '1.6.2' +version = '1.7.0' -def openssl_version = '1.1.1j' +def openssl_version = '1.1.1m' def opensslSrcPath = "${buildDir}/openssl/openssl-${openssl_version}" configurations { diff --git a/openssl.sha256 b/openssl.sha256 index 65e92eaa..ab7f2b9f 100644 --- a/openssl.sha256 +++ b/openssl.sha256 @@ -1 +1 @@ -aaf2fcb575cdf6491b98ab4829abf78a3dec8402b8b81efc8f23c00d443981bf openssl-1.1.1j.tar.gz +f89199be8b23ca45fc7cb9f1d8d3ee67312318286ad030f5316aca6462db6c96 openssl-1.1.1m.tar.gz From 80b3fa918a4787dd5d1d4e3b2451f7e0e5ae5aad Mon Sep 17 00:00:00 2001 From: Greg Rubin Date: Tue, 11 Jan 2022 15:25:17 -0800 Subject: [PATCH 6/6] Add property to skip bundled lib and improve docs. (#168) * Add property to skip bundled lib and improve docs. * Enables skipping the bundled lib by setting the system property `com.amazon.corretto.crypto.provider.useExternalLib`. (Addresses #140) * Documents all currently defined system properties. (Addresses #83) * Add coverage to more tests * Add documentation for "help" option * Reorder readme sections * Add Changelog message for externalLib --- CHANGELOG.md | 1 + CMakeLists.txt | 28 +++++++++++++-- README.md | 29 +++++++++++++++- .../corretto/crypto/provider/Loader.java | 34 +++++++++++++------ 4 files changed, 79 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44b1df0d..dd829cc0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Current values are: * `VerboseLogging` - Enables more detailed logging. * `ALL` - Enables all of the above (May still require changes to your logging configuration to see the new logs.) +* Enables skipping the bundled lib by setting the system property `com.amazon.corretto.crypto.provider.useExternalLib` [PR #168](https://github.com/corretto/amazon-corretto-crypto-provider/pull/168) ### Patches * Improve zeroization of DRBG output. [PR #162](https://github.com/corretto/amazon-corretto-crypto-provider/pull/162) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8a686682..50979903 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -603,8 +603,12 @@ if (TEST_JAVA_MAJOR_VERSION VERSION_GREATER_EQUAL 17) ) endif() -set(TEST_RUNNER_ARGUMENTS +set(COVERAGE_ARGUMENTS -javaagent:${JACOCO_AGENT_JAR}=destfile=coverage/jacoco.exec,classdumpdir=coverage/classes +) + +set(TEST_RUNNER_ARGUMENTS + ${COVERAGE_ARGUMENTS} ${TEST_ADD_OPENS} -Djava.library.path=$ -Dcom.amazon.corretto.crypto.provider.inTestSuite=hunter2 @@ -667,6 +671,7 @@ add_custom_target(check-junit-extra-checks add_custom_target(check-recursive-init COMMAND ${TEST_JAVA_EXECUTABLE} + ${COVERAGE_ARGUMENTS} -cp $:$:${TEST_CLASSPATH} -Djava.library.path=$ -Dcom.amazon.corretto.crypto.provider.inTestSuite=hunter2 @@ -678,8 +683,25 @@ add_custom_target(check-recursive-init add_custom_target(check-install-via-properties COMMAND ${TEST_JAVA_EXECUTABLE} + ${COVERAGE_ARGUMENTS} + -cp $:$:${TEST_CLASSPATH} + -Djava.library.path=$ + -Dcom.amazon.corretto.crypto.provider.inTestSuite=hunter2 + -Dtest.data.dir=${TEST_DATA_DIR} + -Djava.security.properties=${ORIG_SRCROOT}/etc/amazon-corretto-crypto-provider.security + ${TEST_JAVA_ARGS} + com.amazon.corretto.crypto.provider.test.SecurityPropertyTester + + DEPENDS accp-jar tests-jar) + +add_custom_target(check-external-lib + # Unfortunately we do not have a way to know where the library is loaded from. + # So this test just proves that requesting the external lib does not break things + COMMAND ${TEST_JAVA_EXECUTABLE} + ${COVERAGE_ARGUMENTS} -cp $:$:${TEST_CLASSPATH} -Djava.library.path=$ + -Dcom.amazon.corretto.crypto.provider.useExternalLib=true -Dcom.amazon.corretto.crypto.provider.inTestSuite=hunter2 -Dtest.data.dir=${TEST_DATA_DIR} -Djava.security.properties=${ORIG_SRCROOT}/etc/amazon-corretto-crypto-provider.security @@ -690,6 +712,7 @@ add_custom_target(check-install-via-properties add_custom_target(check-install-via-properties-recursive COMMAND ${TEST_JAVA_EXECUTABLE} + ${COVERAGE_ARGUMENTS} -cp $:$:${TEST_CLASSPATH} -Djava.library.path=$ -Dcom.amazon.corretto.crypto.provider.inTestSuite=hunter2 @@ -702,6 +725,7 @@ add_custom_target(check-install-via-properties-recursive add_custom_target(check-install-via-properties-with-debug COMMAND ${TEST_JAVA_EXECUTABLE} + ${COVERAGE_ARGUMENTS} -cp $:$:${TEST_CLASSPATH} -Djava.library.path=$ -Dcom.amazon.corretto.crypto.provider.inTestSuite=hunter2 @@ -714,7 +738,7 @@ add_custom_target(check-install-via-properties-with-debug DEPENDS accp-jar tests-jar) add_custom_target(check - DEPENDS check-recursive-init check-install-via-properties check-install-via-properties-with-debug check-junit check-junit-SecurityManager) + DEPENDS check-recursive-init check-install-via-properties check-install-via-properties-with-debug check-junit check-junit-SecurityManager check-external-lib) if(ENABLE_NATIVE_TEST_HOOKS) if (CMAKE_SYSTEM_PROCESSOR STREQUAL "x86_64") diff --git a/README.md b/README.md index 78705021..050b213d 100644 --- a/README.md +++ b/README.md @@ -120,7 +120,7 @@ For more information, please see [VERSIONING.rst](https://github.com/corretto/am ### Gradle Add the following to your `build.gradle` file. If you already have a `dependencies` block in your `build.gradle`, you can add the ACCP line to your -existing block. +existing block. This will instruct it to use the most recent 1.x version of ACCP. For more information, please see [VERSIONING.rst](https://github.com/corretto/amazon-corretto-crypto-provider/blob/develop/VERSIONING.rst). @@ -205,6 +205,33 @@ We generally do not recommend this solution as we believe that gracefully fallin AmazonCorrettoCryptoProvider.INSTANCE.assertHealthy(); ``` +### Other system properties +ACCP can be configured via several system properties. +None of these should be needed for standard deployments and we recommend not touching them. +They are of most use to developers needing to test ACCP or experiment with benchmarking. +These are all read early in the load process and may be cached so any changes to them made from within Java may not be respected. +Thus, these should all be set on the JVM command line using `-D`. + +* `com.amazon.corretto.crypto.provider.extrachecks` + Adds exta cryptographic consistency checks which are not necessary on standard systems. + These checks may be computationally expensive and are not normally relevant. + See `ExtraCheck.java` for values and more information. + (Also accepts "ALL" as a value to enable all flags and "help" to print out all flags to STDERR.) +* `com.amazon.corretto.crypto.provider.debug` + Enables extra debugging behavior. + These behaviors may be computationally expensive, produce additional output, or otherwise change the behavior of ACCP. + No values here will lower the security of ACCP or cause it to give incorrect results. + See `DebugFlag.java` for values and more information. + (Also accepts "ALL" as a value to enable all flags and "help" to print out all flags to STDERR.) +* `com.amazon.corretto.crypto.provider.useExternalLib` + Takes in `true` or `false` (defaults to `false`). + If `true` then ACCP skips trying to load the native library bundled within its JAR and goes directly to the system library path. +* `com.amazon.corretto.crypto.provider.janitor.stripes` + Takes *positive integer value* which is the requested minimum number of "stripes" used by the `Janitor` for dividing cleaning tasks (messes) among its workers. + (Current behavior is to default this value to 4 times the CPU core count and then round the value up to the nearest power of two.) + See `Janitor.java` for for more information. + + # License This library is licensed under the Apache 2.0 license although portions of this product include software licensed under the [dual OpenSSL and SSLeay license](https://www.openssl.org/source/license.html). diff --git a/src/com/amazon/corretto/crypto/provider/Loader.java b/src/com/amazon/corretto/crypto/provider/Loader.java index d9cc5165..48f8bf0d 100644 --- a/src/com/amazon/corretto/crypto/provider/Loader.java +++ b/src/com/amazon/corretto/crypto/provider/Loader.java @@ -122,13 +122,19 @@ static String getProperty(String propertyName, String def) { FileSystems.getDefault(); // First, try to find the library in our own jar + final boolean useExternalLib = Boolean.valueOf(getProperty("useExternalLib", "false")); + Exception loadingException = null; + String libraryName = System.mapLibraryName(LIBRARY_NAME); - if (libraryName != null) { + if (useExternalLib) { + loadingException = new RuntimeCryptoException("Skipping bundled library due to system property"); + } else if (libraryName != null) { int index = libraryName.lastIndexOf('.'); final String prefix = libraryName.substring(0, index); final String suffix = libraryName.substring(index, libraryName.length()); final Path libPath = createTmpFile(prefix, suffix); + try (final InputStream is = Loader.class.getResourceAsStream(libraryName); final OutputStream os = Files.newOutputStream(libPath, StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING)) { @@ -141,19 +147,27 @@ static String getProperty(String propertyName, String def) { os.flush(); System.load(libPath.toAbsolutePath().toString()); return true; + } catch (final Exception realException) { + loadingException = realException; } catch (final Throwable realError) { - // We failed to load the library from our JAR but don't know why. - // Try to load it directly off of the system path. - try { - System.loadLibrary(LIBRARY_NAME); - return true; - } catch (final Throwable suppressedError) { - realError.addSuppressed(suppressedError); - throw realError; - } + loadingException = new RuntimeCryptoException("Unable to load native library", realError); } finally { Files.delete(libPath); } + } else { + loadingException = new RuntimeCryptoException("Skipping bundled library null mapped name"); + } + + if (loadingException != null) { + // We failed to load the library from our JAR but don't know why. + // Try to load it directly off of the system path. + try { + System.loadLibrary(LIBRARY_NAME); + return true; + } catch (final Throwable suppressedError) { + loadingException.addSuppressed(suppressedError); + throw loadingException; + } } return false;