Fix X509 validity comparisons depending on process time zone

[koen-lee]

Follow-up to #129394.

Problem

X.509 validity periods (NotBefore/NotAfter) are absolute UTC instants by
definition (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
X509Certificate2 instances are long-lived, a value cached under one time zone can
later be read under different rules.

Tests

Two new tests in ChainTests.TimeZone.Linux.cs, covering the managed paths
rather than the OpenSSL chain:

• FindByTimeValid_StableAcrossTimeZoneChange: a cert inside its validity window
  keeps matching Find(FindByTimeValid) across the change.
• CreateNesting_StableAcrossTimeZoneChange: a leaf that nests inside its issuer's
  validity keeps passing CertificateRequest.Create's nesting check across the
  change.

Both fail on main and pass after the ICertificatePalCore change.
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 DateTimeOffset
instead of round-tripping through local time:

• ICertificatePalCore.NotBefore/NotAfter → DateTimeOffset, returned directly
  by 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 is
  projected fresh at the one remaining public boundary
  (X509Certificate2.NotBefore/NotAfter, unchanged DateTimeKind.Local).
• CertificateRequest's nesting check, the Apple chain processor's verification
  time, and the OpenSSL CRL cache now compare in UTC throughout.
• IFindPal's FindByTimeValid/NotYetValid/Expired => DateTimeOffset, closing
  out the one remaining internal DateTime surface in the find path and deleting
  ManagedCertificateFinder's now-unnecessary Local/UTC normalization shim.

This is an internal-contract change only. X509Certificate2.NotBefore/NotAfter
and X509Certificate2Collection.Find's DateTime parameter are unchanged; no
public API changes.

[dotnet-policy-service]

Tagging subscribers to this area: @bartonjs, @vcsjones, @dotnet/area-system-security
See info in area-owners.md if you want to be subscribed.

[Copilot]

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 use DateTimeOffset for stable UTC comparisons.
• Updates platform backends (Windows/Apple/Android/OpenSSL) and key consumers (managed find, CRL cache, CertificateRequest nesting 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.

[koen-lee]

Right, this is a thing. I'll use an unambiguous long instead.

[bartonjs]

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.

[bartonjs]

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.

[koen-lee]

I agree the juggling is indeed not very elegant.
If a torn read is not an issue here at all, I'd like to revert the change to long and cache the DateTimeOffset.
Passing primitives around re-introduces a bug class the type system is designed to prevent, I think the extra register used is well spent.

[bartonjs]

Well, changing from DateTime (long) to DateTimeOffset makes torn reads possible on 64 bit as well as the 32 bit that they were already possible.

Keeping the values as long (or ulong) keeps torn reads as completely not an issue that needs thinking about, so it's preferred. Just make sure that they're all named with a Utc suffix. Since the Unspecified kind won't flow, it won't have ambiguous meaning.

[github-actions]

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "065f1f4f03c563e9dd0d89e13f4e7b2b696b511f",
  "last_dispatched_base_ref": "main",
  "last_dispatched_base_sha": "6066f76a086cc69d3d04587866f26c2b59122208",
  "last_reviewed_commit": "065f1f4f03c563e9dd0d89e13f4e7b2b696b511f",
  "last_reviewed_base_ref": "main",
  "last_reviewed_base_sha": "6066f76a086cc69d3d04587866f26c2b59122208",
  "last_recorded_worker_run_id": "29679150124",
  "review_attempt_commit": "",
  "review_attempt_base_ref": "",
  "review_attempt_count": 0,
  "max_review_attempts": 5,
  "review_history_format": "holistic-review-disclosure-v1",
  "review_history": [
    {
      "commit": "065f1f4f03c563e9dd0d89e13f4e7b2b696b511f",
      "review_id": 4730528485
    }
  ]
}

[github-actions]

Holistic Review

Motivation: The problem is real and well-scoped. X.509 NotBefore/NotAfter are absolute UTC instants (RFC 5280), but the internal PAL layer round-tripped them through local time (ToLocalTime()/FromFileTime()), and X509Certificate2 caches these values for a long-lived instance. A process time-zone change after caching could shift comparisons, which the two new Linux/RemoteExecutor tests demonstrate failing on main.

Approach: Retyping the internal contract (ICertificatePalCore.NotBefore/NotAfter, IFindPal.FindByTime*, IChainPal.BuildChain, OpenSslCrlCache, Apple chain verification) to carry the UTC instant as DateTimeOffset and projecting local time only at the one public boundary (X509Certificate2.NotBefore/NotAfter, still DateTimeKind.Local) is the right shape. The refactor is thorough and internally consistent: all four PAL backends and every interface consumer/dispatcher were updated, the now-redundant ManagedCertificateFinder.NormalizeDateTime shim and Apple/OpenSSL local-conversion steps were removed, and no public API changes. The FILETIME/CFDate interop helpers were correctly switched to the *Utc variants so the UTC instant is preserved rather than shifted.

Summary: ⚠️ Needs Human Review. I found no new correctness defects in the current base-to-head diff — the code compiles as retyped, the UTC semantics are preserved end-to-end, and the FindPal.cs/X509Chain.cs dispatch sites correctly rely on the DateTime→DateTimeOffset implicit conversion which keeps the prior "assume Local unless Utc" contract while fixing the Utc case. The reason for human review is an unresolved maintainer design discussion (see Detailed Findings), not a bug.

---

Detailed Findings

✅ UTC preservation across backends and interop

Each PAL now returns the decoded UTC instant directly (CertificateData.ManagedDecode drops .UtcDateTime, Windows uses ToDateTimeOffset(), OpenSSL asserts Kind==Utc before wrapping). The interop helpers were switched to DateTime.FromFileTimeUtc/ToFileTimeUtc and DateTimeOffset.UtcDateTime, so no local shift is reintroduced. The X509Certificate cache stores UTC ticks and projects LocalDateTime fresh at the public boundary, which is what makes the value stable across a TZ change.

✅ Thread-safety of the lazy cache

The cache fields were changed from DateTime to long UTC ticks with Volatile read/write, and 0 is used as the "not cached" sentinel (no real cert has a year-1 validity). This keeps the fields pointer-sized/atomic and avoids the torn-read hazard that a raw DateTimeOffset field would have introduced on both 32- and 64-bit. The get-methods return the freshly-read DateTimeOffset on the miss path rather than re-reading the field, avoiding a redundant reload.

✅ CertificateRequest nesting check

The nesting comparison now uses issuerCertificate.GetNotBeforeUtc()/GetNotAfterUtc() against the incoming DateTimeOffset, removing the LocalDateTime hop that was the source of TZ-dependence. The round-down-to-second logic operates on the offset's clock ticks; since the resulting instant is unchanged, the signed notAfter value is equivalent to before. Exception messages still surface notBefore.LocalDateTime/issuerCertificate.NotBefore for user-facing continuity.

✅ Test quality

The two new tests (FindByTimeValid_StableAcrossTimeZoneChange, CreateNesting_StableAcrossTimeZoneChange) exercise the managed find and CertificateRequest.Create nesting paths specifically, read the validity dates before the zone change to populate the cache, then flip to Pacific/Kiritimati (UTC+14) with a validity window narrower than the shift — so a regression would flip the result. Using RemoteExecutor to isolate the TZ change is the correct pattern, and the PR states both fail on main and pass with the fix. Reasonable coverage for the managed paths; the OpenSSL chain path is covered by the pre-existing tests in the same file.

⚠️ Open design discussion — PAL surface long vs DateTimeOffset

There is an unresolved maintainer discussion (bartonjs) proposing the PAL contract expose long NotAfterUtcTicks/NotBeforeUtcTicks directly rather than DateTimeOffset, to avoid the DateTimeOffset→long→DateTimeOffset juggling and keep the value register-sized, while the author prefers retaining the DateTimeOffset type for type-safety. This is a legitimate API-shape decision that affects every PAL backend touched here, and it is not yet settled. A human maintainer should land that direction before merge, since it would change several of the files in this PR. It is a design preference, not a correctness defect — the current code is correct as written.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 199 AIC · ⌖ 11.1 AIC · ⊞ 10K