From b19a2724031bc3d82b6e24158acb162bafd64787 Mon Sep 17 00:00:00 2001 From: Will Rouesnel Date: Wed, 24 May 2023 23:59:38 +1000 Subject: [PATCH 01/13] Fix OpenSSL data parsing OpenSSL data parsing could be confused when parsing certificates which have Country/Org and other parameters in the subject line. This is fixed by writing a more robust parser of the output lines, and using that to do parsing which now correctly handles this case. --- backend/internal/certificate.js | 37 ++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/backend/internal/certificate.js b/backend/internal/certificate.js index c93e2578fa..88f267d67c 100644 --- a/backend/internal/certificate.js +++ b/backend/internal/certificate.js @@ -726,6 +726,28 @@ const internalCertificate = { }); }, + /** + * Parse the X509 subject line as returned by the OpenSSL command when + * invoked with openssl x509 -in -subject -noout + * + * @param {String} line emitted from the openssl command + * @param {String} prefix expected to be removed + * @return {Object} object containing the parsed fields from the subject line + */ + parseX509Output: (line, prefix) => { + // Remove the subject= part + const subject_value = line.slice(prefix.length); + + const subject = subject_value.split(/,(?=(?:(?:[^"]*"){2})*[^"]*$)/) + .map( (e) => { return e.trim().split(' = ', 2); }) + .reduce((obj, [key, value]) => { + obj[key] = value.replace(/^"/, '').replace(/"$/, ''); + return obj; + }, {}); + + return subject; + }, + /** * Uses the openssl command to both validate and get info out of the certificate. * It will save the file to disk first, then run commands on it, then delete the file. @@ -739,28 +761,27 @@ const internalCertificate = { return utils.exec('openssl x509 -in ' + certificate_file + ' -subject -noout') .then((result) => { // subject=CN = something.example.com - const regex = /(?:subject=)?[^=]+=\s+(\S+)/gim; - const match = regex.exec(result); + // subject=C = NoCountry, O = NoOrg, OU = NoOrgUnit, CN = Some Value With Spaces + const subjectParams = internalCertificate.parseX509Output(result, 'subject='); - if (typeof match[1] === 'undefined') { + if (typeof subjectParams.CN === 'undefined') { throw new error.ValidationError('Could not determine subject from certificate: ' + result); } - certData['cn'] = match[1]; + certData['cn'] = subjectParams.CN; }) .then(() => { return utils.exec('openssl x509 -in ' + certificate_file + ' -issuer -noout'); }) .then((result) => { // issuer=C = US, O = Let's Encrypt, CN = Let's Encrypt Authority X3 - const regex = /^(?:issuer=)?(.*)$/gim; - const match = regex.exec(result); + const issuerParams = internalCertificate.parseX509Output(result, 'issuer='); - if (typeof match[1] === 'undefined') { + if (typeof issuerParams.CN === 'undefined') { throw new error.ValidationError('Could not determine issuer from certificate: ' + result); } - certData['issuer'] = match[1]; + certData['issuer'] = issuerParams.CN; }) .then(() => { return utils.exec('openssl x509 -in ' + certificate_file + ' -dates -noout'); From c664e864cea87a9dbd3bc0ba04f2018f4f729bcc Mon Sep 17 00:00:00 2001 From: Will Rouesnel Date: Thu, 25 May 2023 00:21:32 +1000 Subject: [PATCH 02/13] Add storing for Client CA certificates in the database Add initial support for managing Client Certificate Authority public certificates as certificate objects in the database. The new provider type 'clientca' is defined to implement this. --- backend/internal/certificate.js | 21 ++++++++++------- backend/schema/definitions.json | 2 +- frontend/js/app/nginx/certificates/form.ejs | 18 ++++++++++++++- frontend/js/app/nginx/certificates/form.js | 25 +++++++++++++++++---- frontend/js/app/nginx/certificates/main.ejs | 1 + frontend/js/i18n/messages.json | 7 ++++-- 6 files changed, 58 insertions(+), 16 deletions(-) diff --git a/backend/internal/certificate.js b/backend/internal/certificate.js index 88f267d67c..723f94b202 100644 --- a/backend/internal/certificate.js +++ b/backend/internal/certificate.js @@ -552,13 +552,18 @@ const internalCertificate = { }) .then(() => { return new Promise((resolve, reject) => { - fs.writeFile(dir + '/privkey.pem', certificate.meta.certificate_key, function (err) { - if (err) { - reject(err); - } else { - resolve(); - } - }); + if (certificate.provider === 'clientca') { + // Client CAs have no private key associated, so just succeed. + resolve(); + } else { + fs.writeFile(dir + '/privkey.pem', certificate.meta.certificate_key, function (err) { + if (err) { + reject(err); + } else { + resolve(); + } + }); + } }); }); }, @@ -639,7 +644,7 @@ const internalCertificate = { upload: (access, data) => { return internalCertificate.get(access, {id: data.id}) .then((row) => { - if (row.provider !== 'other') { + if (row.provider !== 'other' && row.provider !== 'clientca') { throw new error.ValidationError('Cannot upload certificates for this type of provider'); } diff --git a/backend/schema/definitions.json b/backend/schema/definitions.json index 4b4f3405c7..136b023518 100644 --- a/backend/schema/definitions.json +++ b/backend/schema/definitions.json @@ -219,7 +219,7 @@ }, "ssl_provider": { "type": "string", - "pattern": "^(letsencrypt|other)$" + "pattern": "^(letsencrypt|other|clientca)$" }, "http2_support": { "description": "HTTP2 Protocol Support", diff --git a/frontend/js/app/nginx/certificates/form.ejs b/frontend/js/app/nginx/certificates/form.ejs index 7fc12785b3..6b87261d4a 100644 --- a/frontend/js/app/nginx/certificates/form.ejs +++ b/frontend/js/app/nginx/certificates/form.ejs @@ -173,7 +173,23 @@ - + <% } else if (provider === 'clientca') { %> + +
+
+ + +
+
+
+
+
<%- i18n('certificates', 'clientca-certificate') %>*
+
+ + +
+
+
<% } %> diff --git a/frontend/js/app/nginx/certificates/form.js b/frontend/js/app/nginx/certificates/form.js index a56c3f8e00..eb6fb70879 100644 --- a/frontend/js/app/nginx/certificates/form.js +++ b/frontend/js/app/nginx/certificates/form.js @@ -45,7 +45,9 @@ module.exports = Mn.View.extend({ propagation_seconds: 'input[name="meta[propagation_seconds]"]', other_certificate_key_label: '#other_certificate_key_label', other_intermediate_certificate: '#other_intermediate_certificate', - other_intermediate_certificate_label: '#other_intermediate_certificate_label' + other_intermediate_certificate_label: '#other_intermediate_certificate_label', + clientca_certificate: '#clientca_certificate', + clientca_certificate_label: '#clientca_certificate_label' }, events: { @@ -156,6 +158,18 @@ module.exports = Mn.View.extend({ } ssl_files.push({name: 'intermediate_certificate', file: this.ui.other_intermediate_certificate[0].files[0]}); } + } else if (data.provider === 'clientca' && !this.model.hasSslFiles()) { + // check files are attached + if (!this.ui.clientca_certificate[0].files.length || !this.ui.clientca_certificate[0].files[0].size) { + alert('Certificate file is not attached'); + return; + } else { + if (this.ui.clientca_certificate[0].files[0].size > this.max_file_size) { + alert('Certificate file is too large (> 100kb)'); + return; + } + ssl_files.push({name: 'certificate', file: this.ui.clientca_certificate[0].files[0]}); + } } this.ui.loader_content.show(); @@ -163,14 +177,14 @@ module.exports = Mn.View.extend({ // compile file data let form_data = new FormData(); - if (data.provider === 'other' && ssl_files.length) { + if ((data.provider === 'other' || data.provider === 'clientca') && ssl_files.length) { ssl_files.map(function (file) { form_data.append(file.name, file.file); }); } new Promise(resolve => { - if (data.provider === 'other') { + if (data.provider === 'other' || data.provider === 'clientca') { resolve(App.Api.Nginx.Certificates.validate(form_data)); } else { resolve(); @@ -183,7 +197,7 @@ module.exports = Mn.View.extend({ this.model.set(result); // Now upload the certs if we need to - if (data.provider === 'other') { + if (data.provider === 'other' || data.provider === 'clientca') { return App.Api.Nginx.Certificates.upload(this.model.get('id'), form_data) .then(result => { this.model.set('meta', _.assign({}, this.model.get('meta'), result)); @@ -234,6 +248,9 @@ module.exports = Mn.View.extend({ }, 'change @ui.other_intermediate_certificate': function(e){ this.setFileName("other_intermediate_certificate_label", e) + }, + 'change @ui.clientca_certificate': function(e){ + this.setFileName("clientca_certificate_label", e) } }, setFileName(ui, e){ diff --git a/frontend/js/app/nginx/certificates/main.ejs b/frontend/js/app/nginx/certificates/main.ejs index dbd6fa85d7..5d49c47afe 100644 --- a/frontend/js/app/nginx/certificates/main.ejs +++ b/frontend/js/app/nginx/certificates/main.ejs @@ -20,6 +20,7 @@
<%- i18n('ssl', 'letsencrypt') %> <%- i18n('ssl', 'other') %> + <%- i18n('ssl', 'clientca') %>
<% } %> diff --git a/frontend/js/i18n/messages.json b/frontend/js/i18n/messages.json index 4f12f72b90..148d3261a2 100644 --- a/frontend/js/i18n/messages.json +++ b/frontend/js/i18n/messages.json @@ -99,6 +99,7 @@ "ssl": { "letsencrypt": "Let's Encrypt", "other": "Custom", + "clientca": "Client Certificate Authority", "none": "HTTP only", "letsencrypt-email": "Email Address for Let's Encrypt", "letsencrypt-agree": "I Agree to the Let's Encrypt Terms of Service", @@ -185,7 +186,7 @@ "title": "SSL Certificates", "empty": "There are no SSL Certificates", "add": "Add SSL Certificate", - "form-title": "Add {provider, select, letsencrypt{Let's Encrypt} other{Custom}} Certificate", + "form-title": "Add {provider, select, letsencrypt{Let's Encrypt Certificate} other{Custom Certificate} clientca{Client Certificate Authority}}", "delete": "Delete SSL Certificate", "delete-confirm": "Are you sure you want to delete this SSL Certificate? Any hosts using it will need to be updated later.", "help-title": "SSL Certificates", @@ -193,6 +194,7 @@ "other-certificate": "Certificate", "other-certificate-key": "Certificate Key", "other-intermediate-certificate": "Intermediate Certificate", + "clientca-certificate": "Certificate", "force-renew": "Renew Now", "test-reachability": "Test Server Reachability", "reachability-title": "Test Server Reachability", @@ -231,7 +233,8 @@ "pass-auth": "Pass Auth to Host", "access-add": "Add", "auth-add": "Add", - "search": "Search Access…" + "search": "Search Access…", + "client-certificates": "Client Certificates" }, "users": { "title": "Users", From d5b3e5314033ee9fede6795d3fc10ddd899c4705 Mon Sep 17 00:00:00 2001 From: Will Rouesnel Date: Thu, 25 May 2023 00:37:27 +1000 Subject: [PATCH 03/13] Add frontend support for the new clientca type The frontend is modified to filter certificates from selector lists so only non-clientca certificate types can be set as server certificates. --- frontend/js/app/api.js | 31 +++++++++++++++++++++++ frontend/js/app/nginx/dead/form.js | 2 +- frontend/js/app/nginx/proxy/form.js | 2 +- frontend/js/app/nginx/redirection/form.js | 2 +- 4 files changed, 34 insertions(+), 3 deletions(-) diff --git a/frontend/js/app/api.js b/frontend/js/app/api.js index 6e33a6dca8..10fe721306 100644 --- a/frontend/js/app/api.js +++ b/frontend/js/app/api.js @@ -632,6 +632,37 @@ module.exports = { return getAllObjects('nginx/certificates', expand, query); }, + /** + * Retrieve all certificates which have a type suitable for use as + * server certificates. This filters by provider for returned rows. + * + * @param {Array} [expand] + * @param {String} [query] + * @returns {Promise} + */ + getAllServerCertificates: function (expand, query) { + return getAllObjects('nginx/certificates', expand, query) + .then(rows => { + return rows.filter( row => row.provider !== 'clientca' ); + }) + }, + + /** + * Retrieve all certificates which have a type suitable for use as + * client authentication certificates. This filters by provider for + * returned rows. + * + * @param {Array} [expand] + * @param {String} [query] + * @returns {Promise} + */ + getAllClientCertificates: function (expand, query) { + return getAllObjects('nginx/certificates', expand, query) + .then(rows => { + return rows.filter( row => row.provider === 'clientca' ); + }) + }, + /** * @param {Object} data */ diff --git a/frontend/js/app/nginx/dead/form.js b/frontend/js/app/nginx/dead/form.js index 8f6774f689..52f54c1051 100644 --- a/frontend/js/app/nginx/dead/form.js +++ b/frontend/js/app/nginx/dead/form.js @@ -263,7 +263,7 @@ module.exports = Mn.View.extend({ } }, load: function (query, callback) { - App.Api.Nginx.Certificates.getAll() + App.Api.Nginx.Certificates.getAllServerCertificates() .then(rows => { callback(rows); }) diff --git a/frontend/js/app/nginx/proxy/form.js b/frontend/js/app/nginx/proxy/form.js index 1dfb5c1891..eb786251bf 100644 --- a/frontend/js/app/nginx/proxy/form.js +++ b/frontend/js/app/nginx/proxy/form.js @@ -331,7 +331,7 @@ module.exports = Mn.View.extend({ } }, load: function (query, callback) { - App.Api.Nginx.Certificates.getAll() + App.Api.Nginx.Certificates.getAllServerCertificates() .then(rows => { callback(rows); }) diff --git a/frontend/js/app/nginx/redirection/form.js b/frontend/js/app/nginx/redirection/form.js index 1f81feebc1..4e292e53f4 100644 --- a/frontend/js/app/nginx/redirection/form.js +++ b/frontend/js/app/nginx/redirection/form.js @@ -265,7 +265,7 @@ module.exports = Mn.View.extend({ } }, load: function (query, callback) { - App.Api.Nginx.Certificates.getAll() + App.Api.Nginx.Certificates.getAllServerCertificates() .then(rows => { callback(rows); }) From e5bb50c16481f26e8a77cd5390219ffa16251584 Mon Sep 17 00:00:00 2001 From: Will Rouesnel Date: Sat, 27 May 2023 01:43:15 +1000 Subject: [PATCH 04/13] Add support for adding Client Certificates to access-lists Client certificate support is added as a new separate type of option for access-lists. This commit is the support code to enable access-lists to contain Client Certificate references. --- backend/internal/access-list.js | 77 ++++++++--- ...0526062132_add_clientcas_to_accesslists.js | 50 ++++++++ backend/models/access_list.js | 21 ++- backend/models/access_list_clientcas.js | 62 +++++++++ backend/schema/endpoints/access-lists.json | 14 ++ frontend/js/app/nginx/access/form.ejs | 29 +++++ frontend/js/app/nginx/access/form.js | 121 ++++++++++++++++-- .../js/app/nginx/access/form/clientca.ejs | 18 +++ frontend/js/app/nginx/access/form/clientca.js | 7 + frontend/js/app/nginx/access/list/item.ejs | 3 + frontend/js/app/nginx/access/list/main.ejs | 1 + frontend/js/app/nginx/access/main.js | 4 +- frontend/js/app/nginx/proxy/form.js | 2 +- frontend/js/i18n/messages.json | 5 +- frontend/js/models/access-list.js | 1 + 15 files changed, 374 insertions(+), 41 deletions(-) create mode 100644 backend/migrations/20230526062132_add_clientcas_to_accesslists.js create mode 100644 backend/models/access_list_clientcas.js create mode 100644 frontend/js/app/nginx/access/form/clientca.ejs create mode 100644 frontend/js/app/nginx/access/form/clientca.js diff --git a/backend/internal/access-list.js b/backend/internal/access-list.js index 8457792788..de71d16582 100644 --- a/backend/internal/access-list.js +++ b/backend/internal/access-list.js @@ -1,15 +1,16 @@ -const _ = require('lodash'); -const fs = require('fs'); -const batchflow = require('batchflow'); -const logger = require('../logger').access; -const error = require('../lib/error'); -const utils = require('../lib/utils'); -const accessListModel = require('../models/access_list'); -const accessListAuthModel = require('../models/access_list_auth'); -const accessListClientModel = require('../models/access_list_client'); -const proxyHostModel = require('../models/proxy_host'); -const internalAuditLog = require('./audit-log'); -const internalNginx = require('./nginx'); +const _ = require('lodash'); +const fs = require('fs'); +const batchflow = require('batchflow'); +const logger = require('../logger').access; +const error = require('../lib/error'); +const utils = require('../lib/utils'); +const accessListModel = require('../models/access_list'); +const accessListAuthModel = require('../models/access_list_auth'); +const accessListClientModel = require('../models/access_list_client'); +const accessListClientCAsModel = require('../models/access_list_clientcas'); +const proxyHostModel = require('../models/proxy_host'); +const internalAuditLog = require('./audit-log'); +const internalNginx = require('./nginx'); function omissions () { return ['is_deleted']; @@ -66,13 +67,26 @@ const internalAccessList = { }); } + // Now add the client certificate references + if (typeof data.clientcas !== 'undefined' && data.clientcas) { + data.clientcas.map((certificate_id) => { + promises.push(accessListClientCAsModel + .query() + .insert({ + access_list_id: row.id, + certificate_id: certificate_id + }) + ); + }); + } + return Promise.all(promises); }) .then(() => { // re-fetch with expansions return internalAccessList.get(access, { id: data.id, - expand: ['owner', 'items', 'clients', 'proxy_hosts.access_list.[clients,items]'] + expand: ['owner', 'items', 'clients', 'clientcas', 'proxy_hosts.access_list.[clientcas.certificate,clients,items]'] }, true /* <- skip masking */); }) .then((row) => { @@ -204,6 +218,35 @@ const internalAccessList = { }); } }) + .then(() => { + // Check for client certificates and add/update/remove them + if (typeof data.clientcas !== 'undefined' && data.clientcas) { + let promises = []; + + data.clientcas.map(function (certificate_id) { + promises.push(accessListClientCAsModel + .query() + .insert({ + access_list_id: data.id, + certificate_id: certificate_id + }) + ); + }); + + let query = accessListClientCAsModel + .query() + .delete() + .where('access_list_id', data.id); + + return query + .then(() => { + // Add new items + if (promises.length) { + return Promise.all(promises); + } + }); + } + }) .then(internalNginx.reload) .then(() => { // Add to audit log @@ -218,7 +261,7 @@ const internalAccessList = { // re-fetch with expansions return internalAccessList.get(access, { id: data.id, - expand: ['owner', 'items', 'clients', 'proxy_hosts.[certificate,access_list.[clients,items]]'] + expand: ['owner', 'items', 'clients', 'clientcas', 'proxy_hosts.[certificate,access_list.[clientcas.certificate,clients,items]]'] }, true /* <- skip masking */); }) .then((row) => { @@ -256,7 +299,7 @@ const internalAccessList = { .joinRaw('LEFT JOIN `proxy_host` ON `proxy_host`.`access_list_id` = `access_list`.`id` AND `proxy_host`.`is_deleted` = 0') .where('access_list.is_deleted', 0) .andWhere('access_list.id', data.id) - .allowGraph('[owner,items,clients,proxy_hosts.[certificate,access_list.[clients,items]]]') + .withGraphFetched('[owner,items,clients,clientcas,proxy_hosts.[certificate,access_list.[clientcas.certificate,clients,items]]]') .first(); if (access_data.permission_visibility !== 'all') { @@ -294,7 +337,7 @@ const internalAccessList = { delete: (access, data) => { return access.can('access_lists:delete', data.id) .then(() => { - return internalAccessList.get(access, {id: data.id, expand: ['proxy_hosts', 'items', 'clients']}); + return internalAccessList.get(access, {id: data.id, expand: ['proxy_hosts', 'items', 'clients', 'clientcas']}); }) .then((row) => { if (!row) { @@ -377,7 +420,7 @@ const internalAccessList = { .joinRaw('LEFT JOIN `proxy_host` ON `proxy_host`.`access_list_id` = `access_list`.`id` AND `proxy_host`.`is_deleted` = 0') .where('access_list.is_deleted', 0) .groupBy('access_list.id') - .allowGraph('[owner,items,clients]') + .withGraphFetched('[owner,items,clients,clientcas.certificate]') .orderBy('access_list.name', 'ASC'); if (access_data.permission_visibility !== 'all') { diff --git a/backend/migrations/20230526062132_add_clientcas_to_accesslists.js b/backend/migrations/20230526062132_add_clientcas_to_accesslists.js new file mode 100644 index 0000000000..e8c5a7f401 --- /dev/null +++ b/backend/migrations/20230526062132_add_clientcas_to_accesslists.js @@ -0,0 +1,50 @@ +const migrate_name = 'client_certificates'; +const logger = require('../logger').migrate; + +/** + * Migrate + * + * @see http://knexjs.org/#Schema + * + * @param {Object} knex + * @param {Promise} Promise + * @returns {Promise} + */ +exports.up = function (knex/*, Promise*/) { + + logger.info('[' + migrate_name + '] Migrating Up...'); + + return knex.schema.createTable('access_list_clientcas', (table) => { + table.increments().primary(); + table.dateTime('created_on').notNull(); + table.dateTime('modified_on').notNull(); + table.integer('access_list_id').notNull().unsigned(); + table.integer('certificate_id').notNull().unsigned(); + table.json('meta').notNull(); + }) + .then(function () { + logger.info('[' + migrate_name + '] access_list_clientcas Table created'); + }) + .then(() => { + logger.info('[' + migrate_name + '] Migrating Up Complete'); + }); +}; + +/** + * Undo Migrate + * + * @param {Object} knex + * @param {Promise} Promise + * @returns {Promise} + */ +exports.down = function (knex/*, Promise*/) { + logger.info('[' + migrate_name + '] Migrating Down...'); + + return knex.schema.dropTable('access_list_clientcas') + .then(() => { + logger.info('[' + migrate_name + '] access_list_clientcas Table dropped'); + }) + .then(() => { + logger.info('[' + migrate_name + '] Migrating Down Complete'); + }); +}; diff --git a/backend/models/access_list.js b/backend/models/access_list.js index fbf9bda770..a6f8d8995d 100644 --- a/backend/models/access_list.js +++ b/backend/models/access_list.js @@ -1,12 +1,13 @@ // Objection Docs: // http://vincit.github.io/objection.js/ -const db = require('../db'); -const Model = require('objection').Model; -const User = require('./user'); -const AccessListAuth = require('./access_list_auth'); -const AccessListClient = require('./access_list_client'); -const now = require('./now_helper'); +const db = require('../db'); +const Model = require('objection').Model; +const User = require('./user'); +const AccessListAuth = require('./access_list_auth'); +const AccessListClient = require('./access_list_client'); +const AccessListClientCAs = require('./access_list_clientcas'); +const now = require('./now_helper'); Model.knex(db); @@ -68,6 +69,14 @@ class AccessList extends Model { to: 'access_list_client.access_list_id' } }, + clientcas: { + relation: Model.HasManyRelation, + modelClass: AccessListClientCAs, + join: { + from: 'access_list.id', + to: 'access_list_clientcas.access_list_id' + } + }, proxy_hosts: { relation: Model.HasManyRelation, modelClass: ProxyHost, diff --git a/backend/models/access_list_clientcas.js b/backend/models/access_list_clientcas.js new file mode 100644 index 0000000000..3be537a606 --- /dev/null +++ b/backend/models/access_list_clientcas.js @@ -0,0 +1,62 @@ +// Objection Docs: +// http://vincit.github.io/objection.js/ + +const db = require('../db'); +const Model = require('objection').Model; +const now = require('./now_helper'); + +Model.knex(db); + +class AccessListClientCAs extends Model { + $beforeInsert () { + this.created_on = now(); + this.modified_on = now(); + + // Default for meta + if (typeof this.meta === 'undefined') { + this.meta = {}; + } + } + + $beforeUpdate () { + this.modified_on = now(); + } + + static get name () { + return 'AccessListClientCAs'; + } + + static get tableName () { + return 'access_list_clientcas'; + } + + static get jsonAttributes () { + return ['meta']; + } + + static get relationMappings () { + return { + access_list: { + relation: Model.HasOneRelation, + modelClass: require('./access_list'), + join: { + from: 'access_list_clientcas.access_list_id', + to: 'access_list.id' + }, + modify: function (qb) { + qb.where('access_list.is_deleted', 0); + } + }, + certificate: { + relation: Model.HasOneRelation, + modelClass: require('./certificate'), + join: { + from: 'access_list_clientcas.certificate_id', + to: 'certificate.id' + } + } + }; + } +} + +module.exports = AccessListClientCAs; diff --git a/backend/schema/endpoints/access-lists.json b/backend/schema/endpoints/access-lists.json index 404e323765..6ad77fd265 100644 --- a/backend/schema/endpoints/access-lists.json +++ b/backend/schema/endpoints/access-lists.json @@ -142,6 +142,13 @@ } } }, + "clientcas": { + "type": "array", + "minItems": 0, + "items": { + "type": "integer" + } + }, "meta": { "$ref": "#/definitions/meta" } @@ -209,6 +216,13 @@ } } } + }, + "clientcas": { + "type": "array", + "minItems": 0, + "items": { + "type": "integer" + } } } }, diff --git a/frontend/js/app/nginx/access/form.ejs b/frontend/js/app/nginx/access/form.ejs index 79220b14b9..a15ef7b465 100644 --- a/frontend/js/app/nginx/access/form.ejs +++ b/frontend/js/app/nginx/access/form.ejs @@ -8,6 +8,7 @@ @@ -71,6 +72,34 @@ + +
+

+ Client Certificate Authorization via + + Nginx HTTP SSL + +

+ +
+
+ + +
+
+
+ +
+
+
+ + +
+ +
+
+

diff --git a/frontend/js/app/nginx/access/form.js b/frontend/js/app/nginx/access/form.js index bb0755481b..3b23d61e5f 100644 --- a/frontend/js/app/nginx/access/form.js +++ b/frontend/js/app/nginx/access/form.js @@ -4,8 +4,13 @@ const AccessListModel = require('../../../models/access-list'); const template = require('./form.ejs'); const ItemView = require('./form/item'); const ClientView = require('./form/client'); +const ClientCAView = require('./form/clientca'); require('jquery-serializejson'); +require('selectize'); + +const Helpers = require("../../../lib/helpers"); +const certListItemTemplate = require("../certificates-list-item.ejs"); const ItemsView = Mn.CollectionView.extend({ childView: ItemView @@ -15,39 +20,52 @@ const ClientsView = Mn.CollectionView.extend({ childView: ClientView }); +const ClientCAsView = Mn.CollectionView.extend({ + childView: ClientCAView +}); + module.exports = Mn.View.extend({ template: template, className: 'modal-dialog', ui: { - items_region: '.items', - clients_region: '.clients', - form: 'form', - buttons: '.modal-footer button', - cancel: 'button.cancel', - save: 'button.save', - access_add: 'button.access_add', - auth_add: 'button.auth_add' + items_region: '.items', + clients_region: '.clients', + clientcas_region: '.clientcas', + certificate_select: 'select[id="certificate_search"]', + form: 'form', + buttons: '.modal-footer button', + cancel: 'button.cancel', + save: 'button.save', + access_add: 'button.access_add', + auth_add: 'button.auth_add', + clientca_add: 'button.clientca_add', + clientca_del: 'button.clientca_del' }, regions: { items_region: '@ui.items_region', - clients_region: '@ui.clients_region' + clients_region: '@ui.clients_region', + clientcas_region: '@ui.clientcas_region' }, events: { 'click @ui.save': function (e) { e.preventDefault(); + console.log(this.ui.form); // FIXME + if (!this.ui.form[0].checkValidity()) { $('').hide().appendTo(this.ui.form).click().remove(); return; } let view = this; - let form_data = this.ui.form.serializeJSON(); let items_data = []; let clients_data = []; + let clientcas_data = []; + + let form_data = this.ui.form.serializeJSON(); form_data.username.map(function (val, idx) { if (val.trim().length) { @@ -67,7 +85,13 @@ module.exports = Mn.View.extend({ } }); - if (!items_data.length && !clients_data.length) { + if (form_data.certificate_id !== undefined) { + form_data.certificate_id.map(function (val, idx) { + clientcas_data.push(parseInt(val, 10)) + }); + } + + if (!items_data.length && !clients_data.length && !clientcas_data.length) { alert('You must specify at least 1 Authorization or Access rule'); return; } @@ -77,11 +101,10 @@ module.exports = Mn.View.extend({ satisfy_any: !!form_data.satisfy_any, pass_auth: !!form_data.pass_auth, items: items_data, - clients: clients_data + clients: clients_data, + clientcas: clientcas_data }; - console.log(data); - let method = App.Api.Nginx.AccessLists.create; let is_new = true; @@ -125,16 +148,55 @@ module.exports = Mn.View.extend({ this.showChildView('items_region', new ItemsView({ collection: new Backbone.Collection(items) })); + }, + 'click @ui.clientca_add': function (e) { + e.preventDefault(); + + App.Api.Nginx.Certificates.getAllClientCertificates().then((certificates) => { + let value = this.ui.certificate_select[0].value; + if (value === undefined || value === '') { + return; + } + + let certificate_id = parseInt(this.ui.certificate_select[0].value, 10); + let cert = certificates.filter((cert) => { return cert.id === certificate_id })[0]; + + let clientcas = this.model.get('clientcas'); + clientcas.push({ + certificate: cert + }); + + this.ui.certificate_select[0].selectize.clear(); + + this.showChildView('clientcas_region', new ClientCAsView({ + collection: new Backbone.Collection(clientcas) + })); + }) + }, + 'click @ui.clientca_del': function (e) { + e.preventDefault(); + + let certificate_id = parseInt(e.currentTarget.dataset.value, 10); + + let clientcas = this.model.get('clientcas'); + this.model.set('clientcas', clientcas.filter((e) => { return e.certificate.id !== certificate_id })); + clientcas = this.model.get('clientcas'); + + this.showChildView('clientcas_region', new ClientCAsView({ + collection: new Backbone.Collection(clientcas) + })); } }, onRender: function () { let items = this.model.get('items'); let clients = this.model.get('clients'); + let clientcas = this.model.get('clientcas'); // Ensure at least one field is shown initally if (!items.length) items.push({}); if (!clients.length) clients.push({}); + if (!clientcas.length) clients.push({}); this.showChildView('items_region', new ItemsView({ collection: new Backbone.Collection(items) @@ -143,6 +205,37 @@ module.exports = Mn.View.extend({ this.showChildView('clients_region', new ClientsView({ collection: new Backbone.Collection(clients) })); + + this.showChildView('clientcas_region', new ClientCAsView({ + collection: new Backbone.Collection(clientcas) + })); + + this.ui.certificate_select.selectize({ + valueField: 'id', + labelField: 'nice_name', + searchField: ['nice_name', 'domain_names'], + create: false, + preload: true, + allowEmptyOption: true, + render: { + option: function (item) { + item.i18n = App.i18n; + item.formatDbDate = Helpers.formatDbDate; + return certListItemTemplate(item); + } + }, + load: function (query, callback) { + App.Api.Nginx.Certificates.getAllClientCertificates() + .then(rows => { + callback(rows); + }) + .catch(err => { + console.error(err); + callback(); + }); + }, + onLoad: function () {} + }); }, initialize: function (options) { diff --git a/frontend/js/app/nginx/access/form/clientca.ejs b/frontend/js/app/nginx/access/form/clientca.ejs new file mode 100644 index 0000000000..41b980fe07 --- /dev/null +++ b/frontend/js/app/nginx/access/form/clientca.ejs @@ -0,0 +1,18 @@ + +

+ +
+
+
+ <%= certificate.nice_name %> +
Expires: <%- formatDbDate(certificate.expires_on, 'Do MMMM YYYY, h:mm a') %>
+
+
+
+ <% if (certificate.is_deleted == 1) { %>Deleted<% } %> +
+
+ +
\ No newline at end of file diff --git a/frontend/js/app/nginx/access/form/clientca.js b/frontend/js/app/nginx/access/form/clientca.js new file mode 100644 index 0000000000..acf04b64f8 --- /dev/null +++ b/frontend/js/app/nginx/access/form/clientca.js @@ -0,0 +1,7 @@ +const Mn = require('backbone.marionette'); +const template = require('./clientca.ejs'); + +module.exports = Mn.View.extend({ + template: template, + className: 'row' +}); diff --git a/frontend/js/app/nginx/access/list/item.ejs b/frontend/js/app/nginx/access/list/item.ejs index 2ee37a50af..73bd4eb224 100644 --- a/frontend/js/app/nginx/access/list/item.ejs +++ b/frontend/js/app/nginx/access/list/item.ejs @@ -14,6 +14,9 @@ <%- i18n('access-lists', 'item-count', {count: items.length || 0}) %> + + <%- i18n('access-lists', 'clientca-count', {count: clientcas.length || 0}) %> + <%- i18n('access-lists', 'client-count', {count: clients.length || 0}) %> diff --git a/frontend/js/app/nginx/access/list/main.ejs b/frontend/js/app/nginx/access/list/main.ejs index 7988e0c289..f85dc95a25 100644 --- a/frontend/js/app/nginx/access/list/main.ejs +++ b/frontend/js/app/nginx/access/list/main.ejs @@ -2,6 +2,7 @@   <%- i18n('str', 'name') %> <%- i18n('access-lists', 'authorization') %> + <%- i18n('access-lists', 'client-certificates') %> <%- i18n('access-lists', 'access') %> <%- i18n('access-lists', 'satisfy') %> <%- i18n('proxy-hosts', 'title') %> diff --git a/frontend/js/app/nginx/access/main.js b/frontend/js/app/nginx/access/main.js index 513f58659f..94f99de0ea 100644 --- a/frontend/js/app/nginx/access/main.js +++ b/frontend/js/app/nginx/access/main.js @@ -73,7 +73,7 @@ module.exports = Mn.View.extend({ e.preventDefault(); let query = this.ui.query.val(); - this.fetch(['owner', 'items', 'clients'], query) + this.fetch(['owner', 'items', 'clients', 'clientcas'], query) .then(response => this.showData(response)) .catch(err => { this.showError(err); @@ -88,7 +88,7 @@ module.exports = Mn.View.extend({ onRender: function () { let view = this; - view.fetch(['owner', 'items', 'clients']) + view.fetch(['owner', 'items', 'clients', 'clientcas']) .then(response => { if (!view.isDestroyed()) { if (response && response.length) { diff --git a/frontend/js/app/nginx/proxy/form.js b/frontend/js/app/nginx/proxy/form.js index eb786251bf..d2df532ee6 100644 --- a/frontend/js/app/nginx/proxy/form.js +++ b/frontend/js/app/nginx/proxy/form.js @@ -297,7 +297,7 @@ module.exports = Mn.View.extend({ } }, load: function (query, callback) { - App.Api.Nginx.AccessLists.getAll(['items', 'clients']) + App.Api.Nginx.AccessLists.getAll(['items', 'clients', 'clientcas']) .then(rows => { callback(rows); }) diff --git a/frontend/js/i18n/messages.json b/frontend/js/i18n/messages.json index 148d3261a2..619429db79 100644 --- a/frontend/js/i18n/messages.json +++ b/frontend/js/i18n/messages.json @@ -234,7 +234,10 @@ "access-add": "Add", "auth-add": "Add", "search": "Search Access…", - "client-certificates": "Client Certificates" + "client-certificates": "Client Certificates", + "clientca-add": "Add", + "clientca-del": "Del", + "clientca-count": "{count} {count, select, 1{Authority} other{Authorities}}" }, "users": { "title": "Users", diff --git a/frontend/js/models/access-list.js b/frontend/js/models/access-list.js index 0c2c4abea4..e24e19ab08 100644 --- a/frontend/js/models/access-list.js +++ b/frontend/js/models/access-list.js @@ -11,6 +11,7 @@ const model = Backbone.Model.extend({ name: '', items: [], clients: [], + clientcas: [], // The following are expansions: owner: null }; From fb766d14e9e48e0a7d4fd297a08e2911d4ae4bad Mon Sep 17 00:00:00 2001 From: Will Rouesnel Date: Sat, 27 May 2023 02:21:10 +1000 Subject: [PATCH 05/13] Add support for writing client CAs when access-lists are updated This commit adds the basic support necessary to produce the combined client CA files when certificates are updated. --- backend/internal/access-list.js | 82 +++++++++++++++++-- .../s6-overlay/s6-rc.d/prepare/20-paths.sh | 1 + frontend/js/app/nginx/access/main.js | 4 +- 3 files changed, 78 insertions(+), 9 deletions(-) diff --git a/backend/internal/access-list.js b/backend/internal/access-list.js index de71d16582..74cbbeef84 100644 --- a/backend/internal/access-list.js +++ b/backend/internal/access-list.js @@ -86,7 +86,7 @@ const internalAccessList = { // re-fetch with expansions return internalAccessList.get(access, { id: data.id, - expand: ['owner', 'items', 'clients', 'clientcas', 'proxy_hosts.access_list.[clientcas.certificate,clients,items]'] + expand: ['owner', 'items', 'clients', 'clientcas.certificate', 'proxy_hosts.access_list.[clientcas,clients,items]'] }, true /* <- skip masking */); }) .then((row) => { @@ -247,7 +247,6 @@ const internalAccessList = { }); } }) - .then(internalNginx.reload) .then(() => { // Add to audit log return internalAuditLog.add(access, { @@ -261,10 +260,11 @@ const internalAccessList = { // re-fetch with expansions return internalAccessList.get(access, { id: data.id, - expand: ['owner', 'items', 'clients', 'clientcas', 'proxy_hosts.[certificate,access_list.[clientcas.certificate,clients,items]]'] + expand: ['owner', 'items', 'clients', 'clientcas.certificate', 'proxy_hosts.[certificate,access_list.[clientcas,clients,items]]'] }, true /* <- skip masking */); }) .then((row) => { + console.log(row); return internalAccessList.build(row) .then(() => { if (row.proxy_host_count) { @@ -274,6 +274,11 @@ const internalAccessList = { .then(() => { return internalAccessList.maskItems(row); }); + }) + .then((row) => { + return internalNginx.reload().then(() => { + return row; + }); }); }, @@ -299,7 +304,7 @@ const internalAccessList = { .joinRaw('LEFT JOIN `proxy_host` ON `proxy_host`.`access_list_id` = `access_list`.`id` AND `proxy_host`.`is_deleted` = 0') .where('access_list.is_deleted', 0) .andWhere('access_list.id', data.id) - .withGraphFetched('[owner,items,clients,clientcas,proxy_hosts.[certificate,access_list.[clientcas.certificate,clients,items]]]') + .allowGraph('[owner,items,clients,clientcas.certificate,proxy_hosts.[certificate,access_list.[clientcas,clients,items]]]') .first(); if (access_data.permission_visibility !== 'all') { @@ -420,7 +425,7 @@ const internalAccessList = { .joinRaw('LEFT JOIN `proxy_host` ON `proxy_host`.`access_list_id` = `access_list`.`id` AND `proxy_host`.`is_deleted` = 0') .where('access_list.is_deleted', 0) .groupBy('access_list.id') - .withGraphFetched('[owner,items,clients,clientcas.certificate]') + .allowGraph('[owner,items,clients,clientcas.certificate]') .orderBy('access_list.name', 'ASC'); if (access_data.permission_visibility !== 'all') { @@ -477,6 +482,8 @@ const internalAccessList = { }, /** + * Mask sensitive items in access list responses + * * @param {Object} list * @returns {Object} */ @@ -496,6 +503,24 @@ const internalAccessList = { }); } + // Mask certificates in clientcas responses + if (list && typeof list.clientcas !== 'undefined') { + list.clientcas.map(function(val, idx) { + if (typeof val.certificate !== 'undefined') { + list.clientcas[idx].certificate.meta = {}; + } + }); + } + + // Mask certificates in ProxyHost responses (clear the meta field) + if (list && typeof list.proxy_hosts !== 'undefined') { + list.proxy_hosts.map(function(val, idx) { + if (typeof val.certificate !== 'undefined') { + list.proxy_hosts[idx].certificate.meta = {}; + } + }); + } + return list; }, @@ -508,17 +533,27 @@ const internalAccessList = { return '/data/access/' + list.id; }, + /** + * @param {Object} list + * @param {Integer} list.id + * @returns {String} + */ + getClientCAFilename: (list) => { + return '/data/clientca/' + list.id; + }, + /** * @param {Object} list * @param {Integer} list.id * @param {String} list.name * @param {Array} list.items + * @param {Array} list.clientcas * @returns {Promise} */ build: (list) => { - logger.info('Building Access file #' + list.id + ' for: ' + list.name); - return new Promise((resolve, reject) => { + const htPasswdBuild = new Promise((resolve, reject) => { + logger.info('Building Access file #' + list.id + ' for: ' + list.name); let htpasswd_file = internalAccessList.getFilename(list); // 1. remove any existing access file @@ -566,6 +601,39 @@ const internalAccessList = { }); } }); + + const caCertificateBuild = new Promise((resolve, reject) => { + // TODO: we need to ensure this rebuild is run if any certificates change + logger.info('Building Client CA file #' + list.id + ' for: ' + list.name); + let clientca_file = internalAccessList.getClientCAFilename(list); + + const certificate_bodies = list.clientcas + .filter((clientca) => { + return typeof clientca.certificate.meta !== 'undefined'; + }) + .map((clientca) => { + return clientca.certificate.meta.certificate; + }); + + // Unlink the original file (nginx retains file handle till reload) + try { + fs.unlinkSync(clientca_file); + } catch (err) { + // do nothing + } + + // Write the new file in one shot + try { + fs.writeFileSync(clientca_file, certificate_bodies.join('\n'), {encoding: 'utf8'}); + logger.success('Built Client CA file #' + list.id + ' for: ' + list.name); + resolve(clientca_file); + } catch (err) { + reject(err); + } + }); + + // Execute both promises concurrently + return Promise.all([htPasswdBuild, caCertificateBuild]); } }; diff --git a/docker/rootfs/etc/s6-overlay/s6-rc.d/prepare/20-paths.sh b/docker/rootfs/etc/s6-overlay/s6-rc.d/prepare/20-paths.sh index 2f59ef41ac..ec0ca7349c 100755 --- a/docker/rootfs/etc/s6-overlay/s6-rc.d/prepare/20-paths.sh +++ b/docker/rootfs/etc/s6-overlay/s6-rc.d/prepare/20-paths.sh @@ -20,6 +20,7 @@ mkdir -p \ /data/custom_ssl \ /data/logs \ /data/access \ + /data/clientca \ /data/nginx/default_host \ /data/nginx/default_www \ /data/nginx/proxy_host \ diff --git a/frontend/js/app/nginx/access/main.js b/frontend/js/app/nginx/access/main.js index 94f99de0ea..79f774c74b 100644 --- a/frontend/js/app/nginx/access/main.js +++ b/frontend/js/app/nginx/access/main.js @@ -73,7 +73,7 @@ module.exports = Mn.View.extend({ e.preventDefault(); let query = this.ui.query.val(); - this.fetch(['owner', 'items', 'clients', 'clientcas'], query) + this.fetch(['owner', 'items', 'clients', 'clientcas.certificate'], query) .then(response => this.showData(response)) .catch(err => { this.showError(err); @@ -88,7 +88,7 @@ module.exports = Mn.View.extend({ onRender: function () { let view = this; - view.fetch(['owner', 'items', 'clients', 'clientcas']) + view.fetch(['owner', 'items', 'clients', 'clientcas.certificate']) .then(response => { if (!view.isDestroyed()) { if (response && response.length) { From 366efc8ac2d446708a24ba61612c5a4849bfd578 Mon Sep 17 00:00:00 2001 From: Will Rouesnel Date: Mon, 29 May 2023 02:29:42 +1000 Subject: [PATCH 06/13] Add template support for all host types to do client CA authorization When an access list contains client CAs, the combined CA auth file is added to all location blocks via an `if` statement. This allows LetsEncrypt and other support paths to work, while correctly denying access to the protected resources. --- backend/internal/access-list.js | 2 -- backend/internal/proxy-host.js | 8 ++++---- backend/templates/_access.conf | 6 ++++++ backend/templates/_certificates.conf | 8 +++++++- 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/backend/internal/access-list.js b/backend/internal/access-list.js index 74cbbeef84..8869e6a84a 100644 --- a/backend/internal/access-list.js +++ b/backend/internal/access-list.js @@ -264,7 +264,6 @@ const internalAccessList = { }, true /* <- skip masking */); }) .then((row) => { - console.log(row); return internalAccessList.build(row) .then(() => { if (row.proxy_host_count) { @@ -603,7 +602,6 @@ const internalAccessList = { }); const caCertificateBuild = new Promise((resolve, reject) => { - // TODO: we need to ensure this rebuild is run if any certificates change logger.info('Building Client CA file #' + list.id + ' for: ' + list.name); let clientca_file = internalAccessList.getClientCAFilename(list); diff --git a/backend/internal/proxy-host.js b/backend/internal/proxy-host.js index 02a98da266..284cc7088f 100644 --- a/backend/internal/proxy-host.js +++ b/backend/internal/proxy-host.js @@ -74,7 +74,7 @@ const internalProxyHost = { // re-fetch with cert return internalProxyHost.get(access, { id: row.id, - expand: ['certificate', 'owner', 'access_list.[clients,items]'] + expand: ['certificate', 'owner', 'access_list.[clientcas.certificate,clients,items]'] }); }) .then((row) => { @@ -188,7 +188,7 @@ const internalProxyHost = { .then(() => { return internalProxyHost.get(access, { id: data.id, - expand: ['owner', 'certificate', 'access_list.[clients,items]'] + expand: ['owner', 'certificate', 'access_list.[clientcas.certificate,clients,items]'] }) .then((row) => { if (!row.enabled) { @@ -225,7 +225,7 @@ const internalProxyHost = { .query() .where('is_deleted', 0) .andWhere('id', data.id) - .allowGraph('[owner,access_list,access_list.[clients,items],certificate]') + .allowGraph('[owner,access_list.[clientcas.certificate,clients,items],certificate]') .first(); if (access_data.permission_visibility !== 'all') { @@ -308,7 +308,7 @@ const internalProxyHost = { .then(() => { return internalProxyHost.get(access, { id: data.id, - expand: ['certificate', 'owner', 'access_list'] + expand: ['certificate', 'owner', 'access_list.[clientcas.certificate]'] }); }) .then((row) => { diff --git a/backend/templates/_access.conf b/backend/templates/_access.conf index 447006c0c2..cf1950ad64 100644 --- a/backend/templates/_access.conf +++ b/backend/templates/_access.conf @@ -1,4 +1,10 @@ {% if access_list_id > 0 %} +{% if access_list.clientcas.size > 0 %} + # TLS Client Certificate Authorization + if ($ssl_client_verify != "SUCCESS") { + return 403; + } +{% endif %} {% if access_list.items.length > 0 %} # Authorization auth_basic "Authorization required"; diff --git a/backend/templates/_certificates.conf b/backend/templates/_certificates.conf index 06ca7bb87e..18f0b10c5c 100644 --- a/backend/templates/_certificates.conf +++ b/backend/templates/_certificates.conf @@ -11,4 +11,10 @@ ssl_certificate_key /data/custom_ssl/npm-{{ certificate_id }}/privkey.pem; {% endif %} {% endif %} - +{% if access_list_id > 0 -%} +{% if access_list.clientcas.size > 0 %} + # Client Certificate Authorization ({{access_list.clientcas.size}} CAs) + ssl_client_certificate /data/clientca/{{ access_list_id }}; + ssl_verify_client optional; +{% endif %} +{% endif %} \ No newline at end of file From 34305e04e1b08baae5dd366ad4b7881c0c9c9058 Mon Sep 17 00:00:00 2001 From: Will Rouesnel Date: Mon, 29 May 2023 12:14:30 +1000 Subject: [PATCH 07/13] Add authority count to access-list drop down in proxy host --- frontend/js/app/nginx/proxy/access-list-item.ejs | 2 +- frontend/js/i18n/messages.json | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frontend/js/app/nginx/proxy/access-list-item.ejs b/frontend/js/app/nginx/proxy/access-list-item.ejs index e5a7e1163a..f92938e9f1 100644 --- a/frontend/js/app/nginx/proxy/access-list-item.ejs +++ b/frontend/js/app/nginx/proxy/access-list-item.ejs @@ -3,7 +3,7 @@
<%- name %>
- <%- i18n('access-lists', 'item-count', {count: items.length || 0}) %>, <%- i18n('access-lists', 'client-count', {count: clients.length || 0}) %> – Created: <%- formatDbDate(created_on, 'Do MMMM YYYY, h:mm a') %> + <%- i18n('access-lists', 'item-count', {count: items.length || 0}) %>, <%- i18n('access-lists', 'client-count', {count: clients.length || 0}) %>, <%- i18n('access-lists', 'clientca-count', {count: clientcas.length || 0}) %> – Created: <%- formatDbDate(created_on, 'Do MMMM YYYY, h:mm a') %> <% } else { %>
<%- i18n('access-lists', 'public') %> diff --git a/frontend/js/i18n/messages.json b/frontend/js/i18n/messages.json index 619429db79..f746387411 100644 --- a/frontend/js/i18n/messages.json +++ b/frontend/js/i18n/messages.json @@ -223,6 +223,7 @@ "help-content": "Access Lists provide a blacklist or whitelist of specific client IP addresses along with authentication for the Proxy Hosts via Basic HTTP Authentication.\nYou can configure multiple client rules, usernames and passwords for a single Access List and then apply that to a Proxy Host.\nThis is most useful for forwarded web services that do not have authentication mechanisms built in or that you want to protect from access by unknown clients.", "item-count": "{count} {count, select, 1{User} other{Users}}", "client-count": "{count} {count, select, 1{Rule} other{Rules}}", + "clientca-count": "{count} {count, select, 1{Authority} other{Authorities}}", "proxy-host-count": "{count} {count, select, 1{Proxy Host} other{Proxy Hosts}}", "delete-has-hosts": "This Access List is associated with {count} Proxy Hosts. They will become publicly available upon deletion.", "details": "Details", @@ -236,8 +237,7 @@ "search": "Search Access…", "client-certificates": "Client Certificates", "clientca-add": "Add", - "clientca-del": "Del", - "clientca-count": "{count} {count, select, 1{Authority} other{Authorities}}" + "clientca-del": "Del" }, "users": { "title": "Users", From f601105776b5adf333c3ffb4913fcb37e2f8eacc Mon Sep 17 00:00:00 2001 From: Will Rouesnel Date: Mon, 29 May 2023 14:41:23 +1000 Subject: [PATCH 08/13] Add a development docker-compose file for use with User Namespaces --- docker/docker-compose.dev-user.yml | 70 ++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 docker/docker-compose.dev-user.yml diff --git a/docker/docker-compose.dev-user.yml b/docker/docker-compose.dev-user.yml new file mode 100644 index 0000000000..661805cf6c --- /dev/null +++ b/docker/docker-compose.dev-user.yml @@ -0,0 +1,70 @@ +# WARNING: This is a DEVELOPMENT docker-compose file, it should not be used for production. +# Important: this version is designed to work with user-namespaces, which allows running +# under podman. +version: '3.8' +services: + + npm: + image: nginxproxymanager:dev + container_name: npm_core + build: + context: ./ + dockerfile: ./dev/Dockerfile + ports: + - 3080:80 + - 3081:81 + - 3443:443 + networks: + - nginx_proxy_manager + environment: +# PUID: 1000 +# PGID: 1000 + FORCE_COLOR: 1 + # specifically for dev: + DEBUG: 'true' + DEVELOPMENT: 'true' + LE_STAGING: 'true' + # db: + DB_MYSQL_HOST: 'db' + DB_MYSQL_PORT: '3306' + DB_MYSQL_USER: 'npm' + DB_MYSQL_PASSWORD: 'npm' + DB_MYSQL_NAME: 'npm' + # DB_SQLITE_FILE: "/data/database.sqlite" + # DISABLE_IPV6: "true" + volumes: + - npm_data:/data + - le_data:/etc/letsencrypt + - ../backend:/app + - ../frontend:/app/frontend + - ../global:/app/global + depends_on: + - db + working_dir: /app + + db: + image: jc21/mariadb-aria + container_name: npm_db + ports: + - 33306:3306 + networks: + - nginx_proxy_manager + environment: + MYSQL_ROOT_PASSWORD: 'npm' + MYSQL_DATABASE: 'npm' + MYSQL_USER: 'npm' + MYSQL_PASSWORD: 'npm' + volumes: + - db_data:/var/lib/mysql + +volumes: + npm_data: + name: npm_core_data + le_data: + name: npm_le_data + db_data: + name: npm_db_data + +networks: + nginx_proxy_manager: + name: npm_network From 6cf91a2e708e9c7b4f519c56e8570226549a9a3a Mon Sep 17 00:00:00 2001 From: Will Rouesnel Date: Mon, 29 May 2023 14:43:11 +1000 Subject: [PATCH 09/13] Add drop_unauthorized parameter to proxy hosts drop_unauthorized returns 444 when a client is not authorized as opposed to 403. It can be used with Client Certificate authorization. --- backend/doc/api.swagger.json | 7 ++++ backend/internal/nginx.js | 8 +++- ...411_add_drop_unauthorized_to_proxyhosts.js | 39 +++++++++++++++++++ backend/schema/definitions.json | 5 +++ backend/schema/endpoints/proxy-hosts.json | 12 ++++++ backend/templates/_access.conf | 2 +- frontend/js/app/nginx/proxy/form.ejs | 12 +++++- frontend/js/app/nginx/proxy/form.js | 1 + frontend/js/i18n/messages.json | 1 + frontend/js/models/proxy-host.js | 1 + test/cypress/integration/api/Hosts.spec.js | 1 + 11 files changed, 85 insertions(+), 4 deletions(-) create mode 100644 backend/migrations/20230529030411_add_drop_unauthorized_to_proxyhosts.js diff --git a/backend/doc/api.swagger.json b/backend/doc/api.swagger.json index 3fa19fc4b3..e389b1c9a3 100644 --- a/backend/doc/api.swagger.json +++ b/backend/doc/api.swagger.json @@ -82,6 +82,7 @@ "ssl_forced": 0, "caching_enabled": 0, "block_exploits": 0, + "drop_unauthorized": 0, "advanced_config": "sdfsdfsdf", "meta": { "letsencrypt_agree": false, @@ -124,6 +125,7 @@ "ssl_forced": 0, "caching_enabled": 0, "block_exploits": 0, + "drop_unauthorized": 0, "advanced_config": "", "meta": { "letsencrypt_agree": false, @@ -204,6 +206,7 @@ "ssl_forced": 0, "caching_enabled": 0, "block_exploits": 0, + "drop_unauthorized": 0, "advanced_config": "", "meta": { "letsencrypt_agree": false, @@ -1117,6 +1120,7 @@ "ssl_forced", "caching_enabled", "block_exploits", + "drop_unauthorized", "advanced_config", "meta", "allow_websocket_upgrade", @@ -1184,6 +1188,9 @@ "block_exploits": { "type": "integer" }, + "drop_unauthorized": { + "type": "integer" + }, "advanced_config": { "type": "string" }, diff --git a/backend/internal/nginx.js b/backend/internal/nginx.js index 77933e733b..2df4beabc7 100644 --- a/backend/internal/nginx.js +++ b/backend/internal/nginx.js @@ -153,7 +153,7 @@ const internalNginx = { const locationRendering = async () => { for (let i = 0; i < host.locations.length; i++) { let locationCopy = Object.assign({}, {access_list_id: host.access_list_id}, {certificate_id: host.certificate_id}, - {ssl_forced: host.ssl_forced}, {caching_enabled: host.caching_enabled}, {block_exploits: host.block_exploits}, + {ssl_forced: host.ssl_forced}, {caching_enabled: host.caching_enabled}, {block_exploits: host.block_exploits}, {drop_unauthorized: host.drop_unauthorized}, {allow_websocket_upgrade: host.allow_websocket_upgrade}, {http2_support: host.http2_support}, {hsts_enabled: host.hsts_enabled}, {hsts_subdomains: host.hsts_subdomains}, {access_list: host.access_list}, {certificate: host.certificate}, host.locations[i]); @@ -205,6 +205,12 @@ const internalNginx = { let origLocations; // Manipulate the data a bit before sending it to the template + if (typeof host.drop_unauthorized === 'undefined') { + // Only proxy-hosts can have drop_unauthorized, but all hosts share + // the templates. + host.drop_unauthorized = 0; + } + if (nice_host_type !== 'default') { host.use_default_location = true; if (typeof host.advanced_config !== 'undefined' && host.advanced_config) { diff --git a/backend/migrations/20230529030411_add_drop_unauthorized_to_proxyhosts.js b/backend/migrations/20230529030411_add_drop_unauthorized_to_proxyhosts.js new file mode 100644 index 0000000000..411e1a6f39 --- /dev/null +++ b/backend/migrations/20230529030411_add_drop_unauthorized_to_proxyhosts.js @@ -0,0 +1,39 @@ +const migrate_name = 'drop_unauthorized'; +const logger = require('../logger').migrate; + +/** + * Migrate + * + * @see http://knexjs.org/#Schema + * + * @param {Object} knex + * @param {Promise} Promise + * @returns {Promise} + */ +exports.up = function (knex/*, Promise*/) { + + logger.info('[' + migrate_name + '] Migrating Up...'); + + return knex.schema.table('proxy_host', function(proxy_host) { + proxy_host.integer('drop_unauthorized').notNull().unsigned().defaultTo(0); + }).then(() =>{ + logger.info('[' + migrate_name + '] Migrating Up Complete'); + }); +}; + +/** + * Undo Migrate + * + * @param {Object} knex + * @param {Promise} Promise + * @returns {Promise} + */ +exports.down = function (knex/*, Promise*/) { + logger.info('[' + migrate_name + '] Migrating Down...'); + + return knex.schema.table('proxy_host', function(proxy_host) { + proxy_host.dropColumn('drop_unauthorized'); + }).then(() =>{ + logger.info('[' + migrate_name + '] Migrating Up Complete'); + }); +}; diff --git a/backend/schema/definitions.json b/backend/schema/definitions.json index 136b023518..61bbb6dd0d 100644 --- a/backend/schema/definitions.json +++ b/backend/schema/definitions.json @@ -231,6 +231,11 @@ "example": true, "type": "boolean" }, + "drop_unauthorized": { + "description": "Close TCP connection with no response when authorization fails", + "example": true, + "type": "boolean" + }, "caching_enabled": { "description": "Should we cache assets", "example": true, diff --git a/backend/schema/endpoints/proxy-hosts.json b/backend/schema/endpoints/proxy-hosts.json index 9a3fff2fc7..ec812f1ba4 100644 --- a/backend/schema/endpoints/proxy-hosts.json +++ b/backend/schema/endpoints/proxy-hosts.json @@ -50,6 +50,9 @@ "block_exploits": { "$ref": "../definitions.json#/definitions/block_exploits" }, + "drop_unauthorized": { + "$ref": "../definitions.json#/definitions/drop_unauthorized" + }, "caching_enabled": { "$ref": "../definitions.json#/definitions/caching_enabled" }, @@ -149,6 +152,9 @@ "block_exploits": { "$ref": "#/definitions/block_exploits" }, + "drop_unauthorized": { + "$ref": "#/definitions/drop_unauthorized" + }, "caching_enabled": { "$ref": "#/definitions/caching_enabled" }, @@ -239,6 +245,9 @@ "block_exploits": { "$ref": "#/definitions/block_exploits" }, + "drop_unauthorized": { + "$ref": "#/definitions/drop_unauthorized" + }, "caching_enabled": { "$ref": "#/definitions/caching_enabled" }, @@ -312,6 +321,9 @@ "block_exploits": { "$ref": "#/definitions/block_exploits" }, + "drop_unauthorized": { + "$ref": "#/definitions/drop_unauthorized" + }, "caching_enabled": { "$ref": "#/definitions/caching_enabled" }, diff --git a/backend/templates/_access.conf b/backend/templates/_access.conf index cf1950ad64..f19a822897 100644 --- a/backend/templates/_access.conf +++ b/backend/templates/_access.conf @@ -2,7 +2,7 @@ {% if access_list.clientcas.size > 0 %} # TLS Client Certificate Authorization if ($ssl_client_verify != "SUCCESS") { - return 403; + return {% if drop_unauthorized == 1 %}444{% else %}403{% endif %}; } {% endif %} {% if access_list.items.length > 0 %} diff --git a/frontend/js/app/nginx/proxy/form.ejs b/frontend/js/app/nginx/proxy/form.ejs index 56868f5528..a95e4d5f84 100644 --- a/frontend/js/app/nginx/proxy/form.ejs +++ b/frontend/js/app/nginx/proxy/form.ejs @@ -72,7 +72,7 @@
-
+
- +
+
+ +
+
diff --git a/frontend/js/app/nginx/proxy/form.js b/frontend/js/app/nginx/proxy/form.js index d2df532ee6..5c8f05362f 100644 --- a/frontend/js/app/nginx/proxy/form.js +++ b/frontend/js/app/nginx/proxy/form.js @@ -161,6 +161,7 @@ module.exports = Mn.View.extend({ // Manipulate data.forward_port = parseInt(data.forward_port, 10); data.block_exploits = !!data.block_exploits; + data.drop_unauthorized = !!data.drop_unauthorized; data.caching_enabled = !!data.caching_enabled; data.allow_websocket_upgrade = !!data.allow_websocket_upgrade; data.http2_support = !!data.http2_support; diff --git a/frontend/js/i18n/messages.json b/frontend/js/i18n/messages.json index f746387411..660083954e 100644 --- a/frontend/js/i18n/messages.json +++ b/frontend/js/i18n/messages.json @@ -75,6 +75,7 @@ "domain-names": "Domain Names", "cert-provider": "Certificate Provider", "block-exploits": "Block Common Exploits", + "drop-unauthorized": "Drop Unauthorized (444)", "caching-enabled": "Cache Assets", "ssl-certificate": "SSL Certificate", "none": "None", diff --git a/frontend/js/models/proxy-host.js b/frontend/js/models/proxy-host.js index b82d09fef3..b96de2b255 100644 --- a/frontend/js/models/proxy-host.js +++ b/frontend/js/models/proxy-host.js @@ -20,6 +20,7 @@ const model = Backbone.Model.extend({ caching_enabled: false, allow_websocket_upgrade: false, block_exploits: false, + drop_unauthorized: false, http2_support: false, advanced_config: '', enabled: true, diff --git a/test/cypress/integration/api/Hosts.spec.js b/test/cypress/integration/api/Hosts.spec.js index 4652c8e081..7c9a7bc2a9 100644 --- a/test/cypress/integration/api/Hosts.spec.js +++ b/test/cypress/integration/api/Hosts.spec.js @@ -27,6 +27,7 @@ describe('Hosts endpoints', () => { advanced_config: '', locations: [], block_exploits: false, + drop_unauthorized: false, caching_enabled: false, allow_websocket_upgrade: false, http2_support: false, From f3c740954b3f5c3e6adbd32b3106c0abd242fdc0 Mon Sep 17 00:00:00 2001 From: Will Rouesnel Date: Mon, 29 May 2023 16:40:07 +1000 Subject: [PATCH 10/13] Adapt CI command scripts to also support podman --- scripts/.common.sh | 11 +++++++++++ scripts/buildx | 8 ++++---- scripts/ci/frontend-build | 19 ++++++++++++------- scripts/ci/test-and-build | 17 ++++++++++++++--- scripts/docs-build | 2 +- scripts/start-dev | 4 ++-- scripts/wait-healthy | 2 +- 7 files changed, 45 insertions(+), 18 deletions(-) diff --git a/scripts/.common.sh b/scripts/.common.sh index 3cea091671..1b4866144f 100644 --- a/scripts/.common.sh +++ b/scripts/.common.sh @@ -10,6 +10,17 @@ YELLOW='\E[1;33m' export BLUE CYAN GREEN RED RESET YELLOW +# Identify docker-like command +# Ensure docker exists +if command -v docker 1>/dev/null 2>&1; then + export docker=docker +elif command -v podman 1>/dev/null 2>&1; then + export docker=podman +else + echo -e "${RED}❯ docker or podman command is not available${RESET}" + exit 1 +fi + # Docker Compose COMPOSE_PROJECT_NAME="npmdev" COMPOSE_FILE="docker/docker-compose.dev.yml" diff --git a/scripts/buildx b/scripts/buildx index 4da6c16740..55650e0e47 100755 --- a/scripts/buildx +++ b/scripts/buildx @@ -14,10 +14,10 @@ if [ "$BUILD_COMMIT" == "" ]; then fi # Buildx Builder -docker buildx create --name "${BUILDX_NAME:-npm}" || echo -docker buildx use "${BUILDX_NAME:-npm}" +$docker buildx create --name "${BUILDX_NAME:-npm}" || echo +$docker buildx use "${BUILDX_NAME:-npm}" -docker buildx build \ +$docker buildx build \ --build-arg BUILD_VERSION="${BUILD_VERSION:-dev}" \ --build-arg BUILD_COMMIT="${BUILD_COMMIT:-notset}" \ --build-arg BUILD_DATE="$(date '+%Y-%m-%d %T %Z')" \ @@ -31,6 +31,6 @@ docker buildx build \ . rc=$? -docker buildx rm "${BUILDX_NAME:-npm}" +$docker buildx rm "${BUILDX_NAME:-npm}" echo -e "${BLUE}❯ ${GREEN}Multiarch build Complete${RESET}" exit $rc diff --git a/scripts/ci/frontend-build b/scripts/ci/frontend-build index 2ce19a80b3..f611172ca7 100755 --- a/scripts/ci/frontend-build +++ b/scripts/ci/frontend-build @@ -6,12 +6,17 @@ DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" DOCKER_IMAGE=jc21/nginx-full:certbot-node # Ensure docker exists -if hash docker 2>/dev/null; then - docker pull "${DOCKER_IMAGE}" - cd "${DIR}/../.." - echo -e "${BLUE}❯ ${CYAN}Building Frontend ...${RESET}" - docker run --rm -e CI=true -v "$(pwd)/frontend:/app/frontend" -v "$(pwd)/global:/app/global" -w /app/frontend "$DOCKER_IMAGE" sh -c "yarn install && yarn build && yarn build && chown -R $(id -u):$(id -g) /app/frontend" - echo -e "${BLUE}❯ ${GREEN}Building Frontend Complete${RESET}" +if command -v docker 1>/dev/null 2>&1; then + docker=docker +elif command -v podman 1>/dev/null 2>&1; then + docker=podman else - echo -e "${RED}❯ docker command is not available${RESET}" + echo -e "${RED}❯ docker or podman command is not available${RESET}" + exit 1 fi + +$docker pull "${DOCKER_IMAGE}" +cd "${DIR}/../.." +echo -e "${BLUE}❯ ${CYAN}Building Frontend ...${RESET}" +$docker run --rm -e CI=true -v "$(pwd)/frontend:/app/frontend" -v "$(pwd)/global:/app/global" -w /app/frontend "$DOCKER_IMAGE" sh -c "yarn install && yarn build && yarn build && chown -R $(id -u):$(id -g) /app/frontend" +echo -e "${BLUE}❯ ${GREEN}Building Frontend Complete${RESET}" diff --git a/scripts/ci/test-and-build b/scripts/ci/test-and-build index 0bcf70a881..f3caf856c7 100755 --- a/scripts/ci/test-and-build +++ b/scripts/ci/test-and-build @@ -1,10 +1,21 @@ #!/bin/bash -e DOCKER_IMAGE=jc21/nginx-full:certbot-node -docker pull "${DOCKER_IMAGE}" + +# Ensure docker exists +if command -v docker 1>/dev/null 2>&1; then + docker=docker +elif command -v podman 1>/dev/null 2>&1; then + docker=podman +else + echo -e "${RED}❯ docker or podman command is not available${RESET}" + exit 1 +fi + +$docker pull "${DOCKER_IMAGE}" # Test -docker run --rm \ +$docker run --rm \ -v "$(pwd)/backend:/app" \ -v "$(pwd)/global:/app/global" \ -w /app \ @@ -12,7 +23,7 @@ docker run --rm \ sh -c 'yarn install && yarn eslint . && rm -rf node_modules' # Build -docker build --pull --no-cache --squash --compress \ +$docker build --pull --no-cache --squash --compress \ -t "${IMAGE}:ci-${BUILD_NUMBER}" \ -f docker/Dockerfile \ --build-arg TARGETPLATFORM=linux/amd64 \ diff --git a/scripts/docs-build b/scripts/docs-build index 990313912e..7b38157c8e 100755 --- a/scripts/docs-build +++ b/scripts/docs-build @@ -7,7 +7,7 @@ DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" if hash docker 2>/dev/null; then cd "${DIR}/.." echo -e "${BLUE}❯ ${CYAN}Building Docs ...${RESET}" - docker run --rm -e CI=true -v "$(pwd)/docs:/app/docs" -w /app/docs node:alpine sh -c "yarn install && yarn build && chown -R $(id -u):$(id -g) /app/docs" + $docker run --rm -e CI=true -v "$(pwd)/docs:/app/docs" -w /app/docs node:alpine sh -c "yarn install && yarn build && chown -R $(id -u):$(id -g) /app/docs" echo -e "${BLUE}❯ ${GREEN}Building Docs Complete${RESET}" else echo -e "${RED}❯ docker command is not available${RESET}" diff --git a/scripts/start-dev b/scripts/start-dev index f064a4bdfe..c79ef67177 100755 --- a/scripts/start-dev +++ b/scripts/start-dev @@ -18,10 +18,10 @@ if hash docker-compose 2>/dev/null; then if [ "$1" == "-f" ]; then echo -e "${BLUE}❯ ${YELLOW}Following Backend Container:${RESET}" - docker logs -f npm_core + $docker logs -f npm_core else echo -e "${YELLOW}Hint:${RESET} You can follow the output of some of the containers with:" - echo " docker logs -f npm_core" + echo " $docker logs -f npm_core" fi else echo -e "${RED}❯ docker-compose command is not available${RESET}" diff --git a/scripts/wait-healthy b/scripts/wait-healthy index b8da5d69be..c686decbc3 100755 --- a/scripts/wait-healthy +++ b/scripts/wait-healthy @@ -19,7 +19,7 @@ echo -e "${BLUE}❯ ${CYAN}Waiting for healthy: ${YELLOW}${SERVICE}${RESET}" until [ "${HEALTHY}" = "healthy" ]; do echo -n "." sleep 1 - HEALTHY="$(docker inspect -f '{{.State.Health.Status}}' $SERVICE)" + HEALTHY="$($docker inspect -f '{{.State.Health.Status}}' $SERVICE)" ((LOOPCOUNT++)) if [ "$LOOPCOUNT" == "$LIMIT" ]; then From 4d491b2d767c07481bb58b92cf421717e3ea9451 Mon Sep 17 00:00:00 2001 From: Will Rouesnel Date: Wed, 31 May 2023 01:37:07 +1000 Subject: [PATCH 11/13] Fully support client CAs with access-lists This commit changes access-list IP directives to be implemented using the nginx "geo" directive. This allows IP-based blocks to return 444 (drop connection) on authorization failure when the "Drop Unauthorized" is enabled. It also allows the implementation of "Satisfy Any" with the new client CA certificate support - i.e. Satisfy Any can allow clients from the local network to skip client certificate challenge, or drop down to requesting basic authentication. It should be noted that including basic authentication requirements in Satisfy Any mode does prevent a 444 response from being sent, as the basic auth challenge requires the server to respond. --- backend/internal/access-list.js | 70 ++++++++++++++++++- backend/templates/_access.conf | 48 +++++++------ backend/templates/access.conf | 16 +++++ docker/rootfs/etc/nginx/nginx.conf | 1 + .../s6-overlay/s6-rc.d/prepare/20-paths.sh | 1 + frontend/js/app/nginx/access/form.ejs | 2 +- 6 files changed, 115 insertions(+), 23 deletions(-) create mode 100644 backend/templates/access.conf diff --git a/backend/internal/access-list.js b/backend/internal/access-list.js index 8869e6a84a..d0a67b82a6 100644 --- a/backend/internal/access-list.js +++ b/backend/internal/access-list.js @@ -11,6 +11,7 @@ const accessListClientCAsModel = require('../models/access_list_clientcas'); const proxyHostModel = require('../models/proxy_host'); const internalAuditLog = require('./audit-log'); const internalNginx = require('./nginx'); +const config = require('../lib/config'); function omissions () { return ['is_deleted']; @@ -392,6 +393,26 @@ const internalAccessList = { // do nothing } }) + .then(() => { + // delete the client CA file + let clientca_file = internalAccessList.getClientCAFilename(row); + + try { + fs.unlinkSync(clientca_file); + } catch (err) { + // do nothing + } + }) + .then(() => { + // delete the client geo file file + let client_file = internalAccessList.getClientFilename(row); + + try { + fs.unlinkSync(client_file); + } catch (err) { + // do nothing + } + }) .then(() => { // 4. audit log return internalAuditLog.add(access, { @@ -541,6 +562,15 @@ const internalAccessList = { return '/data/clientca/' + list.id; }, + /** + * @param {Object} list + * @param {Integer} list.id + * @returns {String} + */ + getClientFilename: (list) => { + return '/data/nginx/client/' + list.id + '.conf'; + }, + /** * @param {Object} list * @param {Integer} list.id @@ -550,6 +580,7 @@ const internalAccessList = { * @returns {Promise} */ build: (list) => { + const renderEngine = utils.getRenderEngine(); const htPasswdBuild = new Promise((resolve, reject) => { logger.info('Building Access file #' + list.id + ' for: ' + list.name); @@ -630,8 +661,45 @@ const internalAccessList = { } }); + const clientBuild = new Promise((resolve, reject) => { + logger.info('Building Access client file #' + list.id + ' for: ' + list.name); + + let template = null; + const client_file = internalAccessList.getClientFilename(list); + const data = { + access_list: list + }; + + try { + template = fs.readFileSync(__dirname + '/../templates/access.conf', {encoding: 'utf8'}); + } catch (err) { + reject(new error.ConfigurationError(err.message)); + return; + } + + return renderEngine + .parseAndRender(template, data) + .then((config_text) => { + fs.writeFileSync(client_file, config_text, {encoding: 'utf8'}); + + if (config.debug()) { + logger.success('Wrote config:', client_file, config_text); + } + + resolve(true); + }) + .catch((err) => { + if (config.debug()) { + logger.warn('Could not write ' + client_file + ':', err.message); + } + + reject(new error.ConfigurationError(err.message)); + }); + + }); + // Execute both promises concurrently - return Promise.all([htPasswdBuild, caCertificateBuild]); + return Promise.all([htPasswdBuild, caCertificateBuild, clientBuild]); } }; diff --git a/backend/templates/_access.conf b/backend/templates/_access.conf index f19a822897..c06d8a6f41 100644 --- a/backend/templates/_access.conf +++ b/backend/templates/_access.conf @@ -1,31 +1,37 @@ {% if access_list_id > 0 %} -{% if access_list.clientcas.size > 0 %} - # TLS Client Certificate Authorization - if ($ssl_client_verify != "SUCCESS") { + set $auth_basic "Authorization required"; + {% if access_list.satisfy_any == 1 %} + # Satisfy Any - any check can succeed - so look for success + if ( $access_list_{{ access_list_id }} = 1) { + set $auth_basic off; + } + if ( $ssl_client_verify = "SUCCESS" ) { + set $auth_basic off; + } + {% else %} + # Satisfy All - all checks must succeed (so handle fails) + if ( $access_list_{{ access_list_id }} = 0) { return {% if drop_unauthorized == 1 %}444{% else %}403{% endif %}; } -{% endif %} + if ( $ssl_client_verify != "SUCCESS" ) { + return {% if drop_unauthorized == 1 %}444{% else %}403{% endif %}; + } + {% endif %} + {% if access_list.items.length > 0 %} + # Basic Auth is enabled # Authorization - auth_basic "Authorization required"; + auth_basic $auth_basic; auth_basic_user_file /data/access/{{ access_list_id }}; - - {% if access_list.pass_auth == 0 %} + {% if access_list.pass_auth == 0 %} proxy_set_header Authorization ""; - {% endif %} - - {% endif %} - - # Access Rules: {{ access_list.clients | size }} total - {% for client in access_list.clients %} - {{client | nginxAccessRule}} - {% endfor %} - deny all; - - # Access checks must... - {% if access_list.satisfy_any == 1 %} - satisfy any; + {% endif %} {% else %} - satisfy all; + {% if access_list.satisfy_any == 1 %} + # Satisfy Any without Basic Auth + if ( $auth_basic != "off" ) { + return {% if drop_unauthorized == 1 %}444{% else %}403{% endif %}; + } + {% endif %} {% endif %} {% endif %} diff --git a/backend/templates/access.conf b/backend/templates/access.conf new file mode 100644 index 0000000000..90121fb403 --- /dev/null +++ b/backend/templates/access.conf @@ -0,0 +1,16 @@ +# Access List Clients for {{ access_list.id }} - {{ access_list.name }} +geo $realip_remote_addr $access_list_{{ access_list.id }} { +{% if access_list.client.size == 0 %} + default 1; +{% else %} + default 0; +{% endif %} +{% for client in access_list.clients %} +{% if client.directive == "allow" %} + {{client.address}} 1; +{% endif %} +{% if client.directive == "deny" %} + {{client.address}} 0; +{% endif %} +{% endfor %} +} diff --git a/docker/rootfs/etc/nginx/nginx.conf b/docker/rootfs/etc/nginx/nginx.conf index 8261833787..e099f721aa 100644 --- a/docker/rootfs/etc/nginx/nginx.conf +++ b/docker/rootfs/etc/nginx/nginx.conf @@ -73,6 +73,7 @@ http { # Files generated by NPM include /etc/nginx/conf.d/*.conf; + include /data/nginx/client/*.conf; include /data/nginx/default_host/*.conf; include /data/nginx/proxy_host/*.conf; include /data/nginx/redirection_host/*.conf; diff --git a/docker/rootfs/etc/s6-overlay/s6-rc.d/prepare/20-paths.sh b/docker/rootfs/etc/s6-overlay/s6-rc.d/prepare/20-paths.sh index ec0ca7349c..5e1b8f95d7 100755 --- a/docker/rootfs/etc/s6-overlay/s6-rc.d/prepare/20-paths.sh +++ b/docker/rootfs/etc/s6-overlay/s6-rc.d/prepare/20-paths.sh @@ -21,6 +21,7 @@ mkdir -p \ /data/logs \ /data/access \ /data/clientca \ + /data/nginx/client \ /data/nginx/default_host \ /data/nginx/default_www \ /data/nginx/proxy_host \ diff --git a/frontend/js/app/nginx/access/form.ejs b/frontend/js/app/nginx/access/form.ejs index a15ef7b465..d985d512e1 100644 --- a/frontend/js/app/nginx/access/form.ejs +++ b/frontend/js/app/nginx/access/form.ejs @@ -121,7 +121,7 @@
-
Note that the allow and deny directives will be applied in the order they are defined.
+
Note that the most specific directive is what will be applied to the connection. Order does not matter.
From 0969cd76be7a8dfa83c189e268dfa1cab0adc5d6 Mon Sep 17 00:00:00 2001 From: Will Rouesnel Date: Wed, 31 May 2023 23:57:49 +1000 Subject: [PATCH 12/13] Augment parseX509Output to also work with libressl LibreSSL uses a different output separated and semantics, which broke the X509 parser. With some slight modifications both can be supported. --- backend/internal/certificate.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/backend/internal/certificate.js b/backend/internal/certificate.js index 723f94b202..0b45eab107 100644 --- a/backend/internal/certificate.js +++ b/backend/internal/certificate.js @@ -741,10 +741,11 @@ const internalCertificate = { */ parseX509Output: (line, prefix) => { // Remove the subject= part - const subject_value = line.slice(prefix.length); + const subject_value = line.slice(prefix.length).trim(); - const subject = subject_value.split(/,(?=(?:(?:[^"]*"){2})*[^"]*$)/) - .map( (e) => { return e.trim().split(' = ', 2); }) + const subject = subject_value.split(/[,/](?=(?:(?:[^"]*"){2})*[^"]*$)/) + .filter( (e) => { return e.length > 0; } ) + .map( (e) => { return e.trim().split('=', 2).map( (p) => { return p.trim(); }); }) .reduce((obj, [key, value]) => { obj[key] = value.replace(/^"/, '').replace(/"$/, ''); return obj; From aca8206c306f4add234662aee4ddcc79f1f52282 Mon Sep 17 00:00:00 2001 From: Will Rouesnel Date: Thu, 31 Aug 2023 15:36:53 +1000 Subject: [PATCH 13/13] Fix IP access list control regression IP access list control was implemented as default success for an empty access control list - but this had the effect of an empty list default allowing if "Satisfy Any" was set. Fortunately this was bugged, so empty lists default failed - but this broke empty lists for "Satisfy All". This patch is the correct fix: lists now always default fail, but an empty list removes the check from access control considerations. This restores the original implementations behavior and fixes the bug. --- backend/templates/_access.conf | 11 +++++++++-- backend/templates/access.conf | 4 ---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/backend/templates/_access.conf b/backend/templates/_access.conf index c06d8a6f41..583322b312 100644 --- a/backend/templates/_access.conf +++ b/backend/templates/_access.conf @@ -2,17 +2,24 @@ set $auth_basic "Authorization required"; {% if access_list.satisfy_any == 1 %} # Satisfy Any - any check can succeed - so look for success + {% if access_list.clients.size != 0 %} if ( $access_list_{{ access_list_id }} = 1) { - set $auth_basic off; + set $auth_basic off; } + {% endif %} if ( $ssl_client_verify = "SUCCESS" ) { - set $auth_basic off; + set $auth_basic off; } {% else %} # Satisfy All - all checks must succeed (so handle fails) + {% if access_list.clients.size != 0 %} + # {{ access_list.clients.size }} IP rules if ( $access_list_{{ access_list_id }} = 0) { return {% if drop_unauthorized == 1 %}444{% else %}403{% endif %}; } + {% else %} + # Empty IP rules list so no client IP check + {% endif %} if ( $ssl_client_verify != "SUCCESS" ) { return {% if drop_unauthorized == 1 %}444{% else %}403{% endif %}; } diff --git a/backend/templates/access.conf b/backend/templates/access.conf index 90121fb403..7d2d663d89 100644 --- a/backend/templates/access.conf +++ b/backend/templates/access.conf @@ -1,10 +1,6 @@ # Access List Clients for {{ access_list.id }} - {{ access_list.name }} geo $realip_remote_addr $access_list_{{ access_list.id }} { -{% if access_list.client.size == 0 %} - default 1; -{% else %} default 0; -{% endif %} {% for client in access_list.clients %} {% if client.directive == "allow" %} {{client.address}} 1;