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

[FEATURE] csp: enable tracking and serving of csp reports #323

Merged
merged 11 commits into from
Jun 9, 2020
8 changes: 7 additions & 1 deletion lib/middleware/MiddlewareManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ const MiddlewareUtil = require("./MiddlewareUtil");
*/
class MiddlewareManager {
constructor({tree, resources, options = {
sendSAPTargetCSP: false
sendSAPTargetCSP: false,
serveCSPReports: false
}}) {
if (!tree || !resources || !resources.all || !resources.rootProject || !resources.dependencies) {
throw new Error("[MiddlewareManager]: One or more mandatory parameters not provided");
Expand Down Expand Up @@ -118,6 +119,11 @@ class MiddlewareManager {
defaultPolicy2IsReportOnly: true,
});
}
if (this.options.serveCSPReports) {
Object.assign(oCspConfig, {
serveCSPReports: true,
});
}
return () => {
return cspModule("sap-ui-xx-csp-policy", oCspConfig);
};
Expand Down
97 changes: 83 additions & 14 deletions lib/middleware/csp.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
const parseurl = require("parseurl");
const Router = require("router");
const querystring = require("querystring");

const log = require("@ui5/logger").getLogger("server:middleware:csp");

const HEADER_CONTENT_SECURITY_POLICY = "Content-Security-Policy";
const HEADER_CONTENT_SECURITY_POLICY_REPORT_ONLY = "Content-Security-Policy-Report-Only";
const rPolicy = /^([-_a-zA-Z0-9]+)(:report-only|:ro)?$/i;
Expand All @@ -16,6 +19,27 @@ function addHeader(res, header, value) {
}
}


/**
* @typedef {object} CspConfig
* @property {boolean} allowDynamicPolicySelection
* @property {boolean} allowDynamicPolicyDefinition
* @property {string} defaultPolicy
* @property {boolean} defaultPolicyIsReportOnly
* @property {string} defaultPolicy2
* @property {boolean} defaultPolicy2IsReportOnly
* @property {object} definedPolicies
* @property {boolean} serveCSPReports whether to serve the csp resources
*/

