node-acme-client icon indicating copy to clipboard operation
node-acme-client copied to clipboard

Issue with DNS challenge in auto mode

Open ackava opened this issue 2 months ago • 2 comments

Since DNS challenge contains two tokens, and callback is called one after another, propagating challenges to NS server such as AWS Route 53 fails as two consecutive DNS updates fails some times as they are asynchronous in nature within AWS DNS Cluster (Sometimes only one challenge is successfully saved in DNS). I am using one library in C# for different project which gives both tokens together and I can update AWS DNS in a single update and there has never been any issue.

The issue also persists with remove challenge, one of the challenge remains in AWS DNS.

    cert = await client.auto({
           csr,
           email: maintainerEmail,
           termsOfServiceAgreed: true,
           skipChallengeVerification: false,
           challengePriority: wildcard ? ["dns-01"] :  ["http-01"],
           async challengeCreateFn(authz, challenge, keyAuthorization) {
                if (challenge.type !== "http-01") {
                     await dns.saveChallenge(commonName, keyAuthorization);
                     return;
                 }
                 httpServer.challenges.set(challenge.token, keyAuthorization);
             },
             challengeRemoveFn(authz, challenge, keyAuthorization) {
                 httpServer.challenges.delete(challenge.token);
                 return Promise.resolve();
             },
         });

Is it possible to change callback to receive all tokens in an array? Something like following?

There will be challangesCreateFn and challengesRemoveFn (Plural), and call methods accordingly?

    cert = await client.auto({
           csr,
           email: maintainerEmail,
           termsOfServiceAgreed: true,
           skipChallengeVerification: false,
           challengePriority: wildcard ? ["dns-01"] :  ["http-01"],
           async challengesCreateFn(challenges) {
           },
           async challengesRemoveFn(challenges) {
           },
       });

ackava avatar Apr 26 '24 15:04 ackava

I am having a similar issue (intermittently) using the following code. Both challenges seem to get added and deleted but sometimes I get Error processing ACME client: Error: Incorrect TXT record.

// index.js
import cron from "node-cron";
import { renewCert } from "./modules/renewCert.js";
import { uploadFileToS3 } from "./modules/uploadFileToS3.js";
import fs from "fs/promises";

async function checkFileReady(filePath) {
  try {
    await fs.access(filePath);
    return true; // File exists
  } catch {
    return false; // File does not exist
  }
}

async function attemptUpload(filePath, attempts = 0) {
  const fileReady = await checkFileReady(filePath);
  if (fileReady) {
    console.log("File is ready for upload.");
    await uploadFileToS3("thedomain-certs", filePath);
    console.log("File upload successful.");
  } else if (attempts < 5) {
    console.log(`File not ready for upload, retrying... (${attempts + 1}/5)`);
    setTimeout(() => attemptUpload(filePath, attempts + 1), 5000);
  } else {
    console.error("Failed to upload file after 5 attempts.");
  }
}

async function performTasks(forced = false) {
  console.log("Running scheduled certificate renewal and upload.");
  const certRenewed = await renewCert(forced);
  if (certRenewed) {
    const filePath = "thedomain.pfx";
    attemptUpload(filePath); // Call the function to handle the upload and retries
  }
}

// Execute immediately on start
performTasks(true);

// Schedule to execute every 7 days
cron.schedule("0 0 */7 * *", () => performTasks(), {
  scheduled: true,
  timezone: "America/New_York",
});

console.log("Scheduler activated. Task will run at 00:00 every 7 days.");

// /modules/renewCert.js
import acme from "acme-client";
import { addTxtRecord, deleteTxtRecord } from "./dns_management.js";
import createPfx from "./createPFX.js";
import { getFileAgeInDays } from "./getFileAgeInDays.js";

/**
 * Renews the certificate for thedomain.com domain.
 * 
 * @param {boolean} [forced=false] - Indicates whether the certificate renewal should be forced, regardless of the file age.
 * @returns {Promise<boolean>} - A promise that resolves to `true` if the certificate renewal is successful, or `false` otherwise.
 */
