Skip to content

Remove DNS provider credentials from disk after certbot runs #5814

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
shawnhank wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Remove DNS provider credentials from disk after certbot runs #5814

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
918728f
Remove DNS provider credentials from disk after certbot runs
shawnhank Aug 28, 2026
210366c
Stop recreating DNS credentials files on every backend restart
shawnhank Aug 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 63 additions & 6 deletions backend/internal/certificate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -881,10 +881,20 @@ const internalCertificate = {
const result = await utils.execFile(certbotCommand, args, adds.opts);
logger.info(result);
return result;
} catch (err) {
// Don't fail if file does not exist, so no need for action in the callback
} finally {
// Remove the credentials file whether certbot succeeded or failed.
//
// This cleanup used to sit in a catch block, so it only ran when issuance FAILED.
// A certificate that issued successfully left its DNS provider API credentials in
// /etc/letsencrypt/credentials for the entire life of that certificate. Nothing
// reads the file between certbot runs, so there is no reason to keep it:
// renewLetsEncryptSslWithDnsChallenge() writes it again immediately before each
// renewal.
//
// unlink is fire-and-forget with an empty callback. If the file is already gone
// that is the end state we wanted anyway, and a missing file must never turn a
// successful issuance into a failure.
fs.unlink(credentialsLocation, () => {});
throw err;
}
},

Expand Down Expand Up @@ -981,6 +991,43 @@ const internalCertificate = {
`Renewing LetsEncrypt certificates via ${dnsPlugin.name} for Cert #${certificate.id}: ${certificate.domain_names.join(", ")}`,
);

// certbot reads the DNS credentials back from the path recorded in the renewal config
// it wrote at issuance time, for example:
//
// authenticator = dns-cloudflare
// dns_cloudflare_credentials = /etc/letsencrypt/credentials/credentials-27
//
// so the file has to be present for the duration of this run. Write it here and remove
// it again below rather than leaving it on disk between renewals.
//
// Leaving it is an avoidable exposure. Anything running as root - a compromised
// process, a script, malware - can read the token and use it to issue valid Let's
// Encrypt certificates for the domain. Those certificates are genuinely trusted, so
// traffic presented with them passes TLS inspection, IDS/IPS and DLP that would
// otherwise flag it, and an exfiltration path built on them looks like ordinary
// HTTPS. The exposure window should be one certbot run, not the life of the
// certificate.
//
// The value is not on the certificate object we were handed: renew() sources that from
// internalCertificate.get(), which pipes the row through utils.omitRow(omissions()) so
// meta.dns_provider_credentials can never travel out over the API. Read the row from
// the model directly to get at it.
const row = await certificateModel.query().where("id", certificate.id).first();
const credentials = row?.meta?.dns_provider_credentials;
const credentialsLocation = `/etc/letsencrypt/credentials/credentials-${certificate.id}`;

if (credentials) {
fs.mkdirSync("/etc/letsencrypt/credentials", { recursive: true });
fs.writeFileSync(credentialsLocation, credentials, { mode: 0o600 });
} else {
// Nothing stored to write. A certificate issued under the previous behaviour may
// still have its file on disk; leave it be and let certbot decide. Throwing here
// would break a renewal that would otherwise have succeeded.
logger.warn(
`No stored DNS credentials for Cert #${certificate.id}; relying on any existing ${credentialsLocation}`,
);
}

const args = [
"renew",
"--force-renewal",
Expand Down Expand Up @@ -1008,9 +1055,19 @@ const internalCertificate = {

logger.info(`Command: ${certbotCommand} ${args ? args.join(" ") : ""}`);

const result = await utils.execFile(certbotCommand, args, adds.opts);
logger.info(result);
return result;
try {
const result = await utils.execFile(certbotCommand, args, adds.opts);
logger.info(result);
return result;
} finally {
// Only clean up a file we put there ourselves. If `credentials` came back empty we
// wrote nothing, and an older file left on disk by the previous behaviour is the
// only thing keeping that certificate renewable - deleting it would break the next
// run for no gain.
if (credentials) {
fs.unlink(credentialsLocation, () => {});
}
}
},

/**
Expand Down
32 changes: 11 additions & 21 deletions backend/setup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import fs from "node:fs/promises";
import { installPlugins } from "./lib/certbot.js";
import utils from "./lib/utils.js";
import { setup as logger } from "./logger.js";
Expand Down Expand Up @@ -98,39 +97,30 @@ const setupCertbotPlugins = async () => {

if (certificates?.length) {
const plugins = [];
const promises = [];

certificates.map((certificate) => {
if (certificate.meta && certificate.meta.dns_challenge === true) {
if (plugins.indexOf(certificate.meta.dns_provider) === -1) {
plugins.push(certificate.meta.dns_provider);
}

// Make sure credentials file exists
const credentials_loc = `/etc/letsencrypt/credentials/credentials-${certificate.id}`;
if (typeof certificate.meta.dns_provider_credentials === "string") {
promises.push(
fs
.mkdir("/etc/letsencrypt/credentials", { recursive: true })
.then(() =>
fs.writeFile(credentials_loc, certificate.meta.dns_provider_credentials, {
mode: 0o600,
flag: "wx",
}),
)
.catch((err) => {
if (err.code !== "EEXIST") throw err;
}),
);
}
// Deliberately does NOT write the DNS credentials file here any more.
//
// It used to, so that a later `certbot renew` would find the path recorded in its
// renewal config. The effect was that every backend restart rewrote a plaintext
// DNS provider API token for every DNS-01 certificate, and left it there.
//
// internalCertificate now writes that file immediately before it runs certbot and
// removes it again afterwards, so there is exactly one writer and the credential
// is on disk only for the length of a certbot run. Recreating the files at boot
// would put every one of them straight back.
}
return true;
});

await installPlugins(plugins);

if (promises.length) {
await Promise.all(promises);
if (plugins.length) {
logger.info(`Added Certbot plugins ${plugins.join(", ")}`);
}
}
Expand Down