Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nginx: reject requests with unexpected Host header #809

Merged
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
913ebcc
nginx: reject HTTPS requests with unexpected Host header
Oct 8, 2024
00b5bd7
fix test URL
Oct 9, 2024
eb93b15
Add check/config for HTTP
Oct 9, 2024
c232cb3
circle-ci: try changing DOMAIN to localhost
Oct 9, 2024
c26ee73
circle-ci: revert DOMAIN; force-set Host header
Oct 9, 2024
ff04489
Merge branch 'next' into enforce-host-check-https
Oct 15, 2024
e1c245e
capitalise host header
Oct 15, 2024
77b7505
Merge branch 'next' into enforce-host-check-https
Dec 2, 2024
bea939d
revert code changes
Dec 2, 2024
20660c9
wip
Dec 2, 2024
4bc9b6f
revert nginx dockerfile
Dec 2, 2024
3627e92
expand comment
Dec 2, 2024
b7ebe89
421
Dec 2, 2024
18dad03
untee
Dec 2, 2024
5bcc786
42102
Dec 2, 2024
2792708
421
Dec 2, 2024
eea052d
re-order more like next
Dec 2, 2024
72db5d3
revert circle config change
Dec 2, 2024
d4a27ee
Revert "revert circle config change"
Dec 2, 2024
c6c5ce8
remove time limit for generated key
Dec 2, 2024
bb07343
revert
Dec 2, 2024
b461036
remove logging
Dec 2, 2024
b1d5f23
remove catch-all server names
Dec 2, 2024
13808d6
Revert "revert"
Dec 3, 2024
0fd9b27
Add test for SSL cert validity period.
Dec 3, 2024
c2683ce
increase ssl cert validity period
Dec 3, 2024
c9b376e
rewrite test with TLS
Dec 3, 2024
7791efe
little tls
Dec 3, 2024
6971cda
remove cert from fetch()
Dec 3, 2024
ba096cc
remove stop
Dec 3, 2024
cc95d47
revert magix perl
Dec 3, 2024
5850495
rename CNAME -> CERT_DOMAIN
Dec 3, 2024
0fca92a
reorder vars to match #814
Dec 3, 2024
938bd31
Merge branch 'next' into enforce-host-check-https-bronto-approach
Dec 8, 2024
0ea7207
update comment more
Dec 8, 2024
fb826af
rename sfetch() fn to avoid confusion; remove logging
Dec 8, 2024
a03694d
wip: add failing test: ipv6
Dec 8, 2024
c41b7cb
Passing tests
Dec 8, 2024
d7ad2d7
rename sfetch() as request()
Dec 8, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ jobs:
docker compose up -d
CONTAINER_NAME=$(docker inspect -f '{{.Name}}' $(docker compose ps -q nginx) | cut -c2-)
docker run --network container:$CONTAINER_NAME \
appropriate/curl -4 --insecure --retry 30 --retry-delay 10 --retry-connrefused https://localhost/ \
appropriate/curl -4 --insecure --retry 30 --retry-delay 10 --retry-connrefused https://localhost/ -H 'Host: local' \
| tee /dev/tty \
| grep -q 'ODK Central'
docker run --network container:$CONTAINER_NAME \
appropriate/curl -4 --insecure --retry 20 --retry-delay 2 --retry-connrefused https://localhost/v1/projects \
appropriate/curl -4 --insecure --retry 20 --retry-delay 2 --retry-connrefused https://localhost/v1/projects -H 'Host: local' \
| tee /dev/tty \
| grep -q '\[\]'
- run:
Expand Down
17 changes: 13 additions & 4 deletions files/nginx/odk.conf.template
Original file line number Diff line number Diff line change
@@ -1,10 +1,19 @@
server {
listen 443 default_server ssl;

ssl_certificate /etc/nginx/ssl/nginx.default.crt;
ssl_certificate_key /etc/nginx/ssl/nginx.default.key;

return 421;
}

