From c8b895633c9a66b300d00e115e6304b95261be3b Mon Sep 17 00:00:00 2001 From: AJ ONeal Date: Fri, 14 Jun 2019 03:21:03 -0600 Subject: [PATCH] Fix regression with set challenges being ignored Backwards compatibility madness. When you Greenlock.create({ }), challenges will be set by default if not available. This is good... ish. When you approveDomains() and set opts.challenges, however, those must be able to override the defaults. This was just recently broken and the fix seems to be to make the prior defaults inaccessible, otherwise it becomes really confusing as to why a set DNS challenge for local, wild, or private domains is not being preferred to the (failing) http. All this crap will be cleaned up in v3... --- lib/core.js | 37 ++++++++++++++++++++++++++----------- lib/utils.js | 6 ++++++ package-lock.json | 22 +++++++++++----------- package.json | 4 ++-- 4 files changed, 45 insertions(+), 24 deletions(-) diff --git a/lib/core.js b/lib/core.js index f1d4956..009278a 100644 --- a/lib/core.js +++ b/lib/core.js @@ -365,17 +365,18 @@ module.exports.create = function (gl) { // certReq.setChallenge = function (challenge, done) { log(args.debug, "setChallenge called for '" + challenge.altname + "'"); + // NOTE: First arg takes precedence var copy = utils.merge({ domains: [challenge.altname] }, args); copy = utils.merge(copy, gl); utils.tplCopy(copy); copy.challenge = challenge; - if (1 === gl.challenges[challenge.type].set.length) { - gl.challenges[challenge.type].set(copy).then(function (result) { + if (1 === copy.challenges[challenge.type].set.length) { + copy.challenges[challenge.type].set(copy).then(function (result) { done(null, result); }).catch(done); - } else if (2 === gl.challenges[challenge.type].set.length) { - gl.challenges[challenge.type].set(copy, done); + } else if (2 === copy.challenges[challenge.type].set.length) { + copy.challenges[challenge.type].set(copy, done); } else { Object.keys(challenge).forEach(function (key) { done[key] = challenge[key]; @@ -383,28 +384,42 @@ module.exports.create = function (gl) { // regression bugfix for le-challenge-cloudflare // (_acme-challege => _greenlock-dryrun-XXXX) copy.acmePrefix = (challenge.dnsHost||'').replace(/\.*/, '') || copy.acmePrefix; - gl.challenges[challenge.type].set(copy, challenge.altname, challenge.token, challenge.keyAuthorization, done); + copy.challenges[challenge.type].set(copy, challenge.altname, challenge.token, challenge.keyAuthorization, done); } }; certReq.removeChallenge = function (challenge, done) { log(args.debug, "removeChallenge called for '" + challenge.altname + "'"); - var copy = utils.merge({ domains: [challenge.altname] }, gl); + var copy = utils.merge({ domains: [challenge.altname] }, args); + copy = utils.merge(copy, gl); utils.tplCopy(copy); copy.challenge = challenge; - if (1 === gl.challenges[challenge.type].remove.length) { - gl.challenges[challenge.type].remove(copy).then(function (result) { + if (1 === copy.challenges[challenge.type].remove.length) { + copy.challenges[challenge.type].remove(copy).then(function (result) { done(null, result); }).catch(done); - } else if (2 === gl.challenges[challenge.type].remove.length) { - gl.challenges[challenge.type].remove(copy, done); + } else if (2 === copy.challenges[challenge.type].remove.length) { + copy.challenges[challenge.type].remove(copy, done); } else { Object.keys(challenge).forEach(function (key) { done[key] = challenge[key]; }); - gl.challenges[challenge.type].remove(copy, challenge.altname, challenge.token, done); + copy.challenges[challenge.type].remove(copy, challenge.altname, challenge.token, done); } }; + certReq.getZones = function (challenge) { + var copy = utils.merge({ dnsHosts: args.domains.map(function (x) { return 'xxxx.' + x; }) }, args); + copy = utils.merge(copy, gl); + utils.tplCopy(copy); + copy.challenge = challenge; + + if (!copy.challenges[challenge.type] || ('function' !== typeof copy.challenges[challenge.type].zones)) { + // may not be available, that's fine. + return Promise.resolve([]); + } + + return copy.challenges[challenge.type].zones(copy); + }; log(args.debug, 'calling greenlock.acme.getCertificateAsync', certReq.subject, certReq.domains); diff --git a/lib/utils.js b/lib/utils.js index caa3fb1..46324b5 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -54,6 +54,7 @@ module.exports.merge = function (/*defaults, args*/) { allDefaults.forEach(function (defaults) { Object.keys(defaults).forEach(function (key) { + /* if ('challenges' === key && copy[key] && defaults[key]) { Object.keys(defaults[key]).forEach(function (k) { copy[key][k] = defaults[key][k]; @@ -61,10 +62,13 @@ module.exports.merge = function (/*defaults, args*/) { } else { copy[key] = defaults[key]; } + */ + copy[key] = defaults[key]; }); }); Object.keys(args).forEach(function (key) { + /* if ('challenges' === key && copy[key] && args[key]) { Object.keys(args[key]).forEach(function (k) { copy[key][k] = args[key][k]; @@ -72,6 +76,8 @@ module.exports.merge = function (/*defaults, args*/) { } else { copy[key] = args[key]; } + */ + copy[key] = args[key]; }); return copy; diff --git a/package-lock.json b/package-lock.json index 33ffa7a..58beb36 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,19 +1,19 @@ { "name": "greenlock", - "version": "2.7.23", + "version": "2.8.0", "lockfileVersion": 1, "requires": true, "dependencies": { - "@coolaj86/urequest": { - "version": "1.3.7", - "resolved": "https://registry.npmjs.org/@coolaj86/urequest/-/urequest-1.3.7.tgz", - "integrity": "sha512-PPrVYra9aWvZjSCKl/x1pJ9ZpXda1652oJrPBYy5rQumJJMkmTBN3ux+sK2xAUwVvv2wnewDlaQaHLxLwSHnIA==" - }, "@root/mkdirp": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/@root/mkdirp/-/mkdirp-1.0.0.tgz", "integrity": "sha512-hxGAYUx5029VggfG+U9naAhQkoMSXtOeXtbql97m3Hi6/sQSRL/4khKZPyOF6w11glyCOU38WCNLu9nUcSjOfA==" }, + "@root/request": { + "version": "1.3.11", + "resolved": "https://registry.npmjs.org/@root/request/-/request-1.3.11.tgz", + "integrity": "sha512-3a4Eeghcjsfe6zh7EJ+ni1l8OK9Fz2wL1OjP4UCa0YdvtH39kdXB9RGWuzyNv7dZi0+Ffkc83KfH0WbPMiuJFw==" + }, "acme": { "version": "1.3.0", "resolved": "https://registry.npmjs.org/acme/-/acme-1.3.0.tgz", @@ -28,12 +28,12 @@ "integrity": "sha512-Aa4bUpq6ftX1VODiShOetOY5U0tsXY5EV7+fQwme3Q8Y9rjYBArBXHgFCAVKtK1AF+Ev8pIuF6Z42hzMFa73/w==" }, "acme-v2": { - "version": "1.7.7", - "resolved": "https://registry.npmjs.org/acme-v2/-/acme-v2-1.7.7.tgz", - "integrity": "sha512-Pg0EQ45h8N2e4K2goYedutCgWxAmtcruwDHr6hgPBgAWEORVb5SQEdXjtEhCrn+APtr7MyFPryyzXpYpDD5ecA==", + "version": "1.8.1", + "resolved": "https://registry.npmjs.org/acme-v2/-/acme-v2-1.8.1.tgz", + "integrity": "sha512-1nTyycCnsjTZogQ9JQ1nld9r7nY3G9WguKPXV+YdTEvEl0Q8kUcwkzkohsbRHpaUcSqU+OP7t7qLVSp2aWriPQ==", "requires": { - "@coolaj86/urequest": "^1.3.6", - "rsa-compat": "^2.0.6" + "@root/request": "^1.3.11", + "rsa-compat": "^2.0.8" } }, "cert-info": { diff --git a/package.json b/package.json index f0a8b20..3432abd 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "greenlock", - "version": "2.7.24", + "version": "2.8.0", "description": "Greenlock is Let's Encrypt (ACME) client for node.js", "homepage": "https://greenlock.domains/", "main": "index.js", @@ -37,7 +37,7 @@ "dependencies": { "acme": "^1.3.0", "acme-dns-01-cli": "^3.0.0", - "acme-v2": "^1.7.7", + "acme-v2": "^1.8.1", "cert-info": "^1.5.1", "greenlock-store-fs": "^3.0.2", "keypairs": "^1.2.14",