From de453b693e307d664387ba6d8e3d91c50a2b2981 Mon Sep 17 00:00:00 2001 From: Benjamin Friedman Date: Mon, 18 Dec 2017 14:36:55 -0800 Subject: [PATCH 1/2] scrub passwords with url encoded characters from logs --- spec/LogsRouter.spec.js | 89 +++++++++++++++++++++++++++++ src/Controllers/LoggerController.js | 2 +- 2 files changed, 90 insertions(+), 1 deletion(-) diff --git a/spec/LogsRouter.spec.js b/spec/LogsRouter.spec.js index 36fab14998..04eac524ec 100644 --- a/spec/LogsRouter.spec.js +++ b/spec/LogsRouter.spec.js @@ -62,4 +62,93 @@ describe('LogsRouter', () => { done(); }); }); + + const headers = { + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'rest', + 'X-Parse-Master-Key': 'test' + }; + + /** + * Verifies simple passwords in GET login requests with special characters are scrubbed from the verbose log + */ + it('does scrub simple passwords on GET login', done => { + reconfigureServer({ + verbose: true + }).then(function() { + request.get({ + headers: headers, + url: 'http://localhost:8378/1/login?username=test&password=simplepass.com' + }, () => { + request.get({ + url: 'http://localhost:8378/1/scriptlog?size=4&level=verbose', + json: true, + headers: headers + }, (error, response, body) => { + expect(response.statusCode).toEqual(200); + // 4th entry is our actual GET request + expect(body[3].url).toEqual('/1/login?username=test&password=********'); + expect(body[3].message).toEqual('REQUEST for [GET] /1/login?username=test&password=********: {}'); + done(); + }); + }); + }); + }); + + /** + * Verifies complex passwords in GET login requests with special characters are scrubbed from the verbose log + */ + it('does scrub complex passwords on GET login', done => { + reconfigureServer({ + verbose: true + }).then(function() { + request.get({ + headers: headers, + // using urlencoded password, 'simple @,/?:&=+$#pass.com' + url: 'http://localhost:8378/1/login?username=test&password=simple%20%40%2C%2F%3F%3A%26%3D%2B%24%23pass.com' + }, () => { + request.get({ + url: 'http://localhost:8378/1/scriptlog?size=4&level=verbose', + json: true, + headers: headers + }, (error, response, body) => { + expect(response.statusCode).toEqual(200); + // 4th entry is our actual GET request + expect(body[3].url).toEqual('/1/login?username=test&password=********'); + expect(body[3].message).toEqual('REQUEST for [GET] /1/login?username=test&password=********: {}'); + done(); + }); + }); + }); + }); + + /** + * Verifies fields in POST login requests are NOT present in the verbose log + */ + it('does not have password field in POST login', done => { + reconfigureServer({ + verbose: true + }).then(function() { + request.post({ + headers: headers, + url: 'http://localhost:8378/1/login', + data: { + username: 'test', + password: 'simplepass.com' + } + }, () => { + request.get({ + url: 'http://localhost:8378/1/scriptlog?size=4&level=verbose', + json: true, + headers: headers + }, (error, response, body) => { + expect(response.statusCode).toEqual(200); + // 4th entry is our actual GET request + expect(body[3].url).toEqual('/1/login'); + expect(body[3].message).toEqual('REQUEST for [POST] /1/login: {}'); + done(); + }); + }); + }); + }); }); diff --git a/src/Controllers/LoggerController.js b/src/Controllers/LoggerController.js index 5e5da68363..f8e52bc200 100644 --- a/src/Controllers/LoggerController.js +++ b/src/Controllers/LoggerController.js @@ -49,7 +49,7 @@ export class LoggerController extends AdaptableController { const password = url.parse(urlString, true).query.password; if (password) { - urlString = urlString.replace('password=' + password, 'password=********'); + urlString = urlString.replace('password=' + encodeURIComponent(password), 'password=********'); } return urlString; } From 274658994a9f464c712567c97a8eef60f03e4595 Mon Sep 17 00:00:00 2001 From: Benjamin Friedman Date: Mon, 18 Dec 2017 21:37:27 -0800 Subject: [PATCH 2/2] compose query string from parsed params, redacting based on key if needed --- src/Controllers/LoggerController.js | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/Controllers/LoggerController.js b/src/Controllers/LoggerController.js index f8e52bc200..cc8f5318e3 100644 --- a/src/Controllers/LoggerController.js +++ b/src/Controllers/LoggerController.js @@ -46,12 +46,25 @@ export class LoggerController extends AdaptableController { } maskSensitiveUrl(urlString) { - const password = url.parse(urlString, true).query.password; - - if (password) { - urlString = urlString.replace('password=' + encodeURIComponent(password), 'password=********'); + const urlObj = url.parse(urlString, true); + const query = urlObj.query; + let sanitizedQuery = '?'; + + for(const key in query) { + if(key !== 'password') { + // normal value + sanitizedQuery += key + '=' + query[key] + '&'; + } else { + // password value, redact it + sanitizedQuery += key + '=' + '********' + '&'; + } } - return urlString; + + // trim last character, ? or & + sanitizedQuery = sanitizedQuery.slice(0, -1); + + // return original path name with sanitized params attached + return urlObj.pathname + sanitizedQuery; } maskSensitive(argArray) {