export const renewCert = async (forced = false) => {
  const age = await getFileAgeInDays("s3bucketname-certs", "thedomain.pfx");

  console.log(`File age: ${age} days.`);

  // When not forced and there is either an error because the file doesn't exist or the S3 is not accessible, or the file is not older than 30 days then return
  if (!forced && (age === -1 || age <= 30)) return;

  try {
    /* Init client */
    const client = new acme.Client({
      directoryUrl: process.env.NODE_ENV === "production" ? acme.directory.letsencrypt.production : acme.directory.letsencrypt.staging,
      accountKey: await acme.crypto.createPrivateKey(),
    });

    /* Create CSR */
    const [key, csr] = await acme.crypto.createCsr({
      commonName: "*.thedomain.com",
      altNames: ["*.thedomain.com", "thedomain.com"],
      organizationName: "thedomain, LLC",
      organizationalUnit: "thedomain",
    });

    /* Certificate */
    const cert = await client.auto({
      csr,
      email: "[email protected]",
      termsOfServiceAgreed: true,
      challengePriority: ["dns-01"],
      challengeCreateFn: async (authz, challenge, keyAuthorization) => {
        console.log(`Creating DNS record for ACME challenge: ${authz.identifier.value}`);
        await addTxtRecord({
          node: `_acme-challenge`,
          challenge: keyAuthorization,
        });
      },
      challengeRemoveFn: async (authz) => {
        console.log(`Removing DNS record for ACME challenge: ${authz.identifier.value}`);
        const deleteCount = await deleteTxtRecord({
          node: `_acme-challenge.${authz.identifier.value}`,
        });
        if (deleteCount > 0) {
          console.log(`${deleteCount} TXT record(s) successfully deleted.`);
        } else {
          console.log("No TXT records found for deletion.");
        }
      },
    });

    await createPfx(key, cert, process.env.CERT_PASSWORD, "thedomain - thedomain");
    console.log("PFX file saved successfully.");
    return true;
  } catch (error) {
    console.error("Error processing ACME client:", error);
    return false;
  }
};

Running scheduled certificate renewal and upload.
[cert-manager] [2024-05-10 14:26:42] Scheduler activated. Task will run at 00:00 every 7 days.
[cert-manager] [2024-05-10 14:26:42] File age: 1 days.
[cert-manager] [2024-05-10 14:26:44] Creating DNS record for ACME challenge: thedomain.com
[cert-manager] [2024-05-10 14:26:44] Creating DNS record for ACME challenge: thedomain.com
[cert-manager] [2024-05-10 14:26:44] TXT record successfully added.
[cert-manager] [2024-05-10 14:26:45] TXT record successfully added.
[cert-manager] [2024-05-10 14:26:45] Removing DNS record for ACME challenge: thedomain.com
[cert-manager] [2024-05-10 14:26:46] 2 TXT record(s) successfully deleted.
[cert-manager] [2024-05-10 14:26:50] Removing DNS record for ACME challenge: thedomain.com
[cert-manager] [2024-05-10 14:26:50] No TXT records found for deletion.
[cert-manager] [2024-05-10 14:26:50] Error processing ACME client: Error: Incorrect TXT record "2T2JvDms7I

bchr02 avatar May 10 '24 19:05 bchr02

I think I might have figured out the issue. When calling challengeRemoveFn I would delete all DNS TXT records so this would remove both TXT records. I revised the function that challengeRemoveFn calls so that it passes the keyAuthorization value and only deletes the TXT record with the matching record. This so far seems to work reliably.

We should likely upadate the https://github.com/publishlab/node-acme-client/blob/master/examples/auto.js so the following line indicates to pass the recordValue

like so

// await dnsProvider.removeRecord(dnsRecord, 'TXT', recordValue);

bchr02 avatar May 10 '24 20:05 bchr02