/**
* @module @ui5/server/middleware/csp
* Middleware which enables CSP (content security policy) support
* @see https://www.w3.org/TR/CSP/
* @param {string} sCspUrlParameterName
* @param {CspConfig} oConfig
* @returns {Function} Returns a server middleware closure.
*/
function createMiddleware(sCspUrlParameterName, oConfig) {
const {
allowDynamicPolicySelection = false,
Expand All @@ -24,22 +48,65 @@ function createMiddleware(sCspUrlParameterName, oConfig) {
defaultPolicyIsReportOnly = false,
defaultPolicy2 = null,
defaultPolicy2IsReportOnly = false,
definedPolicies = {}
definedPolicies = {},
serveCSPReports = false
} = oConfig;

return function csp(req, res, next) {
const oParsedURL = parseurl(req);

if (req.method === "POST" ) {
if (req.headers["content-type"] === "application/csp-report" &&
oParsedURL.pathname.endsWith("/dummy.csplog") ) {
// In report-only mode there must be a report-uri defined
// For now just ignore the violation. It will be logged in the browser anyway.
/**
* List of CSP Report entries
*/
const cspReportEntries = [];
const router = new Router();
// .csplog
// body parser is required to parse csp-report in body (json)
if (serveCSPReports) {
const bodyParser = require("body-parser");
router.post("/.ui5/csp/report.csplog", bodyParser.json({type: "application/csp-report"}));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need the body-parser middleware? Can't we just parse the JSON string?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I also thought initially, but the body-parser does way more than that. It reads the request stream, handles encoding, etc., so I don't think we should write that on our own.

But this way it's only done for that specific path, so no conflict with other middlewares and no overhead to handle other body content.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, sounds good 👍

}
router.post("/.ui5/csp/report.csplog", function(req, res, next) {
if (req.headers["content-type"] === "application/csp-report") {
if (!serveCSPReports) {
res.end();
return;
}
// Write the violation into an array
// They can be retrieved via a request to '/.ui5/csp/csp-reports.json'
if (typeof req.body !== "object") {
const error = new Error(`No body content available: ${req.url}`);
log.error(error);
next(error);
return;
}
const cspReportObject = req.body["csp-report"];
if (cspReportObject) {
// extract the csp-report and add it to the cspReportEntries list
cspReportEntries.push(cspReportObject);
}
res.end();
} else {
next();
return;
}
});

// csp-reports.json
if (serveCSPReports) {
router.get("/.ui5/csp/csp-reports.json", (req, res, next) => {
// serve csp reports
const body = JSON.stringify({
"csp-reports": cspReportEntries
}, null, "\t");
res.writeHead(200, {
"Content-Type": "application/json"
});
res.end(body);
});
}

// html get requests
// add csp headers
router.use((req, res, next) => {
const oParsedURL = parseurl(req);

// add CSP headers only to get requests for *.html pages
if (req.method !== "GET" || !oParsedURL.pathname.endsWith(".html")) {
Expand Down Expand Up @@ -81,23 +148,25 @@ function createMiddleware(sCspUrlParameterName, oConfig) {
// collect header values based on configuration
if (policy) {
if (reportOnly) {
// Add dummy report-uri. This is mandatory for the report-only mode.
addHeader(res, HEADER_CONTENT_SECURITY_POLICY_REPORT_ONLY, policy + " report-uri dummy.csplog;");
// Add report-uri. This is mandatory for the report-only mode.
addHeader(res, HEADER_CONTENT_SECURITY_POLICY_REPORT_ONLY, policy + " report-uri /.ui5/csp/report.csplog;");
} else {
addHeader(res, HEADER_CONTENT_SECURITY_POLICY, policy);
}
}
if (policy2) {
if (reportOnly2) {
// Add dummy report-uri. This is mandatory for the report-only mode.
addHeader(res, HEADER_CONTENT_SECURITY_POLICY_REPORT_ONLY, policy2 + " report-uri dummy.csplog;");
// Add report-uri. This is mandatory for the report-only mode.
addHeader(res, HEADER_CONTENT_SECURITY_POLICY_REPORT_ONLY, policy2 + " report-uri /.ui5/csp/report.csplog;");
} else {
addHeader(res, HEADER_CONTENT_SECURITY_POLICY, policy2);
}
}

next();
};
});

return router;
}

module.exports = createMiddleware;
4 changes: 3 additions & 1 deletion lib/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,15 @@ module.exports = {
* aim for (AKA 'target policies'), are send for any requested
* <code>*.html</code> file
* @param {boolean} [options.simpleIndex=false] Use a simplified view for the server directory listing
* @param {boolean} [options.serveCSPReports=false] Enable csp reports serving for request url '/.ui5/csp/csp-reports.json'
* @returns {Promise<object>} Promise resolving once the server is listening.
* It resolves with an object containing the <code>port</code>,
* <code>h2</code>-flag and a <code>close</code> function,
* which can be used to stop the server.
*/
async serve(tree, {
port: requestedPort, changePortIfInUse = false, h2 = false, key, cert,
acceptRemoteConnections = false, sendSAPTargetCSP = false, simpleIndex = false
acceptRemoteConnections = false, sendSAPTargetCSP = false, simpleIndex = false, serveCSPReports = false
}) {
const projectResourceCollections = resourceFactory.createCollectionsForTree(tree);

Expand All @@ -140,6 +141,7 @@ module.exports = {
resources,
options: {
sendSAPTargetCSP,
serveCSPReports,
simpleIndex
}
});
Expand Down
26 changes: 26 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@
"@ui5/builder": "^2.0.3",
"@ui5/fs": "^2.0.1",
"@ui5/logger": "^2.0.0",
"body-parser": "^1.19.0",
"compression": "^1.7.4",
"connect-openui5": "^0.9.0",
"cors": "^2.8.5",
Expand All @@ -114,6 +115,7 @@
"parseurl": "^1.3.3",
"portscanner": "^2.1.1",
"replacestream": "^4.0.3",
"router": "^1.3.5",
"spdy": "^4.0.2",
"treeify": "^1.0.1",
"yesno": "^0.3.1"
Expand Down
55 changes: 55 additions & 0 deletions test/lib/server/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,61 @@ test("CSP (sap policies)", (t) => {
});
});

test("CSP serveCSPReports", (t) => {
const port = 3450;
const request = supertest(`http://localhost:${port}`);
let localServeResult;
return normalizer.generateProjectTree({
cwd: "./test/fixtures/application.a"
}).then((tree) => {
return server.serve(tree, {
port,
serveCSPReports: true,
simpleIndex: false
});
}).then((serveResult) => {
localServeResult = serveResult;
const cspReport = {
"csp-report": {
"document-uri": "https://otherserver:8080/index.html",
"referrer": "",
"violated-directive": "script-src-elem",
"effective-directive": "script-src-elem",
"original-policy": "default-src 'self' myserver:443; report-uri /report-csp-violation",
"disposition": "report",
"blocked-uri": "inline",
"line-number": 17,
"source-file": "https://otherserver:8080/index.html",
"status-code": 0,
"script-sample": ""
}
};
return request.post("/.ui5/csp/report.csplog")
.set("Content-Type", "application/csp-report")
// to allow setting the content type the argument for sending must be a string
.send(JSON.stringify(cspReport))
.expect(200);
}).then(() => {
return request.get("/.ui5/csp/csp-reports.json")
.then((res) => {
t.true(typeof res.body === "object", "the body is an object");
t.true(Array.isArray(res.body["csp-reports"]), "csp-reports is an array");
t.is(res.body["csp-reports"].length, 1, "one csp report in result");
});
}).then(() => {
return new Promise((resolve, reject) => {
localServeResult.close((error) => {
if (error) {
reject(error);
} else {
t.pass("Server closing");
resolve();
}
});
});
});
});

test("Get index of resources", (t) => {
return Promise.all([
request.get("").then((res) => {
Expand Down
Loading