Skip to content

Commit a93a6cc

Browse files
committed
fix: poll DNS for challenge TXT visibility before ACME validation
Replaces the hardcoded 5s challengePropagationDelayMs sleep (too short for _acme-challenge TXT records to reach Let's Encrypt's validators, causing DNS problem: NXDOMAIN failures) with active polling: query both a public resolver (1.1.1.1) and the zone's authoritative nameservers for the exact challenge value, proceeding once either is satisfied, with a bounded per-query timeout so a hung resolver can't hang the whole issuance flow. Configurable via ACME_PROPAGATION_DELAY_MS / ACME_PROPAGATION_TIMEOUT_MS.
1 parent 2d7c00c commit a93a6cc

7 files changed

Lines changed: 344 additions & 10 deletions

File tree

.env.example

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ ACME_EMAIL=ops@tinycloud.xyz
1414
# only once the service has been validated end-to-end:
1515
# https://acme-v02.api.letsencrypt.org/directory
1616
ACME_DIRECTORY=https://acme-staging-v02.api.letsencrypt.org/directory
17+
# How long to wait before the first DNS-01 propagation check, and how long to keep polling
18+
# before giving up. Defaults: 10000ms / 120000ms.
19+
ACME_PROPAGATION_DELAY_MS=
20+
ACME_PROPAGATION_TIMEOUT_MS=
1721

1822
# TEE attestation document (Phala dstack), optional outside of TEE deployments
1923
ATTESTATION_DOCUMENT=

src/acme.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ test("issues a certificate via DNS-01 using a fake acme-client and in-memory DNS
1414
email: "ops@tinycloud.xyz",
1515
dnsProvider,
1616
clientFactory: () => fakeClient,
17+
checkPropagation: async () => {},
1718
});
1819

1920
const domain = "mynode.local.tinycloud.link";
@@ -43,6 +44,7 @@ test("cleans up the DNS-01 TXT record even if the ACME order fails", async () =>
4344
email: "ops@tinycloud.xyz",
4445
dnsProvider,
4546
clientFactory: () => fakeClient,
47+
checkPropagation: async () => {},
4648
});
4749

4850
const domain = "failingnode.local.tinycloud.link";
@@ -51,3 +53,28 @@ test("cleans up the DNS-01 TXT record even if the ACME order fails", async () =>
5153
await assert.rejects(() => issuer.issueCertificate({ csrPem: csr, domain }));
5254
assert.equal(dnsProvider.txtRecords.size, 0);
5355
});
56+
57+
test("cleans up the DNS-01 TXT record and never completes the challenge if propagation never becomes visible", async () => {
58+
const dnsProvider = new InMemoryDnsProvider();
59+
const fakeClient = new FakeAcmeClient();
60+
const issuer = new DnsO1AcmeIssuer({
61+
directoryUrl: "https://fake-acme.test/directory",
62+
accountKeyPem: "fake-account-key",
63+
email: "ops@tinycloud.xyz",
64+
dnsProvider,
65+
clientFactory: () => fakeClient,
66+
checkPropagation: async () => {
67+
throw new Error("timed out after 120000ms waiting for TXT record to propagate");
68+
},
69+
});
70+
71+
const domain = "unpropagated.local.tinycloud.link";
72+
const csr = createTestCsr(domain);
73+
74+
await assert.rejects(
75+
() => issuer.issueCertificate({ csrPem: csr, domain }),
76+
/timed out.*propagate/
77+
);
78+
assert.equal(fakeClient.completedChallenges.length, 0);
79+
assert.equal(dnsProvider.txtRecords.size, 0);
80+
});

src/acme.ts

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { X509Certificate } from "node:crypto";
22
import * as acme from "acme-client";
33
import type { DnsProvider } from "./dns/provider.js";
4+
import { waitForTxtPropagation } from "./dns/propagation.js";
45

