Skip to content

Commit 2759147

Browse files
committed
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.
1 parent f3c7409 commit 2759147

File tree

6 files changed

+115
-23
lines changed

6 files changed

+115
-23
lines changed

backend/internal/access-list.js

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const accessListClientCAsModel = require('../models/access_list_clientcas');
1111
const proxyHostModel = require('../models/proxy_host');
1212
const internalAuditLog = require('./audit-log');
1313
const internalNginx = require('./nginx');
14+
const config = require("../lib/config");
1415

1516
function omissions () {
1617
return ['is_deleted'];
@@ -392,6 +393,26 @@ const internalAccessList = {
392393
// do nothing
393394
}
394395
})
396+
.then(() => {
397+
// delete the client CA file
398+
let clientca_file = internalAccessList.getClientCAFilename(row);
399+
400+
try {
401+
fs.unlinkSync(clientca_file);
402+
} catch (err) {
403+
// do nothing
404+
}
405+
})
406+
.then(() => {
407+
// delete the client geo file file
408+
let client_file = internalAccessList.getClientFilename(row);
409+
410+
try {
411+
fs.unlinkSync(client_file);
412+
} catch (err) {
413+
// do nothing
414+
}
415+
})
395416
.then(() => {
396417
// 4. audit log
397418
return internalAuditLog.add(access, {
@@ -541,6 +562,15 @@ const internalAccessList = {
541562
return '/data/clientca/' + list.id;
542563
},
543564

565+
/**
566+
* @param {Object} list
567+
* @param {Integer} list.id
568+
* @returns {String}
569+
*/
570+
getClientFilename: (list) => {
571+
return '/data/nginx/client/' + list.id + '.conf';
572+
},
573+
544574
/**
545575
* @param {Object} list
546576
* @param {Integer} list.id
@@ -550,6 +580,7 @@ const internalAccessList = {
550580
* @returns {Promise}
551581
*/
552582
build: (list) => {
583+
const renderEngine = utils.getRenderEngine();
553584

554585
const htPasswdBuild = new Promise((resolve, reject) => {
555586
logger.info('Building Access file #' + list.id + ' for: ' + list.name);
@@ -630,8 +661,45 @@ const internalAccessList = {
630661
}
631662
});
632663

664+
const clientBuild = new Promise((resolve, reject) => {
665+
logger.info('Building Access client file #' + list.id + ' for: ' + list.name);
666+
667+
let template = null;
668+
const client_file = internalAccessList.getClientFilename(list);
669+
const data = {
670+
access_list: list
671+
};
672+
673+
try {
674+
template = fs.readFileSync(__dirname + '/../templates/access.conf', {encoding: 'utf8'});
675+
} catch (err) {
676+
reject(new error.ConfigurationError(err.message));
677+
return;
678+
}
679+
680+
return renderEngine
681+
.parseAndRender(template, data)
682+
.then((config_text) => {
683+
fs.writeFileSync(client_file, config_text, {encoding: 'utf8'});
684+
685+
if (config.debug()) {
686+
logger.success('Wrote config:', client_file, config_text);
687+
}
688+
689+
resolve(true);
690+
})
691+
.catch((err) => {
692+
if (config.debug()) {
693+
logger.warn('Could not write ' + client_file + ':', err.message);
694+
}
695+
696+
reject(new error.ConfigurationError(err.message));
697+
});
698+
699+
});
700+
633701
// Execute both promises concurrently
634-
return Promise.all([htPasswdBuild, caCertificateBuild]);
702+
return Promise.all([htPasswdBuild, caCertificateBuild, clientBuild]);
635703
}
636704
};
637705

backend/templates/_access.conf

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,37 @@
11
{% if access_list_id > 0 %}
2-
{% if access_list.clientcas.size > 0 %}
3-
# TLS Client Certificate Authorization
4-
if ($ssl_client_verify != "SUCCESS") {
2+
set $auth_basic "Authorization required";
3+
{% if access_list.satisfy_any == 1 %}
4+
# Satisfy Any - any check can succeed - so look for success
5+
if ( $access_list_{{ access_list_id }} = 1) {
6+
set $auth_basic off;
7+
}
8+
if ( $ssl_client_verify = "SUCCESS" ) {
9+
set $auth_basic off;
10+
}
11+
{% else %}
12+
# Satisfy All - all checks must succeed (so handle fails)
13+
if ( $access_list_{{ access_list_id }} = 0) {
514
return {% if drop_unauthorized == 1 %}444{% else %}403{% endif %};
615
}
7-
{% endif %}
16+
if ( $ssl_client_verify != "SUCCESS" ) {
17+
return {% if drop_unauthorized == 1 %}444{% else %}403{% endif %};
18+
}
19+
{% endif %}
20+
821
{% if access_list.items.length > 0 %}
22+
# Basic Auth is enabled
923
# Authorization
10-
auth_basic "Authorization required";
24+
auth_basic $auth_basic;
1125
auth_basic_user_file /data/access/{{ access_list_id }};
12-
13-
{% if access_list.pass_auth == 0 %}
26+
{% if access_list.pass_auth == 0 %}
1427
proxy_set_header Authorization "";
15-
{% endif %}
16-
17-
{% endif %}
18-
19-
# Access Rules: {{ access_list.clients | size }} total
20-
{% for client in access_list.clients %}
21-
{{client | nginxAccessRule}}
22-
{% endfor %}
23-
deny all;
24-
25-
# Access checks must...
26-
{% if access_list.satisfy_any == 1 %}
27-
satisfy any;
28+
{% endif %}
2829
{% else %}
29-
satisfy all;
30+
{% if access_list.satisfy_any == 1 %}
31+
# Satisfy Any without Basic Auth
32+
if ( $auth_basic != "off" ) {
33+
return {% if drop_unauthorized == 1 %}444{% else %}403{% endif %};
34+
}
35+
{% endif %}
3036
{% endif %}
3137
{% endif %}

backend/templates/access.conf

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# Access List Clients for {{ access_list.id }} - {{ access_list.name }}
2+
geo $realip_remote_addr $access_list_{{ access_list.id }} {
3+
{% if access_list.client.size == 0 %}
4+
default 1;
5+
{% else %}
6+
default 0;
7+
{% endif %}
8+
{% for client in access_list.clients %}
9+
{% if client.directive == "allow" %}
10+
{{client.address}} 1;
11+
{% endif %}
12+
{% if client.directive == "deny" %}
13+
{{client.address}} 0;
14+
{% endif %}
15+
{% endfor %}
16+
}

docker/rootfs/etc/nginx/nginx.conf

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ http {
7373

7474
# Files generated by NPM
7575
include /etc/nginx/conf.d/*.conf;
76+
include /data/nginx/client/*.conf;
7677
include /data/nginx/default_host/*.conf;
7778
include /data/nginx/proxy_host/*.conf;
7879
include /data/nginx/redirection_host/*.conf;

docker/rootfs/etc/s6-overlay/s6-rc.d/prepare/20-paths.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ mkdir -p \
2121
/data/logs \
2222
/data/access \
2323
/data/clientca \
24+
/data/nginx/client \
2425
/data/nginx/default_host \
2526
/data/nginx/default_www \
2627
/data/nginx/proxy_host \

frontend/js/app/nginx/access/form.ejs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@
121121
</div>
122122
</div>
123123
</div>
124-
<div class="text-muted">Note that the <code>allow</code> and <code>deny</code> directives will be applied in the order they are defined.</div>
124+
<div class="text-muted">Note that the most specific directive is what will be applied to the connection. Order does not matter.</div>
125125
<div class="btn-list justify-content-end">
126126
<button type="button" class="btn btn-teal access_add"><%- i18n('access-lists', 'access-add') %></button>
127127
</div>

0 commit comments

Comments
 (0)