From 0f7bad611df1329b3de3305674903a44ee21d011 Mon Sep 17 00:00:00 2001 From: Alex Anderson <191496+alxndrsn@users.noreply.github.com> Date: Tue, 10 Dec 2024 07:10:38 +0300 Subject: [PATCH] nginx: reject requests with unexpected Host header (#809) Co-authored-by: alxndrsn --- .circleci/config.yml | 4 +- files/nginx/odk.conf.template | 9 ++ files/nginx/redirector.conf | 12 ++- files/nginx/setup-odk.sh | 13 ++- test/run-tests.sh | 2 +- test/test-nginx.js | 153 ++++++++++++++++++++++++++++++++-- 6 files changed, 180 insertions(+), 13 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 547304ae..01960b5e 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -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: diff --git a/files/nginx/odk.conf.template b/files/nginx/odk.conf.template index 5fbdaf08..8f268ae4 100644 --- a/files/nginx/odk.conf.template +++ b/files/nginx/odk.conf.template @@ -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}; diff --git a/files/nginx/redirector.conf b/files/nginx/redirector.conf index 08e33bd5..ba56723f 100644 --- a/files/nginx/redirector.conf +++ b/files/nginx/redirector.conf @@ -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 @@ -18,3 +19,10 @@ server { return 301 https://$http_host$request_uri; } } + +server { + listen 80 default_server; + listen [::]:80 default_server; + + return 421; +} diff --git a/files/nginx/setup-odk.sh b/files/nginx/setup-odk.sh index db0f9356..87416c4b 100644 --- a/files/nginx/setup-odk.sh +++ b/files/nginx/setup-odk.sh @@ -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 @@ -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' \ diff --git a/test/run-tests.sh b/test/run-tests.sh index f9ee7828..0c09276f 100755 --- a/test/run-tests.sh +++ b/test/run-tests.sh @@ -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 diff --git a/test/test-nginx.js b/test/test-nginx.js index 2125adc7..0c38aa6c 100644 --- a/test/test-nginx.js +++ b/test/test-nginx.js @@ -1,3 +1,5 @@ +const tls = require('node:tls'); +const { Readable } = require('stream'); const { assert } = require('chai'); describe('nginx config', () => { @@ -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 () => { @@ -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>/ ], [ '/version.txt', /^versions:/ ], @@ -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); @@ -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', }, @@ -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) { @@ -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()); } @@ -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}`); + } +}