From 5fdc9b8fd9d8ce854d7d333838e2d7806024092e Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Wed, 22 May 2024 14:27:51 -0700 Subject: [PATCH] Build against Y2038-compatible glibc for linux arm32 (#102410) This updates our linux arm32 build to build against a more recent glibc that supports _TIME_BITS (which we set to 64). Since openssl may be using either 32-bit or 64-bit time_t, this includes detection logic to determine which case we are in, and avoid passing time values that don't fit in 32 bits to openssl. The arm build image is updated to the latest version of the images added in https://github.com/dotnet/dotnet-buildtools-prereqs-docker/pull/1037. The helix test images are updated to debian images added in https://github.com/dotnet/dotnet-buildtools-prereqs-docker/pull/1041. Additional context: Additional context: Reintroduces the fix for Y2038 support on arm32 linux (https://github.com/dotnet/runtime/pull/102059), which was reverted due to problems running against openssl built with _TIME_BITS=32. Fixes https://github.com/dotnet/runtime/issues/101444 (both the originally reported issue, and the test failures mentioned in https://github.com/dotnet/runtime/issues/101444#issuecomment-2111415106). Supports: https://github.com/dotnet/runtime/issues/91826 --- .../templates/pipeline-with-resources.yml | 2 +- .../coreclr/templates/helix-queues-setup.yml | 4 +- .../libraries/helix-queues-setup.yml | 2 +- .../openssl.c | 14 +++++ .../opensslshim.c | 35 ++++++++++++ .../opensslshim.h | 11 ++-- .../pal_x509.c | 53 +++++++++++-------- 7 files changed, 91 insertions(+), 30 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') }}: diff --git a/src/native/libs/System.Security.Cryptography.Native/openssl.c b/src/native/libs/System.Security.Cryptography.Native/openssl.c index 227cef44faaed0..50a3292d054182 100644 --- a/src/native/libs/System.Security.Cryptography.Native/openssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/openssl.c @@ -964,6 +964,20 @@ int32_t CryptoNative_X509StoreSetVerifyTime(X509_STORE* ctx, return 0; } +#if defined(FEATURE_DISTRO_AGNOSTIC_SSL) && defined(TARGET_ARM) && defined(TARGET_LINUX) + if (g_libSslUses32BitTime) + { + if (verifyTime > INT_MAX || verifyTime < INT_MIN) + { + return 0; + } + + // 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; + } +#endif + X509_VERIFY_PARAM_set_time(verifyParams, 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 cd3e5f46b87da6..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 @@ -41,6 +44,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); @@ -205,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) @@ -215,4 +231,23 @@ void InitializeOpenSSLShim(void) abort(); } } + +#if defined(TARGET_ARM) && defined(TARGET_LINUX) + // 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)) + { + 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..c4aa47d18cfae2 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(FEATURE_DISTRO_AGNOSTIC_SSL) && 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. @@ -618,7 +622,6 @@ int EVP_DigestSqueeze(EVP_MD_CTX *ctx, unsigned char *out, size_t outlen); 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) \ @@ -717,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 @@ -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 @@ -1149,7 +1155,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 04c6ba06cd5dce..d75feeb334ac11 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,28 +958,37 @@ static X509VerifyStatusCode CheckOcspGetExpiry(OCSP_REQUEST* req, if (OCSP_resp_find_status(basicResp, certId, &status, NULL, NULL, &thisupd, &nextupd)) { - // 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); - - // 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 + time_t currentTime = time(NULL); + int nextUpdComparison = 0; +#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)) +#endif { - if (nextupd != NULL && nextUpdComparison <= 0) + // 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_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; + } } } @@ -997,7 +1004,7 @@ static X509VerifyStatusCode CheckOcspGetExpiry(OCSP_REQUEST* req, thisupd != NULL && nextUpdComparison > 0) { - time_t oldest = GetIssuanceWindowStart(); + time_t oldest = GetIssuanceWindowStart(currentTime); if (X509_cmp_time(thisupd, &oldest) > 0) {