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 all 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
9 changes: 9 additions & 0 deletions files/nginx/odk.conf.template
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
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 ${DOMAIN};
Expand Down
12 changes: 10 additions & 2 deletions files/nginx/redirector.conf
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
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
listen [::]:80 reuseport;
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 +19,10 @@ server {
return 301 https://$http_host$request_uri;
}
}

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

return 421;
}
13 changes: 12 additions & 1 deletion 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 the incorrect (catch-all) HTTPS listener. 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,7 +37,9 @@ 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

CERT_DOMAIN=$( [ "$SSL_TYPE" = "customssl" ] && echo "local" || echo "$DOMAIN") \
envsubst '$SSL_TYPE $CERT_DOMAIN $DOMAIN $SENTRY_ORG_SUBDOMAIN $SENTRY_KEY $SENTRY_PROJECT' \
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
153 changes: 146 additions & 7 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,16 @@ 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 forward HTTP to HTTPS (IPv6)', async () => {
// when
const res = await fetchHttp6('/');

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

it('should serve generated client-config.json', async () => {
Expand All @@ -25,6 +36,16 @@ describe('nginx config', () => {
assert.equal(await res.headers.get('cache-control'), 'no-cache');
});

it('should serve generated client-config.json (IPv6)', async () => {
// when
const res = await fetchHttps6('/client-config.json');

// then
assert.equal(res.status, 200);
assert.deepEqual(await res.json(), { oidcEnabled: false });
assert.equal(await res.headers.get('cache-control'), 'no-cache');
});

[
[ '/index.html', /<div id="app"><\/div>/ ],
[ '/version.txt', /^versions:/ ],
Expand Down Expand Up @@ -83,7 +104,7 @@ describe('nginx config', () => {

it('should set x-forwarded-proto header to "https"', async () => {
// when
const res = await fetch(`https://localhost:9001/v1/reflect-headers`);
const res = await fetchHttps('/v1/reflect-headers');
// then
assert.equal(res.status, 200);

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

it('should override supplied x-forwarded-proto header', async () => {
// when
const res = await fetch(`https://localhost:9001/v1/reflect-headers`, {
const res = await fetchHttps('/v1/reflect-headers', {
headers: {
'x-forwarded-proto': 'http',
},
Expand All @@ -108,16 +129,78 @@ 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 HTTP requests with incorrect host header supplied (IPv6)', async () => {
// when
const res = await fetchHttp6('/', { 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 reject HTTPS requests with incorrect host header supplied (IPv6)', async () => {
// when
const res = await fetchHttps6('/', { 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 request(`http://127.0.0.1:9000${path}`, options);
}

function fetchHttp6(path, options) {
if(!path.startsWith('/')) throw new Error('Invalid path.');
return request(`http://[::1]: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 request(`https://127.0.0.1:9001${path}`, options);
}

function fetchHttps6(path, options) {
if(!path.startsWith('/')) throw new Error('Invalid path.');
return request(`https://[::1]:9001${path}`, options);
}

function assertEnketoReceived(...expectedRequests) {
Expand All @@ -129,7 +212,7 @@ function assertBackendReceived(...expectedRequests) {
}

async function assertMockHttpReceived(port, expectedRequests) {
const res = await fetch(`http://localhost:${port}/request-log`);
const res = await request(`http://localhost:${port}/request-log`);
assert.isTrue(res.ok);
assert.deepEqual(expectedRequests, await res.json());
}
Expand All @@ -143,6 +226,62 @@ function resetBackendMock() {
}

async function resetMock(port) {
const res = await fetch(`http://localhost:${port}/reset`);
const res = await request(`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 request(url, { body, ...options }={}) {
if(!options.headers) options.headers = {};
if(!options.headers.host) options.headers.host = 'odk-nginx.example.test';

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