From 4d491b2d767c07481bb58b92cf421717e3ea9451 Mon Sep 17 00:00:00 2001 From: Will Rouesnel Date: Wed, 31 May 2023 01:37:07 +1000 Subject: [PATCH] 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.