Skip to content

Commit 910f46e

Browse files
committed
server: retry transient certificate issuance failures for a day
ZeroSSL fails often and briefly (502s from its proxy, HTML instead of JSON, nonce requests without a nonce), and every issuance job gave up after three attempts within a minute, leaving 43 tunnels without a certificate. - Classify failures: transient ones are retried with exponential backoff and jitter (at most an hour apart, honoring Retry-After) for a day; permanent ones (rejectedIdentifier, badCSR, configuration) fail at once; anything else keeps three attempts. - Nonce requests that come back without a nonce report the HTTP status and are repeated (GET after HEAD); POST-as-GET reads are repeated on transient errors; garbage 2xx bodies are transient. - Attempts are idempotent: a challenge record shared by a hostname and its wildcard is created once, an identical existing Cloudflare record is adopted and deleted afterwards, pending authorizations are retried, and already valid authorizations or finalized orders are not redone. The issued certificate must certify the CSR's key. - The reconcile requeues (compare-and-set) first issuance that failed early on a transient error when the tunnel still waits for it, showing issuing again; a requeued job gets one new window.
1 parent c7cb7f8 commit 910f46e

9 files changed

Lines changed: 1508 additions & 62 deletions

File tree

‎crates/opentunnel-server/src/acme.rs‎

Lines changed: 203 additions & 48 deletions
Large diffs are not rendered by default.

‎crates/opentunnel-server/src/dns.rs‎

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,17 +39,31 @@ struct CloudflareResponse<A> {
3939

4040
#[derive(Deserialize)]
4141
struct CloudflareError {
42+
#[serde(default)]
43+
code: u64,
4244
#[serde(default)]
4345
message: String,
4446
}
4547

48+
#[derive(Deserialize)]
49+
struct Listed {
50+
id: String,
51+
#[serde(default)]
52+
content: String,
53+
}
54+
55+
/// Cloudflare's error for a record that exists with the same name and content.
56+
const CLOUDFLARE_IDENTICAL_RECORD: u64 = 81058;
57+
4658
#[derive(Deserialize)]
4759
struct Created {
4860
id: String,
4961
}
5062

5163
impl DnsProvider {
52-
/// Creates a TXT record `name` (a full name, `_acme-challenge.<host>`) with `value`, TTL 60.
64+
/// Creates a TXT record `name` (a full name, `_acme-challenge.<host>`) with `value`, TTL 60. A record that
65+
/// already exists with that value (left by an interrupted attempt) is used as is, and deleted afterwards
66+
/// like a created one.
5367
pub async fn create_txt(&self, name: &str, value: &str) -> Result<TxtRecord> {
5468
match self {
5569
Self::Cloudflare {
@@ -76,6 +90,21 @@ impl DnsProvider {
7690
value: value.into(),
7791
id: Some(created.id),
7892
}),
93+
_ if body.errors.iter().any(|error| {
94+
error.code == CLOUDFLARE_IDENTICAL_RECORD
95+
|| error.message.contains("identical record already exists")
96+
}) =>
97+
{
98+
let id = self.find_txt(name, value).await.unwrap_or_else(|error| {
99+
warn!(%error, name, "failed to look up an existing challenge record");
100+
None
101+
});
102+
Ok(TxtRecord {
103+
name: name.into(),
104+
value: value.into(),
105+
id,
106+
})
107+
}
79108
_ => {
80109
let messages: Vec<String> = body
81110
.errors
@@ -111,6 +140,39 @@ impl DnsProvider {
111140
}
112141
}
113142

143+
/// The ID of the Cloudflare TXT record `name` with `value`, if there is one.
144+
async fn find_txt(&self, name: &str, value: &str) -> Result<Option<String>> {
145+
let Self::Cloudflare {
146+
http,
147+
api,
148+
zone_id,
149+
token,
150+
} = self
151+
else {
152+
return Ok(None);
153+
};
154+
let mut url = url::Url::parse(&format!("{api}/zones/{zone_id}/dns_records"))?;
155+
url.query_pairs_mut()
156+
.append_pair("type", "TXT")
157+
.append_pair("name", name);
158+
let body: CloudflareResponse<Vec<Listed>> = http
159+
.get(url)
160+
.bearer_auth(token)
161+
.timeout(Duration::from_secs(30))
162+
.send()
163+
.await?
164+
.error_for_status()?
165+
.json()
166+
.await?;
167+
// Cloudflare may show TXT content quoted.
168+
Ok(body
169+
.result
170+
.unwrap_or_default()
171+
.into_iter()
172+
.find(|record| record.content.trim_matches('"') == value)
173+
.map(|record| record.id))
174+
}
175+
114176
/// Deletes a record, logging rather than failing: a leftover TXT record is harmless.
115177
pub async fn delete_txt(&self, record: &TxtRecord) {
116178
let result = match self {

‎crates/opentunnel-server/src/jobs.rs‎

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,13 @@ use crate::acme::Challenge;
1414
use crate::clock::{DAY_MS, HOUR_MS, MINUTE_MS};
1515
use crate::crypto::uuid;
1616
use crate::issuer::Issuer;
17+
use crate::retry;
1718
use crate::service::{RENEW_BEFORE_MS, Service};
1819
use crate::store::{Job, JobKind, LEASE_MS, ServerCertificate};
1920
use crate::tls;
2021

21-
/// Attempts per job, like the Workflow step's two retries.
22+
/// Attempts per job for failures that are not known to be transient, like the Workflow step's two retries.
23+
/// Transient ones are retried for `retry::RETRY_WINDOW_MS`.
2224
pub const ATTEMPTS: u32 = 3;
2325
const PERSIST_FAILED: &str = "Failed to persist certificate state: tunnel or certificate not found";
2426
/// Imported issuance gets this long to finish on the Worker before this server takes it over.
@@ -247,18 +249,19 @@ impl Jobs {
247249
.await
248250
.map(drop);
249251
}
250-
if attempts < ATTEMPTS {
251-
let delay = self.retry_delay_ms * (1 << (attempts - 1));
252+
let class = retry::classify(&error);
253+
if let Some(at) = retry::next_attempt(
254+
class,
255+
attempts,
256+
self.retry_delay_ms,
257+
job.created_at,
258+
now,
259+
retry::jitter(),
260+
) {
261+
info!(certificate = %job.certificate_id, attempt = attempts, ?class, in_ms = at - now, "retrying certificate issuance");
252262
return service
253263
.store
254-
.retry_job(
255-
&job.certificate_id,
256-
owner,
257-
attempts,
258-
now + delay,
259-
reason,
260-
now,
261-
)
264+
.retry_job(&job.certificate_id, owner, attempts, at, reason, now)
262265
.await
263266
.map(drop);
264267
}
@@ -334,6 +337,14 @@ pub async fn run_reconcile(service: Service) {
334337
Ok(resumed) => info!(resumed, "resumed certificate issuance without a job"),
335338
Err(error) => error!(%error, "resuming certificate issuance failed"),
336339
}
340+
match service.retry_failed_issuance().await {
341+
Ok(0) => {}
342+
Ok(requeued) => info!(
343+
requeued,
344+
"queued transiently failed certificate issuance again"
345+
),
346+
Err(error) => error!(%error, "queueing failed certificate issuance again failed"),
347+
}
337348
}
338349
}
339350

‎crates/opentunnel-server/src/lib.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ pub mod net;
2020
pub mod proxy_protocol;
2121
pub mod record;
2222
pub mod relay;
23+
pub mod retry;
2324
pub mod server;
2425
pub mod service;
2526
pub mod sni;

0 commit comments

Comments
 (0)