Skip to content

fix(ssl): serialize ssl operations with a process-wide file lock - #498

Open
mrrobot47 wants to merge 4 commits into
EasyEngine:developfrom
mrrobot47:fix/ssl-concurrency-lock
Open

mrrobot47 wants to merge 4 commits into
EasyEngine:developfrom
mrrobot47:fix/ssl-concurrency-lock

Conversation

@mrrobot47

@mrrobot47 mrrobot47 commented Jun 30, 2026 •

Copy link
Copy Markdown
Member

Problem

Nothing serialized SSL operations. A cron ssl-renew --all running concurrently with a manual ee site ssl-verify/ssl-renew/--ssl=le create or update (or two crons) races on the shared HTTP-01 state: the challenge files under nginx-proxy/html/.well-known/acme-challenge and vhost.d/default, which ssl_verify() wipes wholesale via cleanup(), plus overlapping nginx-proxy reloads. Operations on the same site also race on its acme-conf/var/<domain> state and certificate_order.json. No flock existed anywhere on the SSL path.

Fix

Acquire a single global, exclusive flock (EE_ROOT_DIR/ssl-global.lock) at the three ACME entry points — init_le() (before register()/authorize() write account/order state), ssl_verify(), and ssl_renew(). If another process holds it, the command prints Waiting up to 120s for another SSL operation to finish... and polls the non-blocking lock once a second for up to 120 seconds (600 seconds for ssl-renew, which cron runs), then fails with "Another SSL operation is already in progress on this server. Wait for it to finish and retry." Pending SIGINT/SIGTERM signals are dispatched after each poll (pcntl_signal_dispatch()), so Ctrl-C or a SIGTERM ends the wait: the site type's rollback handler runs and the command exits (during create that removes the partial site, as Ctrl-C does elsewhere in create). The lock is held for the whole operation and released automatically on process exit (advisory flock — crash-safe).

Where the lock still can't be obtained mid-operation once the wait runs out, state is rolled back rather than left half-done: in init_le() the busy lock throws, so ee site create --ssl=le goes through the normal clean-up and ee site update --ssl=le doesn't save the SSL flag; for an alias change on an LE site the lock is taken before the compose file is rewritten, so the site keeps HTTPS and the DB is unchanged. If the lock file can't be opened, the error says so instead of claiming another operation is running.

The handle is a process-level static with a reentrancy short-circuit, and this is load-bearing: ssl-renew --all dispatches each site via EE::run_command in one process (a fresh command instance per site), and flock denies a second lock on the same file via a different fd even within the same process — so an instance-level guard would make site #2 of --all wrongly error. The static handle means the first acquire locks and every later/nested acquire (init_le → ssl_verify; each --all site) returns reentrantly. (It differs from the backup lock's instance handle and explicit release: SSL's --all-in-one-process pattern needs a process-wide handle and no per-site release.)

Testing

Manual: hold the lock from another shell (flock /opt/easyengine/ssl-global.lock sleep 150), then ee site ssl-verify <site> prints the waiting message and exits after 120 seconds with "Another SSL operation is already in progress…"; ee site ssl-renew <site> waits and completes once the lock is released; ee site create <x> --ssl=le that can't get the lock within 120 seconds cleans up completely; Ctrl-C or SIGTERM during the wait rolls back and exits immediately; ee site update <le-site> --add-alias-domains=… that can't get the lock changes nothing. Without contention, an LE create (nested init_le → ssl_verify) and ssl-renew --all across several LE sites work normally.

Tested on Ubuntu 26.04 with EasyEngine 4.12.0 with a real Let's Encrypt certificate (uncontended create, and ssl-renew --all across two sites flagged as Let's Encrypt) and lock contention on renew, verify, alias change, HTML and WordPress create --ssl=le, and update --ssl=le. The bounded wait was tested with the lock held from another shell: ssl-verify failed at 120 seconds, ssl-renew finished as soon as the lock was released (after about 30 seconds), an HTML create --ssl=le rolled back cleanly at the 120-second limit, and Ctrl-C or SIGTERM during the wait (including during create) rolled back and exited right away, leaving nothing behind.

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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

…operation

init_le() now throws when the lock is held, so site create (wp/php/html) and `ee site update --ssl=le` reach their existing catch/rollback paths instead of exiting after the site root, containers and WordPress are already set up but before the site DB entry exists.

update_alias_domains() now takes the lock before it dumps the compose file with HTTPS disabled for the HTTP-01 challenge, so a busy lock can no longer leave an LE site serving without HTTPS and with the new alias only in the compose file.

A failed fopen() of the lock file now reports that instead of claiming another SSL operation is running.
Failing fast made a `--ssl=le` create that overlapped the nightly renewal install the whole site and then roll it back, and made the nightly `ssl-renew --all` skip every site when a manual SSL command was running. Poll the non-blocking flock for up to 120 seconds (600 for `ssl-renew`, which cron runs) with a waiting message, then fail with the same error or roll back as before.
EE_Site_Command registers SIGINT/SIGTERM handlers, but class-ee-site.php has no declare(ticks), so during the wait loop a Ctrl-C stayed pending and sleep() just resumed: the command kept waiting the full 120 s (600 s for ssl-renew). Dispatch pending signals after each poll so the site type's rollback handler runs and the command exits.
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.

2 participants