From 892040dc2f82a3e2abe2824e4b553521b6f894de Mon Sep 17 00:00:00 2001 From: Manuel <5673677+mtrezza@users.noreply.github.com> Date: Thu, 5 Jan 2023 14:26:54 +0100 Subject: [PATCH] fix: The client IP address may be determined incorrectly in some cases; this fixes a security vulnerability in which the Parse Server option `masterKeyIps` may be circumvented, see [GHSA-vm5r-c87r-pf6x](https://github.com/parse-community/parse-server/security/advisories/GHSA-vm5r-c87r-pf6x) (#8372) BREAKING CHANGE: The mechanism to determine the client IP address has been rewritten; to correctly determine the IP address it is now required to set the Parse Server option `trustProxy` accordingly if Parse Server runs behind a proxy server, see the express framework's [trust proxy](https://expressjs.com/en/guide/behind-proxies.html) setting (#8372) --- spec/Middlewares.spec.js | 111 +--------------------------------- src/Options/Definitions.js | 7 +++ src/Options/docs.js | 1 + src/Options/index.js | 3 + src/ParseServer.js | 3 + src/cloud-code/Parse.Cloud.js | 2 +- src/middlewares.js | 17 +----- 7 files changed, 19 insertions(+), 125 deletions(-) diff --git a/spec/Middlewares.spec.js b/spec/Middlewares.spec.js index 74273e2835..c1544c311a 100644 --- a/spec/Middlewares.spec.js +++ b/spec/Middlewares.spec.js @@ -173,72 +173,6 @@ describe('middlewares', () => { expect(fakeReq.auth.isMaster).toBe(true); }); - it('should not succeed if the connection.remoteAddress does not belong to masterKeyIps list', async () => { - AppCache.put(fakeReq.body._ApplicationId, { - masterKey: 'masterKey', - masterKeyIps: ['10.0.0.1', '10.0.0.2'], - }); - fakeReq.connection = { remoteAddress: '127.0.0.1' }; - fakeReq.headers['x-parse-master-key'] = 'masterKey'; - await new Promise(resolve => middlewares.handleParseHeaders(fakeReq, fakeRes, resolve)); - expect(fakeReq.auth.isMaster).toBe(false); - }); - - it('should succeed if the connection.remoteAddress does belong to masterKeyIps list', async () => { - AppCache.put(fakeReq.body._ApplicationId, { - masterKey: 'masterKey', - masterKeyIps: ['10.0.0.1', '10.0.0.2'], - }); - fakeReq.connection = { remoteAddress: '10.0.0.1' }; - fakeReq.headers['x-parse-master-key'] = 'masterKey'; - await new Promise(resolve => middlewares.handleParseHeaders(fakeReq, fakeRes, resolve)); - expect(fakeReq.auth.isMaster).toBe(true); - }); - - it('should not succeed if the socket.remoteAddress does not belong to masterKeyIps list', async () => { - AppCache.put(fakeReq.body._ApplicationId, { - masterKey: 'masterKey', - masterKeyIps: ['10.0.0.1', '10.0.0.2'], - }); - fakeReq.socket = { remoteAddress: '127.0.0.1' }; - fakeReq.headers['x-parse-master-key'] = 'masterKey'; - await new Promise(resolve => middlewares.handleParseHeaders(fakeReq, fakeRes, resolve)); - expect(fakeReq.auth.isMaster).toBe(false); - }); - - it('should succeed if the socket.remoteAddress does belong to masterKeyIps list', async () => { - AppCache.put(fakeReq.body._ApplicationId, { - masterKey: 'masterKey', - masterKeyIps: ['10.0.0.1', '10.0.0.2'], - }); - fakeReq.socket = { remoteAddress: '10.0.0.1' }; - fakeReq.headers['x-parse-master-key'] = 'masterKey'; - await new Promise(resolve => middlewares.handleParseHeaders(fakeReq, fakeRes, resolve)); - expect(fakeReq.auth.isMaster).toBe(true); - }); - - it('should not succeed if the connection.socket.remoteAddress does not belong to masterKeyIps list', async () => { - AppCache.put(fakeReq.body._ApplicationId, { - masterKey: 'masterKey', - masterKeyIps: ['10.0.0.1', '10.0.0.2'], - }); - fakeReq.connection = { socket: { remoteAddress: 'ip3' } }; - fakeReq.headers['x-parse-master-key'] = 'masterKey'; - await new Promise(resolve => middlewares.handleParseHeaders(fakeReq, fakeRes, resolve)); - expect(fakeReq.auth.isMaster).toBe(false); - }); - - it('should succeed if the connection.socket.remoteAddress does belong to masterKeyIps list', async () => { - AppCache.put(fakeReq.body._ApplicationId, { - masterKey: 'masterKey', - masterKeyIps: ['10.0.0.1', '10.0.0.2'], - }); - fakeReq.connection = { socket: { remoteAddress: '10.0.0.1' } }; - fakeReq.headers['x-parse-master-key'] = 'masterKey'; - await new Promise(resolve => middlewares.handleParseHeaders(fakeReq, fakeRes, resolve)); - expect(fakeReq.auth.isMaster).toBe(true); - }); - it('should allow any ip to use masterKey if masterKeyIps is empty', async () => { AppCache.put(fakeReq.body._ApplicationId, { masterKey: 'masterKey', @@ -250,48 +184,9 @@ describe('middlewares', () => { expect(fakeReq.auth.isMaster).toBe(true); }); - it('should succeed if xff header does belong to masterKeyIps', async () => { - AppCache.put(fakeReq.body._ApplicationId, { - masterKey: 'masterKey', - masterKeyIps: ['10.0.0.1'], - }); - fakeReq.headers['x-parse-master-key'] = 'masterKey'; - fakeReq.headers['x-forwarded-for'] = '10.0.0.1, 10.0.0.2, ip3'; - await new Promise(resolve => middlewares.handleParseHeaders(fakeReq, fakeRes, resolve)); - expect(fakeReq.auth.isMaster).toBe(true); - }); - - it('should succeed if xff header with one ip does belong to masterKeyIps', async () => { - AppCache.put(fakeReq.body._ApplicationId, { - masterKey: 'masterKey', - masterKeyIps: ['10.0.0.1'], - }); - fakeReq.headers['x-parse-master-key'] = 'masterKey'; - fakeReq.headers['x-forwarded-for'] = '10.0.0.1'; - await new Promise(resolve => middlewares.handleParseHeaders(fakeReq, fakeRes, resolve)); - expect(fakeReq.auth.isMaster).toBe(true); - }); - - it('should not succeed if xff header does not belong to masterKeyIps', async () => { - AppCache.put(fakeReq.body._ApplicationId, { - masterKey: 'masterKey', - masterKeyIps: ['ip4'], - }); - fakeReq.headers['x-parse-master-key'] = 'masterKey'; - fakeReq.headers['x-forwarded-for'] = '10.0.0.1, 10.0.0.2, ip3'; - await new Promise(resolve => middlewares.handleParseHeaders(fakeReq, fakeRes, resolve)); - expect(fakeReq.auth.isMaster).toBe(false); - }); - - it('should not succeed if xff header is empty and masterKeyIps is set', async () => { - AppCache.put(fakeReq.body._ApplicationId, { - masterKey: 'masterKey', - masterKeyIps: ['10.0.0.1'], - }); - fakeReq.headers['x-parse-master-key'] = 'masterKey'; - fakeReq.headers['x-forwarded-for'] = ''; - await new Promise(resolve => middlewares.handleParseHeaders(fakeReq, fakeRes, resolve)); - expect(fakeReq.auth.isMaster).toBe(false); + it('can set trust proxy', async () => { + const server = await reconfigureServer({ trustProxy: 1 }); + expect(server.app.parent.settings['trust proxy']).toBe(1); }); it('should properly expose the headers', () => { diff --git a/src/Options/Definitions.js b/src/Options/Definitions.js index 75f9e1c0b4..dda9c38ca5 100644 --- a/src/Options/Definitions.js +++ b/src/Options/Definitions.js @@ -486,6 +486,13 @@ module.exports.ParseServerOptions = { help: 'Starts the liveQuery server', action: parsers.booleanParser, }, + trustProxy: { + env: 'PARSE_SERVER_TRUST_PROXY', + help: + 'The trust proxy settings. It is important to understand the exact setup of the reverse proxy, since this setting will trust values provided in the Parse Server API request. See the express trust proxy settings documentation. Defaults to `false`.', + action: parsers.objectParser, + default: [], + }, userSensitiveFields: { env: 'PARSE_SERVER_USER_SENSITIVE_FIELDS', help: diff --git a/src/Options/docs.js b/src/Options/docs.js index 138f5ca76e..12d6e50f8e 100644 --- a/src/Options/docs.js +++ b/src/Options/docs.js @@ -89,6 +89,7 @@ * @property {Number} sessionLength Session duration, in seconds, defaults to 1 year * @property {Boolean} silent Disables console output * @property {Boolean} startLiveQueryServer Starts the liveQuery server + * @property {Any} trustProxy The trust proxy settings. It is important to understand the exact setup of the reverse proxy, since this setting will trust values provided in the Parse Server API request. See the express trust proxy settings documentation. Defaults to `false`. * @property {String[]} userSensitiveFields Personally identifiable information fields in the user table the should be removed for non-authorized users. Deprecated @see protectedFields * @property {Boolean} verbose Set the logging to verbose * @property {Boolean} verifyUserEmails Set to `true` to require users to verify their email address to complete the sign-up process.

Default is `false`. diff --git a/src/Options/index.js b/src/Options/index.js index 962afe07b5..3e60ab3cac 100644 --- a/src/Options/index.js +++ b/src/Options/index.js @@ -241,6 +241,9 @@ export interface ParseServerOptions { cluster: ?NumberOrBoolean; /* middleware for express server, can be string or function */ middleware: ?((() => void) | string); + /* The trust proxy settings. It is important to understand the exact setup of the reverse proxy, since this setting will trust values provided in the Parse Server API request. See the express trust proxy settings documentation. Defaults to `false`. + :DEFAULT: false */ + trustProxy: ?any; /* Starts the liveQuery server */ startLiveQueryServer: ?boolean; /* Live query server configuration options (will start the liveQuery server) */ diff --git a/src/ParseServer.js b/src/ParseServer.js index 2d1a0f19f3..e4f3fad586 100644 --- a/src/ParseServer.js +++ b/src/ParseServer.js @@ -343,6 +343,9 @@ class ParseServer { options ); } + if (options.trustProxy) { + app.set('trust proxy', options.trustProxy); + } /* istanbul ignore next */ if (!process.env.TESTING) { configureListeners(this); diff --git a/src/cloud-code/Parse.Cloud.js b/src/cloud-code/Parse.Cloud.js index db12067ffe..c71fbc7c52 100644 --- a/src/cloud-code/Parse.Cloud.js +++ b/src/cloud-code/Parse.Cloud.js @@ -722,7 +722,7 @@ module.exports = ParseCloud; * @property {Boolean} isChallenge If true, means the current request is originally triggered by an auth challenge. * @property {Parse.User} user If set, the user that made the request. * @property {Parse.Object} object The object triggering the hook. - * @property {String} ip The IP address of the client making the request. + * @property {String} ip The IP address of the client making the request. To ensure retrieving the correct IP address, set the Parse Server option `trustProxy: true` if Parse Server runs behind a proxy server, for example behind a load balancer. * @property {Object} headers The original HTTP headers for the request. * @property {String} triggerName The name of the trigger (`beforeSave`, `afterSave`, ...) * @property {Object} log The current logger inside Parse Server. diff --git a/src/middlewares.js b/src/middlewares.js index 845efe108d..60f7938c91 100644 --- a/src/middlewares.js +++ b/src/middlewares.js @@ -288,22 +288,7 @@ export function handleParseHeaders(req, res, next) { } function getClientIp(req) { - if (req.headers['x-forwarded-for']) { - // try to get from x-forwared-for if it set (behind reverse proxy) - return req.headers['x-forwarded-for'].split(',')[0]; - } else if (req.connection && req.connection.remoteAddress) { - // no proxy, try getting from connection.remoteAddress - return req.connection.remoteAddress; - } else if (req.socket) { - // try to get it from req.socket - return req.socket.remoteAddress; - } else if (req.connection && req.connection.socket) { - // try to get it form the connection.socket - return req.connection.socket.remoteAddress; - } else { - // if non above, fallback. - return req.ip; - } + return req.ip; } function httpAuth(req) {