Fix X509 validity comparisons depending on process time zone#129885
Open
koen-lee wants to merge 5 commits into
Open
Fix X509 validity comparisons depending on process time zone#129885koen-lee wants to merge 5 commits into
koen-lee wants to merge 5 commits into
Conversation
FindByTimeValid and CertificateRequest.Create's issuer-nesting check compare certificate validity against local-kind DateTime, so a mid-process timezone change can flip the result even though the underlying certificate validity instant hasn't moved. These tests fail today; the following commits fix the underlying comparisons.
X.509 validity periods are absolute instants encoded in UTC. The PAL layer was decoding that UTC instant correctly and then immediately converting it to a local-kind DateTime, forcing every internal consumer to convert back to compare correctly. Converting to local time and back is exactly the antipattern behind dotnet#109039: values that are fundamentally UTC instants get round-tripped through local time, so results depend on the machine's time zone and DST. Have every PAL backend (Windows, Apple, Android, OpenSSL) expose the UTC DateTimeOffset it already computes, and update internal consumers (X509Certificate's lazy cache, CertificateRequest's issuer-nesting check, ManagedCertificateFinder's time-based Find, the Apple chain processor's verification-time field, the OpenSSL CRL cache) to compare in UTC instead of local time. The public X509Certificate2.NotBefore/ NotAfter properties are unchanged (DateTimeKind.Local), projected from the cached UTC instant at the single public boundary. This turns the regression tests added in the previous commit green.
FindByTimeValid/NotYetValid/Expired were the last internal PAL surface still typed as DateTime for a validity comparison, requiring ManagedCertificateFinder to keep an explicit Local-vs-Utc normalization shim around an otherwise UTC-based comparison. Retype the interface and its implementations (Windows native find, ManagedCertificateFinder shared by Android/Apple/OpenSSL) to take DateTimeOffset directly, deleting the now-unnecessary normalization helper. The public X509Certificate2Collection.Find(...) contract is unchanged: callers still pass a DateTime, converted to DateTimeOffset at the FindPal.cs boundary via the implicit operator.
dotnet-policy-service
Bot
added
the
community-contribution
Contributor
|
Tagging subscribers to this area: @bartonjs, @vcsjones, @dotnet/area-system-security |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates System.Security.Cryptography.X509Certificates internals so certificate validity instants (NotBefore/NotAfter) are carried and compared as UTC instants (via DateTimeOffset) instead of being cached/compared through local time, preventing mid-process time zone changes from affecting correctness.
Changes:
- Retypes internal PAL validity properties (
ICertificatePalCore.NotBefore/NotAfter) and time-based find paths (IFindPal.FindByTime*) to useDateTimeOffsetfor stable UTC comparisons. - Updates platform backends (Windows/Apple/Android/OpenSSL) and key consumers (managed find, CRL cache,
CertificateRequestnesting check) to compare validity in UTC. - Adds Linux-only regression tests validating stability across an in-process TZ change for both chain and managed certificate validity paths.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Security.Cryptography/tests/X509Certificates/ChainTests.TimeZone.Linux.cs | Adds Linux RemoteExecutor tests to validate time-validity stability across TZ changes. |
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509Certificate2.cs | Projects cached UTC validity to local at the public property boundary. |
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509Certificate.cs | Changes lazy validity cache to store UTC instants as DateTimeOffset. |
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509ChainProcessor.cs | Passes DateTimeOffset verification time through revocation/CRL path. |
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509CertificateReader.cs | Returns OpenSSL validity instants as DateTimeOffset (UTC). |
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslCrlCache.cs | Uses DateTimeOffset for CRL cache expiration comparisons/logging. |
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ManagedCertificateFinder.cs | Compares validity using cached UTC instants (GetNotBeforeUtc/GetNotAfterUtc). |
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/IFindPal.cs | Retypes time-based find contract to DateTimeOffset. |
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ICertificatePalCore.cs | Retypes PAL validity contract to DateTimeOffset. |
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/FindPal.Windows.cs | Converts find-by-time to FILETIME using UTC instants. |
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Apple.cs | Updates Apple chain processing to store verification time as DateTimeOffset. |
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificateRequest.cs | Performs issuer/leaf nesting validity checks in UTC. |
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePal.Windows.cs | Returns Windows validity as DateTimeOffset from FILETIME. |
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificateData.ManagedDecode.cs | Keeps decoded validity as DateTimeOffset (UTC instant). |
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/AppleCertificatePal.TempExportPal.cs | Updates PAL forwarders to DateTimeOffset validity properties. |
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/AppleCertificatePal.cs | Returns Apple validity instants directly as DateTimeOffset. |
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/AndroidCertificatePal.cs | Returns Android validity instants directly as DateTimeOffset. |
| src/libraries/Common/src/Interop/Windows/Crypt32/Interop.CERT_INFO.cs | Adds FILETIME → DateTimeOffset conversion helper for Windows PAL. |
|
|
||
| internal static FILETIME FromDateTime(DateTime dt) | ||
| { | ||
| long fileTime = dt.ToFileTime(); |
Contributor
Author
There was a problem hiding this comment.
I'll complete the refactor and make this method obsolete.
Comment on lines
26
to
29
| private volatile bool _lazyKeyAlgorithmParametersCreated; | ||
| private DateTime _lazyNotBefore = DateTime.MinValue; | ||
| private DateTime _lazyNotAfter = DateTime.MinValue; | ||
| private DateTimeOffset _lazyNotBefore = DateTimeOffset.MinValue; | ||
| private DateTimeOffset _lazyNotAfter = DateTimeOffset.MinValue; | ||
|
|
Contributor
Author
There was a problem hiding this comment.
Right, this is a thing. I'll use an unambiguous long instead.
Member
There was a problem hiding this comment.
The previous DateTime values were 64-bit values not written with Volatile, so changing them to a long doesn't need to use Volatile (it isn't changing anything about torn state).
And since neither X509Certificate nor X509Certificate2 are documented as thread-safe, hoops don't need to be jumped through.
This was referenced Jun 26, 2026
Open
bartonjs
reviewed
Jun 26, 2026
| @@ -280,21 +280,21 @@ public string SignatureAlgorithm | |||
| } | |||
| } | |||
|
|
|||
| public DateTime NotAfter | |||
| public DateTimeOffset NotAfter | |||
Member
There was a problem hiding this comment.
Since the value ultimately being stored here is now UTC Ticks, I'd change the cert PAL to be public long NotAfterUtcTicks and avoid a lot of "turn this into a DateTimeOffset, then back into a long, then back into a DateTimeOffset".
Plus, a long return fits in a register and the DateTimeOffset doesn't.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #129394.
Problem
X.509 validity periods (
NotBefore/NotAfter) are absolute UTC instants bydefinition (RFC 5280). The internal PAL layer decodes that instant and
then immediately converts it to a local-kind
DateTime.Each hop through local time is a chance to get the time zone wrong, and because
X509Certificate2instances are long-lived, a value cached under one time zone canlater be read under different rules.
Tests
Two new tests in
ChainTests.TimeZone.Linux.cs, covering the managed pathsrather than the OpenSSL chain:
FindByTimeValid_StableAcrossTimeZoneChange: a cert inside its validity windowkeeps matching
Find(FindByTimeValid)across the change.CreateNesting_StableAcrossTimeZoneChange: a leaf that nests inside its issuer'svalidity keeps passing
CertificateRequest.Create's nesting check across thechange.
Both fail on
mainand pass after theICertificatePalCorechange.Linux is chosen as test platform because it allows programmatic TZ change, but the caching issue exists on all platforms.
Fix
Retype the internal PAL contract to carry the UTC instant as
DateTimeOffsetinstead of round-tripping through local time:
ICertificatePalCore.NotBefore/NotAfter→DateTimeOffset, returned directlyby all four backends (Windows, Apple, Android, OpenSSL) instead of via
ToLocalTime()/FromFileTime().X509Certificate's lazy cache (GetNotBefore()/GetNotAfter()=>GetNotBeforeUtc()/GetNotAfterUtc()) stores the UTC instant; local time isprojected fresh at the one remaining public boundary
(
X509Certificate2.NotBefore/NotAfter, unchangedDateTimeKind.Local).CertificateRequest's nesting check, the Apple chain processor's verificationtime, and the OpenSSL CRL cache now compare in UTC throughout.
IFindPal'sFindByTimeValid/NotYetValid/Expired=>DateTimeOffset, closingout the one remaining internal
DateTimesurface in the find path and deletingManagedCertificateFinder's now-unnecessary Local/UTC normalization shim.This is an internal-contract change only.
X509Certificate2.NotBefore/NotAfterand
X509Certificate2Collection.Find'sDateTimeparameter are unchanged; nopublic API changes.