Skip to content

Fix certificate expiration during verification - #10510

Merged
auvipy merged 5 commits into
celery:mainfrom
Namraa310806:fix/certificate-expiration-verification
Aug 26, 2026
Merged

auvipy merged 5 commits into
celery:mainfrom
Namraa310806:fix/certificate-expiration-verification

Conversation

@Namraa310806

Copy link
Copy Markdown
Contributor

Summary

Fixes #10509

A certificate could be valid when loaded into FSCertStore, later expire while the worker remained running, and still be used to successfully verify signed messages.

This PR enforces certificate expiration at verification time.

Root Cause

FSCertStore already checks has_expired() when certificates are loaded. However, Certificate.verify() only performed cryptographic signature verification and did not re-check the certificate's validity.

This created a time-of-use gap:

  1. Certificate is valid when the worker starts.
  2. Certificate is loaded and stored.
  3. Certificate later expires.
  4. Certificate.verify() is called for a new message.
  5. Signature verification succeeds even though the certificate is expired.

Changes

  • Added an expiration check at the beginning of Certificate.verify().
  • Expired certificates now raise SecurityError before cryptographic signature verification.
  • Kept the existing FSCertStore load-time expiration check unchanged.
  • Added a regression test covering verification before and after certificate expiration.
  • Updated the existing serialization test setup because its historical test certificates are expired and those tests are intended to exercise serialization behavior rather than certificate validity.

Testing

  • All security tests pass: 37 passed, 1 skipped
  • Flake8 passes for the modified files.
  • The regression test verifies that:
    • a valid certificate can successfully verify a valid signature;
    • the same certificate is rejected once it is considered expired.

Scope

This change is intentionally limited to certificate expiration during verification. It does not change signature algorithms, certificate loading, certificate parsing, or unrelated exception handling.

@codecov

codecov Bot commented Aug 23, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.76%. Comparing base (7c846b6) to head (31b7716).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10510      +/-   ##
==========================================
+ Coverage   88.72%   88.76%   +0.04%     
==========================================
  Files         153      153              
  Lines       19938    19956      +18     
  Branches     2331     2334       +3     
==========================================
+ Hits        17689    17713      +24     
+ Misses       1951     1945       -6     
  Partials      298      298              
Flag Coverage Δ
unittests 88.73% <100.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@auvipy
auvipy requested review from auvipy and a lite review from Copilot August 23, 2026 12:39

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 pull request fixes a time-of-use gap in Celery’s security certificate handling by enforcing X.509 certificate expiration at verification time, not just when certificates are loaded into a store. This prevents long-running workers from continuing to accept signatures from certificates that have expired since startup.

Changes:

  • Added an explicit has_expired() check in Certificate.verify() that raises SecurityError for expired certs before signature verification.
  • Added a regression test that verifies successful validation pre-expiry and rejection post-expiry (via has_expired patching).
  • Updated serialization tests to avoid failing due to historically expired bundled test certificates (those tests target serialization behavior, not validity windows).

Reviewed changes

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

File Description
celery/security/certificate.py Enforces certificate expiry at verification time by raising SecurityError when expired.
t/unit/security/test_certificate.py Adds regression coverage ensuring Certificate.verify() rejects expired certificates.
t/unit/security/test_serialization.py Adjusts test setup so expired historical cert fixtures don’t break serialization-focused tests.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread t/unit/security/test_serialization.py Resolved
@auvipy auvipy added this to the 5.7.0 milestone Aug 23, 2026

@auvipy auvipy 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.

can you also check this PR #10511 and cross check if any thing can be incorporate here?

@Namraa310806

Copy link
Copy Markdown
Contributor Author

@auvipy I cross-checked PR #10511 against my PR #10510. I don't think anything from #10511 should be incorporated into #10510.

The certificate-expiration change in #10511 is the same core fix already implemented here: checking has_expired() at the Certificate.verify() boundary while retaining the existing FSCertStore load-time check.

The other changes in #10511, specifically the LimitedSet changes in celery/utils/collections.py and their tests, address a separate issue and are unrelated to this PR, so I intentionally kept them out to preserve the scope of #10510.

I also compared the certificate regression tests. My test exercises the actual signing and verification path using a real key/signature, while the alternative test in #10511 mocks the public-key verification for the valid case. I believe the current test is appropriate for this regression because it verifies the actual behavior being fixed.

So I recommend keeping #10510 focused on certificate expiration during verification, with no additional changes from #10511.

@Namraa310806

Copy link
Copy Markdown
Contributor Author

Missed one thing from #10511 — adding it now: test_serialization.pyuses an autouse fixture to patch has_expired instead of the manualper-instance override in _get_s(). Pushed that update.

@auvipy
auvipy merged commit 681ce00 into celery:main Aug 26, 2026
1 of 3 checks passed
@Namraa310806
Namraa310806 deleted the fix/certificate-expiration-verification branch October 13, 2026 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Certificate.verify() accepts signatures from certificates that expired after being loaded

3 participants