56
export interface AcmeIssueResult {
67
certChainPem: string;
@@ -63,8 +64,12 @@ export interface DnsO1AcmeIssuerOptions {
6364
email: string;
6465
dnsProvider: DnsProvider;
6566
clientFactory?: AcmeClientFactory;
66-
/** Delay after publishing the DNS-01 TXT record, before asking the CA to validate. */
67-
challengePropagationDelayMs?: number;
67+
/**
68+
* Confirms the _acme-challenge TXT record is actually visible in DNS before asking the CA to
69+
* validate it. Defaults to polling real DNS (see ./dns/propagation.ts); tests inject a fake to
70+
* avoid network calls.
71+
*/
72+
checkPropagation?: (recordName: string, expectedValue: string) => Promise<void>;
6873
}
6974

7075
/** Issues a certificate via ACME DNS-01, brokering the challenge through a DnsProvider. */
@@ -104,9 +109,7 @@ export class DnsO1AcmeIssuer implements AcmeIssuer {
104109
const record = await this.opts.dnsProvider.createTxtRecord(recordName, keyAuthorization);
105110
cleanups.push(() => this.opts.dnsProvider.deleteTxtRecord(recordName, record.id));
106111

107-
if (this.opts.challengePropagationDelayMs) {
108-
await sleep(this.opts.challengePropagationDelayMs);
109-
}
112+
await (this.opts.checkPropagation ?? waitForTxtPropagation)(recordName, keyAuthorization);
110113

111114
await client.completeChallenge(challenge);
112115
await client.waitForValidStatus(challenge);
@@ -125,10 +128,6 @@ export class DnsO1AcmeIssuer implements AcmeIssuer {
125128
}
126129
}
127130

128-
function sleep(ms: number): Promise<void> {
129-
return new Promise((resolve) => setTimeout(resolve, ms));
130-
}
131-
132131
function extractNotAfter(certChainPem: string): string {
133132
const leafEnd = certChainPem.indexOf("-----END CERTIFICATE-----");
134133
const leafPem =

src/dns-propagation.test.ts

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
import assert from "node:assert/strict";
2+
import test from "node:test";
3+
import { waitForTxtPropagation } from "./dns/propagation.js";
4+
5+
const RECORD_NAME = "_acme-challenge.example.local.tinycloud.link";
6+
const EXPECTED_VALUE = "expected-value";
7+
8+
/** Deterministic fake clock: sleep() advances the clock instead of waiting in real time. */
9+
function fakeClock() {
10+
let time = 0;
11+
return {
12+
now: () => time,
13+
sleep: async (ms: number) => {
14+
time += ms;
15+
},
16+
};
17+
}
18+
19+
test("resolves on the first poll when the public resolver already shows the value", async () => {
20+
const clock = fakeClock();
21+
let publicCalls = 0;
22+
let authoritativeFactoryCalls = 0;
23+
24+
await waitForTxtPropagation(RECORD_NAME, EXPECTED_VALUE, {
25+
publicResolver: {
26+
resolveTxt: async () => {
27+
publicCalls += 1;
28+
return [[EXPECTED_VALUE]];
29+
},
30+
},
31+
authoritativeResolverFactory: async () => {
32+
authoritativeFactoryCalls += 1;
33+
return { resolveTxt: async () => [] };
34+
},
35+
initialDelayMs: 10000,
36+
pollIntervalMs: 2000,
37+
timeoutMs: 120000,
38+
now: clock.now,
39+
sleep: clock.sleep,
40+
});
41+
42+
assert.equal(publicCalls, 1);
43+
assert.equal(
44+
authoritativeFactoryCalls,
45+
0,
46+
"authoritative resolver should not be needed once the public resolver already shows the value"
47+
);
48+
});
49+
50+
test("resolves once the authoritative servers have shown the value for the full grace period", async () => {
51+
const clock = fakeClock();
52+
let authoritativeCalls = 0;
53+
54+
await waitForTxtPropagation(RECORD_NAME, EXPECTED_VALUE, {
55+
publicResolver: { resolveTxt: async () => [] },
56+
authoritativeResolverFactory: async () => ({
57+
resolveTxt: async () => {
58+
authoritativeCalls += 1;
59+
return [[EXPECTED_VALUE]];
60+
},
61+
}),
62+
initialDelayMs: 10000,
63+
pollIntervalMs: 2000,
64+
authoritativeGraceMs: 6000,
65+
timeoutMs: 120000,
66+
now: clock.now,
67+
sleep: clock.sleep,
68+
});
69+
70+
// authoritativeGraceMs (6s) / pollIntervalMs (2s) means the value must be observed across
71+
// several successive polls (not just once) before we're willing to proceed on it alone.
72+
assert.equal(authoritativeCalls, 4);
73+
});
74+
75+
test("times out with a clear error when the value never becomes visible anywhere", async () => {
76+
const clock = fakeClock();
77+
78+
await assert.rejects(
79+
() =>
80+
waitForTxtPropagation(RECORD_NAME, EXPECTED_VALUE, {
81+
publicResolver: { resolveTxt: async () => [] },
82+
authoritativeResolverFactory: async () => ({ resolveTxt: async () => [] }),
83+
initialDelayMs: 1000,
84+
pollIntervalMs: 1000,
85+
timeoutMs: 5000,
86+
now: clock.now,
87+
sleep: clock.sleep,
88+
}),
89+
new RegExp(`timed out after 5000ms waiting for ${RECORD_NAME.replace(/\./g, "\\.")}`)
90+
);
91+
});
92+
93+
test("treats a resolver query that never returns as not-visible, instead of hanging the whole flow", async () => {
94+
const clock = fakeClock();
95+
const hungResolver = {
96+
// Never resolves; only the per-query timeout can move this along.
97+
resolveTxt: () => new Promise<string[][]>(() => {}),
98+
};
99+
100+
await assert.rejects(
101+
() =>
102+
waitForTxtPropagation(RECORD_NAME, EXPECTED_VALUE, {
103+
publicResolver: hungResolver,
104+
authoritativeResolverFactory: async () => ({ resolveTxt: async () => [] }),
105+
initialDelayMs: 0,
106+
pollIntervalMs: 10,
107+
timeoutMs: 50,
108+
queryTimeoutMs: 20,
109+
now: clock.now,
110+
sleep: clock.sleep,
111+
}),
112+
/timed out/
113+
);
114+
});

0 commit comments

Comments
 (0)