Skip to content

Commit 19086ce

Browse files
committed
fix: check access list before force SSL redirect
Fixes #5207 - Security issue where Force SSL leaks host existence When both Force SSL and an Access List are active on a Proxy Host, HTTP requests from unauthorized IPs were receiving a 301 redirect instead of being blocked. This allowed attackers to enumerate valid hosts by brute-forcing the Host header. Solution: Use nginx geo module to check IP access before the SSL redirect. Only allowed IPs get redirected to HTTPS; denied IPs fall through to the access phase and receive 403. Changes: - Add geo block template for IP-based access control - Modify _forced_ssl.conf to check geo variable before redirecting - Generate geo config files when access lists are created/updated - Include geo configs at http level in nginx.conf - Create access_geo directory on startup
1 parent 7747db9 commit 19086ce

5 files changed

Lines changed: 89 additions & 0 deletions

File tree

backend/internal/access-list.js

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import fs from "node:fs";
2+
import { dirname } from "node:path";
3+
import { fileURLToPath } from "node:url";
24
import batchflow from "batchflow";
35
import _ from "lodash";
46
import errs from "../lib/error.js";
@@ -11,6 +13,9 @@ import proxyHostModel from "../models/proxy_host.js";
1113
import internalAuditLog from "./audit-log.js";
1214
import internalNginx from "./nginx.js";
1315

16+
const __filename = fileURLToPath(import.meta.url);
17+
const __dirname = dirname(__filename);
18+
1419
const omissions = () => {
1520
return ["is_deleted"];
1621
};
@@ -311,6 +316,13 @@ const internalAccessList = {
311316
// do nothing
312317
}
313318

319+
// delete the geo config file
320+
try {
321+
fs.unlinkSync(`/data/nginx/access_geo/${row.id}.conf`);
322+
} catch (_err) {
323+
// do nothing
324+
}
325+
314326
// 4. audit log
315327
await internalAuditLog.add(access, {
316328
action: "deleted",
@@ -432,6 +444,7 @@ const internalAccessList = {
432444
* @param {Integer} list.id
433445
* @param {String} list.name
434446
* @param {Array} list.items
447+
* @param {Array} list.clients
435448
* @returns {Promise}
436449
*/
437450
build: async (list) => {
@@ -482,6 +495,50 @@ const internalAccessList = {
482495
});
483496
});
484497
}
498+
499+
// 4. Build geo config file for IP-based access control (used by Force SSL)
500+
await internalAccessList.buildGeoConfig(list);
501+
},
502+
503+
/**
504+
* Build geo config file for IP-based access control
505+
* This is used to check IP access before Force SSL redirect
506+
* @param {Object} list
507+
* @param {Integer} list.id
508+
* @param {String} list.name
509+
* @param {Array} list.clients
510+
* @returns {Promise}
511+
*/
512+
buildGeoConfig: async (list) => {
513+
const geoDir = '/data/nginx/access_geo';
514+
const geoFile = `${geoDir}/${list.id}.conf`;
515+
516+
// Ensure directory exists
517+
try {
518+
fs.mkdirSync(geoDir, { recursive: true });
519+
} catch (_err) {
520+
// do nothing
521+
}
522+
523+
// Remove existing geo file
524+
try {
525+
fs.unlinkSync(geoFile);
526+
} catch (_err) {
527+
// do nothing
528+
}
529+
530+
// Only create geo config if there are client IP rules
531+
if (list.clients && list.clients.length > 0) {
532+
logger.info(`Building Geo config file #${list.id} for: ${list.name}`);
533+
534+
const renderEngine = utils.getRenderEngine();
535+
const template = fs.readFileSync(`${__dirname}/../templates/access_list_geo.conf`, { encoding: 'utf8' });
536+
537+
const config = await renderEngine.parseAndRender(template, list);
538+
fs.writeFileSync(geoFile, config, { encoding: 'utf8' });
539+
540+
logger.success(`Built Geo config file #${list.id} for: ${list.name}`);
541+
}
485542
}
486543
}
487544

backend/templates/_forced_ssl.conf

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,24 @@
11
{% if certificate and certificate_id > 0 -%}
22
{% if ssl_forced == 1 or ssl_forced == true %}
3+
{% if access_list %}{% assign clients_size = access_list.clients | size %}{% else %}{% assign clients_size = 0 %}{% endif %}
4+
{% if access_list_id > 0 and clients_size > 0 %}
5+
# Force SSL (only for allowed IPs - denied IPs will get 403 from access rules)
6+
set $redirect_to_ssl 0;
7+
if ($scheme = "http") {
8+
set $redirect_to_ssl 1;
9+
}
10+
if ($access_list_{{ access_list_id }}_allowed = 0) {
11+
set $redirect_to_ssl 0;
12+
}
13+
if ($request_uri = /.well-known/acme-challenge/test-challenge) {
14+
set $redirect_to_ssl 0;
15+
}
16+
if ($redirect_to_ssl = 1) {
17+
return 301 https://$host$request_uri;
18+
}
19+
{% else %}
320
# Force SSL
421
include conf.d/include/force-ssl.conf;
522
{% endif %}
23+
{% endif %}
624
{% endif %}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
# Geo-based access control for access list {{ id }}
2+
# Used to check IP access before Force SSL redirect
3+
geo $access_list_{{ id }}_allowed {
4+
default 0;
5+
{% for client in clients %}
6+
{% if client.directive == "allow" %}
7+
{{ client.address }} 1;
8+
{% endif %}
9+
{% endfor %}
10+
}

docker/rootfs/etc/nginx/nginx.conf

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ http {
7272
# Custom
7373
include /data/nginx/custom/http_top[.]conf;
7474

75+
# Access list geo definitions for Force SSL
76+
include /data/nginx/access_geo/*.conf;
77+
7578
# Files generated by NPM
7679
include /etc/nginx/conf.d/*.conf;
7780
include /data/nginx/default_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
@@ -27,6 +27,7 @@ mkdir -p \
2727
/data/nginx/stream \
2828
/data/nginx/dead_host \
2929
/data/nginx/temp \
30+
/data/nginx/access_geo \
3031
/data/letsencrypt-acme-challenge \
3132
/run/nginx \
3233
/tmp/nginx/body \

0 commit comments

Comments
 (0)