diff --git a/CHANGES b/CHANGES index 05fb77f2f..fa1fa0fc6 100644 --- a/CHANGES +++ b/CHANGES @@ -2057,4 +2057,5 @@ * route53: fix issue where verification failed if user had more than 100 zones * rework task workers to run them in a separate cgroup * backups: now much faster thanks to reworking of task worker +* When custom fallback cert is set, make sure it's used over LE certs diff --git a/src/cert/fallback.js b/src/cert/fallback.js deleted file mode 100644 index 01e82e395..000000000 --- a/src/cert/fallback.js +++ /dev/null @@ -1,22 +0,0 @@ -'use strict'; - -exports = module.exports = { - getCertificate: getCertificate, - - // testing - _name: 'fallback' -}; - -var assert = require('assert'), - debug = require('debug')('box:cert/fallback.js'); - -function getCertificate(hostname, domain, options, callback) { - assert.strictEqual(typeof hostname, 'string'); - assert.strictEqual(typeof domain, 'string'); - assert.strictEqual(typeof options, 'object'); - assert.strictEqual(typeof callback, 'function'); - - debug('getCertificate: using fallback certificate', hostname); - - return callback(null, '', ''); -} diff --git a/src/cert/interface.js b/src/cert/interface.js deleted file mode 100644 index 86d52411a..000000000 --- a/src/cert/interface.js +++ /dev/null @@ -1,24 +0,0 @@ -'use strict'; - -// ------------------------------------------- -// This file just describes the interface -// -// New backends can start from here -// ------------------------------------------- - -exports = module.exports = { - getCertificate: getCertificate -}; - -var assert = require('assert'), - BoxError = require('../boxerror.js'); - -function getCertificate(hostname, domain, options, callback) { - assert.strictEqual(typeof hostname, 'string'); - assert.strictEqual(typeof domain, 'string'); - assert.strictEqual(typeof options, 'object'); - assert.strictEqual(typeof callback, 'function'); - - return callback(new BoxError(BoxError.NOT_IMPLEMENTED, 'getCertificate is not implemented')); -} - diff --git a/src/reverseproxy.js b/src/reverseproxy.js index 10554b96e..a9f32eda7 100644 --- a/src/reverseproxy.js +++ b/src/reverseproxy.js @@ -27,7 +27,7 @@ exports = module.exports = { removeAppConfigs: removeAppConfigs, // exported for testing - _getCertApi: getCertApi + _getAcmeApi: getAcmeApi }; var acme2 = require('./cert/acme2.js'), @@ -41,7 +41,6 @@ var acme2 = require('./cert/acme2.js'), domains = require('./domains.js'), ejs = require('ejs'), eventlog = require('./eventlog.js'), - fallback = require('./cert/fallback.js'), fs = require('fs'), mail = require('./mail.js'), os = require('os'), @@ -58,15 +57,13 @@ var acme2 = require('./cert/acme2.js'), var NGINX_APPCONFIG_EJS = fs.readFileSync(__dirname + '/nginxconfig.ejs', { encoding: 'utf8' }), RELOAD_NGINX_CMD = path.join(__dirname, 'scripts/reloadnginx.sh'); -function getCertApi(domainObject, callback) { +function getAcmeApi(domainObject, callback) { assert.strictEqual(typeof domainObject, 'object'); assert.strictEqual(typeof callback, 'function'); - if (domainObject.tlsConfig.provider === 'fallback') return callback(null, fallback, { fallback: true }); + const api = acme2; - var api = acme2; - - var options = { prod: false, performHttpAuthorization: false, wildcard: false, email: '' }; + let options = { prod: false, performHttpAuthorization: false, wildcard: false, email: '' }; options.prod = domainObject.tlsConfig.provider.match(/.*-prod/) !== null; // matches 'le-prod' or 'letsencrypt-prod' options.performHttpAuthorization = domainObject.provider.match(/noop|manual|wildcard/) !== null; options.wildcard = !!domainObject.tlsConfig.wildcard; @@ -105,8 +102,6 @@ function providerMatchesSync(domainObject, certFilePath, apiOptions) { if (!fs.existsSync(certFilePath)) return false; // not found - if (apiOptions.fallback) return certFilePath.includes('.host.cert'); - const subjectAndIssuer = safe.child_process.execSync(`/usr/bin/openssl x509 -noout -subject -issuer -in "${certFilePath}"`, { encoding: 'utf8' }); if (!subjectAndIssuer) return false; // something bad happenned @@ -251,15 +246,12 @@ function setAppCertificateSync(location, domainObject, certificate) { return null; } -function getCertificateByHostname(hostname, domainObject, callback) { +function getAcmeCertificate(hostname, domainObject, callback) { assert.strictEqual(typeof hostname, 'string'); assert.strictEqual(typeof domainObject, 'object'); assert.strictEqual(typeof callback, 'function'); - let certFilePath = path.join(paths.APP_CERTS_DIR, `${hostname}.user.cert`); - let keyFilePath = path.join(paths.APP_CERTS_DIR, `${hostname}.user.key`); - - if (fs.existsSync(certFilePath) && fs.existsSync(keyFilePath)) return callback(null, { certFilePath, keyFilePath }); + let certFilePath, keyFilePath; if (hostname !== domainObject.domain && domainObject.tlsConfig.wildcard) { // bare domain is not part of wildcard SAN let certName = domains.makeWildcard(hostname).replace('*.', '_.'); @@ -282,10 +274,22 @@ function getCertificate(fqdn, domain, callback) { assert.strictEqual(typeof domain, 'string'); assert.strictEqual(typeof callback, 'function'); + // 1. user cert always wins + // 2. if using fallback provider, return that cert + // 3. look for LE certs + domains.get(domain, function (error, domainObject) { if (error) return callback(error); - getCertificateByHostname(fqdn, domainObject, function (error, result) { + // user cert always wins + let certFilePath = path.join(paths.APP_CERTS_DIR, `${fqdn}.user.cert`); + let keyFilePath = path.join(paths.APP_CERTS_DIR, `${fqdn}.user.key`); + + if (fs.existsSync(certFilePath) && fs.existsSync(keyFilePath)) return callback(null, { certFilePath, keyFilePath }); + + if (domainObject.tlsConfig.provider === 'fallback') return getFallbackCertificate(domain, callback); + + getAcmeCertificate(fqdn, domainObject, function (error, result) { if (error || result) return callback(error, result); return getFallbackCertificate(domain, callback); @@ -313,14 +317,27 @@ function ensureCertificate(vhost, domain, auditSource, callback) { domains.get(domain, function (error, domainObject) { if (error) return callback(error); - getCertApi(domainObject, function (error, api, apiOptions) { + // user cert always wins + let certFilePath = path.join(paths.APP_CERTS_DIR, `${vhost}.user.cert`); + let keyFilePath = path.join(paths.APP_CERTS_DIR, `${vhost}.user.key`); + + if (fs.existsSync(certFilePath) && fs.existsSync(keyFilePath)) return callback(null, { certFilePath, keyFilePath }, { renewed: false }); + + if (domainObject.tlsConfig.provider === 'fallback') { + return getFallbackCertificate(domain, function (error, bundle) { + if (error) return callback(error); + + callback(null, bundle, { renewed: false }); + }); + } + + getAcmeApi(domainObject, function (error, acmeApi, apiOptions) { if (error) return callback(error); - getCertificateByHostname(vhost, domainObject, function (_error, currentBundle) { + getAcmeCertificate(vhost, domainObject, function (_error, currentBundle) { if (currentBundle) { debug(`ensureCertificate: ${vhost} certificate already exists at ${currentBundle.keyFilePath}`); - if (currentBundle.certFilePath.endsWith('.user.cert')) return callback(null, currentBundle, { renewed: false }); // user certs cannot be renewed if (!isExpiringSync(currentBundle.certFilePath, 24 * 30) && providerMatchesSync(domainObject, currentBundle.certFilePath, apiOptions)) return callback(null, currentBundle, { renewed: false }); debug(`ensureCertificate: ${vhost} cert require renewal`); } else { @@ -329,7 +346,7 @@ function ensureCertificate(vhost, domain, auditSource, callback) { debug('ensureCertificate: getting certificate for %s with options %j', vhost, apiOptions); - api.getCertificate(vhost, domain, apiOptions, function (error, certFilePath, keyFilePath) { + acmeApi.getCertificate(vhost, domain, apiOptions, function (error, certFilePath, keyFilePath) { debug(`ensureCertificate: error: ${error ? error.message : 'null'} cert: ${certFilePath || 'null'}`); eventlog.add(currentBundle ? eventlog.ACTION_CERTIFICATE_RENEWAL : eventlog.ACTION_CERTIFICATE_NEW, auditSource, { domain: vhost, errorMessage: error ? error.message : '' }); @@ -346,7 +363,6 @@ function ensureCertificate(vhost, domain, auditSource, callback) { debug(`ensureCertificate: renewal of ${vhost} failed. using fallback certificates for ${domain}`); - // if no cert was returned use fallback. the fallback provider will not provide any for example getFallbackCertificate(domain, function (error, bundle) { if (error) return callback(error); diff --git a/src/test/reverseproxy-test.js b/src/test/reverseproxy-test.js index 344cf6e0b..34b63667d 100644 --- a/src/test/reverseproxy-test.js +++ b/src/test/reverseproxy-test.js @@ -193,7 +193,7 @@ describe('Certificates', function () { after(cleanup); it('returns prod acme in prod cloudron', function (done) { - reverseProxy._getCertApi(DOMAIN_0, function (error, api, options) { + reverseProxy._getAcmeApi(DOMAIN_0, function (error, api, options) { expect(error).to.be(null); expect(api._name).to.be('acme'); expect(options.prod).to.be(true); @@ -202,7 +202,7 @@ describe('Certificates', function () { }); it('returns prod acme in dev cloudron', function (done) { - reverseProxy._getCertApi(DOMAIN_0, function (error, api, options) { + reverseProxy._getAcmeApi(DOMAIN_0, function (error, api, options) { expect(error).to.be(null); expect(api._name).to.be('acme'); expect(options.prod).to.be(true); @@ -224,7 +224,7 @@ describe('Certificates', function () { after(cleanup); it('returns staging acme in prod cloudron', function (done) { - reverseProxy._getCertApi(DOMAIN_0, function (error, api, options) { + reverseProxy._getAcmeApi(DOMAIN_0, function (error, api, options) { expect(error).to.be(null); expect(api._name).to.be('acme'); expect(options.prod).to.be(false); @@ -233,7 +233,7 @@ describe('Certificates', function () { }); it('returns staging acme in dev cloudron', function (done) { - reverseProxy._getCertApi(DOMAIN_0, function (error, api, options) { + reverseProxy._getAcmeApi(DOMAIN_0, function (error, api, options) { expect(error).to.be(null); expect(api._name).to.be('acme'); expect(options.prod).to.be(false);