Skip to content

Commit

Permalink
nginx: reject requests with unexpected Host header (#809)
Browse files Browse the repository at this point in the history
Co-authored-by: alxndrsn <alxndrsn>
  • Loading branch information
alxndrsn authored Dec 10, 2024
1 parent 515bfb9 commit 0f7bad6
Show file tree
Hide file tree
Showing 6 changed files with 180 additions and 13 deletions.
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;
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

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()
// 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}`);
}
}

0 comments on commit 0f7bad6

Please sign in to comment.