Always build the OpenSSL shim in portable mode on macOS.
authorJeremy Barton <jbarton@microsoft.com>
Fri, 10 Jan 2020 16:13:53 +0000 (08:13 -0800)
committerGitHub <noreply@github.com>
Fri, 10 Jan 2020 16:13:53 +0000 (08:13 -0800)
src/libraries/Native/Unix/System.Security.Cryptography.Native/CMakeLists.txt
src/libraries/Native/Unix/System.Security.Cryptography.Native/openssl.c
src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.c

index bdf4772..cacf8a3 100644 (file)
@@ -49,11 +49,14 @@ set(NATIVECRYPTO_SOURCES
     pal_x509ext.c
 )
 
-if (FEATURE_DISTRO_AGNOSTIC_SSL)
-    if (NOT CMAKE_SYSTEM_NAME STREQUAL Linux)
-        message(FATAL_ERROR "FEATURE_DISTRO_AGNOSTIC_SSL can only be enabled for Linux")
-    endif()
+# Always build portable on macOS because OpenSSL is not a system component
+# and our prebuilts should not assume a specific ABI version for the types
+# that use OpenSSL at runtime.
+if (APPLE)
+    set(FEATURE_DISTRO_AGNOSTIC_SSL True)
+endif()
 
+if (FEATURE_DISTRO_AGNOSTIC_SSL)
     list(APPEND NATIVECRYPTO_SOURCES
         opensslshim.c
     )
@@ -80,13 +83,20 @@ set_target_properties(System.Security.Cryptography.Native.OpenSsl-Static PROPERT
 set_target_properties(System.Security.Cryptography.Native.OpenSsl-Static PROPERTIES OUTPUT_NAME System.Security.Cryptography.Native.OpenSsl CLEAN_DIRECT_OUTPUT 1)
 
 if (FEATURE_DISTRO_AGNOSTIC_SSL)
-    add_custom_command(TARGET System.Security.Cryptography.Native.OpenSsl POST_BUILD
-        COMMENT "Verifying System.Security.Cryptography.Native.OpenSsl.so dependencies"
-        COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/../verify-so.sh 
-            $<TARGET_FILE:System.Security.Cryptography.Native.OpenSsl> 
-            "Verification failed. System.Security.Cryptography.Native.OpenSsl.so has undefined dependencies. These are likely OpenSSL APIs that need to be added to opensslshim.h"
-        VERBATIM
-    )
+    # on macOS the link step fails with undefined symbols, and the script doesn't run.
+    # if the build succeeds, the script would succeed, except it uses a Linux-only command.
+    #
+    # on Linux, the build will succeed with undefined symbols, then the script reports them
+    # and fails the build for us.
+    if (NOT APPLE)
+        add_custom_command(TARGET System.Security.Cryptography.Native.OpenSsl POST_BUILD
+            COMMENT "Verifying System.Security.Cryptography.Native.OpenSsl.so dependencies"
+            COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/../verify-so.sh 
+                $<TARGET_FILE:System.Security.Cryptography.Native.OpenSsl> 
+                "Verification failed. System.Security.Cryptography.Native.OpenSsl.so has undefined dependencies. These are likely OpenSSL APIs that need to be added to opensslshim.h"
+            VERBATIM
+        )
+    endif()
 
     # Link with libdl.so to get the dlopen / dlsym / dlclose
     target_link_libraries(System.Security.Cryptography.Native.OpenSsl
@@ -97,22 +107,6 @@ else()
       ${OPENSSL_CRYPTO_LIBRARY}
       ${OPENSSL_SSL_LIBRARY}
     )
-  
-    # On OS X every library emits the manner in which it should be referenced.
-    # All of our libraries are referenced via @rpath, which is similar to how Linux and Windows
-    # libraries are loaded. The homebrew installation of OpenSSL (libcrypto, libssl) uses the
-    # full path to the library installation. This means that this library is not flexible to
-    # users installing newer libcrypto in the working directory, or to systems which do not
-    # install to the same path as homebrew does.
-    #
-    # So, after compiling, rewrite the references to libcrypto to be more flexible.
-    if (APPLE)
-        add_custom_command(TARGET System.Security.Cryptography.Native.OpenSsl POST_BUILD
-            COMMAND ${CMAKE_INSTALL_NAME_TOOL} -change /usr/local/opt/openssl/lib/libcrypto.1.0.0.dylib @rpath/libcrypto.1.0.0.dylib $<TARGET_FILE:System.Security.Cryptography.Native.OpenSsl>
-            COMMAND ${CMAKE_INSTALL_NAME_TOOL} -change /usr/local/opt/openssl/lib/libssl.1.0.0.dylib @rpath/libssl.1.0.0.dylib $<TARGET_FILE:System.Security.Cryptography.Native.OpenSsl>
-            COMMAND ${CMAKE_INSTALL_NAME_TOOL} -add_rpath @loader_path $<TARGET_FILE:System.Security.Cryptography.Native.OpenSsl>
-            )
-    endif()
 endif()
 
 include(configure.cmake)
index b59943b..1a9ea04 100644 (file)
@@ -1159,24 +1159,6 @@ static void LockingCallback(int mode, int n, const char* file, int line)
 #pragma clang diagnostic pop
 }
 
