Skip to content

[Issue #181] Compare certificate expiration timestamps in UTC - #269

Merged
jsf9k merged 6 commits into
cisagov:developfrom
arpitjain099:chore/fix-cert-expiration-consistency
Jun 24, 2026
Merged

[Issue #181] Compare certificate expiration timestamps in UTC#269
jsf9k merged 6 commits into
cisagov:developfrom
arpitjain099:chore/fix-cert-expiration-consistency

Conversation

@arpitjain099

Copy link
Copy Markdown
Contributor

Fixes #181

I tracked this down to how expiration is compared in https_check: we compare certificate timestamps against local naive time, while SSLyze data is UTC-based. That can drift by timezone and produce inconsistent expired/not-expired results.

This change adds a small helper (certificate_is_expired) and uses one UTC now value for both leaf and chain certificate checks. It prefers not_valid_after_utc when available and falls back to treating naive timestamps as UTC for older certificate objects.

I also added unit coverage for both paths in tests/test_pshtt.py.

What I ran locally:

  • python3 -m py_compile src/pshtt/pshtt.py tests/test_pshtt.py
  • focused runtime assertions for certificate_is_expired (UTC-aware and naive fallback cases)

I could not run pytest on this macOS environment because sslyze pulls nassl and fails to import with _SSLv2_method missing. So I validated the helper behavior directly and left the new unit tests for CI.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
@jsf9k jsf9k self-assigned this Jun 24, 2026
@jsf9k jsf9k added python Pull requests that update Python code test This issue or pull request adds or otherwise modifies test code bug This issue or pull request addresses broken functionality labels Jun 24, 2026
@github-project-automation github-project-automation Bot moved this to In progress in BOD 18-01 Jun 24, 2026
@jsf9k jsf9k moved this to In Progress in CyHy System Jun 24, 2026
@jsf9k jsf9k added the version bump This issue or pull request increments the version number label Jun 24, 2026
@jsf9k jsf9k moved this from In progress to Review in progress in BOD 18-01 Jun 24, 2026
@jsf9k
jsf9k requested a review from Copilot June 24, 2026 14:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses Issue #181 by making certificate-expiration evaluation consistent with SSLyze’s UTC-based timestamps, avoiding timezone-dependent drift when comparing certificate validity.

Changes:

  • Adds a certificate_is_expired() helper that compares certificate “not valid after” timestamps in UTC.
  • Updates https_check() to compute a single UTC now value and reuse it for leaf/chain expiry checks.
  • Adds unit tests covering UTC-aware and naive timestamp handling, and bumps the package version.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/pshtt/pshtt.py Introduces UTC-based expiry helper and uses a single now_utc for consistent comparisons in https_check().
tests/test_pshtt.py Adds unit tests for UTC-aware vs naive certificate timestamp handling.
src/pshtt/_version.py Bumps module version to 0.7.5.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/pshtt/pshtt.py Outdated
Comment thread src/pshtt/pshtt.py
Comment thread tests/test_pshtt.py Outdated
jsf9k and others added 3 commits June 24, 2026 10:28
not_valid_after is typically the last valid moment; once now == not_valid_after the certificate should be considered expired. Using a strict < comparison can produce off-by-one-second results vs the common not_after <= now semantics (and the query in Issue #181).

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
In the former, test the cases where not_valid_after_utc is TZ-aware
and naive.  In the latter, test the cases where not_valid_after is
TZ-aware and naive.
This makes the TZ code handle the case where not_valid_after_utc is
present but is a naive timestamp.  I'm not sure if that ever happens,
but now if it does we are covered.

@dav3r dav3r left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🚀

@github-project-automation github-project-automation Bot moved this from Review in progress to Reviewer approved in BOD 18-01 Jun 24, 2026
@jsf9k
jsf9k merged commit 8d6e947 into cisagov:develop Jun 24, 2026
37 checks passed
@github-project-automation github-project-automation Bot moved this from Reviewer approved to Done in BOD 18-01 Jun 24, 2026
@github-project-automation github-project-automation Bot moved this from In Progress to Done in CyHy System Jun 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug This issue or pull request addresses broken functionality python Pull requests that update Python code test This issue or pull request adds or otherwise modifies test code version bump This issue or pull request increments the version number

Projects

Status: Done
Status: Done

Development

Successfully merging this pull request may close these issues.

pshtt and sslyze appear to be inconsistent with respect to certificate expiration

4 participants