use OCSP files for stapling responses#3317
Conversation
mkf-sysangels
requested review from
TheophileDiot and
fl0ppy-d1sk
as code owners
|
Hello @TheophileDiot, I need a hint on why the job "ocsp-refresh" does not run. Currently I need to start the job manually to feed the stapling files in the destination directories. Thank you! |
TODO: Check SSL_USE_OCSP_STAPLING variable when variable loading is available in ssl_certificate phase
Do not fetch OCSP response when TTL of current is more than 3 days
- add customcert processing - use refresh TTL at 50% of lifetime
- job cleaned up existing directories - run as main()
|
Hello @TheophileDiot - found out the reason: the py must call main() to run correctly as a job. |
- migrate to native python cryptography module so we don't have openssl dependency - add cleanup function when SSL_USE_OCSP_STAPLING=no - add symlinks in case of multiple SAN entries - fix race condition during service reload: use database for certs extraction
after successful order of a LE cert or a custom cert upload trigger an OCSP stapling refresh so we can use OCSP stapling instantly
Addressing the high-severity security issue where OCSP response signatures were not being cryptographically verified.
- SSRF checks when verifying the OCSP Issuer - protections against Path Traversal via untrusted certificate Subject Alternative Names (SANs).
- Path traversal via tarball cert_name and DB restore cert - Redundant import - Debug log data leak, unbounded HTTP response reads
- Path Traversal Prevention - Temporary File Permission Hardening - Debug Logging Reduction
- Acquire a file-based lock for a specific certificate to prevent race conditions when multiple scheduler instances fetch OCSP responses concurrently. - HTTP error code — OCSP responder returned an error - End-of-job verification: ensure all OCSP files on disk match database checksums. Restores missing files from database to prevent OCSP stapling failures. This handles cases where OCSP cache directories are externally deleted or corrupted.
- before: each OCSP request blocked the database - after: use a single insert into the database
- add certificate checksum check - issue OCSP to new certs first, then changed certs, calculate TTL for the rest and run OCSP refresh if needed
There was a problem hiding this comment.
Pull request overview
This PR introduces OCSP stapling support to the SSL core by adding a scheduler job to fetch/refresh OCSP responses, caching them on disk/DB, and wiring NGINX’s ssl_certificate_by_lua phase to staple cached OCSP responses during TLS handshakes.
Changes:
- Add
SSL_USE_OCSP_STAPLINGmultisite setting and register a new hourlyocsp-refreshjob in the SSL plugin. - Add a new
ocsp-refresh.pyjob that fetches, verifies, caches, and restores OCSP responses (disk + database). - Trigger OCSP refresh after certificate issuance/renewal/custom-cert changes, and load/staple OCSP responses during TLS handshakes from cached
ocsp.der.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| src/common/core/ssl/plugin.json | Adds SSL_USE_OCSP_STAPLING setting and registers the new ocsp-refresh job. |
| src/common/core/ssl/jobs/ocsp-refresh.py | New OCSP refresh job: fetch + verify OCSP, persist to DB/disk, manage locks, restore/verify cache. |
| src/common/core/letsencrypt/jobs/certbot-renew.py | Adds certbot timeout handling and triggers OCSP refresh after successful renewals. |
| src/common/core/letsencrypt/jobs/certbot-new.py | Triggers OCSP refresh after successful certificate issuance. |
| src/common/core/customcert/jobs/custom-cert.py | Tracks changed domains and triggers OCSP refresh after custom cert updates. |
| src/common/confs/server-http/ssl-certificate-lua.conf | Loads OCSP response from cache/disk and staples it via ngx.ocsp during TLS handshake. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
Hello @TheophileDiot Thank you for the audit. I published the fixes so we can rerun the audit again. |
we need to verify that the OCSP file is for the current certificate in case the certificate changes but the old OCSP is still cached. use resty.openssl with fallback to openssl set debug level to messages so we don't fill up logs
… on OCSP errors - Serial number uppercase comparison - Both certificate and OCSP response serials are converted to uppercase before comparison to ensure consistent matching - Early certificate reading - Certificate is read immediately using ngx.ssl.cert_pem() at the start of set_ocsp_from_cache() - OCSP responder URL extraction - New get_ocsp_responder_url() function that: -- Tries native resty.openssl first (fast) -- Falls back to openssl x509 -ocsp_uri command -- Returns the OCSP responder URL if found -- Skip OCSP if no responder URL - If certificate doesn't have an OCSP responder URL, function returns early without loading OCSP files - Any error at any stage is logged but doesn't break the TLS handshake. The connection always continues with native TLS if OCSP fails for any reason. The certificate is already served before OCSP is even attempted.
on fast concurrent connections the filenames could create a race condition
- load modules with fail safe - check if certificate has OCSP Must-Staple extension - remove redis for a simple config
- add option to force a fresh fetch of the OCSP status files. - cleanup old files in case of a previous script run error
use cert fingerprint for storing and loading use native lua optimizations optimize TTL for OCSP responses
|
The OCSP-stapling PR is ready for code audit and public preview set this variable to enable the new OCSP module: |
|
running the ocsp-refresh.py alone:
|
- verify ocsp response file vailidity using ngx.ocsp.validate_ocsp_response() - optimize ocsp lookup logic (leaf cert first) - add more debug information - add test scenarios
- optimize orphan cleanup - add hash collision check (a collision "should" never happen but we need to catch that situation)
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 2 minutes and 12 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (8)
Comment |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
use OCSP files for stapling responses #1592
Lua needs an external OCSP status file to provide OCSP stapling without re-fetching the OCSP status from upstream PKI on every connect.