starred/nginx-proxy-manager
1
0
Fork 0
mirror of https://github.com/NginxProxyManager/nginx-proxy-manager.git synced 2026-09-24 19:10:26 +01:00
Code Issues Packages Projects Releases 10 Wiki Activity

Merge pull request #5814 from shawnhank/fix/dns-credentials-lifetime

Browse Source 
Remove DNS provider credentials from disk after certbot runs
This commit is contained in:
jc21
2026-09-23 14:08:29 +10:00
committed by GitHub
parent 61af8e23d8 210366cca0
commit da12ee5b0c
2 changed files with 74 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/internal/certificate.js
+63 -6
View File
@@ -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;
}
},
@@ -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",
@@ -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, () => {});
}
}
},
/**
backend/setup.js
+11 -21
View File
@@ -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";
@@ -98,7 +97,6 @@ const setupCertbotPlugins = async () => {
if (certificates?.length) {
const plugins = [];
const promises = [];
certificates.map((certificate) => {
if (certificate.meta && certificate.meta.dns_challenge === true) {
@@ -106,31 +104,23 @@ const setupCertbotPlugins = async () => {
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(", ")}`);
}
}