Skip to content

Commit af64f1d

Browse files
alxndrsnalxndrsn
authored
nginx/csp: enforce policy for central-backend (#1859)
Switch from Content-Security-Policy-Report-Only to Content-Security-Policy.
1 parent d3eb696 commit af64f1d

6 files changed

Lines changed: 70 additions & 78 deletions

File tree

‎files/nginx/backend.conf‎

Lines changed: 0 additions & 8 deletions
This file was deleted.

‎files/nginx/odk.conf.template‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,12 @@ map $request_uri $central_frontend_csp {
9797
"default-src 'report-sample' 'none'; connect-src 'self' https://translate.google.com https://translate.googleapis.com; font-src 'self'; form-action 'self'; frame-ancestors 'none'; frame-src 'self' https://getodk.github.io/central/; img-src data: https:; manifest-src 'self'; media-src 'none'; object-src 'none'; script-src 'report-sample' 'self'; style-src 'report-sample' 'self'; style-src-attr 'unsafe-inline'; worker-src 'report-sample' blob:; report-uri /csp-report";
9898
}
9999

100+
map $upstream_http_content_security_policy $central_backend_csp {
101+
# pass through any Content-Security-Policy received from upstream services (central-backend, enketo)
102+
"" "default-src 'report-sample' 'none'; form-action 'none'; frame-ancestors 'none'; img-src http://${DOMAIN}/favicon.ico; report-uri /csp-report";
103+
default $upstream_http_content_security_policy;
104+
}
105+
100106
server {
101107
listen 443 ssl;
102108
http2 on;
@@ -176,17 +182,20 @@ server {
176182
}
177183
# End of Enketo Configuration.
178184

179-
location ~ ^/v\d+/oidc/callback$ {
180-
include /usr/share/odk/nginx/common-headers.conf;
181-
include /usr/share/odk/nginx/backend.conf;
182-
}
183-
184185
location ~ ^/v\d {
185-
proxy_hide_header Content-Security-Policy-Report-Only;
186-
add_header Content-Security-Policy-Report-Only "default-src 'report-sample' 'none'; form-action 'none'; frame-ancestors 'none'; img-src http://${DOMAIN}/favicon.ico; report-uri /csp-report" always;
186+
proxy_hide_header Content-Security-Policy;
187+
add_header Content-Security-Policy $central_backend_csp always;
187188

188189
include /usr/share/odk/nginx/common-headers.conf;
189-
include /usr/share/odk/nginx/backend.conf;
190+
191+
proxy_set_header X-Forwarded-Proto $scheme;
192+
proxy_pass http://service:8383;
193+
proxy_redirect off;
194+
195+
# buffer requests, but not responses, so streaming out works.
196+
proxy_request_buffering on;
197+
proxy_buffering off;
198+
proxy_read_timeout 2m;
190199
}
191200

192201
location @blank.html {

‎files/nginx/setup-odk.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ else
5959
# strip out all ssl_* directives
6060
perl -i -ne 's/listen 443.*/listen 80;/; print if ! /\bssl_/' /etc/nginx/conf.d/odk.conf
6161
# force https because we expect SSL upstream
62-
perl -i -pe 's/X-Forwarded-Proto \$scheme/X-Forwarded-Proto https/;' /usr/share/odk/nginx/backend.conf
62+
perl -i -pe 's/X-Forwarded-Proto \$scheme/X-Forwarded-Proto https/;' /etc/nginx/conf.d/odk.conf
6363
echo "starting nginx for upstream ssl..."
6464
else
6565
# remove letsencrypt challenge reply, but keep 80 to 443 redirection

‎nginx.dockerfile‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ COPY files/nginx/setup-odk.sh \
3232
/scripts/
3333

3434
COPY files/nginx/redirector.conf /usr/share/odk/nginx/
35-
COPY files/nginx/backend.conf /usr/share/odk/nginx/
3635
COPY files/nginx/common-headers.conf /usr/share/odk/nginx/
3736
COPY files/nginx/robots.txt /usr/share/nginx/html
3837
COPY --from=intermediate client/dist/ /usr/share/nginx/html

‎test/nginx/mock-http-server/index.js‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,23 +9,17 @@ const app = express();
99

1010
app.use((req, res, next) => {
1111
console.log(new Date(), req.method, req.originalUrl);
12-
13-
// always set CSP header to detect (or allow) leaks from backend through to the client
14-
const topLevelDirectives = [
15-
'default-src',
16-
'form-action',
17-
'frame-ancestors',
18-
];
19-
res.set('Content-Security-Policy', topLevelDirectives.map(d => `${d} NOTE:FROM-BACKEND:block`) .join('; '));
20-
res.set('Content-Security-Policy-Report-Only', topLevelDirectives.map(d => `${d} NOTE:FROM-BACKEND:reportOnly`).join('; '));
21-
2212
next();
2313
});
2414

2515
// Enketo express returns response with Vary and Cache-Control headers
2616
app.use('/-/', (req, res, next) => {
2717
res.set('Vary', 'Accept-Encoding');
2818
res.set('Cache-Control', 'public, max-age=0');
19+
20+
// Set both CSP headers from enketo. Eventually nginx should be confident to override both.
21+
res.set('Content-Security-Policy', `NOTE:FROM-BACKEND:block`);
22+
res.set('Content-Security-Policy-Report-Only', `NOTE:FROM-BACKEND:reportOnly`);
2923
next();
3024
});
3125

@@ -45,6 +39,14 @@ app.get('/v1/projects', (_, res) => {
4539
res.send('OK');
4640
});
4741

42+
app.get('/v1/oidc/callback', (req, res) => {
43+
// This endpoint is 100% responsible for its own headers. Set both, and test they both get through.
44+
res.set('Content-Security-Policy', `NOTE:FROM-BACKEND:block`);
45+
res.set('Content-Security-Policy-Report-Only', `NOTE:FROM-BACKEND:reportOnly`);
46+
47+
res.send('OK');
48+
});
49+
4850
[
4951
'delete',
5052
'get',

‎test/nginx/src/mocha/nginx.spec.js‎

Lines changed: 40 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,6 @@ const allowGoogleTranslate = ({ 'connect-src':connectSrc, 'img-src':imgSrc, ...o
4343
const contentSecurityPolicies = {
4444
'backend-strict': {
4545
block: {
46-
'default-src': 'NOTE:FROM-BACKEND:block',
47-
'form-action': 'NOTE:FROM-BACKEND:block',
48-
'frame-ancestors': 'NOTE:FROM-BACKEND:block',
49-
},
50-
reportOnly: {
5146
'default-src': [
5247
reportSample,
5348
none,
@@ -59,16 +54,8 @@ const contentSecurityPolicies = {
5954
},
6055
},
6156
'backend-unmodified': {
62-
block: {
63-
'default-src': 'NOTE:FROM-BACKEND:block',
64-
'form-action': 'NOTE:FROM-BACKEND:block',
65-
'frame-ancestors': 'NOTE:FROM-BACKEND:block',
66-
},
67-
reportOnly: {
68-
'default-src': 'NOTE:FROM-BACKEND:reportOnly',
69-
'form-action': 'NOTE:FROM-BACKEND:reportOnly',
70-
'frame-ancestors': 'NOTE:FROM-BACKEND:reportOnly',
71-
},
57+
block: 'NOTE:FROM-BACKEND:block',
58+
reportOnly: 'NOTE:FROM-BACKEND:reportOnly',
7259
},
7360
'blank-html': {
7461
block: allowGoogleTranslate({
@@ -122,11 +109,7 @@ const contentSecurityPolicies = {
122109
}),
123110
},
124111
enketo: {
125-
block: {
126-
'default-src': 'NOTE:FROM-BACKEND:block',
127-
'form-action': 'NOTE:FROM-BACKEND:block',
128-
'frame-ancestors': 'NOTE:FROM-BACKEND:block',
129-
},
112+
block: 'NOTE:FROM-BACKEND:block',
130113
reportOnly: allowGoogleTranslate({
131114
'default-src': [
132115
reportSample,
@@ -262,36 +245,39 @@ describe('Content-Security-Policy definitions', () => {
262245
if(!policy) continue;
263246

264247
describe(`header: ${headerNames[headerType]}`, () => {
265-
it(`should have required directives: ${requiredDirectives}`, () => {
266-
assert.containsAllKeys(policy, requiredDirectives);
267-
});
268-
269-
Object.entries(policy)
270-
.map (([ key, directive ]) => [ key, asArray(directive) ])
271-
.filter (([ key, directive ]) => !(directive.length === 1 && directive[0] === `NOTE:FROM-BACKEND:${headerType}`)) // eslint-disable-line no-unused-vars
272-
.forEach(([ key, directive ]) => {
273-
describe(`directive: ${key}`, () => {
274-
if(supportsReportSample.includes(key)) {
275-
if(key.startsWith('style-src') && directive.includes(`'unsafe-inline'`)) {
276-
// For style-* directives, report-sample will only provide a sample of inline violations.
277-
it(`should not include 'report-sample' in directive '${key}' when 'unsafe-inline' is allowed`, () => {
278-
// expect
279-
assert.notInclude(directive, "'report-sample'");
280-
});
248+
if(typeof policy === 'string') {
249+
if(!policy.startsWith('NOTE:FROM-BACKEND:')) throw new Error(`Unexpected policy string: '${policy}'`);
250+
} else {
251+
it(`should have required directives: ${requiredDirectives}`, () => {
252+
assert.containsAllKeys(policy, requiredDirectives);
253+
});
254+
255+
Object.entries(policy)
256+
.map (([ key, directive ]) => [ key, asArray(directive) ])
257+
.forEach(([ key, directive ]) => {
258+
describe(`directive: ${key}`, () => {
259+
if(supportsReportSample.includes(key)) {
260+
if(key.startsWith('style-src') && directive.includes(`'unsafe-inline'`)) {
261+
// For style-* directives, report-sample will only provide a sample of inline violations.
262+
it(`should not include 'report-sample' in directive '${key}' when 'unsafe-inline' is allowed`, () => {
263+
// expect
264+
assert.notInclude(directive, "'report-sample'");
265+
});
266+
} else {
267+
it(`should include 'report-sample' in directive '${key}'`, () => {
268+
// expect
269+
assert.include(directive, "'report-sample'");
270+
});
271+
}
281272
} else {
282-
it(`should include 'report-sample' in directive '${key}'`, () => {
273+
it(`should not include 'report-sample' in directive '${key}'`, () => {
283274
// expect
284-
assert.include(directive, "'report-sample'");
275+
assert.notInclude(directive, "'report-sample'");
285276
});
286277
}
287-
} else {
288-
it(`should not include 'report-sample' in directive '${key}'`, () => {
289-
// expect
290-
assert.notInclude(directive, "'report-sample'");
291-
});
292-
}
278+
});
293279
});
294-
});
280+
}
295281
});
296282
}
297283
});
@@ -1182,9 +1168,13 @@ function assertSecurityHeaders(res, { csp }) {
11821168
function assertCsp(actual, expected) {
11831169
if(!expected) return assert.isNull(actual);
11841170

1185-
assert.deepEqualInAnyOrder(
1186-
actual?.split('; '),
1187-
Object.entries(expected)
1188-
.map(([ k, v ]) => `${k} ${asArray(v).join(' ')}`),
1189-
);
1171+
if(typeof expected === 'string') {
1172+
assert.equal(actual, expected);
1173+
} else {
1174+
assert.deepEqualInAnyOrder(
1175+
actual?.split('; '),
1176+
Object.entries(expected)
1177+
.map(([ k, v ]) => `${k} ${asArray(v).join(' ')}`),
1178+
);
1179+
}
11901180
}

0 commit comments

Comments
 (0)