From e5a5aff76b6a78c1b6083548e23546ee6062ac85 Mon Sep 17 00:00:00 2001 From: Alex Weibel Date: Mon, 20 Jun 2022 17:18:31 -0700 Subject: [PATCH 01/13] Dynamically load ACCP with AWS-LC using RPath --- CMakeLists.txt | 27 +- build.gradle | 14 +- csrc/accp_lc_loader.cpp | 76 ------ csrc/loader.cpp | 24 +- .../corretto/crypto/provider/AesGcmSpi.java | 1 + .../AmazonCorrettoCryptoProvider.java | 9 +- .../corretto/crypto/provider/EcGen.java | 1 + .../corretto/crypto/provider/EvpKey.java | 1 + .../crypto/provider/EvpKeyFactory.java | 1 + .../crypto/provider/LibCryptoRng.java | 1 + .../corretto/crypto/provider/Loader.java | 236 +++++++++++------- 11 files changed, 206 insertions(+), 185 deletions(-) delete mode 100644 csrc/accp_lc_loader.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 6a6af3a7..c210ab22 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -86,7 +86,8 @@ string(REPLACE ":" ";" TEST_CLASSPATH_LIST "${TEST_CLASSPATH}") # Needed as we abuse some of the test compile macros to test shared lib links set(CMAKE_POSITION_INDEPENDENT_CODE ON) - +# CMake's default RPath handling includes the full build directory path, which isn't needed. Turn it off. +set(CMAKE_SKIP_RPATH TRUE) set(GENERATE_HASHERS ${CMAKE_CURRENT_SOURCE_DIR}/build-tools/bin/generate-java-hash-spi) set(GENERATED_JAVA_DIR ${CMAKE_CURRENT_BINARY_DIR}/generated-java/com/amazon/corretto/crypto/provider) set(JNI_HEADER_DIR ${CMAKE_CURRENT_BINARY_DIR}/generated-include) @@ -233,7 +234,6 @@ else() COMMAND ${CMAKE_COMMAND} -E make_directory ${CMAKE_CURRENT_BINARY_DIR}/tmplib/com/amazon/corretto/crypto/provider/ COMMAND ${CMAKE_COMMAND} -E copy ${OPENSSL_CRYPTO_LIBRARY} ${CMAKE_CURRENT_BINARY_DIR}/tmplib/com/amazon/corretto/crypto/provider/ COMMAND ${CMAKE_COMMAND} -E copy $ ${CMAKE_CURRENT_BINARY_DIR}/tmplib/com/amazon/corretto/crypto/provider/ - COMMAND ${CMAKE_COMMAND} -E copy $ ${CMAKE_CURRENT_BINARY_DIR}/tmplib/com/amazon/corretto/crypto/provider/ COMMAND ${Java_JAR_EXECUTABLE} uf ${ACCP_JAR_TMP} -C ${CMAKE_CURRENT_BINARY_DIR}/tmplib/ com/amazon/corretto/crypto/provider/ COMMAND ${Java_JAR_EXECUTABLE} uf ${ACCP_JAR_TMP} -C ${CMAKE_CURRENT_BINARY_DIR}/generated-java/ com/amazon/corretto/crypto/provider/version.properties COMMAND ${CMAKE_COMMAND} -E copy ${ACCP_JAR_TMP} ${ACCP_JAR} @@ -266,12 +266,6 @@ ADD_CUSTOM_COMMAND( ### Native library configuration include_directories(${OPENSSL_INCLUDE_DIR} ${JNI_INCLUDE_DIRS} ${JNI_HEADER_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/src/cpp) -add_library( - accpLcLoader SHARED - csrc/accp_lc_loader.cpp - ${JNI_HEADER_DIR}/generated-headers.h -) - add_library( amazonCorrettoCryptoProvider SHARED csrc/aes_gcm.cpp @@ -300,6 +294,11 @@ add_library( ${JNI_HEADER_DIR}/generated-headers.h ) +# We MUST set ACCP's RPATH to "$ORIGIN" so that the runtime dynamic loader will look in the same directory as ACCP for +# any shared library dependencies BEFORE looking for a system-installed depencencies (such as libcrypto). If RPath is +# not set, the loader may attempt to load an incompatible system-installed libcrypto and cause runtime crashes. +set_target_properties(amazonCorrettoCryptoProvider PROPERTIES LINK_FLAGS "-Wl,-rpath,\$ORIGIN") + add_custom_command( OUTPUT ${ACCP_JAR_SOURCE} COMMAND ${Java_JAR_EXECUTABLE} cf ${ACCP_JAR_SOURCE} -C ${CMAKE_CURRENT_SOURCE_DIR}/src . @@ -373,7 +372,6 @@ CHECK_LIBRARY_EXISTS(m "sqrt" "" HAVE_LIBM) if(HAVE_LIBDL) target_link_libraries(amazonCorrettoCryptoProvider dl) - target_link_libraries(accpLcLoader dl) endif() if(HAVE_LIBM) @@ -460,9 +458,6 @@ int main(int, char **) noexcept {} set(CMAKE_REQUIRED_FLAGS "${OLD_CMAKE_REQUIRED_FLAGS}") set(CMAKE_EXE_LINKER_FLAGS "${OLD_CMAKE_EXE_LINKER_FLAGS}") -# We do not declare an explicit on libcrypto to ensure we're using our bundled version. -# If this ends up being a problem on some platforms we can figure out a different way to manage this. - # Miscellaneous linker flag tests. Unfortunately cmake doesn't have built-in # functionality for a linker flag test, so we have to roll our own. @@ -545,11 +540,13 @@ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DPROVIDER_VERSION_STRING=${PROVIDER_VER string(STRIP "${PROBED_LINKED_FLAGS}" PROBED_LINKED_FLAGS) MESSAGE(STATUS "probed flags ${PROBED_LINKED_FLAGS}") target_link_libraries(amazonCorrettoCryptoProvider ${PROBED_LINKED_FLAGS}) -target_link_libraries(accpLcLoader ${PROBED_LINKED_FLAGS}) # Add pthread support target_link_libraries(amazonCorrettoCryptoProvider Threads::Threads) +# Take a dependency on our libcrypto.so file +target_link_libraries(amazonCorrettoCryptoProvider ${OPENSSL_CRYPTO_LIBRARY}) + ## Tests add_jar( tests-code-jar @@ -642,7 +639,7 @@ else() endif() if(ALWAYS_ALLOW_EXTERNAL_LIB) - set(EXTERNAL_LIB_PROPERTY "-Djava.library.path=$:$") + set(EXTERNAL_LIB_PROPERTY "-Djava.library.path=$") endif() set(TEST_RUNNER_ARGUMENTS @@ -743,7 +740,7 @@ add_custom_target(check-external-lib ${TEST_FIPS_PROPERTY} -cp $:$:${TEST_CLASSPATH} # Since this tests external loading we always provide this property - -Djava.library.path=$:$ + -Djava.library.path=$ -Dcom.amazon.corretto.crypto.provider.useExternalLib=true -Dcom.amazon.corretto.crypto.provider.inTestSuite=hunter2 -Dtest.data.dir=${TEST_DATA_DIR} diff --git a/build.gradle b/build.gradle index 99cfc7b9..6c46916d 100644 --- a/build.gradle +++ b/build.gradle @@ -91,6 +91,16 @@ task buildAwsLc { } } +// Need to copy libcrypto to same directory where we will be building libACCP, so that we can run ACCP tests with +// "-Djava.library.path=${buildDir}/cmake" and "-Dcom.amazon.corretto.crypto.provider.useExternalLib=true" flags +task copyLibCryptoForTests(type: Copy) { + dependsOn buildAwsLc + from("${buildDir}/awslc/bin/lib") { + include 'libcrypto.*' + } + into "${buildDir}/cmake" +} + task executeCmake(type: Exec) { outputs.dir("${buildDir}/cmake") inputs.dir("${buildDir}/awslc/bin/") @@ -103,9 +113,10 @@ task executeCmake(type: Exec) { inputs.dir("${projectDir}/test-data") inputs.dir("${projectDir}/template-src") - dependsOn buildAwsLc + dependsOn buildAwsLc, copyLibCryptoForTests workingDir "${buildDir}/cmake" + def prebuiltJar = null if (project.hasProperty('stagingUrl')) { mkdir "${buildDir}/tmp" @@ -123,6 +134,7 @@ task executeCmake(type: Exec) { args "-DOPENSSL_ROOT_DIR=${buildDir}/awslc/bin", '-DCMAKE_BUILD_TYPE=Release', '-DPROVIDER_VERSION_STRING=' + version args "-DTEST_RUNNER_JAR=${configurations.testRunner.singleFile}" args "-DCHECKSTYLE_CP=${configurations.checkstyle.asPath}" + args "-DCMAKE_VERBOSE_MAKEFILE:BOOL=ON" if (isFips) { args "-DFIPS=ON" } diff --git a/csrc/accp_lc_loader.cpp b/csrc/accp_lc_loader.cpp deleted file mode 100644 index c2753faf..00000000 --- a/csrc/accp_lc_loader.cpp +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -#include "generated-headers.h" -#include "compiler.h" // This is minimal so is safe to include -#include -#include - -#ifndef AWSLC_MANGLE -#define AWSLC_MANGLE -#endif - -// The function in the symbol table we use to recognize that libcrypto has been loaded. -#define CANARY_FUNCTION_NAME STRINGIFY(AWSLC_MANGLE) "CRYPTO_library_init" - -// We're purposefully taking no standard ACCP dependencies in this file -// so that it is minimal and stand alone. -namespace { - void throw_java_exception(JNIEnv* env, const char* className, const char* message) { - if (env->ExceptionCheck()) { - // There already is a queued exception - return; - } - jclass ex_class = env->FindClass(className); - // If ex_class is null, then java implicitly threw a ClassDefNotFoundError - if (ex_class != nullptr) - { - int rv = env->ThrowNew(ex_class, message); - if (rv != 0) { - env->FatalError("ThrowNew returned error"); - abort(); - } - } - } - - // Returns nullptr if the library is properly loaded. - // Otherwise returns a string containing the error seen when trying to access CANARY_FUNCTION_NAME. - // This string does *not* need to be freed. - char* libraryLoadError() { - dlerror(); // Clear errors - dlsym(RTLD_DEFAULT, CANARY_FUNCTION_NAME); // Return value doesn't matter - return dlerror(); // Did the lookup succeed? - } -} - -JNIEXPORT jboolean JNICALL Java_com_amazon_corretto_crypto_provider_Loader_loadLibCrypto( - JNIEnv* pEnv, jclass, jstring libPath) -{ - // Have we already loaded a properly mangled version of AWS-LC? - if (libraryLoadError() == nullptr) { - // Already loaded - return JNI_FALSE; // We didn't do anything - } - - // We need to load it now - if (libPath == nullptr) { - throw_java_exception(pEnv, "java/lang/NullPointerException", "Library file was null"); - return JNI_FALSE; - } - const char* nativePath = pEnv->GetStringUTFChars(libPath, NULL); - void* libPtr = dlopen(nativePath, RTLD_LAZY | RTLD_GLOBAL); - // Immediately release the string regardless of if we were successful - pEnv->ReleaseStringUTFChars(libPath, nativePath); - - if (libPtr == nullptr) { - throw_java_exception(pEnv, "com/amazon/corretto/crypto/provider/RuntimeCryptoException", dlerror()); - return JNI_FALSE; - } - char* loadError = libraryLoadError(); - if (loadError != nullptr) { - throw_java_exception(pEnv, "com/amazon/corretto/crypto/provider/RuntimeCryptoException", loadError); - return JNI_FALSE; - } - - return JNI_TRUE; -} diff --git a/csrc/loader.cpp b/csrc/loader.cpp index 0cd884c7..e13bb588 100644 --- a/csrc/loader.cpp +++ b/csrc/loader.cpp @@ -20,7 +20,6 @@ // Right now we only support PTHREAD #include - using namespace AmazonCorrettoCryptoProvider; namespace { @@ -60,3 +59,26 @@ JNIEXPORT jstring JNICALL Java_com_amazon_corretto_crypto_provider_Loader_getNat } } + + +JNIEXPORT jboolean JNICALL Java_com_amazon_corretto_crypto_provider_Loader_validateLibcryptoExactVersionMatch(JNIEnv* pEnv, jclass) +{ + char msg_buffer[256] = {0}; + + try { + const unsigned long libcrypto_compiletime_version = OPENSSL_VERSION_NUMBER; + const unsigned long libcrypto_runtime_version = OpenSSL_version_num(); + + if (libcrypto_compiletime_version != libcrypto_runtime_version) { + snprintf(msg_buffer, sizeof(msg_buffer), "Runtime libcrypto version does not match compile-time version. " + "Expected: 0x%08lX , Actual: 0x%08lX", libcrypto_compiletime_version, libcrypto_runtime_version); + throw java_ex(EX_RUNTIME_CRYPTO, msg_buffer); + } + + return JNI_TRUE; + } catch (java_ex &ex) { + ex.throw_to_java(pEnv); + } + + return JNI_FALSE; +} diff --git a/src/com/amazon/corretto/crypto/provider/AesGcmSpi.java b/src/com/amazon/corretto/crypto/provider/AesGcmSpi.java index cbc921b4..bf23838e 100644 --- a/src/com/amazon/corretto/crypto/provider/AesGcmSpi.java +++ b/src/com/amazon/corretto/crypto/provider/AesGcmSpi.java @@ -211,6 +211,7 @@ private static native int encryptDoFinal( private final AccessibleByteArrayOutputStream decryptAADBuf = new AccessibleByteArrayOutputStream(); AesGcmSpi(final AmazonCorrettoCryptoProvider provider) { + Loader.checkNativeLibraryAvailability(); this.provider = provider; } diff --git a/src/com/amazon/corretto/crypto/provider/AmazonCorrettoCryptoProvider.java b/src/com/amazon/corretto/crypto/provider/AmazonCorrettoCryptoProvider.java index edee4e3d..660215c3 100644 --- a/src/com/amazon/corretto/crypto/provider/AmazonCorrettoCryptoProvider.java +++ b/src/com/amazon/corretto/crypto/provider/AmazonCorrettoCryptoProvider.java @@ -266,15 +266,16 @@ public AmazonCorrettoCryptoProvider() { Utils.optionsFromProperty(ExtraCheck.class, extraChecks, "extrachecks"); - if (!Loader.IS_AVAILABLE && DebugFlag.VERBOSELOGS.isEnabled()) { - getLogger("AmazonCorrettoCryptoProvider").fine("Native JCE libraries are unavailable - disabling"); + if (!Loader.IS_AVAILABLE) { + if (DebugFlag.VERBOSELOGS.isEnabled()) { + getLogger("AmazonCorrettoCryptoProvider").fine("Native JCE libraries are unavailable - disabling"); + } - // Don't implement anything + // If Loading failed, do not register any algorithms return; } buildServiceMap(); - initializeSelfTests(); } diff --git a/src/com/amazon/corretto/crypto/provider/EcGen.java b/src/com/amazon/corretto/crypto/provider/EcGen.java index b638315c..0ff53eca 100644 --- a/src/com/amazon/corretto/crypto/provider/EcGen.java +++ b/src/com/amazon/corretto/crypto/provider/EcGen.java @@ -62,6 +62,7 @@ protected NativeParams initialValue() { private ECInfo ecInfo = null; EcGen(AmazonCorrettoCryptoProvider provider) { + Loader.checkNativeLibraryAvailability(); provider_ = provider; } diff --git a/src/com/amazon/corretto/crypto/provider/EvpKey.java b/src/com/amazon/corretto/crypto/provider/EvpKey.java index 0d177770..d02cac46 100644 --- a/src/com/amazon/corretto/crypto/provider/EvpKey.java +++ b/src/com/amazon/corretto/crypto/provider/EvpKey.java @@ -36,6 +36,7 @@ abstract class EvpKey implements Key, Destroyable { protected static native byte[] getDerEncodedParams(long ptr); EvpKey(final InternalKey key, final EvpKeyType type, final boolean isPublicKey) { + Loader.checkNativeLibraryAvailability(); this.internalKey = key; this.type = type; this.isPublicKey = isPublicKey; diff --git a/src/com/amazon/corretto/crypto/provider/EvpKeyFactory.java b/src/com/amazon/corretto/crypto/provider/EvpKeyFactory.java index 83cc863d..ad43299e 100644 --- a/src/com/amazon/corretto/crypto/provider/EvpKeyFactory.java +++ b/src/com/amazon/corretto/crypto/provider/EvpKeyFactory.java @@ -39,6 +39,7 @@ abstract class EvpKeyFactory extends KeyFactorySpi { private static native long ec2Evp(byte[] s, byte[] wx, byte[] wy, byte[] params, boolean checkPrivate) throws InvalidKeySpecException; protected EvpKeyFactory(EvpKeyType type, AmazonCorrettoCryptoProvider provider) { + Loader.checkNativeLibraryAvailability(); this.type = type; this.provider = provider; if (this.type == null) { diff --git a/src/com/amazon/corretto/crypto/provider/LibCryptoRng.java b/src/com/amazon/corretto/crypto/provider/LibCryptoRng.java index 205f0e81..b7a540fa 100644 --- a/src/com/amazon/corretto/crypto/provider/LibCryptoRng.java +++ b/src/com/amazon/corretto/crypto/provider/LibCryptoRng.java @@ -18,6 +18,7 @@ public class LibCryptoRng extends SecureRandom { public LibCryptoRng() { super(new SPI(), AmazonCorrettoCryptoProvider.INSTANCE); + Loader.checkNativeLibraryAvailability(); } @Override diff --git a/src/com/amazon/corretto/crypto/provider/Loader.java b/src/com/amazon/corretto/crypto/provider/Loader.java index 545686b0..86b81b73 100644 --- a/src/com/amazon/corretto/crypto/provider/Loader.java +++ b/src/com/amazon/corretto/crypto/provider/Loader.java @@ -45,8 +45,10 @@ */ final class Loader { static final String PROPERTY_BASE = "com.amazon.corretto.crypto.provider."; - private static final String LIBRARY_NAME = "amazonCorrettoCryptoProvider"; + private static final String TEMP_DIR_PREFIX = "amazonCorrettoCryptoProvider."; + private static final String JNI_LIBRARY_NAME = "amazonCorrettoCryptoProvider"; private static final String LIBCRYPTO_NAME = "crypto"; + private static final String[] JAR_RESOURCES = {JNI_LIBRARY_NAME, LIBCRYPTO_NAME}; 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"); @@ -122,28 +124,11 @@ static String getProperty(String propertyName, String def) { available = AccessController.doPrivileged((PrivilegedExceptionAction) () -> { // This is to work a JVM runtime bug where FileSystems.getDefault() and - // System.loadLibrary() can deadlock. Calling this explicitly shoulf prevent + // System.loadLibrary() can deadlock. Calling this explicitly should prevent // the problem from happening, but since we don't know what other threads are // doing, we cannot promise success. FileSystems.getDefault(); - - final Path libCryptoPath = writeResourceToTemporaryFile(System.mapLibraryName(LIBCRYPTO_NAME)); - tryLoadLibrary("accpLcLoader"); - // Yes, this next bit is horribly evil but we need a way to lock such that it really is global to - // everything running in the JVM even if multiple classloaders have loaded multiple copies of this - // same class. - // We intentionally have nothing in this synchronized block except for a single method call. - boolean loadCompleted = false; - final String pathAsString = libCryptoPath.toAbsolutePath().toString(); - synchronized (ClassLoader.getSystemClassLoader()) { - loadCompleted = loadLibCrypto(pathAsString); - } - maybeDelete(libCryptoPath); - if (!loadCompleted) { - LOG.info("Already loaded libcrypto"); - } - - tryLoadLibrary(LIBRARY_NAME); + tryLoadLibrary(); return true; }); } catch (final Throwable t) { @@ -187,15 +172,12 @@ private static void maybeDelete(final Path path) { } } - private static Path writeResourceToTemporaryFile(final String resourceName) throws IOException { - final int index = resourceName.lastIndexOf('.'); - final String prefix = resourceName.substring(0, index); - final String suffix = resourceName.substring(index, resourceName.length()); - final Path libPath = createTmpFile(prefix, suffix); + private static Path writeJarResourceToTemporaryFile(Path tempDirectory, final String resourceFileName) throws IOException { + final Path tempResourceFilePath = tempDirectory.resolve(resourceFileName); - try (InputStream is = Loader.class.getResourceAsStream(resourceName); - OutputStream os = Files.newOutputStream(libPath, StandardOpenOption.CREATE, + try (InputStream is = Loader.class.getResourceAsStream(resourceFileName); + OutputStream os = Files.newOutputStream(tempResourceFilePath, StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING)) { final byte[] buffer = new byte[16 * 1024]; int read = is.read(buffer); @@ -205,36 +187,98 @@ private static Path writeResourceToTemporaryFile(final String resourceName) thro } os.flush(); } - return libPath; + + // Ensure we delete any temp files on exit + tempResourceFilePath.toFile().deleteOnExit(); + return tempResourceFilePath; + } + + private static void tryLoadLibraryFromJar() throws IOException { + Path privateTempDirectory = createPrivateTmpDir(TEMP_DIR_PREFIX); + + for (String jarResource: JAR_RESOURCES) { + String resourceFileName = System.mapLibraryName(jarResource); + writeJarResourceToTemporaryFile(privateTempDirectory, resourceFileName); + } + + /** + * Java will internally call dlopen() on libamazonCorrettoCryptoProvider from within this System.load() call. + * This will cause the runtime dynamic loader to load ACCP's shared library into a new LOCAL object group that + * is a child of the current Java LOCAL object group. If the library being loaded requires new transitive shared + * library dependencies to be loaded (libcrypto), the system's regular dynamic loading rules are followed + * (namely, any RPath values present will be used), and those shared library dependencies will also be loaded + * into the same child's LOCAL object group recursively until all shared library dependencies are loaded. + * + * At runtime the Java process's object group hierarchy would look like this: + * + * #0: GLOBAL Object Group: Usually empty unless using LD_PRELOAD or dlopen(..., RTLD_GLOBAL) + * ↳ #1: LOCAL Object Group: Java and JVM symbols (libjli), libc, ld-linux, linux-vdso + * ↳ #2: LOCAL Object Group: libamazonCorrettoCryptoProvider, libcrypto + * ↳ #3: LOCAL Object Group: Any other JNI libraries (Eg. with potentially different libcrypto) + * + * Any shared libraries that are not present in Java's LOCAL object group (Group #1), will be loaded into a + * new child LOCAL object group, lower in the object group hierarchy. This can be done multiple times for + * multiple different JNI libraries. Note that since both Groups #2 and #3 have Group #1 as a parent, their + * library symbols will not conflict with each another since each group only see's symbols from their own + * parents in the hierarchy above them. This means it is possible for multiple different JNI libraries to be + * loaded at runtime that use different libcrypto implementations so long as those JNI libraries configure + * their RPath values correctly. + * + * Once a LOCAL object group is created and the recursive library loading is complete, that LOCAL object group + * can no longer be modified at runtime other than to be deleted entirely with dlclose(). Subsequent + * System.load() or dlopen(..., RTLD_LOCAL) calls will only create new child LOCAL Object Groups below the + * current object group in the hierarchy, and will only load in shared libraries that are not present in that + * specific object group's hierarchy. + * + * Links: + * - https://docs.oracle.com/cd/E19957-01/806-0641/6j9vuquj2/index.html + * - http://people.redhat.com/drepper/dsohowto.pdf + * - https://docs.oracle.com/cd/E23824_01/pdf/819-0690.pdf + */ + Path rootSharedLibraryPath = privateTempDirectory.resolve(System.mapLibraryName(JNI_LIBRARY_NAME)).toAbsolutePath(); + System.load(rootSharedLibraryPath.toString()); + + // If loading library from JAR file, then the compile-time and run-time libcrypto versions should be an exact match. + validateLibcryptoExactVersionMatch(); + System.err.println("Successfully Loaded from JAR Libraries."); } - private static void tryLoadLibrary(final String libraryName) throws Exception { + + private static void tryLoadLibraryFromSystem() { + /** + * Attempt to load library using system's default shared library lookup paths + */ + System.loadLibrary(JNI_LIBRARY_NAME); + System.err.println("Successfully Loaded from System Libraries."); + } + + private static void tryLoadLibrary() throws Exception { // First, try to find the library in our own jar - final boolean useExternalLib = Boolean.valueOf(getProperty("useExternalLib", "false")); + final boolean useExternalLib = Boolean.parseBoolean(getProperty("useExternalLib", "false")); + boolean successfullyLoadedLibrary = false; Exception loadingException = null; - if (useExternalLib) { - loadingException = new RuntimeCryptoException("Skipping bundled library due to system property"); - } else if (libraryName != null) { - final Path libPath = writeResourceToTemporaryFile(System.mapLibraryName(libraryName)); + if (!useExternalLib) { try { - System.load(libPath.toAbsolutePath().toString()); - } catch (final Exception ex) { - loadingException = ex; - } finally { - maybeDelete(libPath); + tryLoadLibraryFromJar(); + successfullyLoadedLibrary = true; + } catch (Exception e) { + System.err.println("Failed to Load from JAR: " + e.getMessage()); + e.printStackTrace(); + loadingException = e; } - } else { - loadingException = new RuntimeCryptoException("Skipping bundled library null mapped name"); } - if (loadingException != null) { + if (!successfullyLoadedLibrary) { // We failed to load the library from our JAR but don't know why. // Try to load it directly off of the system path. + if (loadingException == null) { + loadingException = new RuntimeCryptoException("Skipping bundled library due to system property"); + } try { - System.loadLibrary(libraryName); - return; + tryLoadLibraryFromSystem(); } catch (final Throwable suppressedError) { + System.err.println("Failed to Load from System: " + suppressedError.getMessage()); loadingException.addSuppressed(suppressedError); throw loadingException; } @@ -257,12 +301,13 @@ static void checkNativeLibraryAvailability() { } private static native String getNativeLibraryVersion(); + /** - * Attempts to load (mangled) lib crypto from the specified path. - * @param libraryPath the absolute path to load the library or - * @return true if the library was loaded + * Validates that the LibCrypto available at runtime is exactly the same as what was available at compile time. + * This should only be done if loading ACCP from a JAR file, since if loading from system libraries then minor + * version upgrades to libcrypto may cause breakages. */ - private static native boolean loadLibCrypto(String libraryPath); + private static native boolean validateLibcryptoExactVersionMatch(); /** * Indicates if libcrypto is a FIPS build or not. * Equivalent to {@code FIPS_mode() == 1} @@ -289,20 +334,41 @@ private static void assertVersionMatch() { } } + private static byte[] bootstrapRng(int numBytes) throws IOException { + final Path urandomPath = Paths.get("/dev/urandom"); + if (!Files.exists(urandomPath)) { + throw new AssertionError("/dev/urandom must exist for bootstrapping"); + } + + final byte[] rndBytes = new byte[numBytes]; // Default java tmp files use this much entropy + final int RETRY_LIMIT = 10; + try (InputStream rndStream = Files.newInputStream(urandomPath, StandardOpenOption.READ)) { + int attempt = 0; + // We keep doing this until we can create something new or fail badly + while (attempt < RETRY_LIMIT) { + attempt++; + if (rndStream.read(rndBytes) == rndBytes.length) { + return rndBytes; + } + } + } + throw new AssertionError("Unable to read enough entropy"); + } + /** * Unfortunately, we cannot actually use Files.createTempFile, because that internally depends on * SecureRandom, which results in a circular dependency. So, for now, we just read from * /dev/urandom. Clearly this won't work when we start supporting windows systems. We are intentionally taking * as few dependencies here as possible. */ - private static synchronized Path createTmpFile(final String prefix, final String suffix) throws IOException { + private static synchronized Path createPrivateTmpDir(final String prefix) throws IOException { final Path urandomPath = Paths.get("/dev/urandom"); if (!Files.exists(urandomPath)) { throw new AssertionError("/dev/urandom must exist for bootstrapping"); } - final Path tmpDir = Paths.get(System.getProperty("java.io.tmpdir")); - if (!Files.isDirectory(tmpDir)) { - throw new AssertionError("java.io.tmpdir is not valid: " + tmpDir); + final Path systemTempDir = Paths.get(System.getProperty("java.io.tmpdir")); + if (!Files.isDirectory(systemTempDir)) { + throw new AssertionError("java.io.tmpdir is not valid: " + systemTempDir); } final FileAttribute> permissions = @@ -312,45 +378,39 @@ private static synchronized Path createTmpFile(final String prefix, final String PosixFilePermission.OWNER_EXECUTE ))); - final byte[] rndBytes = new byte[Long.BYTES]; // Default java tmp files use this much entropy - final int RETRY_LIMIT = 1000; - try (InputStream rndStream = Files.newInputStream(urandomPath, StandardOpenOption.READ)) { - int attempt = 0; - // We keep doing this until we can create something new or fail badly - while (attempt < RETRY_LIMIT) { - attempt++; - if (rndStream.read(rndBytes) != rndBytes.length) { - throw new AssertionError("Unable to read enough entropy"); - } + final byte[] rndBytes = bootstrapRng(Long.BYTES); // Default java tmp files use this much entropy + final StringBuilder privateTempDir = new StringBuilder(prefix); - final StringBuilder fileName = new StringBuilder(prefix); - for (byte b : rndBytes) { - // We convert to an unsigned integer first to avoid sign-bit extension when converting to hex. - String hexByte = Integer.toHexString(Byte.toUnsignedInt(b)); - if (hexByte.length() == 1) { - fileName.append('0'); - } - fileName.append(hexByte); - } - fileName.append(suffix); - - final Path tmpFile = tmpDir.resolve(fileName.toString()); - - try { - final Path result = Files.createFile(tmpFile, permissions); - if (DebugFlag.VERBOSELOGS.isEnabled()) { - LOG.log(Level.FINE, "Created temporary library file after " + attempt + " attempts"); - } - result.toFile().deleteOnExit(); - return result; - } catch (final FileAlreadyExistsException ex) { - // We ignore and retry this exception - } catch (final Exception ex) { - // Any other exception is bad and we may need to quash. - throw new AssertionError("Unable to create temporary file"); + for (byte b : rndBytes) { + // We convert to an unsigned integer first to avoid sign-bit extension when converting to hex. + String hexByte = Integer.toHexString(Byte.toUnsignedInt(b)); + if (hexByte.length() == 1) { + privateTempDir.append('0'); + } + privateTempDir.append(hexByte); + } + + final Path privateDirFullPath = systemTempDir.resolve(privateTempDir.toString()); + final int RETRY_LIMIT = 10; + int attempt = 0; + + while(attempt < RETRY_LIMIT) { + attempt++; + try { + final Path result = Files.createDirectory(privateDirFullPath, permissions); + if (DebugFlag.VERBOSELOGS.isEnabled()) { + LOG.log(Level.FINE, "Created temporary library directory"); } + result.toFile().deleteOnExit(); + return result; + } catch (final FileAlreadyExistsException ex) { + // We ignore and retry this exception + } catch (final Exception ex) { + // Any other exception is bad and we may need to quash. + throw new AssertionError("Unable to create temporary directory"); } } - throw new AssertionError("Unable to create temporary file. Retries exceeded."); + + throw new AssertionError("Unable to create temporary directory. Retries exceeded."); } } From 2fac3d43223dd618ffee10cc434175143b6b5533 Mon Sep 17 00:00:00 2001 From: Alex Weibel Date: Tue, 21 Jun 2022 13:25:37 -0700 Subject: [PATCH 02/13] Remove debugging print statements --- src/com/amazon/corretto/crypto/provider/Loader.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/com/amazon/corretto/crypto/provider/Loader.java b/src/com/amazon/corretto/crypto/provider/Loader.java index 86b81b73..40015c79 100644 --- a/src/com/amazon/corretto/crypto/provider/Loader.java +++ b/src/com/amazon/corretto/crypto/provider/Loader.java @@ -240,7 +240,6 @@ private static void tryLoadLibraryFromJar() throws IOException { // If loading library from JAR file, then the compile-time and run-time libcrypto versions should be an exact match. validateLibcryptoExactVersionMatch(); - System.err.println("Successfully Loaded from JAR Libraries."); } @@ -249,7 +248,6 @@ private static void tryLoadLibraryFromSystem() { * Attempt to load library using system's default shared library lookup paths */ System.loadLibrary(JNI_LIBRARY_NAME); - System.err.println("Successfully Loaded from System Libraries."); } private static void tryLoadLibrary() throws Exception { @@ -263,8 +261,6 @@ private static void tryLoadLibrary() throws Exception { tryLoadLibraryFromJar(); successfullyLoadedLibrary = true; } catch (Exception e) { - System.err.println("Failed to Load from JAR: " + e.getMessage()); - e.printStackTrace(); loadingException = e; } } @@ -278,7 +274,6 @@ private static void tryLoadLibrary() throws Exception { try { tryLoadLibraryFromSystem(); } catch (final Throwable suppressedError) { - System.err.println("Failed to Load from System: " + suppressedError.getMessage()); loadingException.addSuppressed(suppressedError); throw loadingException; } From 1c3b9426e4b7139beafe1abc372c9ea691cc1b67 Mon Sep 17 00:00:00 2001 From: Alex Weibel Date: Tue, 21 Jun 2022 15:20:37 -0700 Subject: [PATCH 03/13] Update documentation in Loader.java --- .../corretto/crypto/provider/Loader.java | 44 ++++++++++++++----- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/src/com/amazon/corretto/crypto/provider/Loader.java b/src/com/amazon/corretto/crypto/provider/Loader.java index 40015c79..4627e0d4 100644 --- a/src/com/amazon/corretto/crypto/provider/Loader.java +++ b/src/com/amazon/corretto/crypto/provider/Loader.java @@ -211,10 +211,33 @@ private static void tryLoadLibraryFromJar() throws IOException { * * At runtime the Java process's object group hierarchy would look like this: * - * #0: GLOBAL Object Group: Usually empty unless using LD_PRELOAD or dlopen(..., RTLD_GLOBAL) - * ↳ #1: LOCAL Object Group: Java and JVM symbols (libjli), libc, ld-linux, linux-vdso - * ↳ #2: LOCAL Object Group: libamazonCorrettoCryptoProvider, libcrypto - * ↳ #3: LOCAL Object Group: Any other JNI libraries (Eg. with potentially different libcrypto) + * +------------------------------------------------------------+ + * |Group #0: GLOBAL Object Group | + * | - Usually empty unless using LD_PRELOAD or dlopen() with | + * | RTLD_GLOBAL | + * | | + * +------------------------------------------------------------+ + * ^ + * | + * | + * +----------------------------+-------------------------------+ + * |Group #1: LOCAL Object Group | + * | - Java and JVM Symbols (libjli), libc, ld-linux, etc | + * | | + * | | + * +------------------------------------------------------------+ + * ^ ^ + * | | + * | | + * | | + * | | + * | | + * +------------------+--------------------+ +---+-----------------------------------+ + * |Group #2: LOCAL Object Group | |Group #3: LOCAL Object Group | + * | - libamazonCorrettoCryptoProvider, | | - Any other JNI libraries. (Eg with | + * | libcrypto | | potentially different libcrypto) | + * | | | | + * +---------------------------------------+ +---------------------------------------+ * * Any shared libraries that are not present in Java's LOCAL object group (Group #1), will be loaded into a * new child LOCAL object group, lower in the object group hierarchy. This can be done multiple times for @@ -329,6 +352,11 @@ private static void assertVersionMatch() { } } + /** + * We need a source of entropy to create a random temporary directory name at startup, before we've loaded native + * crypto libraries. So, for now, we just read from /dev/urandom. Clearly this won't work when we start supporting + * Windows systems. We are intentionally taking as few dependencies here as possible. + */ private static byte[] bootstrapRng(int numBytes) throws IOException { final Path urandomPath = Paths.get("/dev/urandom"); if (!Files.exists(urandomPath)) { @@ -352,15 +380,9 @@ private static byte[] bootstrapRng(int numBytes) throws IOException { /** * Unfortunately, we cannot actually use Files.createTempFile, because that internally depends on - * SecureRandom, which results in a circular dependency. So, for now, we just read from - * /dev/urandom. Clearly this won't work when we start supporting windows systems. We are intentionally taking - * as few dependencies here as possible. + * SecureRandom, which results in a circular dependency. */ private static synchronized Path createPrivateTmpDir(final String prefix) throws IOException { - final Path urandomPath = Paths.get("/dev/urandom"); - if (!Files.exists(urandomPath)) { - throw new AssertionError("/dev/urandom must exist for bootstrapping"); - } final Path systemTempDir = Paths.get(System.getProperty("java.io.tmpdir")); if (!Files.isDirectory(systemTempDir)) { throw new AssertionError("java.io.tmpdir is not valid: " + systemTempDir); From 260bec637004df1dc6fe92b7363be0be1b4d0808 Mon Sep 17 00:00:00 2001 From: Alex Weibel Date: Tue, 21 Jun 2022 15:52:40 -0700 Subject: [PATCH 04/13] More documentation updates --- .../corretto/crypto/provider/Loader.java | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/com/amazon/corretto/crypto/provider/Loader.java b/src/com/amazon/corretto/crypto/provider/Loader.java index 4627e0d4..a91be111 100644 --- a/src/com/amazon/corretto/crypto/provider/Loader.java +++ b/src/com/amazon/corretto/crypto/provider/Loader.java @@ -205,11 +205,11 @@ private static void tryLoadLibraryFromJar() throws IOException { * Java will internally call dlopen() on libamazonCorrettoCryptoProvider from within this System.load() call. * This will cause the runtime dynamic loader to load ACCP's shared library into a new LOCAL object group that * is a child of the current Java LOCAL object group. If the library being loaded requires new transitive shared - * library dependencies to be loaded (libcrypto), the system's regular dynamic loading rules are followed - * (namely, any RPath values present will be used), and those shared library dependencies will also be loaded - * into the same child's LOCAL object group recursively until all shared library dependencies are loaded. + * library dependencies to be loaded (such as libcrypto), the system's regular dynamic loading rules are + * followed (namely, any RPath values present will be used), and those shared library dependencies will also be + * loaded into the same child's LOCAL object group recursively until all shared library dependencies are loaded. * - * At runtime the Java process's object group hierarchy would look like this: + * This means at runtime the Java process's object group hierarchy would look like this: * * +------------------------------------------------------------+ * |Group #0: GLOBAL Object Group | @@ -220,19 +220,15 @@ private static void tryLoadLibraryFromJar() throws IOException { * ^ * | * | - * +----------------------------+-------------------------------+ + * +------------------------------------------------------------+ * |Group #1: LOCAL Object Group | * | - Java and JVM Symbols (libjli), libc, ld-linux, etc | * | | - * | | * +------------------------------------------------------------+ - * ^ ^ - * | | - * | | - * | | - * | | - * | | - * +------------------+--------------------+ +---+-----------------------------------+ + * ^ ^ + * | | + * | | + * +---------------------------------------+ +---------------------------------------+ * |Group #2: LOCAL Object Group | |Group #3: LOCAL Object Group | * | - libamazonCorrettoCryptoProvider, | | - Any other JNI libraries. (Eg with | * | libcrypto | | potentially different libcrypto) | @@ -245,7 +241,8 @@ private static void tryLoadLibraryFromJar() throws IOException { * library symbols will not conflict with each another since each group only see's symbols from their own * parents in the hierarchy above them. This means it is possible for multiple different JNI libraries to be * loaded at runtime that use different libcrypto implementations so long as those JNI libraries configure - * their RPath values correctly. + * their RPath values correctly, and are compatible with any libraries that have already been loaded above + * them in the hierarchy. * * Once a LOCAL object group is created and the recursive library loading is complete, that LOCAL object group * can no longer be modified at runtime other than to be deleted entirely with dlclose(). Subsequent @@ -257,6 +254,7 @@ private static void tryLoadLibraryFromJar() throws IOException { * - https://docs.oracle.com/cd/E19957-01/806-0641/6j9vuquj2/index.html * - http://people.redhat.com/drepper/dsohowto.pdf * - https://docs.oracle.com/cd/E23824_01/pdf/819-0690.pdf + * - https://man7.org/linux/man-pages/man3/dlopen.3.html */ Path rootSharedLibraryPath = privateTempDirectory.resolve(System.mapLibraryName(JNI_LIBRARY_NAME)).toAbsolutePath(); System.load(rootSharedLibraryPath.toString()); From 03d6c8e82a70d373290f482a73a63efb49583c20 Mon Sep 17 00:00:00 2001 From: Alex Weibel Date: Tue, 21 Jun 2022 16:55:47 -0700 Subject: [PATCH 05/13] Fix coverage and test_keyutil targets to load in correct libcrypto --- CMakeLists.txt | 2 ++ build.gradle | 14 +++----------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c210ab22..2aca22b0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -232,6 +232,7 @@ else() COMMAND ${Java_JAR_EXECUTABLE} uf ${ACCP_JAR_TMP} -C ${CMAKE_CURRENT_SOURCE_DIR}/extra-jar-files . COMMAND ${Java_JAR_EXECUTABLE} uf ${ACCP_JAR_TMP} -C $ module-info.class COMMAND ${CMAKE_COMMAND} -E make_directory ${CMAKE_CURRENT_BINARY_DIR}/tmplib/com/amazon/corretto/crypto/provider/ + COMMAND ${CMAKE_COMMAND} -E copy ${OPENSSL_CRYPTO_LIBRARY} $ COMMAND ${CMAKE_COMMAND} -E copy ${OPENSSL_CRYPTO_LIBRARY} ${CMAKE_CURRENT_BINARY_DIR}/tmplib/com/amazon/corretto/crypto/provider/ COMMAND ${CMAKE_COMMAND} -E copy $ ${CMAKE_CURRENT_BINARY_DIR}/tmplib/com/amazon/corretto/crypto/provider/ COMMAND ${Java_JAR_EXECUTABLE} uf ${ACCP_JAR_TMP} -C ${CMAKE_CURRENT_BINARY_DIR}/tmplib/ com/amazon/corretto/crypto/provider/ @@ -314,6 +315,7 @@ if(ENABLE_NATIVE_TEST_HOOKS) add_executable(test_keyutils EXCLUDE_FROM_ALL csrc/test_keyutils.cpp ) + set_target_properties(test_keyutils PROPERTIES LINK_FLAGS "-Wl,-rpath,\$ORIGIN") target_link_libraries(test_keyutils ${OPENSSL_CRYPTO_LIBRARY}) target_link_libraries(test_keyutils amazonCorrettoCryptoProvider) endif() diff --git a/build.gradle b/build.gradle index 6c46916d..d6bc18bd 100644 --- a/build.gradle +++ b/build.gradle @@ -79,6 +79,7 @@ task buildAwsLc { args '-DBUILD_SHARED_LIBS=ON' args '-DCMAKE_BUILD_TYPE=RelWithDebInfo' args "-DCMAKE_INSTALL_PREFIX=${sharedObjectOutDir}" + args "-DCMAKE_VERBOSE_MAKEFILE:BOOL=ON" if (isFips) { args '-DFIPS=1' @@ -91,16 +92,6 @@ task buildAwsLc { } } -// Need to copy libcrypto to same directory where we will be building libACCP, so that we can run ACCP tests with -// "-Djava.library.path=${buildDir}/cmake" and "-Dcom.amazon.corretto.crypto.provider.useExternalLib=true" flags -task copyLibCryptoForTests(type: Copy) { - dependsOn buildAwsLc - from("${buildDir}/awslc/bin/lib") { - include 'libcrypto.*' - } - into "${buildDir}/cmake" -} - task executeCmake(type: Exec) { outputs.dir("${buildDir}/cmake") inputs.dir("${buildDir}/awslc/bin/") @@ -113,7 +104,7 @@ task executeCmake(type: Exec) { inputs.dir("${projectDir}/test-data") inputs.dir("${projectDir}/template-src") - dependsOn buildAwsLc, copyLibCryptoForTests + dependsOn buildAwsLc workingDir "${buildDir}/cmake" @@ -313,6 +304,7 @@ task coverage_cmake(type: Exec) { args '-DCMAKE_BUILD_TYPE=Coverage', '-DCOVERAGE=ON', '-DENABLE_NATIVE_TEST_HOOKS=ON' args '-DPROVIDER_VERSION_STRING=' + version, projectDir args "-DTEST_RUNNER_JAR=${configurations.testRunner.singleFile}" + args "-DCMAKE_VERBOSE_MAKEFILE:BOOL=ON" if (isFips) { args "-DFIPS=ON" } From 78113eca9f26828a495a6bf4bfd853c4d0b80d93 Mon Sep 17 00:00:00 2001 From: Alex Weibel Date: Tue, 21 Jun 2022 17:24:35 -0700 Subject: [PATCH 06/13] Always copy libcrypto to test_keyutil before use --- CMakeLists.txt | 1 + build.gradle | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2aca22b0..55c028ae 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -795,6 +795,7 @@ add_custom_target(checkstyle if(ENABLE_NATIVE_TEST_HOOKS) add_custom_target(check-keyutils + COMMAND ${CMAKE_COMMAND} -E copy ${OPENSSL_CRYPTO_LIBRARY} $ COMMAND $ ) add_dependencies(check check-keyutils) diff --git a/build.gradle b/build.gradle index d6bc18bd..b88daece 100644 --- a/build.gradle +++ b/build.gradle @@ -107,7 +107,6 @@ task executeCmake(type: Exec) { dependsOn buildAwsLc workingDir "${buildDir}/cmake" - def prebuiltJar = null if (project.hasProperty('stagingUrl')) { mkdir "${buildDir}/tmp" From 0ebff9dd82e6f216196a83a4f2851e9eb93c7b5c Mon Sep 17 00:00:00 2001 From: Alex Weibel Date: Wed, 22 Jun 2022 15:32:42 -0700 Subject: [PATCH 07/13] Delete Native resources immediately after loading them --- .../corretto/crypto/provider/Loader.java | 94 +++++++++++-------- 1 file changed, 53 insertions(+), 41 deletions(-) diff --git a/src/com/amazon/corretto/crypto/provider/Loader.java b/src/com/amazon/corretto/crypto/provider/Loader.java index a91be111..a10d9d79 100644 --- a/src/com/amazon/corretto/crypto/provider/Loader.java +++ b/src/com/amazon/corretto/crypto/provider/Loader.java @@ -45,7 +45,7 @@ */ final class Loader { static final String PROPERTY_BASE = "com.amazon.corretto.crypto.provider."; - private static final String TEMP_DIR_PREFIX = "amazonCorrettoCryptoProvider."; + private static final String TEMP_DIR_PREFIX = "amazonCorrettoCryptoProviderNativeLibraries."; private static final String JNI_LIBRARY_NAME = "amazonCorrettoCryptoProvider"; private static final String LIBCRYPTO_NAME = "crypto"; private static final String[] JAR_RESOURCES = {JNI_LIBRARY_NAME, LIBCRYPTO_NAME}; @@ -162,37 +162,6 @@ static String getProperty(String propertyName, String def) { RESOURCE_JANITOR = new Janitor(); } - private static void maybeDelete(final Path path) { - if (!DebugFlag.PRESERVE_NATIVE_LIBRARIES.isEnabled()) { - try { - Files.delete(path); - } catch (IOException ex) { - LOG.warning("Unable to delete native library: " + ex); - } - } - } - - - private static Path writeJarResourceToTemporaryFile(Path tempDirectory, final String resourceFileName) throws IOException { - final Path tempResourceFilePath = tempDirectory.resolve(resourceFileName); - - try (InputStream is = Loader.class.getResourceAsStream(resourceFileName); - OutputStream os = Files.newOutputStream(tempResourceFilePath, StandardOpenOption.CREATE, - StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING)) { - final byte[] buffer = new byte[16 * 1024]; - int read = is.read(buffer); - while (read >= 0) { - os.write(buffer, 0, read); - read = is.read(buffer); - } - os.flush(); - } - - // Ensure we delete any temp files on exit - tempResourceFilePath.toFile().deleteOnExit(); - return tempResourceFilePath; - } - private static void tryLoadLibraryFromJar() throws IOException { Path privateTempDirectory = createPrivateTmpDir(TEMP_DIR_PREFIX); @@ -204,10 +173,13 @@ private static void tryLoadLibraryFromJar() throws IOException { /** * Java will internally call dlopen() on libamazonCorrettoCryptoProvider from within this System.load() call. * This will cause the runtime dynamic loader to load ACCP's shared library into a new LOCAL object group that - * is a child of the current Java LOCAL object group. If the library being loaded requires new transitive shared - * library dependencies to be loaded (such as libcrypto), the system's regular dynamic loading rules are - * followed (namely, any RPath values present will be used), and those shared library dependencies will also be - * loaded into the same child's LOCAL object group recursively until all shared library dependencies are loaded. + * is a child of the current Java LOCAL object group. Any shared library dependencies of the library being + * loaded (such as libcrypto) will also be loaded recursively into the same LOCAL object group until all + * transitive shared library dependencies are loaded. The system's regular dynamic loading rules are followed + * (namely, any RPATH values present will be used). Since libamazonCorrettoCryptoProvider is built with an RPATH + * value of "$ORIGIN", the runtime dynamic loader will always look in the same directory as libACCP for any + * shared library dependencies BEFORE attempting to look in any system directories containing potentially + * conflicting libraries with the same name. * * This means at runtime the Java process's object group hierarchy would look like this: * @@ -247,8 +219,8 @@ private static void tryLoadLibraryFromJar() throws IOException { * Once a LOCAL object group is created and the recursive library loading is complete, that LOCAL object group * can no longer be modified at runtime other than to be deleted entirely with dlclose(). Subsequent * System.load() or dlopen(..., RTLD_LOCAL) calls will only create new child LOCAL Object Groups below the - * current object group in the hierarchy, and will only load in shared libraries that are not present in that - * specific object group's hierarchy. + * current object group in the hierarchy, and will only load in shared libraries that are not present in the + * caller's object group's hierarchy. * * Links: * - https://docs.oracle.com/cd/E19957-01/806-0641/6j9vuquj2/index.html @@ -256,11 +228,13 @@ private static void tryLoadLibraryFromJar() throws IOException { * - https://docs.oracle.com/cd/E23824_01/pdf/819-0690.pdf * - https://man7.org/linux/man-pages/man3/dlopen.3.html */ - Path rootSharedLibraryPath = privateTempDirectory.resolve(System.mapLibraryName(JNI_LIBRARY_NAME)).toAbsolutePath(); - System.load(rootSharedLibraryPath.toString()); + Path accpJniSharedLibraryPath = privateTempDirectory.resolve(System.mapLibraryName(JNI_LIBRARY_NAME)).toAbsolutePath(); + System.load(accpJniSharedLibraryPath.toString()); // If loading library from JAR file, then the compile-time and run-time libcrypto versions should be an exact match. validateLibcryptoExactVersionMatch(); + + maybeDeletePrivateTempDir(privateTempDirectory); } @@ -350,8 +324,46 @@ private static void assertVersionMatch() { } } + private static void maybeDelete(final Path path) { + if (!DebugFlag.PRESERVE_NATIVE_LIBRARIES.isEnabled()) { + try { + Files.delete(path); + } catch (IOException ex) { + LOG.warning("Unable to delete native library: " + ex); + } + } + } + + private static void maybeDeletePrivateTempDir(final Path tmpDirectory) { + for (String jarResource: JAR_RESOURCES) { + String resourceFileName = System.mapLibraryName(jarResource); + maybeDelete(tmpDirectory.resolve(resourceFileName)); + } + maybeDelete(tmpDirectory); + } + + private static Path writeJarResourceToTemporaryFile(Path tempDirectory, final String resourceFileName) throws IOException { + final Path tempResourceFilePath = tempDirectory.resolve(resourceFileName); + + try (InputStream is = Loader.class.getResourceAsStream(resourceFileName); + OutputStream os = Files.newOutputStream(tempResourceFilePath, StandardOpenOption.CREATE, + StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING)) { + final byte[] buffer = new byte[16 * 1024]; + int read = is.read(buffer); + while (read >= 0) { + os.write(buffer, 0, read); + read = is.read(buffer); + } + os.flush(); + } + + // Ensure we delete any temp files on exit + tempResourceFilePath.toFile().deleteOnExit(); + return tempResourceFilePath; + } + /** - * We need a source of entropy to create a random temporary directory name at startup, before we've loaded native + * We need a source of entropy to create a random temporary directory name at startup before we've loaded native * crypto libraries. So, for now, we just read from /dev/urandom. Clearly this won't work when we start supporting * Windows systems. We are intentionally taking as few dependencies here as possible. */ From eeccd8b9162dff41ba665bcb00d316bc56333ad6 Mon Sep 17 00:00:00 2001 From: Alex Weibel Date: Thu, 23 Jun 2022 18:53:25 -0700 Subject: [PATCH 08/13] Add Fuzzy version check, and File owner permissions --- csrc/loader.cpp | 28 +++++++++++++++- .../corretto/crypto/provider/Loader.java | 32 +++++++++++++------ 2 files changed, 50 insertions(+), 10 deletions(-) diff --git a/csrc/loader.cpp b/csrc/loader.cpp index e13bb588..ee87b489 100644 --- a/csrc/loader.cpp +++ b/csrc/loader.cpp @@ -20,6 +20,11 @@ // Right now we only support PTHREAD #include +// https://www.openssl.org/docs/man1.1.1/man3/OPENSSL_VERSION_NUMBER.html +// 0xMNNFFPPS : major minor fix patch status +// 0x1010107f == v1.1.1g release +#define LIBCRYPTO_MAJOR_MINOR_VERSION_MASK 0xFFF00000 + using namespace AmazonCorrettoCryptoProvider; namespace { @@ -60,7 +65,6 @@ JNIEXPORT jstring JNICALL Java_com_amazon_corretto_crypto_provider_Loader_getNat } - JNIEXPORT jboolean JNICALL Java_com_amazon_corretto_crypto_provider_Loader_validateLibcryptoExactVersionMatch(JNIEnv* pEnv, jclass) { char msg_buffer[256] = {0}; @@ -82,3 +86,25 @@ JNIEXPORT jboolean JNICALL Java_com_amazon_corretto_crypto_provider_Loader_valid return JNI_FALSE; } + +JNIEXPORT jboolean JNICALL Java_com_amazon_corretto_crypto_provider_Loader_validateLibcryptoFuzzyVersionMatch(JNIEnv* pEnv, jclass) +{ + char msg_buffer[256] = {0}; + + try { + const unsigned long libcrypto_compiletime_version = (OPENSSL_VERSION_NUMBER & LIBCRYPTO_MAJOR_MINOR_VERSION_MASK); + const unsigned long libcrypto_runtime_version = (OpenSSL_version_num() & LIBCRYPTO_MAJOR_MINOR_VERSION_MASK); + + if (libcrypto_compiletime_version != libcrypto_runtime_version) { + snprintf(msg_buffer, sizeof(msg_buffer), "Runtime libcrypto version does not match compile-time version. " + "Expected: 0x%08lX , Actual: 0x%08lX", libcrypto_compiletime_version, libcrypto_runtime_version); + throw java_ex(EX_RUNTIME_CRYPTO, msg_buffer); + } + + return JNI_TRUE; + } catch (java_ex &ex) { + ex.throw_to_java(pEnv); + } + + return JNI_FALSE; +} diff --git a/src/com/amazon/corretto/crypto/provider/Loader.java b/src/com/amazon/corretto/crypto/provider/Loader.java index a10d9d79..73a9afa0 100644 --- a/src/com/amazon/corretto/crypto/provider/Loader.java +++ b/src/com/amazon/corretto/crypto/provider/Loader.java @@ -50,6 +50,13 @@ final class Loader { private static final String LIBCRYPTO_NAME = "crypto"; private static final String[] JAR_RESOURCES = {JNI_LIBRARY_NAME, LIBCRYPTO_NAME}; private static final Pattern TEST_FILENAME_PATTERN = Pattern.compile("[-a-zA-Z0-9]+(\\.[a-zA-Z0-9]+)*"); + private static final FileAttribute> SELF_OWNER_FILE_PERMISSIONS = + PosixFilePermissions.asFileAttribute(new HashSet<>(Arrays.asList( + PosixFilePermission.OWNER_READ, + PosixFilePermission.OWNER_WRITE, + PosixFilePermission.OWNER_EXECUTE + ))); + private static final Logger LOG = Logger.getLogger("AmazonCorrettoCryptoProvider"); // Version strings live in the loader because we want to be able to access them before @@ -243,6 +250,9 @@ private static void tryLoadLibraryFromSystem() { * Attempt to load library using system's default shared library lookup paths */ System.loadLibrary(JNI_LIBRARY_NAME); + + // If loading from system directory, ensure compile-time and run-time libcrypto have same major and minor version + validateLibcryptoFuzzyVersionMatch(); } private static void tryLoadLibrary() throws Exception { @@ -298,6 +308,14 @@ static void checkNativeLibraryAvailability() { * version upgrades to libcrypto may cause breakages. */ private static native boolean validateLibcryptoExactVersionMatch(); + + /** + * Validates that the LibCrypto available at runtime has the same Major and Minor version as compile time, but allow + * the Patch version to be different. This should only be done if loading ACCP from a system directory to allow for + * patch security updates to libcrypto to be made independently from ACCP. + */ + private static native boolean validateLibcryptoFuzzyVersionMatch(); + /** * Indicates if libcrypto is a FIPS build or not. * Equivalent to {@code FIPS_mode() == 1} @@ -345,6 +363,9 @@ private static void maybeDeletePrivateTempDir(final Path tmpDirectory) { private static Path writeJarResourceToTemporaryFile(Path tempDirectory, final String resourceFileName) throws IOException { final Path tempResourceFilePath = tempDirectory.resolve(resourceFileName); + // Create a new temporary file. If one already exists this will throw FileAlreadyExistsException + Files.createFile(tempResourceFilePath, SELF_OWNER_FILE_PERMISSIONS); + try (InputStream is = Loader.class.getResourceAsStream(resourceFileName); OutputStream os = Files.newOutputStream(tempResourceFilePath, StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING)) { @@ -373,7 +394,7 @@ private static byte[] bootstrapRng(int numBytes) throws IOException { throw new AssertionError("/dev/urandom must exist for bootstrapping"); } - final byte[] rndBytes = new byte[numBytes]; // Default java tmp files use this much entropy + final byte[] rndBytes = new byte[numBytes]; final int RETRY_LIMIT = 10; try (InputStream rndStream = Files.newInputStream(urandomPath, StandardOpenOption.READ)) { int attempt = 0; @@ -398,13 +419,6 @@ private static synchronized Path createPrivateTmpDir(final String prefix) throws throw new AssertionError("java.io.tmpdir is not valid: " + systemTempDir); } - final FileAttribute> permissions = - PosixFilePermissions.asFileAttribute(new HashSet<>(Arrays.asList( - PosixFilePermission.OWNER_READ, - PosixFilePermission.OWNER_WRITE, - PosixFilePermission.OWNER_EXECUTE - ))); - final byte[] rndBytes = bootstrapRng(Long.BYTES); // Default java tmp files use this much entropy final StringBuilder privateTempDir = new StringBuilder(prefix); @@ -424,7 +438,7 @@ private static synchronized Path createPrivateTmpDir(final String prefix) throws while(attempt < RETRY_LIMIT) { attempt++; try { - final Path result = Files.createDirectory(privateDirFullPath, permissions); + final Path result = Files.createDirectory(privateDirFullPath, SELF_OWNER_FILE_PERMISSIONS); if (DebugFlag.VERBOSELOGS.isEnabled()) { LOG.log(Level.FINE, "Created temporary library directory"); } From 43866be78acbd5d82bd8901d42c4668b2a3534e1 Mon Sep 17 00:00:00 2001 From: Alex Weibel Date: Fri, 24 Jun 2022 14:37:38 -0700 Subject: [PATCH 09/13] Refactor version matching, remove unnecessary File CREATE flag, honor DebugFlag.PRESERVE_NATIVE_LIBRARIES, generate new temp directory names on retries --- csrc/loader.cpp | 47 ++++++++++--------- .../corretto/crypto/provider/Loader.java | 47 +++++++++++-------- 2 files changed, 53 insertions(+), 41 deletions(-) diff --git a/csrc/loader.cpp b/csrc/loader.cpp index ee87b489..09053a29 100644 --- a/csrc/loader.cpp +++ b/csrc/loader.cpp @@ -25,6 +25,11 @@ // 0x1010107f == v1.1.1g release #define LIBCRYPTO_MAJOR_MINOR_VERSION_MASK 0xFFF00000 +#define LIBCRYPTO_EXACT_VERSION_MATCH 0 +#define LIBCRYPTO_FUZZY_VERSION_MATCH 1 + +static char accp_loader_exception_msg[256] = {0}; + using namespace AmazonCorrettoCryptoProvider; namespace { @@ -65,20 +70,28 @@ JNIEXPORT jstring JNICALL Java_com_amazon_corretto_crypto_provider_Loader_getNat } -JNIEXPORT jboolean JNICALL Java_com_amazon_corretto_crypto_provider_Loader_validateLibcryptoExactVersionMatch(JNIEnv* pEnv, jclass) -{ - char msg_buffer[256] = {0}; +void accpValidateLibcryptoVersion(bool fuzzyMatch) { + unsigned long libcrypto_compiletime_version = OPENSSL_VERSION_NUMBER; + unsigned long libcrypto_runtime_version = OpenSSL_version_num(); - try { - const unsigned long libcrypto_compiletime_version = OPENSSL_VERSION_NUMBER; - const unsigned long libcrypto_runtime_version = OpenSSL_version_num(); + if (fuzzyMatch) { + libcrypto_compiletime_version &= LIBCRYPTO_MAJOR_MINOR_VERSION_MASK; + libcrypto_runtime_version &= LIBCRYPTO_MAJOR_MINOR_VERSION_MASK; + } - if (libcrypto_compiletime_version != libcrypto_runtime_version) { - snprintf(msg_buffer, sizeof(msg_buffer), "Runtime libcrypto version does not match compile-time version. " - "Expected: 0x%08lX , Actual: 0x%08lX", libcrypto_compiletime_version, libcrypto_runtime_version); - throw java_ex(EX_RUNTIME_CRYPTO, msg_buffer); - } + if (libcrypto_compiletime_version != libcrypto_runtime_version) { + memset(accp_loader_exception_msg, 0, sizeof(accp_loader_exception_msg)); + snprintf(accp_loader_exception_msg, sizeof(accp_loader_exception_msg), + "Runtime libcrypto version does not match compile-time version. Expected: 0x%08lX , Actual: 0x%08lX", + libcrypto_compiletime_version, libcrypto_runtime_version); + throw java_ex(EX_RUNTIME_CRYPTO, accp_loader_exception_msg); + } +} +JNIEXPORT jboolean JNICALL Java_com_amazon_corretto_crypto_provider_Loader_validateLibcryptoExactVersionMatch(JNIEnv* pEnv, jclass) +{ + try { + accpValidateLibcryptoVersion(LIBCRYPTO_EXACT_VERSION_MATCH); return JNI_TRUE; } catch (java_ex &ex) { ex.throw_to_java(pEnv); @@ -89,18 +102,8 @@ JNIEXPORT jboolean JNICALL Java_com_amazon_corretto_crypto_provider_Loader_valid JNIEXPORT jboolean JNICALL Java_com_amazon_corretto_crypto_provider_Loader_validateLibcryptoFuzzyVersionMatch(JNIEnv* pEnv, jclass) { - char msg_buffer[256] = {0}; - try { - const unsigned long libcrypto_compiletime_version = (OPENSSL_VERSION_NUMBER & LIBCRYPTO_MAJOR_MINOR_VERSION_MASK); - const unsigned long libcrypto_runtime_version = (OpenSSL_version_num() & LIBCRYPTO_MAJOR_MINOR_VERSION_MASK); - - if (libcrypto_compiletime_version != libcrypto_runtime_version) { - snprintf(msg_buffer, sizeof(msg_buffer), "Runtime libcrypto version does not match compile-time version. " - "Expected: 0x%08lX , Actual: 0x%08lX", libcrypto_compiletime_version, libcrypto_runtime_version); - throw java_ex(EX_RUNTIME_CRYPTO, msg_buffer); - } - + accpValidateLibcryptoVersion(LIBCRYPTO_FUZZY_VERSION_MATCH); return JNI_TRUE; } catch (java_ex &ex) { ex.throw_to_java(pEnv); diff --git a/src/com/amazon/corretto/crypto/provider/Loader.java b/src/com/amazon/corretto/crypto/provider/Loader.java index 73a9afa0..77d7dbfb 100644 --- a/src/com/amazon/corretto/crypto/provider/Loader.java +++ b/src/com/amazon/corretto/crypto/provider/Loader.java @@ -217,7 +217,7 @@ private static void tryLoadLibraryFromJar() throws IOException { * Any shared libraries that are not present in Java's LOCAL object group (Group #1), will be loaded into a * new child LOCAL object group, lower in the object group hierarchy. This can be done multiple times for * multiple different JNI libraries. Note that since both Groups #2 and #3 have Group #1 as a parent, their - * library symbols will not conflict with each another since each group only see's symbols from their own + * library symbols will not conflict with each other since each group only see's symbols from their own * parents in the hierarchy above them. This means it is possible for multiple different JNI libraries to be * loaded at runtime that use different libcrypto implementations so long as those JNI libraries configure * their RPath values correctly, and are compatible with any libraries that have already been loaded above @@ -363,12 +363,15 @@ private static void maybeDeletePrivateTempDir(final Path tmpDirectory) { private static Path writeJarResourceToTemporaryFile(Path tempDirectory, final String resourceFileName) throws IOException { final Path tempResourceFilePath = tempDirectory.resolve(resourceFileName); - // Create a new temporary file. If one already exists this will throw FileAlreadyExistsException + // Create a new temporary file with ourselves as the owner. We need to ensure that we create the file and set + // permissions atomically so that an adversary cannot put a symlink or file here which would cause us to write + // (or read) to an arbitrary attacker controlled location. If this file already exists at this location, then + // a FileAlreadyExistsException will be thrown. Files.createFile(tempResourceFilePath, SELF_OWNER_FILE_PERMISSIONS); try (InputStream is = Loader.class.getResourceAsStream(resourceFileName); - OutputStream os = Files.newOutputStream(tempResourceFilePath, StandardOpenOption.CREATE, - StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING)) { + OutputStream os = Files.newOutputStream(tempResourceFilePath, StandardOpenOption.WRITE, + StandardOpenOption.TRUNCATE_EXISTING)) { final byte[] buffer = new byte[16 * 1024]; int read = is.read(buffer); while (read >= 0) { @@ -379,7 +382,9 @@ private static Path writeJarResourceToTemporaryFile(Path tempDirectory, final St } // Ensure we delete any temp files on exit - tempResourceFilePath.toFile().deleteOnExit(); + if (!DebugFlag.PRESERVE_NATIVE_LIBRARIES.isEnabled()) { + tempResourceFilePath.toFile().deleteOnExit(); + } return tempResourceFilePath; } @@ -419,30 +424,34 @@ private static synchronized Path createPrivateTmpDir(final String prefix) throws throw new AssertionError("java.io.tmpdir is not valid: " + systemTempDir); } - final byte[] rndBytes = bootstrapRng(Long.BYTES); // Default java tmp files use this much entropy - final StringBuilder privateTempDir = new StringBuilder(prefix); - - for (byte b : rndBytes) { - // We convert to an unsigned integer first to avoid sign-bit extension when converting to hex. - String hexByte = Integer.toHexString(Byte.toUnsignedInt(b)); - if (hexByte.length() == 1) { - privateTempDir.append('0'); - } - privateTempDir.append(hexByte); - } - - final Path privateDirFullPath = systemTempDir.resolve(privateTempDir.toString()); final int RETRY_LIMIT = 10; int attempt = 0; while(attempt < RETRY_LIMIT) { attempt++; try { + final byte[] rndBytes = bootstrapRng(Long.BYTES); // Default java tmp files use this much entropy + final StringBuilder privateTempDir = new StringBuilder(prefix); + + for (byte b : rndBytes) { + // We convert to an unsigned integer first to avoid sign-bit extension when converting to hex. + String hexByte = Integer.toHexString(Byte.toUnsignedInt(b)); + if (hexByte.length() == 1) { + privateTempDir.append('0'); + } + privateTempDir.append(hexByte); + } + + final Path privateDirFullPath = systemTempDir.resolve(privateTempDir.toString()); + final Path result = Files.createDirectory(privateDirFullPath, SELF_OWNER_FILE_PERMISSIONS); if (DebugFlag.VERBOSELOGS.isEnabled()) { LOG.log(Level.FINE, "Created temporary library directory"); } - result.toFile().deleteOnExit(); + + if (!DebugFlag.PRESERVE_NATIVE_LIBRARIES.isEnabled()) { + result.toFile().deleteOnExit(); + } return result; } catch (final FileAlreadyExistsException ex) { // We ignore and retry this exception From 04527ef76ca0f70285a6199011ca1bfc4ad5857c Mon Sep 17 00:00:00 2001 From: Alex Weibel Date: Fri, 24 Jun 2022 14:49:00 -0700 Subject: [PATCH 10/13] Always delete temp files on exit --- src/com/amazon/corretto/crypto/provider/Loader.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/com/amazon/corretto/crypto/provider/Loader.java b/src/com/amazon/corretto/crypto/provider/Loader.java index 77d7dbfb..254e92aa 100644 --- a/src/com/amazon/corretto/crypto/provider/Loader.java +++ b/src/com/amazon/corretto/crypto/provider/Loader.java @@ -382,9 +382,8 @@ private static Path writeJarResourceToTemporaryFile(Path tempDirectory, final St } // Ensure we delete any temp files on exit - if (!DebugFlag.PRESERVE_NATIVE_LIBRARIES.isEnabled()) { - tempResourceFilePath.toFile().deleteOnExit(); - } + tempResourceFilePath.toFile().deleteOnExit(); + return tempResourceFilePath; } @@ -449,9 +448,7 @@ private static synchronized Path createPrivateTmpDir(final String prefix) throws LOG.log(Level.FINE, "Created temporary library directory"); } - if (!DebugFlag.PRESERVE_NATIVE_LIBRARIES.isEnabled()) { - result.toFile().deleteOnExit(); - } + result.toFile().deleteOnExit(); return result; } catch (final FileAlreadyExistsException ex) { // We ignore and retry this exception From 87bb83415f2df98d18346cf9ca7ce53d47d4cbde Mon Sep 17 00:00:00 2001 From: Alex Weibel Date: Mon, 27 Jun 2022 15:15:12 -0700 Subject: [PATCH 11/13] Update csrc/loader.cpp Co-authored-by: Greg Rubin --- csrc/loader.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/csrc/loader.cpp b/csrc/loader.cpp index 09053a29..fa9066e7 100644 --- a/csrc/loader.cpp +++ b/csrc/loader.cpp @@ -25,8 +25,8 @@ // 0x1010107f == v1.1.1g release #define LIBCRYPTO_MAJOR_MINOR_VERSION_MASK 0xFFF00000 -#define LIBCRYPTO_EXACT_VERSION_MATCH 0 -#define LIBCRYPTO_FUZZY_VERSION_MATCH 1 +#define LIBCRYPTO_EXACT_VERSION_MATCH false +#define LIBCRYPTO_FUZZY_VERSION_MATCH true static char accp_loader_exception_msg[256] = {0}; From ab495e9af817f35cecc74d957f07904e26eb0674 Mon Sep 17 00:00:00 2001 From: Alex Weibel Date: Tue, 28 Jun 2022 09:27:12 -0700 Subject: [PATCH 12/13] Create local string copy for Java exceptions and refactor libcrypto version checks --- csrc/env.cpp | 7 +-- csrc/env.h | 20 ++++--- csrc/loader.cpp | 53 ++++++------------- .../corretto/crypto/provider/Loader.java | 17 ++---- 4 files changed, 37 insertions(+), 60 deletions(-) diff --git a/csrc/env.cpp b/csrc/env.cpp index 4e6e4522..f2455db8 100644 --- a/csrc/env.cpp +++ b/csrc/env.cpp @@ -119,11 +119,8 @@ void java_ex::throw_to_java(JNIEnv *env) { if (likely(ex_class != NULL)) { std::ostringstream oss; - if (m_message_cstr) { - oss << m_message_cstr; - } else { - oss << m_message; - } + oss << m_message_cstr; + #ifdef BACKTRACE_ON_EXCEPTION format_trace(oss, m_trace); #endif diff --git a/csrc/env.h b/csrc/env.h index 47aeeb4a..4d921afd 100644 --- a/csrc/env.h +++ b/csrc/env.h @@ -14,6 +14,7 @@ #include #include #include +#include #ifdef HAVE_IS_TRIVIALLY_COPYABLE #include @@ -50,8 +51,7 @@ class java_ex { jthrowable m_java_exception; const char *m_java_classname; - const std::string m_message; - const char *m_message_cstr; + char m_message_cstr[256] = {0}; #ifdef BACKTRACE_ON_EXCEPTION std::vector m_trace; void capture_trace() COLD { AmazonCorrettoCryptoProvider::capture_trace(m_trace); } @@ -61,16 +61,22 @@ class java_ex { public: java_ex(jthrowable exception) COLD - : m_java_exception(exception), m_java_classname(nullptr), m_message(), m_message_cstr("") + : m_java_exception(exception), m_java_classname(nullptr) { } java_ex(const char *java_classname, const char *message) COLD - : m_java_exception(nullptr), m_java_classname(java_classname), m_message(), m_message_cstr(message) - { capture_trace(); } + : m_java_exception(nullptr), m_java_classname(java_classname) { + + capture_trace(); + + if (message != NULL) { + size_t amount_to_copy = strnlen(message, (sizeof(m_message_cstr) - 1)); + std::strncpy(m_message_cstr, message, amount_to_copy); + } + } java_ex(const char *java_classname, const std::string &message) COLD - : m_java_exception(nullptr), m_java_classname(java_classname), m_message(message), m_message_cstr(nullptr) - { capture_trace(); } + : java_ex(java_classname, message.c_str()) {} /** * Constructs an exception based on the openssl error code. diff --git a/csrc/loader.cpp b/csrc/loader.cpp index fa9066e7..3f9ded0a 100644 --- a/csrc/loader.cpp +++ b/csrc/loader.cpp @@ -25,11 +25,6 @@ // 0x1010107f == v1.1.1g release #define LIBCRYPTO_MAJOR_MINOR_VERSION_MASK 0xFFF00000 -#define LIBCRYPTO_EXACT_VERSION_MATCH false -#define LIBCRYPTO_FUZZY_VERSION_MATCH true - -static char accp_loader_exception_msg[256] = {0}; - using namespace AmazonCorrettoCryptoProvider; namespace { @@ -70,40 +65,26 @@ JNIEXPORT jstring JNICALL Java_com_amazon_corretto_crypto_provider_Loader_getNat } -void accpValidateLibcryptoVersion(bool fuzzyMatch) { - unsigned long libcrypto_compiletime_version = OPENSSL_VERSION_NUMBER; - unsigned long libcrypto_runtime_version = OpenSSL_version_num(); - - if (fuzzyMatch) { - libcrypto_compiletime_version &= LIBCRYPTO_MAJOR_MINOR_VERSION_MASK; - libcrypto_runtime_version &= LIBCRYPTO_MAJOR_MINOR_VERSION_MASK; - } - - if (libcrypto_compiletime_version != libcrypto_runtime_version) { - memset(accp_loader_exception_msg, 0, sizeof(accp_loader_exception_msg)); - snprintf(accp_loader_exception_msg, sizeof(accp_loader_exception_msg), - "Runtime libcrypto version does not match compile-time version. Expected: 0x%08lX , Actual: 0x%08lX", - libcrypto_compiletime_version, libcrypto_runtime_version); - throw java_ex(EX_RUNTIME_CRYPTO, accp_loader_exception_msg); - } -} - -JNIEXPORT jboolean JNICALL Java_com_amazon_corretto_crypto_provider_Loader_validateLibcryptoExactVersionMatch(JNIEnv* pEnv, jclass) +JNIEXPORT jboolean JNICALL Java_com_amazon_corretto_crypto_provider_Loader_validateLibcryptoVersion(JNIEnv* pEnv, jclass, jboolean jFuzzyMatch) { - try { - accpValidateLibcryptoVersion(LIBCRYPTO_EXACT_VERSION_MATCH); - return JNI_TRUE; - } catch (java_ex &ex) { - ex.throw_to_java(pEnv); - } + bool fuzzyMatch = (jFuzzyMatch == JNI_TRUE); - return JNI_FALSE; -} - -JNIEXPORT jboolean JNICALL Java_com_amazon_corretto_crypto_provider_Loader_validateLibcryptoFuzzyVersionMatch(JNIEnv* pEnv, jclass) -{ try { - accpValidateLibcryptoVersion(LIBCRYPTO_FUZZY_VERSION_MATCH); + unsigned long libcrypto_compiletime_version = OPENSSL_VERSION_NUMBER; + unsigned long libcrypto_runtime_version = OpenSSL_version_num(); + + if (fuzzyMatch) { + libcrypto_compiletime_version &= LIBCRYPTO_MAJOR_MINOR_VERSION_MASK; + libcrypto_runtime_version &= LIBCRYPTO_MAJOR_MINOR_VERSION_MASK; + } + + if (libcrypto_compiletime_version != libcrypto_runtime_version) { + char accp_loader_exception_msg[256] = {0}; + snprintf(accp_loader_exception_msg, sizeof(accp_loader_exception_msg), + "Runtime libcrypto version does not match compile-time version. Expected: 0x%08lX , Actual: 0x%08lX", + libcrypto_compiletime_version, libcrypto_runtime_version); + throw java_ex(EX_RUNTIME_CRYPTO, accp_loader_exception_msg); + } return JNI_TRUE; } catch (java_ex &ex) { ex.throw_to_java(pEnv); diff --git a/src/com/amazon/corretto/crypto/provider/Loader.java b/src/com/amazon/corretto/crypto/provider/Loader.java index 254e92aa..2c800e1c 100644 --- a/src/com/amazon/corretto/crypto/provider/Loader.java +++ b/src/com/amazon/corretto/crypto/provider/Loader.java @@ -239,7 +239,7 @@ private static void tryLoadLibraryFromJar() throws IOException { System.load(accpJniSharedLibraryPath.toString()); // If loading library from JAR file, then the compile-time and run-time libcrypto versions should be an exact match. - validateLibcryptoExactVersionMatch(); + validateLibcryptoVersion(false); maybeDeletePrivateTempDir(privateTempDirectory); } @@ -252,7 +252,7 @@ private static void tryLoadLibraryFromSystem() { System.loadLibrary(JNI_LIBRARY_NAME); // If loading from system directory, ensure compile-time and run-time libcrypto have same major and minor version - validateLibcryptoFuzzyVersionMatch(); + validateLibcryptoVersion(true); } private static void tryLoadLibrary() throws Exception { @@ -303,18 +303,11 @@ static void checkNativeLibraryAvailability() { private static native String getNativeLibraryVersion(); /** - * Validates that the LibCrypto available at runtime is exactly the same as what was available at compile time. - * This should only be done if loading ACCP from a JAR file, since if loading from system libraries then minor - * version upgrades to libcrypto may cause breakages. + * Validates that the LibCrypto available at runtime is the same as what was available at compile time. If + * fuzzyMatch is true, then only the major and minor version values of libcrypto's version number is compared. */ - private static native boolean validateLibcryptoExactVersionMatch(); + private static native boolean validateLibcryptoVersion(boolean fuzzyMatch); - /** - * Validates that the LibCrypto available at runtime has the same Major and Minor version as compile time, but allow - * the Patch version to be different. This should only be done if loading ACCP from a system directory to allow for - * patch security updates to libcrypto to be made independently from ACCP. - */ - private static native boolean validateLibcryptoFuzzyVersionMatch(); /** * Indicates if libcrypto is a FIPS build or not. From b968468f4e20e2847d69fa94bc22ecea0aafcc16 Mon Sep 17 00:00:00 2001 From: Alex Weibel Date: Tue, 28 Jun 2022 11:46:21 -0700 Subject: [PATCH 13/13] Switch Java Exception handling to unbounded std::string --- csrc/env.cpp | 2 +- csrc/env.h | 18 ++++++------------ 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/csrc/env.cpp b/csrc/env.cpp index f2455db8..96dc14dd 100644 --- a/csrc/env.cpp +++ b/csrc/env.cpp @@ -119,7 +119,7 @@ void java_ex::throw_to_java(JNIEnv *env) { if (likely(ex_class != NULL)) { std::ostringstream oss; - oss << m_message_cstr; + oss << m_message; #ifdef BACKTRACE_ON_EXCEPTION format_trace(oss, m_trace); diff --git a/csrc/env.h b/csrc/env.h index 4d921afd..eecf7e64 100644 --- a/csrc/env.h +++ b/csrc/env.h @@ -14,7 +14,6 @@ #include #include #include -#include #ifdef HAVE_IS_TRIVIALLY_COPYABLE #include @@ -51,7 +50,7 @@ class java_ex { jthrowable m_java_exception; const char *m_java_classname; - char m_message_cstr[256] = {0}; + const std::string m_message; #ifdef BACKTRACE_ON_EXCEPTION std::vector m_trace; void capture_trace() COLD { AmazonCorrettoCryptoProvider::capture_trace(m_trace); } @@ -61,22 +60,17 @@ class java_ex { public: java_ex(jthrowable exception) COLD - : m_java_exception(exception), m_java_classname(nullptr) + : m_java_exception(exception), m_java_classname(nullptr), m_message() { } java_ex(const char *java_classname, const char *message) COLD - : m_java_exception(nullptr), m_java_classname(java_classname) { - - capture_trace(); - - if (message != NULL) { - size_t amount_to_copy = strnlen(message, (sizeof(m_message_cstr) - 1)); - std::strncpy(m_message_cstr, message, amount_to_copy); - } + : java_ex(java_classname, std::string(message)) { } java_ex(const char *java_classname, const std::string &message) COLD - : java_ex(java_classname, message.c_str()) {} + : m_java_exception(nullptr), m_java_classname(java_classname), m_message(message) { + capture_trace(); + } /** * Constructs an exception based on the openssl error code.