Repository navigation
fix: check external PKI as a set, fix WireGuard naming and rollback, drain on SIGTERM - #111
Merged
Merged
Conversation
docker-compose.yaml defaulted every CULVERT_* setting, and env beats the profile, so CULVERT_PROFILE=corporate ran with OIDC, the TCP listener and full tunnel off, and edge-fleet ran without routing control on 10.8.0.0. A copied .env.example also cleared the corporate group gate. Settings now pass through only when the operator sets them. The TLS paths onto the compose-mounted volume keep their defaults.
The allocation lock covered only allocations.json, so a reissue that read a peer before a concurrent revoke could write the revoked key back into wg0.conf and sync it to the running interface after the revoke reported success. generate-client, revoke-client and the server's own wg0.conf rebuild now hold a lifecycle lock from the first read of peers to the live sync.
Full-tunnel client configs and the server's full-tunnel push redirected IPv4 only, so a dual-stack client sent IPv6 straight out its own uplink. Both now add route-ipv6 2000::/3 and block-ipv6, so IPv6 enters the tunnel and is refused there. redirect-gateway ipv6 is not used: it installs no route without ifconfig-ipv6, and ifconfig-ipv6 makes OpenVPN exit on a client with IPv6 disabled. The e2e full-tunnel test now runs a dual-stack client and checks IPv6 routes into tun0.
The metrics collector parses tab-separated status-version 3, but no server template set it, so OpenVPN wrote version 1 and every listener reported zero connected clients. All three templates now set status-version 3, and the status file paths live in one map the collector and the template test share. The e2e session count reads status.log with the collector's own parser.
allocate_peer_ip and deallocate_peer_ip rewrote allocations.json in place, so a crash or full disk mid-write left a truncated file that every later issue, revoke and server start failed to parse. Both now write a temp sibling, fsync it, rename it over the old file and fsync the directory. write_file_atomic gained the two fsyncs, so the external PKI fetch gets the same guarantee.
Client alice's tls-crypt-v2 key was private/alice-tc.key, which is also the private key of a client named alice-tc. Issuing alice-tc overwrote alice's key, and revoking either deleted the other's. Per-client tls-crypt-v2 keys now live in tls-crypt-v2/<name>.key, a path no other client name can produce. generate-client also refuses to issue a name whose private key already exists without a certificate, since Easy-RSA in batch mode overwrites it.
generate-client discarded sync_running_interface's result, so after --rotate a refused wg syncconf left the old key live in the kernel while the files and the operator both believed it retired. A refused sync now raises, and generate-client restores every file the issue touched (peer keys, allocations.json, wg0.conf, the client's configs) and exits 1 saying the server still accepts the keys it had. Any other failure part way through restores the same files.
The external fetch wrote each file the moment it arrived, after a check that only looked for "-----BEGIN ", so a failure on the server key left a new CA beside the old certificate, and a key from another server or a CRL from another CA was accepted. All files are now fetched first, then checked together with openssl: the certificate verifies against the CA and is in date, the key matches it, the CRL is signed by the CA and the tls-crypt-v2 key is a server key. Nothing is written unless the whole set passes. The CRL refresh checks a re-fetched CRL against the CA on disk the same way.
With CULVERT_SECRETS_TC_KEY_PATH set and the provider unreachable at startup, init_pki fell back to the cached certificates and then minted a local tc.key, so the server came up healthy on a key no sibling shares and refused their clients. It now exits naming the setting unless a previously fetched tc.key is cached.
revoke-client returns once it has sent quit, before the stand-in's thread records it, so the assertion read ['kill alice'] about one run in six. The test now waits on the stand-in's handled event, as test_management already does.
…g new Every CRL refresh sent SIGHUP, which restarts every OpenVPN listener and drops every connected client, though OpenVPN re-reads the CRL on each handshake. The refresh loop now compares the revoked serials before and after, and reloads only when the new CRL revokes a certificate the old one did not, so that session ends before it renegotiates. A refresh that only moves nextUpdate leaves the server alone.
With CULVERT_PKI_MODE=external the CA key lives upstream, yet generate-client still ran Easy-RSA's sign-req and revoke-client its revoke, which fail with an Easy-RSA error at best. Both now refuse with the way through: issue at the CA, place the pair under issued/ and private/, and run generate-client --config-only, which now mints the client's tls-crypt-v2 key when it has none. Revocation is the CA's, picked up by the CRL refresh. The README's External PKI section says so.
The entrypoint wrote /etc/logrotate.d/vpn and nothing ever ran it, because PID 1 is Python and the image has no cron, so the OpenVPN, stunnel, wstunnel and oauth2 logs grew without bound. The server now runs logrotate hourly from a daemon thread, with copytruncate because every writer holds an O_APPEND descriptor it never reopens, daily or past 100M, 7 kept. The status files are left out: OpenVPN rewrites them in place.
wg-quick and the Android app name the interface after the config file, and Linux allows 15 characters, so <client>-wg-https-full.conf failed for every client name, the 15-character default vpn-example-com included. WireGuard configs now go in a directory named for the client, as wg-split.conf, wg-full.conf, wg-https-split.conf and wg-https-full.conf, with the slot number after "wg" from slot 2. generate-client refuses a slot count whose names would not fit (over HTTPS from slot 10). revoke-client, the zip bundle and the download server follow the directory, and the download server serves only a client name as a directory.
…TTPS A full tunnel puts the default route in wg-quick's own table, so the wstunnel connection to the server that carries wg0 was itself routed into wg0 and the tunnel never came up. The full-tunnel HTTPS config's wstunnel command now carries --socket-so-mark 51820, wg-quick's fwmark, which its policy rule sends out the normal route, and the config says what to do on platforms without SO_MARK. The e2e helper now runs the command the config documents, and a new e2e case brings up the full tunnel over HTTPS with the plain ports blocked.
SIGTERM dropped readiness and in the same instant took WireGuard down and terminated every OpenVPN listener, although the README and the Kubernetes guide promise a drain. Readiness now drops first and connected clients keep their tunnels for CULVERT_DRAIN_SECONDS (10 by default) while load balancers stop sending new ones, ending early once none is connected. A second signal skips the rest. The children then share one 15 s stop deadline instead of 10 s each, so the whole stop fits the 30 s grace period. docker-compose.yaml sets stop_grace_period to match.
ty reports the Iterator return annotation on a @contextmanager function as deprecated. The lock yields once and returns, which is what Generator[None] says.
…dings ty flagged the fetched secrets as bytes | None where pki_set_problems takes bytes, so the set passed on is now only what was fetched. The signal handler's frame parameter carries its vulture mark on its own line, and two Raises sections and one summary line follow the docstring rules.
The socket mark the previous commit added keeps wstunnel's packets out of wg0, but wg-quick restores its mark on UDP replies only, so on a host with strict rp_filter the server's TCP replies fail the reverse-path check and the tunnel still never comes up. The full-tunnel HTTPS config now tells the user to add a host route for the server through their current gateway, with the Linux, macOS and Windows commands, which is wstunnel's own documented fix and survives strict rp_filter. The e2e case now reaches the server through the default route, as a real client does, and passes only with that route in place.
catinspace-au
changed the title
OpenSSL before 3.2, the 3.0.13 in the image and on ubuntu-latest included, exits 0 from openssl crl -CAfile when the signature does not verify and reports the failure only on stderr. A CRL signed by a different key under the CA's name passed the check there. The check now also requires verify OK on stderr.
The schedule thread looped forever, so the unit test's 50 ms schedule kept running logrotate against a deleted tmp directory for the rest of the session and logged a warning on every pass. start_log_rotation now returns an event that stops it, and the test sets it.
ruff D413 wants a blank line after the last docstring section of start_log_rotation.
scalo's generator produced deploy/compose/culvert.yaml with `${CULVERT_VERSION:-latest}`. An unset version pulled whatever image latest referenced that day. The generator rewrites it to `${CULVERT_VERSION:?...}` and rejects fragments without that tag, so compose stops if the release is unset. A unit test checks the committed fragment requires the tag. hyperi-ci's compose-pins gate misses deploy/compose/culvert.yaml because it only scans files named docker-compose or compose.
The server can't know a client's gateway. The full-tunnel HTTPS config includes the host-route command. A unit test opens the client's zip and checks wg-https-full.conf contains the Linux host-route command. The README slot example names Zaphod to match its paragraph.
Contributor
|
Released in v2.1.30 -- https://github.com/hyperi-io/culvert/releases/tag/v2.1.30 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Regression tests cover each original failure.
P1s:
.env.exampleno longer override profile settings. Only values the operator sets pass through.route-ipv6 2000::/3andblock-ipv6, in the client configs and the server push.P2s:
Client tls-crypt-v2 keys move to
pki/tls-crypt-v2/<name>.key.private/alice-tc.keywas also clientalice-tc's private key.generate-clientrefuses a name that has a private key but no certificate.Existing clients get fresh tls-crypt-v2 keys on their next
--config-onlyrun. Configs already handed out keep working.WireGuard issuance exits 1 and restores every file it changed when the live interface rejects the new peers. A rejected
--rotateleft the old key live while the files said it was gone.allocations.jsonis written to a temp file, fsynced and renamed, then the directory is fsynced. This prevents crashes from leaving it truncated.External PKI is fetched in full before validation. No files are written until all checks pass.
The certificate must verify against the CA. The key must match the certificate. The CRL must be signed by the CA.
tc.keymust be a server key.CRL refresh checks a re-fetched CRL against the CA on disk.
With
CULVERT_SECRETS_TC_KEY_PATHset and nothing fetched or cached, the server exits instead of generating a key no sibling server shares.A CRL refresh reloads OpenVPN only when the new CRL revokes a serial the old one didn't. OpenVPN re-reads the CRL on every handshake. The daily SIGHUP dropped every client.
External PKI refuses OpenVPN client signing and revocation.
generate-clientandrevoke-clientprint the steps documented in the README's External PKI section.Issue at the CA. Install the certificate and key. Run
--config-onlyto generate the client's tls-crypt-v2 key.logrotate runs hourly from the server process, with copytruncate. Nothing ran it before because PID 1 is Python.
WireGuard configs move into a directory per client as
<client>/wg-split.conf,wg-https-full.confand so on. wg-quick and Android take the interface name from the file name, 15 characters at most.<client>-wg-https-fullexceeded that limit. Revocation, the zip and the download server use the new paths.A WireGuard full tunnel over HTTPS routed wstunnel's own connection into wg0 and never came up. The full-tunnel HTTPS config includes Linux, macOS and Windows commands to route the server through the current gateway, wstunnel's documented fix. The client's zip includes that config.
A socket mark alone fails because wg-quick restores its mark on UDP replies only. Strict rp_filter then drops the server's TCP replies.
An e2e case requires this route to pass with a full tunnel over HTTPS, plain ports blocked and the server reached through the default route.
All three server templates set
status-version 3, which the metrics parser needs. Every listener reported zero clients without it.SIGTERM drops readiness and keeps connected clients up for
CULVERT_DRAIN_SECONDS(10). The wait ends early once no client is connected. The children then share one 15 s stop deadline.A stop with clients connected takes up to 25 s. Compose sets
stop_grace_period: 30sto match.Also:
quitcaused failures about one run in six.deploy/compose/culvert.yamlrequiresCULVERT_VERSIONinstead of falling back tolatest.Generator. ty flags theIteratorform as deprecated.Validation:
hyperi-ci check(857 unit tests, at da70b7a).