diff --git a/CHANGES b/CHANGES index eb2797590..23cd7c32f 100644 --- a/CHANGES +++ b/CHANGES @@ -1621,4 +1621,5 @@ * Make all cloudron env vars have CLOUDRON_ prefix * Update manifest version to 2 * Fix issue where DKIM keys were inaccessible +* Fix DKIM selector conflict when adding same domain across multiple cloudrons diff --git a/migrations/20190610195323-mail-add-dkimSelector.js b/migrations/20190610195323-mail-add-dkimSelector.js new file mode 100644 index 000000000..75e2ae411 --- /dev/null +++ b/migrations/20190610195323-mail-add-dkimSelector.js @@ -0,0 +1,15 @@ +'use strict'; + +exports.up = function(db, callback) { + db.runSql('ALTER TABLE mail ADD COLUMN dkimSelector VARCHAR(128) NOT NULL DEFAULT "cloudron"', function (error) { + if (error) console.error(error); + callback(error); + }); +}; + +exports.down = function(db, callback) { + db.runSql('ALTER TABLE mail DROP COLUMN dkimSelector', function (error) { + if (error) console.error(error); + callback(error); + }); +}; diff --git a/migrations/schema.sql b/migrations/schema.sql index cc3351eb8..f42931593 100644 --- a/migrations/schema.sql +++ b/migrations/schema.sql @@ -174,6 +174,8 @@ CREATE TABLE IF NOT EXISTS mail( catchAllJson TEXT, relayJson TEXT, + dkimSelector VARCHAR(128) NOT NULL DEFAULT "cloudron", + FOREIGN KEY(domain) REFERENCES domains(domain), PRIMARY KEY(domain)) diff --git a/src/constants.js b/src/constants.js index ddc13e069..7c39a44e2 100644 --- a/src/constants.js +++ b/src/constants.js @@ -17,7 +17,6 @@ exports = module.exports = { ], ADMIN_LOCATION: 'my', - DKIM_SELECTOR: 'cloudron', NGINX_DEFAULT_CONFIG_FILE_NAME: 'default.conf', diff --git a/src/mail.js b/src/mail.js index 8396f4193..2f664420c 100644 --- a/src/mail.js +++ b/src/mail.js @@ -210,10 +210,14 @@ function verifyRelay(relay, callback) { }); } -function checkDkim(domain, callback) { - var dkim = { - domain: `${constants.DKIM_SELECTOR}._domainkey.${domain}`, - name: `${constants.DKIM_SELECTOR}._domainkey`, +function checkDkim(mailDomain, callback) { + assert.strictEqual(typeof mailDomain, 'object'); + assert.strictEqual(typeof callback, 'function'); + + const domain = mailDomain.domain; + let dkim = { + domain: `${mailDomain.dkimSelector}._domainkey.${domain}`, + name: `${mailDomain.dkimSelector}._domainkey`, type: 'TXT', expected: null, value: null, @@ -495,28 +499,28 @@ function getStatus(domain, callback) { const mailFqdn = config.mailFqdn(); - getDomain(domain, function (error, result) { + getDomain(domain, function (error, mailDomain) { if (error) return callback(error); let checks = []; - if (result.enabled) { + if (mailDomain.enabled) { checks.push( recordResult('dns.mx', checkMx.bind(null, domain, mailFqdn)), recordResult('dns.dmarc', checkDmarc.bind(null, domain)) ); } - if (result.relay.provider === 'cloudron-smtp') { + if (mailDomain.relay.provider === 'cloudron-smtp') { // these tests currently only make sense when using Cloudron's SMTP server at this point checks.push( recordResult('dns.spf', checkSpf.bind(null, domain, mailFqdn)), - recordResult('dns.dkim', checkDkim.bind(null, domain)), + recordResult('dns.dkim', checkDkim.bind(null, mailDomain)), recordResult('dns.ptr', checkPtr.bind(null, mailFqdn)), recordResult('relay', checkOutboundPort25), recordResult('rbl', checkRblStatus.bind(null, domain)) ); - } else if (result.relay.provider !== 'noop') { - checks.push(recordResult('relay', checkSmtpRelay.bind(null, result.relay))); + } else if (mailDomain.relay.provider !== 'noop') { + checks.push(recordResult('relay', checkSmtpRelay.bind(null, mailDomain.relay))); } async.parallel(checks, function () { @@ -770,9 +774,10 @@ function txtRecordsWithSpf(domain, mailFqdn, callback) { }); } -function ensureDkimKeySync(domain) { - assert.strictEqual(typeof domain, 'string'); +function ensureDkimKeySync(mailDomain) { + assert.strictEqual(typeof mailDomain, 'object'); + const domain = mailDomain.domain; const dkimPath = path.join(paths.MAIL_DATA_DIR, `dkim/${domain}`); const dkimPrivateKeyFile = path.join(dkimPath, 'private'); const dkimPublicKeyFile = path.join(dkimPath, 'public'); @@ -795,7 +800,7 @@ function ensureDkimKeySync(domain) { if (!safe.child_process.execSync('openssl genrsa -out ' + dkimPrivateKeyFile + ' 1024')) return new MailError(MailError.INTERNAL_ERROR, safe.error); if (!safe.child_process.execSync('openssl rsa -in ' + dkimPrivateKeyFile + ' -out ' + dkimPublicKeyFile + ' -pubout -outform PEM')) return new MailError(MailError.INTERNAL_ERROR, safe.error); - if (!safe.fs.writeFileSync(dkimSelectorFile, constants.DKIM_SELECTOR, 'utf8')) return new MailError(MailError.INTERNAL_ERROR, safe.error); + if (!safe.fs.writeFileSync(dkimSelectorFile, mailDomain.dkimSelector, 'utf8')) return new MailError(MailError.INTERNAL_ERROR, safe.error); // if the 'yellowtent' user of OS and the 'cloudron' user of mail container don't match, the keys become inaccessible by mail code if (!safe.fs.chmodSync(dkimPrivateKeyFile, 0o644)) return new MailError(MailError.INTERNAL_ERROR, safe.error); @@ -829,11 +834,11 @@ function upsertDnsRecords(domain, mailFqdn, callback) { debug(`upsertDnsRecords: updating mail dns records of domain ${domain} and mail fqdn ${mailFqdn}`); - maildb.get(domain, function (error, result) { + maildb.get(domain, function (error, mailDomain) { if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new MailError(MailError.NOT_FOUND)); if (error) return callback(new MailError(MailError.INTERNAL_ERROR, error)); - error = ensureDkimKeySync(domain); + error = ensureDkimKeySync(mailDomain); if (error) return callback(error); if (process.env.BOX_ENV === 'test') return callback(); @@ -842,11 +847,11 @@ function upsertDnsRecords(domain, mailFqdn, callback) { if (!dkimKey) return callback(new MailError(MailError.INTERNAL_ERROR, new Error('Failed to read dkim public key'))); // t=s limits the domainkey to this domain and not it's subdomains - var dkimRecord = { subdomain: `${constants.DKIM_SELECTOR}._domainkey`, domain: domain, type: 'TXT', values: [ '"v=DKIM1; t=s; p=' + dkimKey + '"' ] }; + var dkimRecord = { subdomain: `${mailDomain.dkimSelector}._domainkey`, domain: domain, type: 'TXT', values: [ '"v=DKIM1; t=s; p=' + dkimKey + '"' ] }; var records = [ ]; records.push(dkimRecord); - if (result.enabled) { + if (mailDomain.enabled) { records.push({ subdomain: '_dmarc', domain: domain, type: 'TXT', values: [ '"v=DMARC1; p=reject; pct=100"' ] }); records.push({ subdomain: '', domain: domain, type: 'MX', values: [ '10 ' + mailFqdn + '.' ] }); } @@ -904,7 +909,9 @@ function addDomain(domain, callback) { assert.strictEqual(typeof domain, 'string'); assert.strictEqual(typeof callback, 'function'); - maildb.add(domain, function (error) { + const dkimSelector = domain === config.adminDomain() ? 'cloudron' : (config.adminDomain().replace(/\./g, '') + '-cloudron'); + + maildb.add(domain, { dkimSelector }, function (error) { if (error && error.reason === DatabaseError.ALREADY_EXISTS) return callback(new MailError(MailError.ALREADY_EXISTS, 'Domain already exists')); if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new MailError(MailError.NOT_FOUND, 'No such domain')); if (error) return callback(new MailError(MailError.INTERNAL_ERROR, error)); diff --git a/src/maildb.js b/src/maildb.js index 82acd3187..8e21695a1 100644 --- a/src/maildb.js +++ b/src/maildb.js @@ -19,7 +19,7 @@ var assert = require('assert'), DatabaseError = require('./databaseerror.js'), safe = require('safetydance'); -var MAILDB_FIELDS = [ 'domain', 'enabled', 'mailFromValidation', 'catchAllJson', 'relayJson' ].join(','); +var MAILDB_FIELDS = [ 'domain', 'enabled', 'mailFromValidation', 'catchAllJson', 'relayJson', 'dkimSelector' ].join(','); function postProcess(data) { data.enabled = !!data.enabled; // int to boolean @@ -34,10 +34,12 @@ function postProcess(data) { return data; } -function add(domain, callback) { +function add(domain, data, callback) { assert.strictEqual(typeof domain, 'string'); + assert.strictEqual(typeof data, 'object'); + assert.strictEqual(typeof callback, 'function'); - database.query('INSERT INTO mail (domain) VALUES (?)', [ domain ], function (error) { + database.query('INSERT INTO mail (domain, dkimSelector) VALUES (?, ?)', [ domain, data.dkimSelector || 'cloudron' ], function (error) { if (error && error.code === 'ER_DUP_ENTRY') return callback(new DatabaseError(DatabaseError.ALREADY_EXISTS, 'mail domain already exists')); if (error && error.code === 'ER_NO_REFERENCED_ROW_2') return callback(new DatabaseError(DatabaseError.NOT_FOUND), 'no such domain'); if (error) return callback(new DatabaseError(DatabaseError.INTERNAL_ERROR, error)); diff --git a/src/provision.js b/src/provision.js index 59ef6a9c7..79feee39d 100644 --- a/src/provision.js +++ b/src/provision.js @@ -171,7 +171,7 @@ function setup(dnsConfig, backupConfig, auditSource, callback) { autoRegister.bind(null, domain), domains.prepareDashboardDomain.bind(null, domain, auditSource, (progress) => setProgress('setup', progress.message, NOOP_CALLBACK)), cloudron.setDashboardDomain.bind(null, domain, auditSource), // this sets up the config.fqdn() - mail.addDomain.bind(null, domain), // this relies on config.mailFqdn() + mail.addDomain.bind(null, domain), // this relies on config.mailFqdn() and config.adminDomain() setProgress.bind(null, 'setup', 'Applying auto-configuration'), (next) => { if (!backupConfig) return next(); settings.setBackupConfig(backupConfig, next); }, setProgress.bind(null, 'setup', 'Done'), diff --git a/src/routes/test/mail-test.js b/src/routes/test/mail-test.js index 69de351bf..958f4d01f 100644 --- a/src/routes/test/mail-test.js +++ b/src/routes/test/mail-test.js @@ -239,7 +239,7 @@ describe('Mail API', function () { callback(null, dnsAnswerQueue[hostname][type]); }; - dkimDomain = 'cloudron._domainkey.' + DOMAIN_0.domain; + dkimDomain = 'admincom-cloudron._domainkey.' + DOMAIN_0.domain; spfDomain = DOMAIN_0.domain; mxDomain = DOMAIN_0.domain; dmarcDomain = '_dmarc.' + DOMAIN_0.domain; diff --git a/src/test/database-test.js b/src/test/database-test.js index b92f9e4a1..a8356ef5d 100644 --- a/src/test/database-test.js +++ b/src/test/database-test.js @@ -2044,7 +2044,7 @@ describe('database', function () { before(function (done) { async.series([ domaindb.add.bind(null, DOMAIN_0.domain, { zoneName: DOMAIN_0.zoneName, provider: DOMAIN_0.provider, config: DOMAIN_0.config, tlsConfig: DOMAIN_0.tlsConfig }), - maildb.add.bind(null, DOMAIN_0.domain) + maildb.add.bind(null, DOMAIN_0.domain, {}) ], done); }); @@ -2206,7 +2206,8 @@ describe('database', function () { enabled: false, relay: { provider: 'cloudron-smtp' }, catchAll: [ ], - mailFromValidation: true + mailFromValidation: true, + dkimSelector: 'cloudron' }; before(function (done) { @@ -2218,7 +2219,7 @@ describe('database', function () { }); it('cannot add non-existing domain', function (done) { - maildb.add(MAIL_DOMAIN_0.domain + 'nope', function (error) { + maildb.add(MAIL_DOMAIN_0.domain + 'nope', {}, function (error) { expect(error).to.be.ok(); expect(error.reason).to.be(DatabaseError.NOT_FOUND); @@ -2227,7 +2228,7 @@ describe('database', function () { }); it('can add domain', function (done) { - maildb.add(MAIL_DOMAIN_0.domain, function (error) { + maildb.add(MAIL_DOMAIN_0.domain, {}, function (error) { expect(error).to.equal(null); done(); diff --git a/src/test/ldap-test.js b/src/test/ldap-test.js index db34a083d..5fabf1781 100644 --- a/src/test/ldap-test.js +++ b/src/test/ldap-test.js @@ -100,7 +100,7 @@ function setup(done) { database._clear.bind(null), ldapServer.start.bind(null), domains.add.bind(null, DOMAIN_0.domain, DOMAIN_0, AUDIT_SOURCE), - maildb.add.bind(null, DOMAIN_0.domain), + maildb.add.bind(null, DOMAIN_0.domain, {}), function (callback) { users.createOwner(USER_0.username, USER_0.password, USER_0.email, USER_0.displayName, AUDIT_SOURCE, function (error, result) { if (error) return callback(error);