diff --git a/src/utils/middleware/csp.js b/src/utils/middleware/csp.js index 2c56b995..705bb66b 100644 --- a/src/utils/middleware/csp.js +++ b/src/utils/middleware/csp.js @@ -34,7 +34,7 @@ const detectErrorsMiddleware = (logger) => { next() } } -const captureEventMiddleware = (ignoredStrings, logger, sentry) => { +const captureEventMiddleware = (ignoredStrings, logger, Sentry) => { return (req, res) => { let reports = undefined if (Array.isArray(req.body) && req.body.length > 0) { @@ -54,12 +54,13 @@ const captureEventMiddleware = (ignoredStrings, logger, sentry) => { return res.status(400).end() } else { if (hasSubstr(ignoredStrings, blockedUri)) return res.status(204).end() - sentry.captureEvent({ + + Sentry.captureEvent({ message: `Blocked ${violatedDirective} from ${blockedUri}`, level: 'warning', extra: { cspReport: body, - userAgent: userAgent + userAgent } }) } @@ -73,6 +74,7 @@ const captureEventMiddleware = (ignoredStrings, logger, sentry) => { } module.exports = { + hasSubstr, rateLimitMiddleware, captureEventMiddleware, requestParseMiddleware, diff --git a/src/utils/middleware/csp.test.js b/src/utils/middleware/csp.test.js index 2e973b36..eeea1152 100644 --- a/src/utils/middleware/csp.test.js +++ b/src/utils/middleware/csp.test.js @@ -7,8 +7,8 @@ const { expect } = require('chai') const loggingSpy = { info: sinon.spy() } const sentrySpy = { captureEvent: sinon.spy() } - const { + hasSubstr, rateLimitMiddleware, captureEventMiddleware, requestParseMiddleware, @@ -26,165 +26,194 @@ const cspMiddlewareStack = [ sentrySpy) ] -describe('CSP report endpoint', () => { - const underTest = express() - underTest.enable('trust proxy') - underTest.set('trust proxy', 3) - underTest.use('/test', cspMiddlewareStack) +describe('CSP middleware', () => { + describe('hasSubstr', () => { + const testCase = 'lorem ipsum dolor sit amet' + + it('returns true when substring is detected', () => { + const lookupStrings = ['ipsum'] + const result = hasSubstr(lookupStrings, testCase) + expect(result).to.equal(true) + }) + + it('returns false when substring is not detected', () => { + const lookupStrings = ['consectetur'] + const result = hasSubstr(lookupStrings, testCase) + expect(result).to.equal(false) + }) - beforeEach(() => { - loggingSpy.info.resetHistory() - sentrySpy.captureEvent.resetHistory() + it('ignores case when comparing', () => { + const lookupStrings = ['DOLOR'] + const result = hasSubstr(lookupStrings, testCase) + expect(result).to.equal(true) + }) + + it('handles multiple lookup strings', () => { + const lookupStrings = ['consectetur', 'LOREM'] + const result = hasSubstr(lookupStrings, testCase) + expect(result).to.equal(true) + }) }) - const validReportUriPayload = { - 'csp-report': { - 'document-uri': 'https://example.com/page-with-violation', - 'referrer': '', - 'violated-directive': 'script-src \'self\'', - 'effective-directive': 'script-src', - 'original-policy': 'default-src \'none\'; script-src \'self\'; object-src \'self\'; report-to: default', - 'disposition': 'enforce', - 'blocked-uri': 'https://evil.example.com/malicious.js', - 'line-number': 22, - 'column-number': 13, - 'source-file': 'https://example.com/script.js', - 'status-code': 200, - 'script-sample': '' - } - } + describe('CSP report endpoint', () => { + const underTest = express() + underTest.enable('trust proxy') + underTest.set('trust proxy', 3) + underTest.use('/test', cspMiddlewareStack) + + beforeEach(() => { + loggingSpy.info.resetHistory() + sentrySpy.captureEvent.resetHistory() + }) - const validReportingAPIPayload = [ - { - 'age': 2, - 'body': { - 'blockedURL': 'https://site2.example/script.js', + const validReportUriPayload = { + 'csp-report': { + 'document-uri': 'https://example.com/page-with-violation', + 'referrer': '', + 'violated-directive': 'script-src \'self\'', + 'effective-directive': 'script-src', + 'original-policy': 'default-src \'none\'; script-src \'self\'; object-src \'self\'; report-to: default', 'disposition': 'enforce', - 'documentURL': 'https://site.example', - 'effectiveDirective': 'script-src-elem', - 'originalPolicy': 'script-src \'self\'; object-src \'none\'; report-to main-endpoint;', - 'referrer': 'https://site.example', - 'sample': '', - 'statusCode': 200 - }, - 'type': 'csp-violation', - 'url': 'https://site.example', - 'user_agent': 'Mozilla/5.0... Chrome/92.0.4504.0' + 'blocked-uri': 'https://evil.example.com/malicious.js', + 'line-number': 22, + 'column-number': 13, + 'source-file': 'https://example.com/script.js', + 'status-code': 200, + 'script-sample': '' + } } - ] - const validPayloads = [ - { arg: validReportUriPayload, expectedMessage: 'Blocked script-src \'self\' from https://evil.example.com/malicious.js', expectedReport: validReportUriPayload['csp-report'], type: 'report-uri' }, - { arg: validReportingAPIPayload, expectedMessage: 'Blocked script-src-elem from https://site2.example/script.js', expectedReport: validReportingAPIPayload[0]['body'], type: 'reporting api' } - ] + const validReportingAPIPayload = [ + { + 'age': 2, + 'body': { + 'blockedURL': 'https://site2.example/script.js', + 'disposition': 'enforce', + 'documentURL': 'https://site.example', + 'effectiveDirective': 'script-src-elem', + 'originalPolicy': 'script-src \'self\'; object-src \'none\'; report-to main-endpoint;', + 'referrer': 'https://site.example', + 'sample': '', + 'statusCode': 200 + }, + 'type': 'csp-violation', + 'url': 'https://site.example', + 'user_agent': 'Mozilla/5.0... Chrome/92.0.4504.0' + } + ] + + const validPayloads = [ + { arg: validReportUriPayload, expectedMessage: 'Blocked script-src \'self\' from https://evil.example.com/malicious.js', expectedReport: validReportUriPayload['csp-report'], type: 'report-uri' }, + { arg: validReportingAPIPayload, expectedMessage: 'Blocked script-src-elem from https://site2.example/script.js', expectedReport: validReportingAPIPayload[0]['body'], type: 'reporting api' } + ] + + validPayloads.forEach(test => { + it(`should return 204 and send to sentry if a valid ${test.type} csp report is present`, async () => { + await request(underTest) + .post('/test') + .set('user-agent', 'supertest') + .send(test.arg) + .expect(204) + + expect(sentrySpy.captureEvent.calledOnce).to.be.true + expect(sentrySpy.captureEvent.calledWith({ + message: test.expectedMessage, + level: 'warning', + extra: { + 'cspReport': test.expectedReport, + 'userAgent': 'supertest' + } + })).to.be.true + }) + }) - validPayloads.forEach(test => { - it(`should return 204 and send to sentry if a valid ${test.type} csp report is present`, async () => { + it('should return 204 and not send to sentry if a valid csp report is present but the blocked-uri is ignored', async () => { await request(underTest) .post('/test') .set('user-agent', 'supertest') - .send(test.arg) + .send({ + 'csp-report': { + 'blocked-uri': 'https://www.banned.com', + 'violated-directive': 'connect-src' + } + }) .expect(204) - expect(sentrySpy.captureEvent.calledOnce).to.be.true - expect(sentrySpy.captureEvent.calledWith({ - message: test.expectedMessage, - level: 'warning', - extra: { - 'cspReport': test.expectedReport, - 'userAgent': 'supertest' - } - })).to.be.true + expect(sentrySpy.captureEvent.notCalled).to.be.true }) - }) - it('should return 204 and not send to sentry if a valid csp report is present but the blocked-uri is ignored', async () => { - await request(underTest) - .post('/test') - .set('user-agent', 'supertest') - .send({ - 'csp-report': { - 'blocked-uri': 'https://www.banned.com', - 'violated-directive': 'connect-src' - } - }) - .expect(204) + it('should return 400 if content-type is unexpected', async () => { + await request(underTest) + .post('/test') + .set('content-type', 'application/x-www-form-urlencoded') + .send(validReportUriPayload) + .expect(400) + }) - expect(sentrySpy.captureEvent.notCalled).to.be.true - }) + it('should return 400 if payload is too large', async () => { - it('should return 400 if content-type is unexpected', async () => { - await request(underTest) - .post('/test') - .set('content-type', 'application/x-www-form-urlencoded') - .send(validReportUriPayload) - .expect(400) - }) + const largePayload = { + ...validReportUriPayload, + 'large_value': 'some text here x1000'.repeat(1000) + } - it('should return 400 if payload is too large', async () => { + await request(underTest) + .post('/test') + .send(largePayload) + .expect(400) - const largePayload = { - ...validReportUriPayload, - 'large_value': 'some text here x1000'.repeat(1000) - } + expect(loggingSpy.info.calledOnce).to.be.true + expect(loggingSpy.info.calledWith('CSP violation request payload exceeds maximum size')).to.be.true + }) - await request(underTest) - .post('/test') - .send(largePayload) - .expect(400) + it('should return 400 if request is not JSON', async () => { + await request(underTest) + .post('/test') + .set('content-type', 'application/json') + .send('notJSON') + .expect(400) - expect(loggingSpy.info.calledOnce).to.be.true - expect(loggingSpy.info.calledWith('CSP violation request payload exceeds maximum size')).to.be.true - }) + expect(loggingSpy.info.calledOnce).to.be.true + expect(loggingSpy.info.calledWith('CSP violation request payload did not match expected content type')).to.be.true + }) - it('should return 400 if request is not JSON', async () => { - await request(underTest) - .post('/test') - .set('content-type', 'application/json') - .send('notJSON') - .expect(400) + it('should return 400 if json does not included expected values', async () => { + await request(underTest) + .post('/test') + .send({ key: 'value' }) + .expect(400) + }) - expect(loggingSpy.info.calledOnce).to.be.true - expect(loggingSpy.info.calledWith('CSP violation request payload did not match expected content type')).to.be.true - }) + it('should return 429 if too many requests are made and respect trust proxy settings', async () => { - it('should return 400 if json does not included expected values', async () => { - await request(underTest) - .post('/test') - .send({ key: 'value' }) - .expect(400) - }) + // X-Forwarded-For: , , , https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For + // make 10 requests for each 'IP', the 11th should be rate limited - it('should return 429 if too many requests are made and respect trust proxy settings', async () => { + for (let i = 0; i < 10; i++) { + await request(underTest) + .post('/test') + .set('x-forwarded-for', '1.2.3.4, 2.2.2.2, 3.3.3.3') + .send(validReportingAPIPayload) + .expect(204) - // X-Forwarded-For: , , , https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For - // make 10 requests for each 'IP', the 11th should be rate limited + await request(underTest) + .post('/test') + .set('x-forwarded-for', '5.6.7.8, 2.2.2.2, 3.3.3.3') + .send(validReportUriPayload) + .expect(204) + } - for (let i = 0; i < 10; i++) { await request(underTest) .post('/test') .set('x-forwarded-for', '1.2.3.4, 2.2.2.2, 3.3.3.3') - .send(validReportingAPIPayload) - .expect(204) + .send(validReportUriPayload) + .expect(429) await request(underTest) .post('/test') .set('x-forwarded-for', '5.6.7.8, 2.2.2.2, 3.3.3.3') - .send(validReportUriPayload) - .expect(204) - } - - await request(underTest) - .post('/test') - .set('x-forwarded-for', '1.2.3.4, 2.2.2.2, 3.3.3.3') - .send(validReportUriPayload) - .expect(429) - - await request(underTest) - .post('/test') - .set('x-forwarded-for', '5.6.7.8, 2.2.2.2, 3.3.3.3') - .send(validReportingAPIPayload) - .expect(429) + .send(validReportingAPIPayload) + .expect(429) + }) }) }) -