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 c93e2578..88f267d6 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 88f267d6..723f94b2 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 4b4f3405..136b0235 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 7fc12785..6b87261d 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 a56c3f8e..eb6fb708 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 dbd6fa85..5d49c47a 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 4f12f72b..148d3261 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 6e33a6dc..10fe7213 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 8f6774f6..52f54c10 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 1dfb5c18..eb786251 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 1f81feeb..4e292e53 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 84577927..de71d165 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 00000000..e8c5a7f4 --- /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 fbf9bda7..a6f8d899 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 00000000..3be537a6 --- /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 404e3237..6ad77fd2 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 79220b14..a15ef7b4 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 bb075548..3b23d61e 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 00000000..41b980fe --- /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 00000000..acf04b64 --- /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 2ee37a50..73bd4eb2 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 7988e0c2..f85dc95a 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 513f5865..94f99de0 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 eb786251..d2df532e 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 148d3261..619429db 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 0c2c4abe..e24e19ab 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 de71d165..74cbbeef 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 2f59ef41..ec0ca734 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 94f99de0..79f774c7 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 74cbbeef..8869e6a8 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 02a98da2..284cc708 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 447006c0..cf1950ad 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 06ca7bb8..18f0b10c 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 e5a7e116..f92938e9 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 619429db..f7463874 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 00000000..661805cf --- /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 3fa19fc4..e389b1c9 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 77933e73..2df4beab 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 00000000..411e1a6f --- /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 136b0235..61bbb6dd 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 9a3fff2f..ec812f1b 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 cf1950ad..f19a8228 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 56868f55..a95e4d5f 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 d2df532e..5c8f0536 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 f7463874..66008395 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 b82d09fe..b96de2b2 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 4652c8e0..7c9a7bc2 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 3cea0916..1b486614 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 4da6c167..55650e0e 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 2ce19a80..f611172c 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 0bcf70a8..f3caf856 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 99031391..7b38157c 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 f064a4bd..c79ef671 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 b8da5d69..c686decb 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 8869e6a8..d0a67b82 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 f19a8228..c06d8a6f 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 00000000..90121fb4 --- /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 82618337..e099f721 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 ec0ca734..5e1b8f95 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 a15ef7b4..d985d512 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 723f94b2..0b45eab1 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 c06d8a6f..583322b3 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 90121fb4..7d2d663d 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;