-#ifdef __APPLE__
-/*
-Function:
-GetCurrentThreadId
-
-Called back by OpenSSL to get the current thread id.
-
-This is necessary because OSX uses an earlier version of
-OpenSSL, which requires setting the CRYPTO_set_id_callback.
-*/
-static unsigned long GetCurrentThreadId()
-{
-    uint64_t tid;
-    pthread_threadid_np(pthread_self(), &tid);
-    return tid;
-}
-#endif // __APPLE__
-
 /*
 Function:
 EnsureOpenSslInitialized
@@ -1239,11 +1221,6 @@ static int32_t EnsureOpenSsl10Initialized()
     // Initialize the callback
     CRYPTO_set_locking_callback(LockingCallback);
 
-#ifdef __APPLE__
-    // OSX uses an earlier version of OpenSSL which requires setting the CRYPTO_set_id_callback
-    CRYPTO_set_id_callback(GetCurrentThreadId);
-#endif
-
     // Initialize the random number generator seed
     randPollResult = RAND_poll();
     if (randPollResult < 1)
index 2d3fe03..b33aa12 100644 (file)
@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 //
 
+#include <assert.h>
 #include <dlfcn.h>
 #include <stdio.h>
 #include <stdbool.h>
@@ -27,10 +28,24 @@ FOR_ALL_OPENSSL_FUNCTIONS
 
 // x.x.x, considering the max number of decimal digits for each component
 #define MaxVersionStringLength 32
-#define SONAME_BASE "libssl.so."
 
 static void* libssl = NULL;
 
+#ifdef __APPLE__
+#define DYLIBNAME_PREFIX "libssl."
+#define DYLIBNAME_SUFFIX ".dylib"
+#define MAKELIB(v) DYLIBNAME_PREFIX v DYLIBNAME_SUFFIX
+#else
+#define SONAME_BASE "libssl.so."
+#define MAKELIB(v)  SONAME_BASE v
+#endif
+
+static void DlOpen(const char* libraryName)
+{
+    assert(libssl == NULL);
+    libssl = dlopen(libraryName, RTLD_LAZY);
+}
+
 static bool OpenLibrary()
 {
     // If there is an override of the version specified using the CLR_OPENSSL_VERSION_OVERRIDE
@@ -41,16 +56,25 @@ static bool OpenLibrary()
 
     if ((versionOverride != NULL) && strnlen(versionOverride, MaxVersionStringLength + 1) <= MaxVersionStringLength)
     {
+#ifdef __APPLE__
+        char soName[sizeof(DYLIBNAME_PREFIX) + MaxVersionStringLength + sizeof(DYLIBNAME_SUFFIX)] =
+            DYLIBNAME_PREFIX;
+
+        strcat(soName, versionOverride);
+        strcat(soName, DYLIBNAME_SUFFIX);
+#else
         char soName[sizeof(SONAME_BASE) + MaxVersionStringLength] = SONAME_BASE;
 
         strcat(soName, versionOverride);
-        libssl = dlopen(soName, RTLD_LAZY);
+#endif
+
+        DlOpen(soName);
     }
 
     if (libssl == NULL)
     {
         // Prefer OpenSSL 1.1.x
-        libssl = dlopen("libssl.so.1.1", RTLD_LAZY);
+        DlOpen(MAKELIB("1.1"));
     }
 
     if (libssl == NULL)
@@ -58,19 +82,19 @@ static bool OpenLibrary()
         // Debian 9 has dropped support for SSLv3 and so they have bumped their soname. Let's try it
         // before trying the version 1.0.0 to make it less probable that some of our other dependencies 
         // end up loading conflicting version of libssl.
-        libssl = dlopen("libssl.so.1.0.2", RTLD_LAZY);
+        DlOpen(MAKELIB("1.0.2"));
     }
 
     if (libssl == NULL)
     {
         // Now try the default versioned so naming as described in the OpenSSL doc
-        libssl = dlopen("libssl.so.1.0.0", RTLD_LAZY);
+        DlOpen(MAKELIB("1.0.0"));
     }
 
     if (libssl == NULL)
     {
         // Fedora derived distros use different naming for the version 1.0.0
-        libssl = dlopen("libssl.so.10", RTLD_LAZY);
+        DlOpen(MAKELIB("10"));
     }
 
     return libssl != NULL;