Fix OpenSSL X509Chain time validity depending on process time zone#129394
Fix OpenSSL X509Chain time validity depending on process time zone#129394koen-lee wants to merge 5 commits into
Conversation
…ocess TZ change) Linux-only tests (RemoteExecutor + TZ) covering both directions of the OpenSSL chain verification-time bug, as a theory over Kind=Utc and Kind=Local verify times: a valid certificate must stay time-valid, and an expired certificate must stay NotTimeValid, across a mid-process time-zone change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The OpenSSL chain backend passed the verification time to native code as broken-down local fields and reconstructed it with mktime(), which re-resolves them against the process's current time zone. Because a certificate's validity window is a fixed UTC instant, a mid-process time-zone change shifted the effective verification instant and could flip the time-validity verdict (a valid cert reported NotTimeValid, or an expired cert no longer reported NotTimeValid). X509_VERIFY_PARAM_set_time takes a time_t (UTC epoch seconds), so the local round-trip was unnecessary. Pass the verification time as a UTC Unix timestamp: - Interop X509StoreSetVerifyTime now takes DateTimeOffset -> ToUnixTimeSeconds(). - CryptoNative_X509StoreSetVerifyTime takes int64_t and forwards it straight to X509_VERIFY_PARAM_set_time; MakeTimeT/mktime removed. The 32-bit time_t ARM guard is preserved. - The OpenSSL chain processor carries the verification time as a DateTimeOffset. Internal + native only; X509ChainPolicy.VerificationTime stays a public DateTime. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR fixes X509 chain verification-time handling on Linux/OpenSSL so that certificate time validity doesn’t depend on the process time zone.
Changes:
- Switch native verify-time API from local-time components to a Unix-time (UTC) instant.
- Update OpenSSL chain processor to carry verification time as
DateTimeOffsetand pass the instant to native. - Add Linux-only regression tests that toggle
TZin a child process to ensure verdict stability.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/native/libs/System.Security.Cryptography.Native/openssl.h | Updates native API signature to accept Unix time. |
| src/native/libs/System.Security.Cryptography.Native/openssl.c | Implements Unix-time based verify time, removing local-time conversion. |
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.OpenSsl.cs | Converts DateTime verificationTime into a DateTimeOffset instant for OpenSSL. |
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509ChainProcessor.cs | Stores verification time as DateTimeOffset; adjusts revocation call site. |
| src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Crypto.cs | Updates P/Invoke to pass Unix time seconds to native. |
| src/libraries/System.Security.Cryptography/tests/X509Certificates/ChainTests.TimeZone.Linux.cs | Adds regression tests for time-zone independence on Linux. |
| src/libraries/System.Security.Cryptography/tests/System.Security.Cryptography.Tests.csproj | Includes the new Linux time-zone test file in the test project. |
Comments suppressed due to low confidence (1)
src/native/libs/System.Security.Cryptography.Native/openssl.c:1
- Casting
int64_t unixTimedirectly totime_tcan overflow or wrap on platforms with 32-bittime_t(or for out-of-range inputs), potentially setting an incorrect verification instant rather than failing. This function used to reject invalid conversions (viamktimereturning-1), but now it always proceeds. Consider validating that the cast is lossless (e.g., round-trip compareunixTimevs(int64_t)(time_t)unixTime) and returning 0 on overflow/out-of-range so callers don’t get silently wrong chain results.
// Licensed to the .NET Foundation under one or more agreements.
|
Tagging subscribers to this area: @bartonjs, @vcsjones, @dotnet/area-system-security |
|
@dotnet-policy-service agree company="Logiqs b.v." |
bartonjs
left a comment
There was a problem hiding this comment.
Generally looks good. Since I generally love UTC, I wonder if OSSL 0.9.6 (the lowest version we ever supported, IIRC) had some TZ/local bias here. Even if it did, we've moved on.
Co-authored-by: Jeremy Barton <jbarton@microsoft.com>
| { | ||
| return 0; | ||
| } | ||
| time_t verifyTime = (time_t)unixTime; |
There was a problem hiding this comment.
time_t is asserted 8 bytes on
There was a problem hiding this comment.
Our portable compile will see the time as a t64, and if it's running on a known t32 system then the guard will correctly apply.
For a pure t32 system in direct compile, it's possible that dates in excess of 2038 would previously have ended up as -1 (I believe), and now will end up as the lower bits of their time value.
I'm not really sure of a good way to handle that, honestly. But t32 is running out of sane existence...
| Return values: | ||
| 0 if ctx is NULL, if ctx has no X509_VERIFY_PARAM, or the date inputs don't produce a valid time_t; | ||
| 0 if ctx is NULL or if ctx has no X509_VERIFY_PARAM; | ||
| 1 on success. |
| chain.Build(cert); | ||
| return (chain.AllStatusFlags() & X509ChainStatusFlags.NotTimeValid) == 0; |
Fixes #109039.
Problem
On the OpenSSL chain backend, the chain verification time was passed to native code as broken-down local fields (resolved against .NET's cached time zone) and reconstructed with
mktime()(which resolves them against libc's cached zone). These are two independent zone resolutions, so a system time-zone change while the process runs can desync them and the verdict flips:NotTimeValid(Certificate validation routine fails with NotTimeValid when changing time zone on Linux while client app is still running #109039);NotTimeValid.This is independent of how the caller supplied the verification time (
DateTime.Nowdefault, or an explicitVerificationTimeof either Kind).Fix
X509_VERIFY_PARAM_set_timetakes atime_t, so the local round-trip is not required (as noted in the issue). Instead, pass the verification time straight through as a UTC Unix timestamp:Interop.Crypto.X509StoreSetVerifyTimenow takes aDateTimeOffsetand passesToUnixTimeSeconds().CryptoNative_X509StoreSetVerifyTimetakes anint64_tand forwards it directly toX509_VERIFY_PARAM_set_time;MakeTimeT/mktimeare removed. The 32-bit-time_tARM guard (g_libSslUses32BitTime) is preserved.ChainPal.OpenSslno longer normalizes the verification time to local.This is an internal + native change only.
X509ChainPolicy.VerificationTimeremains a publicDateTime; no public API changes.Scope
Windows is unaffected; macOS already normalizes to UTC. This fix is OpenSSL/Linux-only.
OpenSslCrlCachereads cert validity through the same local-time path (ExtractValidityDateTime); that's a separate read path and considered out of scope here. I'm happy to follow up to carryDateTimeOffsetthrough the OpenSSL path /ICertificatePalfor consistency.Tests
Two Linux-only regression test classes (RemoteExecutor +
TZenvironment var, so the time-zone change is isolated to a child process), each a theory overKind=Utc/Kind=Localverification instants:ChainTimeZoneTests, a valid certificate stays valid across a mid-process time-zone change.ChainTimeZoneExpiredTests, an expired certificate staysNotTimeValidacross a mid-process time-zone change.I don't know how a unit test can deterministically change the process's system zone the way the field repro did (libc picks up the change, .NET stays stale), so the tests invert the desync:
ClearCachedData()re-syncs .NET to the newTZand leaves libc stale. Both tests fail before the fix and pass after.