From dde5a18d1c27ed2ce62f86bf17c35cc538475812 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Sat, 18 May 2024 00:18:10 +0000 Subject: [PATCH 1/9] Revert "Revert "Unpin arm build image, use Debian 12 helix images (#102059)" (#102324)" This reverts commit 9a18cf5d88f1a43097646f147140a7078760ec4e. --- eng/pipelines/common/templates/pipeline-with-resources.yml | 2 +- eng/pipelines/coreclr/templates/helix-queues-setup.yml | 4 ++-- eng/pipelines/libraries/helix-queues-setup.yml | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/eng/pipelines/common/templates/pipeline-with-resources.yml b/eng/pipelines/common/templates/pipeline-with-resources.yml index dd10e2a4bbe08d..bf668d6441fe66 100644 --- a/eng/pipelines/common/templates/pipeline-with-resources.yml +++ b/eng/pipelines/common/templates/pipeline-with-resources.yml @@ -17,7 +17,7 @@ extends: containers: linux_arm: - image: mcr.microsoft.com/dotnet-buildtools/prereqs:azurelinux-3.0-cross-arm-net9.0-20240507035943-1390eea + image: mcr.microsoft.com/dotnet-buildtools/prereqs:azurelinux-3.0-cross-arm-net9.0 env: ROOTFS_DIR: /crossrootfs/arm diff --git a/eng/pipelines/coreclr/templates/helix-queues-setup.yml b/eng/pipelines/coreclr/templates/helix-queues-setup.yml index c8558ac117ece6..7c43d1d992a1cc 100644 --- a/eng/pipelines/coreclr/templates/helix-queues-setup.yml +++ b/eng/pipelines/coreclr/templates/helix-queues-setup.yml @@ -63,9 +63,9 @@ jobs: # Linux arm - ${{ if eq(parameters.platform, 'linux_arm') }}: - ${{ if eq(variables['System.TeamProject'], 'public') }}: - - (Ubuntu.1804.Arm32.Open)Ubuntu.2004.Armarch.Open@mcr.microsoft.com/dotnet-buildtools/prereqs:ubuntu-18.04-helix-arm32v7 + - (Debian.12.Arm32.Open)Ubuntu.2004.ArmArch.Open@mcr.microsoft.com/dotnet-buildtools/prereqs:debian-12-helix-arm32v7 - ${{ if eq(variables['System.TeamProject'], 'internal') }}: - - (Ubuntu.1804.Arm32)Ubuntu.2004.Armarch@mcr.microsoft.com/dotnet-buildtools/prereqs:ubuntu-18.04-helix-arm32v7 + - (Debian.12.Arm32)Ubuntu.2004.ArmArch@mcr.microsoft.com/dotnet-buildtools/prereqs:debian-12-helix-arm32v7 # Linux arm64 - ${{ if eq(parameters.platform, 'linux_arm64') }}: diff --git a/eng/pipelines/libraries/helix-queues-setup.yml b/eng/pipelines/libraries/helix-queues-setup.yml index 91fe25701b07be..dd7e155a4afeba 100644 --- a/eng/pipelines/libraries/helix-queues-setup.yml +++ b/eng/pipelines/libraries/helix-queues-setup.yml @@ -26,7 +26,7 @@ jobs: # Linux arm - ${{ if eq(parameters.platform, 'linux_arm') }}: - ${{ if or(eq(parameters.jobParameters.isExtraPlatformsBuild, true), eq(parameters.jobParameters.includeAllPlatforms, true)) }}: - - (Debian.11.Arm32.Open)Ubuntu.2004.ArmArch.Open@mcr.microsoft.com/dotnet-buildtools/prereqs:debian-11-helix-arm32v7 + - (Debian.12.Arm32.Open)Ubuntu.2004.ArmArch.Open@mcr.microsoft.com/dotnet-buildtools/prereqs:debian-12-helix-arm32v7 # Linux armv6 - ${{ if eq(parameters.platform, 'linux_armv6') }}: From 84bb05073f0048623c617c07ca56b2a96084b79e Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Sat, 18 May 2024 00:17:12 +0000 Subject: [PATCH 2/9] Fix ARM ABI issue calling openssl function --- .../System.Security.Cryptography.Native/openssl.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/native/libs/System.Security.Cryptography.Native/openssl.c b/src/native/libs/System.Security.Cryptography.Native/openssl.c index 227cef44faaed0..898c9ba2925862 100644 --- a/src/native/libs/System.Security.Cryptography.Native/openssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/openssl.c @@ -964,7 +964,21 @@ int32_t CryptoNative_X509StoreSetVerifyTime(X509_STORE* ctx, return 0; } +#if defined(TARGET_ARM) && defined(TARGET_LINUX) + // We support ARM32 linux distros that have Y2038-compatible glibc (those which support _TIME_BITS). + // Some such distros have not yet switched to _TIME_BITS=64 by default, so we may be running against an openssl + // that expects 32-bit time_t even though our time_t is 64-bit. + + // Builds of openssl that use 32-bit time_t expect it to be passed to X509_VERIFY_PARAM_set_time in r1. + // Builds that use 64-bit time_t expect it to be passed in r2:r3, because the ARM ABI specifies: + // > If the argument requires double-word alignment (8-byte), the NCRN is rounded up to the next even register number. + + // Let's pretend we're calling a function that takes both an int32_t and a time_t. This ensures that we set both + // r1 (to the low bits of our time_t) and r2:r3 (to the full time_t) which will satisfy either ABI. + ((void (*)(X509_VERIFY_PARAM*, int32_t, time_t))(void*)(X509_VERIFY_PARAM_set_time))(verifyParams, (int32_t)verifyTime, verifyTime); +#else X509_VERIFY_PARAM_set_time(verifyParams, verifyTime); +#endif return 1; } From e5880d003230a39b8f731f8d6721bc6e377aea6c Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Mon, 20 May 2024 21:51:54 +0000 Subject: [PATCH 3/9] Detect openssl time_t bitness And avoid passing values that can't be represented in 32 bits when running against openssl that expects 32-bit time_t. --- .../openssl.c | 25 ++++++++++-------- .../opensslshim.c | 26 +++++++++++++++++++ .../opensslshim.h | 6 +++++ .../pal_x509.c | 17 +++++++++++- 4 files changed, 62 insertions(+), 12 deletions(-) diff --git a/src/native/libs/System.Security.Cryptography.Native/openssl.c b/src/native/libs/System.Security.Cryptography.Native/openssl.c index 898c9ba2925862..2bb2d4d73959d3 100644 --- a/src/native/libs/System.Security.Cryptography.Native/openssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/openssl.c @@ -965,20 +965,23 @@ int32_t CryptoNative_X509StoreSetVerifyTime(X509_STORE* ctx, } #if defined(TARGET_ARM) && defined(TARGET_LINUX) - // We support ARM32 linux distros that have Y2038-compatible glibc (those which support _TIME_BITS). - // Some such distros have not yet switched to _TIME_BITS=64 by default, so we may be running against an openssl - // that expects 32-bit time_t even though our time_t is 64-bit. + if (g_libSslUses32BitTime) + { + if (verifyTime > INT_MAX || verifyTime < INT_MIN) + { + return 0; + } - // Builds of openssl that use 32-bit time_t expect it to be passed to X509_VERIFY_PARAM_set_time in r1. - // Builds that use 64-bit time_t expect it to be passed in r2:r3, because the ARM ABI specifies: - // > If the argument requires double-word alignment (8-byte), the NCRN is rounded up to the next even register number. + // Builds of openssl that use 32-bit time_t expect it to be passed to X509_VERIFY_PARAM_set_time in r1. + // Builds that use 64-bit time_t expect it to be passed in r2:r3, because the ARM ABI specifies: + // > If the argument requires double-word alignment (8-byte), the NCRN is rounded up to the next even register number. + // Casting to a signature that takes int32_t ensures it will be passed in r1, not r2:r3. + ((void (*)(X509_VERIFY_PARAM*, int32_t))(void*)(X509_VERIFY_PARAM_set_time))(verifyParams, (int32_t)verifyTime); + return 1; + } +#endif - // Let's pretend we're calling a function that takes both an int32_t and a time_t. This ensures that we set both - // r1 (to the low bits of our time_t) and r2:r3 (to the full time_t) which will satisfy either ABI. - ((void (*)(X509_VERIFY_PARAM*, int32_t, time_t))(void*)(X509_VERIFY_PARAM_set_time))(verifyParams, (int32_t)verifyTime, verifyTime); -#else X509_VERIFY_PARAM_set_time(verifyParams, verifyTime); -#endif return 1; } diff --git a/src/native/libs/System.Security.Cryptography.Native/opensslshim.c b/src/native/libs/System.Security.Cryptography.Native/opensslshim.c index cd3e5f46b87da6..6db9547017414e 100644 --- a/src/native/libs/System.Security.Cryptography.Native/opensslshim.c +++ b/src/native/libs/System.Security.Cryptography.Native/opensslshim.c @@ -41,6 +41,15 @@ FOR_ALL_OPENSSL_FUNCTIONS #define MAKELIB(v) SONAME_BASE v #endif +#if defined(TARGET_ARM) && defined(TARGET_LINUX) +// We support ARM32 linux distros that have Y2038-compatible glibc (those which support _TIME_BITS). +// Some such distros have not yet switched to _TIME_BITS=64 by default, so we may be running against an openssl +// that expects 32-bit time_t even though our time_t is 64-bit. +// This can be deleted once the minimum supported Linux Arm32 distros are +// at least Debian 13 and Ubuntu 24.04. +bool g_libSslUses32BitTime = false; +#endif + static void DlOpen(const char* libraryName) { void* libsslNew = dlopen(libraryName, RTLD_LAZY); @@ -215,4 +224,21 @@ void InitializeOpenSSLShim(void) abort(); } } + +#if defined(TARGET_ARM) && defined(TARGET_LINUX) + // Detect whether openssl uses 32-bit or 64-bit time_t. + // This value will represent a time in year 2038 if 64-bit time is used, + // or 1901 if the lower 32 bits are interpreted as a 32-bit time_t value. + time_t timeVal = (time_t)INT_MAX + 1; + struct tm tmVal = { 0 }; + + // tm_year is the number of years since 1900. + if (!OPENSSL_gmtime(&timeVal, &tmVal) || (tmVal.tm_year != 138 && tmVal.tm_year != 1)) + { + fprintf(stderr, "Cannot determine the time_t size used by libssl\n"); + abort(); + } + + g_libSslUses32BitTime = (tmVal.tm_year == 1); +#endif } diff --git a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h index 7837810de715e2..b54226af605946 100644 --- a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h +++ b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h @@ -187,6 +187,10 @@ int EVP_DigestSqueeze(EVP_MD_CTX *ctx, unsigned char *out, size_t outlen); #define API_EXISTS(fn) (fn != NULL) +#if defined(TARGET_ARM) && defined(TARGET_LINUX) +extern bool g_libSslUses32BitTime; +#endif + // List of all functions from the libssl that are used in the System.Security.Cryptography.Native. // Forgetting to add a function here results in build failure with message reporting the function // that needs to be added. @@ -490,6 +494,7 @@ int EVP_DigestSqueeze(EVP_MD_CTX *ctx, unsigned char *out, size_t outlen); REQUIRED_FUNCTION(OCSP_RESPONSE_new) \ LEGACY_FUNCTION(OPENSSL_add_all_algorithms_conf) \ REQUIRED_FUNCTION(OPENSSL_cleanse) \ + REQUIRED_FUNCTION(OPENSSL_gmtime) \ REQUIRED_FUNCTION_110(OPENSSL_init_ssl) \ RENAMED_FUNCTION(OPENSSL_sk_free, sk_free) \ RENAMED_FUNCTION(OPENSSL_sk_new_null, sk_new_null) \ @@ -1018,6 +1023,7 @@ FOR_ALL_OPENSSL_FUNCTIONS #define OCSP_RESPONSE_new OCSP_RESPONSE_new_ptr #define OPENSSL_add_all_algorithms_conf OPENSSL_add_all_algorithms_conf_ptr #define OPENSSL_cleanse OPENSSL_cleanse_ptr +#define OPENSSL_gmtime OPENSSL_gmtime_ptr #define OPENSSL_init_ssl OPENSSL_init_ssl_ptr #define OPENSSL_sk_free OPENSSL_sk_free_ptr #define OPENSSL_sk_new_null OPENSSL_sk_new_null_ptr diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_x509.c b/src/native/libs/System.Security.Cryptography.Native/pal_x509.c index 04c6ba06cd5dce..6532b8caaa6866 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_x509.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_x509.c @@ -999,7 +999,22 @@ static X509VerifyStatusCode CheckOcspGetExpiry(OCSP_REQUEST* req, { time_t oldest = GetIssuanceWindowStart(); - if (X509_cmp_time(thisupd, &oldest) > 0) + int cmp = 0; +#if defined(TARGET_ARM) && defined(TARGET_LINUX) + if (g_libSslUses32BitTime) + { + if (oldest <= INT_MAX && oldest >= INT_MIN) + { + cmp = X509_cmp_time(thisupd, &oldest); + } + } + else +#endif + { + cmp = X509_cmp_time(thisupd, &oldest); + } + + if (cmp > 0) { *canCache = 1; From f3f50587bf2338111f2143d6e406a1a44a5d7a75 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Tue, 21 May 2024 21:02:08 -0700 Subject: [PATCH 4/9] Return failure code if for invalid time --- .../opensslshim.h | 2 -- .../pal_x509.c | 36 ++++++++----------- 2 files changed, 14 insertions(+), 24 deletions(-) diff --git a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h index b54226af605946..2bff9b816a3737 100644 --- a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h +++ b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h @@ -623,7 +623,6 @@ extern bool g_libSslUses32BitTime; REQUIRED_FUNCTION(SSL_version) \ FALLBACK_FUNCTION(X509_check_host) \ REQUIRED_FUNCTION(X509_check_purpose) \ - REQUIRED_FUNCTION(X509_cmp_current_time) \ REQUIRED_FUNCTION(X509_cmp_time) \ REQUIRED_FUNCTION(X509_CRL_free) \ FALLBACK_FUNCTION(X509_CRL_get0_nextUpdate) \ @@ -1155,7 +1154,6 @@ FOR_ALL_OPENSSL_FUNCTIONS #define TLS_method TLS_method_ptr #define X509_check_host X509_check_host_ptr #define X509_check_purpose X509_check_purpose_ptr -#define X509_cmp_current_time X509_cmp_current_time_ptr #define X509_cmp_time X509_cmp_time_ptr #define X509_CRL_free X509_CRL_free_ptr #define X509_CRL_get0_nextUpdate X509_CRL_get0_nextUpdate_ptr diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_x509.c b/src/native/libs/System.Security.Cryptography.Native/pal_x509.c index 6532b8caaa6866..a47542546b4257 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_x509.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_x509.c @@ -893,14 +893,12 @@ static OCSP_CERTID* MakeCertId(X509* subject, X509* issuer) return OCSP_cert_to_id(EVP_sha1(), subject, issuer); } -static time_t GetIssuanceWindowStart(void) +static time_t GetIssuanceWindowStart(time_t currentTime) { // time_t granularity is seconds, so subtract 4 days worth of seconds. // The 4 day policy is based on the CA/Browser Forum Baseline Requirements // (version 1.6.3) section 4.9.10 (On-Line Revocation Checking Requirements) - time_t t = time(NULL); - t -= 4 * 24 * 60 * 60; - return t; + return currentTime - 4 * 24 * 60 * 60; } static X509VerifyStatusCode CheckOcspGetExpiry(OCSP_REQUEST* req, @@ -960,10 +958,18 @@ static X509VerifyStatusCode CheckOcspGetExpiry(OCSP_REQUEST* req, if (OCSP_resp_find_status(basicResp, certId, &status, NULL, NULL, &thisupd, &nextupd)) { + time_t currentTime = time(NULL); + int nextUpdComparison = 0; +#if defined(TARGET_ARM) && defined(TARGET_LINUX) + // If openssl uses 32-bit time_t and the current time doesn't fit in 32 bits, + // skip checking the status/nextupd, and fall through to return PAL_X509_V_ERR_UNABLE_TO_GET_CRL. + if (!g_libSslUses32BitTime || (currentTime >= INT_MIN && currentTime <= INT_MAX)) +#endif + { // X509_cmp_current_time uses 0 for error already, so we can use it when there's a null value. // 1 means the nextupd value is in the future, -1 means it is now-or-in-the-past. // Following with OpenSSL conventions, we'll accept "now" as "the past". - int nextUpdComparison = nextupd == NULL ? 0 : X509_cmp_current_time(nextupd); + nextUpdComparison = nextupd == NULL ? 0 : X509_cmp_time(nextupd, ¤tTime); // Un-revoking is rare, so reporting revoked on an expired response has a low chance // of a false-positive. @@ -982,6 +988,7 @@ static X509VerifyStatusCode CheckOcspGetExpiry(OCSP_REQUEST* req, else if (status == V_OCSP_CERTSTATUS_GOOD) { ret = PAL_X509_V_OK; + } } } @@ -997,24 +1004,9 @@ static X509VerifyStatusCode CheckOcspGetExpiry(OCSP_REQUEST* req, thisupd != NULL && nextUpdComparison > 0) { - time_t oldest = GetIssuanceWindowStart(); - - int cmp = 0; -#if defined(TARGET_ARM) && defined(TARGET_LINUX) - if (g_libSslUses32BitTime) - { - if (oldest <= INT_MAX && oldest >= INT_MIN) - { - cmp = X509_cmp_time(thisupd, &oldest); - } - } - else -#endif - { - cmp = X509_cmp_time(thisupd, &oldest); - } + time_t oldest = GetIssuanceWindowStart(currentTime); - if (cmp > 0) + if (X509_cmp_time(thisupd, &oldest) > 0) { *canCache = 1; From b25bb1ded92fea060a0f8e7501ea3fdba9e2dea9 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Tue, 21 May 2024 21:11:03 -0700 Subject: [PATCH 5/9] Adjust comments --- .../libs/System.Security.Cryptography.Native/openssl.c | 5 +---- .../libs/System.Security.Cryptography.Native/opensslshim.c | 4 +++- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/native/libs/System.Security.Cryptography.Native/openssl.c b/src/native/libs/System.Security.Cryptography.Native/openssl.c index 2bb2d4d73959d3..e6e6c5efcaa31b 100644 --- a/src/native/libs/System.Security.Cryptography.Native/openssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/openssl.c @@ -972,10 +972,7 @@ int32_t CryptoNative_X509StoreSetVerifyTime(X509_STORE* ctx, return 0; } - // Builds of openssl that use 32-bit time_t expect it to be passed to X509_VERIFY_PARAM_set_time in r1. - // Builds that use 64-bit time_t expect it to be passed in r2:r3, because the ARM ABI specifies: - // > If the argument requires double-word alignment (8-byte), the NCRN is rounded up to the next even register number. - // Casting to a signature that takes int32_t ensures it will be passed in r1, not r2:r3. + // Cast to a signature that takes a 32-bit value for the time. ((void (*)(X509_VERIFY_PARAM*, int32_t))(void*)(X509_VERIFY_PARAM_set_time))(verifyParams, (int32_t)verifyTime); return 1; } diff --git a/src/native/libs/System.Security.Cryptography.Native/opensslshim.c b/src/native/libs/System.Security.Cryptography.Native/opensslshim.c index 6db9547017414e..77a6c1b222abb2 100644 --- a/src/native/libs/System.Security.Cryptography.Native/opensslshim.c +++ b/src/native/libs/System.Security.Cryptography.Native/opensslshim.c @@ -226,12 +226,14 @@ void InitializeOpenSSLShim(void) } #if defined(TARGET_ARM) && defined(TARGET_LINUX) - // Detect whether openssl uses 32-bit or 64-bit time_t. // This value will represent a time in year 2038 if 64-bit time is used, // or 1901 if the lower 32 bits are interpreted as a 32-bit time_t value. time_t timeVal = (time_t)INT_MAX + 1; struct tm tmVal = { 0 }; + // Detect whether openssl is using 32-bit or 64-bit time_t. + // If it uses 32-bit time_t, little-endianness means that the pointer + // will be interpreted as a pointer to the lower 32 bits of timeVal. // tm_year is the number of years since 1900. if (!OPENSSL_gmtime(&timeVal, &tmVal) || (tmVal.tm_year != 138 && tmVal.tm_year != 1)) { From 45c2bc7775dd5d82bd1429cfc84123567f301ef5 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Tue, 21 May 2024 21:13:41 -0700 Subject: [PATCH 6/9] Fix whitespace --- .../pal_x509.c | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_x509.c b/src/native/libs/System.Security.Cryptography.Native/pal_x509.c index a47542546b4257..0fb577e66d13c8 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_x509.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_x509.c @@ -966,28 +966,28 @@ static X509VerifyStatusCode CheckOcspGetExpiry(OCSP_REQUEST* req, if (!g_libSslUses32BitTime || (currentTime >= INT_MIN && currentTime <= INT_MAX)) #endif { - // X509_cmp_current_time uses 0 for error already, so we can use it when there's a null value. - // 1 means the nextupd value is in the future, -1 means it is now-or-in-the-past. - // Following with OpenSSL conventions, we'll accept "now" as "the past". + // X509_cmp_current_time uses 0 for error already, so we can use it when there's a null value. + // 1 means the nextupd value is in the future, -1 means it is now-or-in-the-past. + // Following with OpenSSL conventions, we'll accept "now" as "the past". nextUpdComparison = nextupd == NULL ? 0 : X509_cmp_time(nextupd, ¤tTime); - // Un-revoking is rare, so reporting revoked on an expired response has a low chance - // of a false-positive. - // - // For non-revoked responses, a next-update value in the past counts as expired. - if (status == V_OCSP_CERTSTATUS_REVOKED) - { - ret = PAL_X509_V_ERR_CERT_REVOKED; - } - else - { - if (nextupd != NULL && nextUpdComparison <= 0) + // Un-revoking is rare, so reporting revoked on an expired response has a low chance + // of a false-positive. + // + // For non-revoked responses, a next-update value in the past counts as expired. + if (status == V_OCSP_CERTSTATUS_REVOKED) { - ret = PAL_X509_V_ERR_CRL_HAS_EXPIRED; + ret = PAL_X509_V_ERR_CERT_REVOKED; } - else if (status == V_OCSP_CERTSTATUS_GOOD) + else { - ret = PAL_X509_V_OK; + if (nextupd != NULL && nextUpdComparison <= 0) + { + ret = PAL_X509_V_ERR_CRL_HAS_EXPIRED; + } + else if (status == V_OCSP_CERTSTATUS_GOOD) + { + ret = PAL_X509_V_OK; } } } From c5719ffb880640740e934c04ead7aaa7261396e6 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Wed, 22 May 2024 08:01:47 -0700 Subject: [PATCH 7/9] Move OPENSSL_gmtime out of FOR_ALL_OPENSSL_FUNCTIONS Apparently this symbol did not exist in the builds of openssl in Ubuntu 16.04. --- .../libs/System.Security.Cryptography.Native/opensslshim.c | 7 +++++++ .../libs/System.Security.Cryptography.Native/opensslshim.h | 5 +++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/native/libs/System.Security.Cryptography.Native/opensslshim.c b/src/native/libs/System.Security.Cryptography.Native/opensslshim.c index 77a6c1b222abb2..f3b425bf36ce3b 100644 --- a/src/native/libs/System.Security.Cryptography.Native/opensslshim.c +++ b/src/native/libs/System.Security.Cryptography.Native/opensslshim.c @@ -25,6 +25,9 @@ FOR_ALL_OPENSSL_FUNCTIONS #undef LIGHTUP_FUNCTION #undef REQUIRED_FUNCTION_110 #undef REQUIRED_FUNCTION +#if defined(TARGET_ARM) && defined(TARGET_LINUX) +TYPEOF(OPENSSL_gmtime) OPENSSL_gmtime_ptr; +#endif // x.x.x, considering the max number of decimal digits for each component #define MaxVersionStringLength 32 @@ -214,6 +217,10 @@ void InitializeOpenSSLShim(void) #undef LIGHTUP_FUNCTION #undef REQUIRED_FUNCTION_110 #undef REQUIRED_FUNCTION +#if defined(TARGET_ARM) && defined(TARGET_LINUX) + if (!(OPENSSL_gmtime_ptr = (TYPEOF(OPENSSL_gmtime))(dlsym(libssl, "OPENSSL_gmtime")))) { fprintf(stderr, "Cannot get required symbol OPENSSL_gmtime from libssl\n"); abort(); } +#endif + // Sanity check that we have at least one functioning way of reporting errors. if (ERR_put_error_ptr == &local_ERR_put_error) diff --git a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h index 2bff9b816a3737..7353f8a4e5712e 100644 --- a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h +++ b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h @@ -494,7 +494,6 @@ extern bool g_libSslUses32BitTime; REQUIRED_FUNCTION(OCSP_RESPONSE_new) \ LEGACY_FUNCTION(OPENSSL_add_all_algorithms_conf) \ REQUIRED_FUNCTION(OPENSSL_cleanse) \ - REQUIRED_FUNCTION(OPENSSL_gmtime) \ REQUIRED_FUNCTION_110(OPENSSL_init_ssl) \ RENAMED_FUNCTION(OPENSSL_sk_free, sk_free) \ RENAMED_FUNCTION(OPENSSL_sk_new_null, sk_new_null) \ @@ -721,7 +720,9 @@ FOR_ALL_OPENSSL_FUNCTIONS #undef LIGHTUP_FUNCTION #undef REQUIRED_FUNCTION_110 #undef REQUIRED_FUNCTION - +#if defined(TARGET_ARM) && defined(TARGET_LINUX) +extern TYPEOF(OPENSSL_gmtime)* OPENSSL_gmtime_ptr; +#endif // Redefine all calls to OpenSSL functions as calls through pointers that are set // to the functions from the libssl.so selected by the shim. #define a2d_ASN1_OBJECT a2d_ASN1_OBJECT_ptr From 414f24e18cf814a5946b99d5edad6b2a75c8e3f8 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Wed, 22 May 2024 11:04:06 -0700 Subject: [PATCH 8/9] Check FEATURE_DISTRO_AGNOSTIC_SSL Except in opensslshim.c, which is only built for that feature. --- .../libs/System.Security.Cryptography.Native/openssl.c | 5 +---- .../libs/System.Security.Cryptography.Native/opensslshim.h | 2 +- .../libs/System.Security.Cryptography.Native/pal_x509.c | 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/native/libs/System.Security.Cryptography.Native/openssl.c b/src/native/libs/System.Security.Cryptography.Native/openssl.c index e6e6c5efcaa31b..efe3829ef8a28d 100644 --- a/src/native/libs/System.Security.Cryptography.Native/openssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/openssl.c @@ -7,10 +7,7 @@ #include "pal_safecrt.h" #include "pal_x509.h" #include "openssl.h" - -#ifdef FEATURE_DISTRO_AGNOSTIC_SSL #include "opensslshim.h" -#endif #include #include @@ -964,7 +961,7 @@ int32_t CryptoNative_X509StoreSetVerifyTime(X509_STORE* ctx, return 0; } -#if defined(TARGET_ARM) && defined(TARGET_LINUX) +#if defined(FEATURE_DISTRO_AGNOSTIC_SSL) && defined(TARGET_ARM) && defined(TARGET_LINUX) if (g_libSslUses32BitTime) { if (verifyTime > INT_MAX || verifyTime < INT_MIN) diff --git a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h index 7353f8a4e5712e..c4aa47d18cfae2 100644 --- a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h +++ b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h @@ -187,7 +187,7 @@ int EVP_DigestSqueeze(EVP_MD_CTX *ctx, unsigned char *out, size_t outlen); #define API_EXISTS(fn) (fn != NULL) -#if defined(TARGET_ARM) && defined(TARGET_LINUX) +#if defined(FEATURE_DISTRO_AGNOSTIC_SSL) && defined(TARGET_ARM) && defined(TARGET_LINUX) extern bool g_libSslUses32BitTime; #endif diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_x509.c b/src/native/libs/System.Security.Cryptography.Native/pal_x509.c index 0fb577e66d13c8..d75feeb334ac11 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_x509.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_x509.c @@ -960,7 +960,7 @@ static X509VerifyStatusCode CheckOcspGetExpiry(OCSP_REQUEST* req, { time_t currentTime = time(NULL); int nextUpdComparison = 0; -#if defined(TARGET_ARM) && defined(TARGET_LINUX) +#if defined(FEATURE_DISTRO_AGNOSTIC_SSL) && defined(TARGET_ARM) && defined(TARGET_LINUX) // If openssl uses 32-bit time_t and the current time doesn't fit in 32 bits, // skip checking the status/nextupd, and fall through to return PAL_X509_V_ERR_UNABLE_TO_GET_CRL. if (!g_libSslUses32BitTime || (currentTime >= INT_MIN && currentTime <= INT_MAX)) From bac65d4e2070402d3b09041246374efe714343f5 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Wed, 22 May 2024 11:33:23 -0700 Subject: [PATCH 9/9] PR feedback - Revert unintentional change --- src/native/libs/System.Security.Cryptography.Native/openssl.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/native/libs/System.Security.Cryptography.Native/openssl.c b/src/native/libs/System.Security.Cryptography.Native/openssl.c index efe3829ef8a28d..50a3292d054182 100644 --- a/src/native/libs/System.Security.Cryptography.Native/openssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/openssl.c @@ -7,7 +7,10 @@ #include "pal_safecrt.h" #include "pal_x509.h" #include "openssl.h" + +#ifdef FEATURE_DISTRO_AGNOSTIC_SSL #include "opensslshim.h" +#endif #include #include