Auto-issue wildcard TLS for port shares (ACME phase 1) - #26
Auto-issue wildcard TLS for port shares (ACME phase 1)#26abhishek-anand wants to merge 25 commits into
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds in-process wildcard TLS termination via the ACME DNS-01 challenge, introducing database schemas for certificates, accounts, and jobs, as well as an ACME worker and TLS server configuration. The review feedback identifies a potential infinite failure loop during ACME order retries due to uncleared order URLs, unnecessary string allocations in the hot path of CertStore::select, and a failure to reset the job state to 'pending' when reclaiming ACME jobs in the database.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| async fn record_failure(&self, job: &AcmeJob, error: ManagerError) { | ||
| let mut updated = job.clone(); | ||
| updated.state = AcmeJobState::Failed; | ||
| updated.attempt = updated.attempt.saturating_add(1); | ||
| let attempt = u32::try_from(job.attempt).unwrap_or(u32::MAX); | ||
| let retry_seconds = i64::try_from(backoff_after(attempt).as_secs()).unwrap_or(3_600); | ||
| updated.next_attempt_at = Some(Utc::now() + chrono::Duration::seconds(retry_seconds)); | ||
| updated.last_error = Some(error.to_string()); | ||
| updated.updated_at = Utc::now(); | ||
|
|
||
| match self.fleet.save_acme_job(&updated, job.fence).await { |
There was a problem hiding this comment.
Infinite Failure Loop on Unresumable ACME Orders
When an ACME order fails (e.g., due to a DNS propagation timeout, or entering an Invalid/Processing/Valid state where the private key is lost), run_dns01_order will reject resuming it on subsequent attempts with an UnexpectedOrderState error.
Because record_failure clones the job and persists the same order_url back to the database, the state machine gets stuck in an infinite loop of immediate failures trying to resume the unresumable order.
Fix: Clear the order_url on failure so that the next retry starts a fresh ACME order.
| async fn record_failure(&self, job: &AcmeJob, error: ManagerError) { | |
| let mut updated = job.clone(); | |
| updated.state = AcmeJobState::Failed; | |
| updated.attempt = updated.attempt.saturating_add(1); | |
| let attempt = u32::try_from(job.attempt).unwrap_or(u32::MAX); | |
| let retry_seconds = i64::try_from(backoff_after(attempt).as_secs()).unwrap_or(3_600); | |
| updated.next_attempt_at = Some(Utc::now() + chrono::Duration::seconds(retry_seconds)); | |
| updated.last_error = Some(error.to_string()); | |
| updated.updated_at = Utc::now(); | |
| match self.fleet.save_acme_job(&updated, job.fence).await { | |
| async fn record_failure(&self, job: &AcmeJob, error: ManagerError) { | |
| let mut updated = job.clone(); | |
| updated.state = AcmeJobState::Failed; | |
| updated.attempt = updated.attempt.saturating_add(1); | |
| updated.order_url = None; | |
| let attempt = u32::try_from(job.attempt).unwrap_or(u32::MAX); | |
| let retry_seconds = i64::try_from(backoff_after(attempt).as_secs()).unwrap_or(3_600); | |
| updated.next_attempt_at = Some(Utc::now() + chrono::Duration::seconds(retry_seconds)); | |
| updated.last_error = Some(error.to_string()); | |
| updated.updated_at = Utc::now(); | |
| match self.fleet.save_acme_job(&updated, job.fence).await { |
| pub fn select(&self, sni: Option<&str>) -> Option<Arc<CertifiedKey>> { | ||
| let sni = normalize_sni(sni?)?; | ||
|
|
||
| if let Some(key) = self.exact.get(&sni) { | ||
| return Some(Arc::clone(key)); | ||
| } | ||
|
|
||
| let (base_domain, key) = self.wildcard.as_ref()?; | ||
| let label = sni.strip_suffix(&format!(".{base_domain}"))?; | ||
|
|
||
| (!label.is_empty() && !label.contains('.')).then(|| Arc::clone(key)) | ||
| } |
There was a problem hiding this comment.
Avoid String Allocation in Hot Path
CertStore::select is called on every incoming TLS connection during the ClientHello phase. Using format!(\".{base_domain}\") allocates a new String on every single handshake, which can severely degrade performance under high load.
Fix: Use string slicing and basic suffix checks to avoid any dynamic allocation.
| pub fn select(&self, sni: Option<&str>) -> Option<Arc<CertifiedKey>> { | |
| let sni = normalize_sni(sni?)?; | |
| if let Some(key) = self.exact.get(&sni) { | |
| return Some(Arc::clone(key)); | |
| } | |
| let (base_domain, key) = self.wildcard.as_ref()?; | |
| let label = sni.strip_suffix(&format!(".{base_domain}"))?; | |
| (!label.is_empty() && !label.contains('.')).then(|| Arc::clone(key)) | |
| } | |
| pub fn select(&self, sni: Option<&str>) -> Option<Arc<CertifiedKey>> { | |
| let sni = normalize_sni(sni?)?; | |
| if let Some(key) = self.exact.get(&sni) { | |
| return Some(Arc::clone(key)); | |
| } | |
| let (base_domain, key) = self.wildcard.as_ref()?; | |
| if sni.len() > base_domain.len() + 1 | |
| && sni.ends_with(base_domain) | |
| && sni.as_bytes()[sni.len() - base_domain.len() - 1] == b'.' | |
| { | |
| let label = &sni[..sni.len() - base_domain.len() - 1]; | |
| if !label.is_empty() && !label.contains('.') { | |
| return Some(Arc::clone(key)); | |
| } | |
| } | |
| None | |
| } |
| let row = client | ||
| .query_opt( | ||
| "INSERT INTO fleet_acme_jobs ( | ||
| id, identifier, state, fence, lease_holder, lease_expires_at, | ||
| attempt, next_attempt_at, updated_at | ||
| ) VALUES ($1,$2,'pending',1,$3,now() + $4::text::interval,0,NULL,now()) | ||
| ON CONFLICT (identifier) DO UPDATE SET | ||
| fence = fleet_acme_jobs.fence + 1, | ||
| lease_holder = EXCLUDED.lease_holder, | ||
| lease_expires_at = now() + $4::text::interval, | ||
| updated_at = now() |
There was a problem hiding this comment.
Reset Job State to Pending on Reclaim
When a previously failed or active ACME job is reclaimed by a node, the DO UPDATE SET clause does not update the state column. As a result, the job will continue to show as Failed or Active in the database while the node is actively working on it.
Fix: Set state = EXCLUDED.state (which is 'pending') in the DO UPDATE clause so that the database accurately reflects the job's active status.
let row = client
.query_opt(
"INSERT INTO fleet_acme_jobs (
id, identifier, state, fence, lease_holder, lease_expires_at,
attempt, next_attempt_at, updated_at
) VALUES ($1,$2,'pending',1,$3,now() + $4::text::interval,0,NULL,now())
ON CONFLICT (identifier) DO UPDATE SET
fence = fleet_acme_jobs.fence + 1,
state = EXCLUDED.state,
lease_holder = EXCLUDED.lease_holder,
lease_expires_at = now() + $4::text::interval,
updated_at = now()"There was a problem hiding this comment.
Pull request overview
This PR introduces phase-1 support for in-process wildcard TLS for port shares by adding an ACME DNS-01 issuance/renewal worker, storing encrypted key material in the fleet database, and serving HTTPS shares via a dynamic SNI cert resolver.
Changes:
- Add ACME DNS-01 issuance + renewal worker with Cloudflare/Route53 DNS providers and envelope-encrypted key storage.
- Add an in-process TLS listener and share-gateway logic to enforce TLS routing expectations.
- Add fleet schema + APIs for certificates/ACME accounts/jobs, plus a new hermetic e2e gate.
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| orch/tests/e2e_acme.sh | New hermetic Pebble DNS-01 + wildcard TLS end-to-end gate. |
| orch/docs/CONFIGURATION.md | Documents new ACME + TLS configuration and operational requirements. |
| orch/docs/API.md | Notes in-process HTTPS share support and links to configuration. |
| orch/crates/taritd/src/tls.rs | New TLS server accept loop and per-connection TLS metadata injection. |
| orch/crates/taritd/src/supervisor.rs | Updates test config scaffolding for new ACME/TLS config fields. |
| orch/crates/taritd/src/share_gateway.rs | Enforces TLS scheme/SNI behavior and adds MISDIRECTED response. |
| orch/crates/taritd/src/ops.rs | Updates test config scaffolding for new ACME/TLS config fields. |
| orch/crates/taritd/src/metrics.rs | Updates test config scaffolding for new ACME/TLS config fields. |
| orch/crates/taritd/src/main.rs | Wires ACME worker + TLS listener into taritd startup. |
| orch/crates/taritd/src/gateway.rs | Updates test config scaffolding for new ACME/TLS config fields. |
| orch/crates/taritd/src/config.rs | Adds ACME/TLS env parsing + validation and share-token-key requirement. |
| orch/crates/taritd/src/api.rs | Updates test config scaffolding for new ACME/TLS config fields. |
| orch/crates/taritd/src/acme/mod.rs | New ACME module entry points. |
| orch/crates/taritd/src/acme/resolver.rs | Dynamic cert store + rustls SNI resolver. |
| orch/crates/taritd/src/acme/order.rs | DNS-01 order driver and cert parsing utilities. |
| orch/crates/taritd/src/acme/manager.rs | Fleet-backed reconcile/renew loop with fenced lease coordination. |
| orch/crates/taritd/src/acme/dns.rs | Cloudflare/Route53 TXT upsert/delete + propagation checks. |
| orch/crates/taritd/src/acme/crypto.rs | Envelope encryption helper for sealing key material. |
| orch/crates/taritd/Cargo.toml | Adds ACME/TLS/DNS/encryption dependencies. |
| orch/crates/tarit-fleet/src/lib.rs | Adds schema + APIs for certificates, ACME accounts, and job leasing. |
| orch/crates/tarit-fleet/Cargo.toml | Adds tokio dependency for new listener wiring. |
| orch/Cargo.toml | Adds workspace dependencies for ACME/TLS/DNS/encryption stack. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let tls = request.extensions().get::<crate::tls::TlsInfo>(); | ||
| let host = format!("{slug}.{domain}"); | ||
| if !host_matches_sni(&host, tls.and_then(|info| info.sni.as_deref())) { | ||
| return Err(GatewayError::Misdirected); | ||
| } |
| pub async fn publish_certificate( | ||
| &self, | ||
| certificate: &CertRecord, | ||
| identifier: &str, | ||
| fence: i64, | ||
| ) -> Result<bool, FleetError> { | ||
| let sans = serialize_json(&certificate.sans, "sans")?; |
| if (plaintext_listen.is_some() || tls_listen.is_some()) && token_key.is_none() { | ||
| bail!( | ||
| "TARIT_SHARE_TOKEN_KEY must decode to exactly 32 bytes when a share listener is enabled" | ||
| ); | ||
| } |
| the fleet and refresh when notified. Renewal is automatic, ARI-aware when the CA | ||
| advertises it, and otherwise begins at one third of the certificate lifetime. |
Scope the Route53 client to the modern rustls client and drop the unused aws-smithy test deps, replace unmaintained rustls-pemfile with rustls-pki-types, and bump hickory-resolver to the patched 0.26 release.
Auto-issue and renew a wildcard TLS certificate (
*.<share-domain>) so portshares serve over full HTTPS at
https://<slug>.<domain>.What this adds
providers, additive RRset writes, injectable API base for tests).
reconcile and renewal worker using a fenced lease for single-writer safety.
over HTTP/1.1 and enforce
Host == SNI.TARIT_ACME_*and share TLS settings.Design notes
http/1.1ALPN only. The share pipeline is HTTP/1.1 (Hostrouting, origin-form URIs, WebSocket upgrades), so h2 would break it.
TARIT_SHARE_TOKEN_KEYwhenever any share listener (plaintext or TLS)is set, so HTTPS-only deployments cannot start with unusable private shares.
Validation
orch/tests/e2e_acme.sh: hermetic Pebble DNS-01 gate. Layer A covers issuance,fleet distribution, unknown-SNI rejection, and fenced-lease failover. Layer B
optionally provisions a real KVM share. Verified
ACME_PASSon a c8i host.cargo fmt --check,cargo clippy -D warnings, and the taritd/fleet testsuites pass.
Real-VM two-node validation on bare metal remains a separate migration gate.