server {
listen 443 ssl;
server_name ${CNAME};
server_name ${DOMAIN};

ssl_certificate /etc/${SSL_TYPE}/live/${CNAME}/fullchain.pem;
ssl_certificate_key /etc/${SSL_TYPE}/live/${CNAME}/privkey.pem;
ssl_trusted_certificate /etc/${SSL_TYPE}/live/${CNAME}/fullchain.pem;
ssl_certificate /etc/${SSL_TYPE}/live/${CERT_DOMAIN}/fullchain.pem;
ssl_certificate_key /etc/${SSL_TYPE}/live/${CERT_DOMAIN}/privkey.pem;
ssl_trusted_certificate /etc/${SSL_TYPE}/live/${CERT_DOMAIN}/fullchain.pem;
alxndrsn marked this conversation as resolved.
Show resolved Hide resolved

ssl_protocols TLSv1.2 TLSv1.3;
ssl_ciphers ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384;
Expand Down
11 changes: 9 additions & 2 deletions files/nginx/redirector.conf
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
server {
# Listen on plain old HTTP and catch all requests so they can be redirected
# to HTTPS instead.
listen 80 default_server reuseport;
listen [::]:80 default_server reuseport;
listen 80 reuseport;
alxndrsn marked this conversation as resolved.
Show resolved Hide resolved
server_name ${DOMAIN};

# Anything requesting this particular URL should be served content from
# Certbot's folder so the HTTP-01 ACME challenges can be completed for the
Expand All @@ -18,3 +18,10 @@ server {
return 301 https://$http_host$request_uri;
}
}

server {
listen 80 default_server;
listen [::]:80 default_server;

return 421;
}
17 changes: 14 additions & 3 deletions files/nginx/setup-odk.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,15 @@ fi

envsubst < /usr/share/odk/nginx/client-config.json.template > /usr/share/nginx/html/client-config.json

# Generate self-signed keys for incorrect (catch-all) HTTP listeners. This cert
# should never be seen by legitimate users, so it's not a big deal that it's
# self-signed and won't expire for 1,000 years.
mkdir -p /etc/nginx/ssl
openssl req -x509 -nodes -newkey rsa:2048 \
-subj "/" \
-keyout /etc/nginx/ssl/nginx.default.key \
-out /etc/nginx/ssl/nginx.default.crt \
-days 365000
brontolosone marked this conversation as resolved.
Show resolved Hide resolved

DH_PATH=/etc/dh/nginx.pem
if [ "$SSL_TYPE" != "upstream" ] && [ ! -s "$DH_PATH" ]; then
Expand All @@ -28,10 +37,12 @@ fi
# start from fresh templates in case ssl type has changed
echo "writing fresh nginx templates..."
# redirector.conf gets deleted if using upstream SSL so copy it back
cp /usr/share/odk/nginx/redirector.conf /etc/nginx/conf.d/redirector.conf
envsubst '$DOMAIN' \
< /usr/share/odk/nginx/redirector.conf \
> /etc/nginx/conf.d/redirector.conf

CNAME=$( [ "$SSL_TYPE" = "customssl" ] && echo "local" || echo "$DOMAIN") \
envsubst '$SSL_TYPE $CNAME $SENTRY_ORG_SUBDOMAIN $SENTRY_KEY $SENTRY_PROJECT' \
CERT_DOMAIN=$( [ "$SSL_TYPE" = "customssl" ] && echo "local" || echo "$DOMAIN") \
envsubst '$SSL_TYPE $DOMAIN $CERT_DOMAIN $SENTRY_ORG_SUBDOMAIN $SENTRY_KEY $SENTRY_PROJECT' \
< /usr/share/odk/nginx/odk.conf.template \
> /etc/nginx/conf.d/odk.conf

Expand Down
2 changes: 1 addition & 1 deletion test/run-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ wait_for_http_response 5 localhost:8383/health 200
log "Waiting for mock enketo..."
wait_for_http_response 5 localhost:8005/health 200
log "Waiting for nginx..."
wait_for_http_response 90 localhost:9000 301
wait_for_http_response 90 localhost:9000 421

npm run test:nginx

Expand Down
102 changes: 99 additions & 3 deletions test/test-nginx.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const tls = require('node:tls');
const { Readable } = require('stream');
const { assert } = require('chai');

describe('nginx config', () => {
Expand All @@ -12,7 +14,7 @@ describe('nginx config', () => {

// then
assert.equal(res.status, 301);
assert.equal(res.headers.get('location'), 'https://localhost:9000/');
assert.equal(res.headers.get('location'), 'https://odk-nginx.example.test/');
});

it('should serve generated client-config.json', async () => {
Expand Down Expand Up @@ -108,16 +110,52 @@ describe('nginx config', () => {
// then
assert.equal(body['x-forwarded-proto'], 'https');
});

it('should reject HTTP requests with incorrect host header supplied', async () => {
// when
const res = await fetchHttp('/', { headers:{ host:'bad.example.com' } });

// then
assert.equal(res.status, 421);
});

it('should reject HTTPS requests with incorrect host header supplied', async () => {
// when
const res = await fetchHttps('/', { headers:{ host:'bad.example.com' } });

// then
assert.equal(res.status, 421);
});

it('should serve long-lived certificate to HTTPS requests with incorrect host header', () => new Promise((resolve, reject) => {
const socket = tls.connect(9001, { host:'localhost', servername:'bad.example.com', rejectUnauthorized:false }, () => {
try {
const certificate = socket.getPeerCertificate();
const validUntilRaw = certificate.valid_to;

// Dates look like RFC-822 format - probably direct output of `openssl`. NodeJS Date.parse()
alxndrsn marked this conversation as resolved.
Show resolved Hide resolved
// seems to support this format.
const validUntil = new Date(validUntilRaw);
assert.isFalse(isNaN(validUntil), `Could not parse certificate's valid_to value as a date ('${validUntilRaw}')`);
assert.isAbove(validUntil.getFullYear(), 3000, 'The provided certificate expires too soon.');
socket.end();
} catch(err) {
socket.destroy(err);
}
});
socket.on('end', resolve);
socket.on('error', reject);
}));
});

function fetchHttp(path, options) {
if(!path.startsWith('/')) throw new Error('Invalid path.');
return fetch(`http://localhost:9000${path}`, { redirect:'manual', ...options });
return fetch(`http://localhost:9000${path}`, options);
}

function fetchHttps(path, options) {
if(!path.startsWith('/')) throw new Error('Invalid path.');
return fetch(`https://localhost:9001${path}`, { redirect:'manual', ...options });
return fetch(`https://localhost:9001${path}`, options);
}

function assertEnketoReceived(...expectedRequests) {
Expand Down Expand Up @@ -146,3 +184,61 @@ async function resetMock(port) {
const res = await fetch(`http://localhost:${port}/reset`);
assert.isTrue(res.ok);
}

// Similar to fetch() but:
//
// 1. do not follow redirects
// 2. allow overriding of fetch's "forbidden" headers: https://developer.mozilla.org/en-US/docs/Glossary/Forbidden_header_name
// 3. allow access to server SSL certificate
function fetch(url, { body, ...options }={}) {
alxndrsn marked this conversation as resolved.
Show resolved Hide resolved
if(!options.headers) options.headers = {};
if(!options.headers.host) options.headers.host = 'odk-nginx.example.test';

console.log('fetch()', url, 'headers:', options.headers);

return new Promise((resolve, reject) => {
try {
const req = getProtocolImplFrom(url).request(url, options, res => {
res.on('error', reject);

const body = new Readable({ _read: () => {} });
res.on('error', err => body.destroy(err));
res.on('data', data => body.push(data));
res.on('end', () => body.push(null));

const text = () => new Promise((resolve, reject) => {
const chunks = [];
body.on('error', reject);
body.on('data', data => chunks.push(data))
body.on('end', () => resolve(Buffer.concat(chunks).toString('utf8')));
});

const status = res.statusCode;

resolve({
status,
ok: status >= 200 && status < 300,
statusText: res.statusText,
body,
text,
json: async () => JSON.parse(await text()),
headers: new Headers(res.headers),
});
});
req.on('error', reject);
if(body !== undefined) req.write(body);
req.end();
} catch(err) {
reject(err);
}
});
}

function getProtocolImplFrom(url) {
const { protocol } = new URL(url);
switch(protocol) {
case 'http:': return require('node:http');
case 'https:': return require('node:https');
default: throw new Error(`Unsupported protocol: ${protocol}`);
}
}
Loading