From ae8e20b8e1e8a6f9fae118dfae6e090a40e88896 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Thu, 13 May 2021 10:23:22 -0400 Subject: [PATCH 1/6] refactor: Create services folder in desktop/electron-app --- ui/desktop/electron-app/src/ipc/handlers.js | 2 +- .../src/{session/manager.js => services/session-manager.js} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename ui/desktop/electron-app/src/{session/manager.js => services/session-manager.js} (100%) diff --git a/ui/desktop/electron-app/src/ipc/handlers.js b/ui/desktop/electron-app/src/ipc/handlers.js index e727a4b48..befb10dda 100644 --- a/ui/desktop/electron-app/src/ipc/handlers.js +++ b/ui/desktop/electron-app/src/ipc/handlers.js @@ -3,7 +3,7 @@ const isDev = require('electron-is-dev'); const handle = require('./ipc-handler.js'); const boundaryCli = require('../cli/index.js'); const origin = require('../origin/index.js'); -const sessionManager = require('../session/manager.js'); +const sessionManager = require('../services/session-manager.js'); /** * Returns the current runtime origin, which is used by the main thread to diff --git a/ui/desktop/electron-app/src/session/manager.js b/ui/desktop/electron-app/src/services/session-manager.js similarity index 100% rename from ui/desktop/electron-app/src/session/manager.js rename to ui/desktop/electron-app/src/services/session-manager.js From 9f32b8cdc3654387ab7f09decf219d701c9380ee Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Thu, 13 May 2021 10:36:28 -0400 Subject: [PATCH 2/6] refactor: Origin runtime settings into origin service desktop --- .../src/config/content-security-policy.js | 2 +- .../src/helpers/net-request-promise.js | 24 ++++++++++++++ ui/desktop/electron-app/src/index.js | 2 +- ui/desktop/electron-app/src/ipc/handlers.js | 2 +- .../index.js => models/runtime-settings.js} | 33 ++----------------- .../electron-app/src/services/origin.js | 8 +++++ 6 files changed, 37 insertions(+), 34 deletions(-) create mode 100644 ui/desktop/electron-app/src/helpers/net-request-promise.js rename ui/desktop/electron-app/src/{origin/index.js => models/runtime-settings.js} (59%) create mode 100644 ui/desktop/electron-app/src/services/origin.js diff --git a/ui/desktop/electron-app/src/config/content-security-policy.js b/ui/desktop/electron-app/src/config/content-security-policy.js index 8895178ef..1de13924e 100644 --- a/ui/desktop/electron-app/src/config/content-security-policy.js +++ b/ui/desktop/electron-app/src/config/content-security-policy.js @@ -1,4 +1,4 @@ -const origin = require('../origin/index.js'); +const origin = require('../services/origin.js'); const isDev = require('electron-is-dev'); const csp = { diff --git a/ui/desktop/electron-app/src/helpers/net-request-promise.js b/ui/desktop/electron-app/src/helpers/net-request-promise.js new file mode 100644 index 000000000..eb48426c7 --- /dev/null +++ b/ui/desktop/electron-app/src/helpers/net-request-promise.js @@ -0,0 +1,24 @@ +const { net } = require('electron'); + +const requestTimeoutSeconds = 10; +// Simple promise wrapper around Electron's net.request feature. +// https://www.electronjs.org/docs/api/client-request +const netRequestPromise = (url) => + new Promise((resolve, reject) => { + try { + const request = net.request(url); + // We don't worry about canceling the request timeout timer once a + // request ends because: request.abort() has no effect after a request + // ends and the promise will already have been completed, so a rejection + // has no effect either. + const requestTimeout = setTimeout(() => { + request.abort(); + reject(new Error('Request timeout')); + }, requestTimeoutSeconds * 1000); + request.on('error', reject); + request.on('response', resolve); + request.end(); + } catch (e) { + reject(e); + } + }); diff --git a/ui/desktop/electron-app/src/index.js b/ui/desktop/electron-app/src/index.js index 3ff94bf64..8e61bea48 100644 --- a/ui/desktop/electron-app/src/index.js +++ b/ui/desktop/electron-app/src/index.js @@ -15,7 +15,7 @@ const { } = require('electron'); require('./ipc/handlers.js'); -const origin = require('./origin/index.js'); +const origin = require('./services/origin.js'); const { generateCSPHeader } = require('./config/content-security-policy.js'); const menu = require('./config/menu.js'); diff --git a/ui/desktop/electron-app/src/ipc/handlers.js b/ui/desktop/electron-app/src/ipc/handlers.js index befb10dda..f32a00e28 100644 --- a/ui/desktop/electron-app/src/ipc/handlers.js +++ b/ui/desktop/electron-app/src/ipc/handlers.js @@ -2,7 +2,7 @@ const { shell } = require('electron'); const isDev = require('electron-is-dev'); const handle = require('./ipc-handler.js'); const boundaryCli = require('../cli/index.js'); -const origin = require('../origin/index.js'); +const origin = require('../services/origin.js'); const sessionManager = require('../services/session-manager.js'); /** diff --git a/ui/desktop/electron-app/src/origin/index.js b/ui/desktop/electron-app/src/models/runtime-settings.js similarity index 59% rename from ui/desktop/electron-app/src/origin/index.js rename to ui/desktop/electron-app/src/models/runtime-settings.js index 5cc150c32..fdd91608c 100644 --- a/ui/desktop/electron-app/src/origin/index.js +++ b/ui/desktop/electron-app/src/models/runtime-settings.js @@ -1,34 +1,7 @@ -const { net } = require('electron'); - -const requestTimeoutSeconds = 10; -// Simple promise wrapper around Electron's net.request feature. -// https://www.electronjs.org/docs/api/client-request -const netRequestPromise = (url) => - new Promise((resolve, reject) => { - try { - const request = net.request(url); - // We don't worry about canceling the request timeout timer once a - // request ends because: request.abort() has no effect after a request - // ends and the promise will already have been completed, so a rejection - // has no effect either. - const requestTimeout = setTimeout(() => { - request.abort(); - reject(new Error('Request timeout')); - }, requestTimeoutSeconds * 1000); - request.on('error', reject); - request.on('response', resolve); - request.end(); - } catch (e) { - reject(e); - } - }); - -// Provides a singleton class instance to enable a consistent view of -// runtime settings across the application. +const netRequestPromise = require('../helpers/net-request-promise.js'); // Runtime settings are any configuration, especially user-specified, // set up at runtime. For example, the Boundary origin is a runtime setting. - class RuntimeSettings { // Internal private origin is exposed via getter/setter below. #origin = undefined; @@ -84,6 +57,4 @@ class RuntimeSettings { } } -const runtimeSettings = new RuntimeSettings(); - -module.exports = runtimeSettings; +module.exports = RuntimeSettings; diff --git a/ui/desktop/electron-app/src/services/origin.js b/ui/desktop/electron-app/src/services/origin.js new file mode 100644 index 000000000..8d33f04fc --- /dev/null +++ b/ui/desktop/electron-app/src/services/origin.js @@ -0,0 +1,8 @@ +const RuntimeSesttings = require('../models/runtime-settings.js'); + +// Provides a singleton class instance to enable a consistent view of +// runtime settings across the application. + +const runtimeSettings = new RuntimeSettings(); + +module.exports = runtimeSettings; From cc01bddea32c27788ad00eb858cd8421e346b2f3 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Thu, 13 May 2021 12:50:01 -0400 Subject: [PATCH 3/6] refactor: Create net-request-promise helper for runtimesettin --- ui/desktop/electron-app/src/cli/index.js | 2 +- .../electron-app/src/helpers/net-request-promise.js | 2 ++ ui/desktop/electron-app/src/models/session.js | 11 +++++------ ui/desktop/electron-app/src/services/origin.js | 2 +- .../src/utils/{base62.js => sanitizer.js} | 10 +++++++++- 5 files changed, 18 insertions(+), 9 deletions(-) rename ui/desktop/electron-app/src/utils/{base62.js => sanitizer.js} (58%) diff --git a/ui/desktop/electron-app/src/cli/index.js b/ui/desktop/electron-app/src/cli/index.js index cfb5153a5..0aaad4e31 100644 --- a/ui/desktop/electron-app/src/cli/index.js +++ b/ui/desktop/electron-app/src/cli/index.js @@ -1,7 +1,7 @@ const path = require('path'); const { spawnSync } = require('../helpers/spawn-promise.js'); -const cliPath = async () => path.resolve(__dirname, '..', 'cli', 'boundary'); +const cliPath = async () => path.resolve(__dirname, '..', '..', 'cli', 'boundary'); module.exports = { // Check boundary cli existence diff --git a/ui/desktop/electron-app/src/helpers/net-request-promise.js b/ui/desktop/electron-app/src/helpers/net-request-promise.js index eb48426c7..c86030c79 100644 --- a/ui/desktop/electron-app/src/helpers/net-request-promise.js +++ b/ui/desktop/electron-app/src/helpers/net-request-promise.js @@ -22,3 +22,5 @@ const netRequestPromise = (url) => reject(e); } }); + +module.exports = netRequestPromise; diff --git a/ui/desktop/electron-app/src/models/session.js b/ui/desktop/electron-app/src/models/session.js index 105b7d723..20d5b37a4 100644 --- a/ui/desktop/electron-app/src/models/session.js +++ b/ui/desktop/electron-app/src/models/session.js @@ -1,5 +1,5 @@ const { spawnAsyncJSONPromise } = require('../helpers/spawn-promise.js'); -const base62 = require('../utils/base62.js'); +const sanitizer = require('../utils/sanitizer.js'); class Session { #id; @@ -77,21 +77,20 @@ class Session { */ cliCommand() { const sanitized = { - target_id: base62.escapeAndValidate(this.#targetId), - token: base62.escapeAndValidate(this.#token), - addr: encodeURI(this.#addr), + target_id: sanitizer.base62EscapeAndValidate(this.#targetId), + token: sanitizer.base62EscapeAndValidate(this.#token), }; const command = [ 'connect', `-target-id=${sanitized.target_id}`, `-token=${sanitized.token}`, - `-addr=${sanitized.addr}`, + `-addr=${this.#addr}`, '-format=json', ]; if (this.#hostId) { - sanitized.host_id = base62.escapeAndValidate(this.#hostId); + sanitized.host_id = sanitizer.base62EscapeAndValidate(this.#hostId); command.push(`-host-id=${sanitized.host_id}`); } return command; diff --git a/ui/desktop/electron-app/src/services/origin.js b/ui/desktop/electron-app/src/services/origin.js index 8d33f04fc..8c5634079 100644 --- a/ui/desktop/electron-app/src/services/origin.js +++ b/ui/desktop/electron-app/src/services/origin.js @@ -1,4 +1,4 @@ -const RuntimeSesttings = require('../models/runtime-settings.js'); +const RuntimeSettings = require('../models/runtime-settings.js'); // Provides a singleton class instance to enable a consistent view of // runtime settings across the application. diff --git a/ui/desktop/electron-app/src/utils/base62.js b/ui/desktop/electron-app/src/utils/sanitizer.js similarity index 58% rename from ui/desktop/electron-app/src/utils/base62.js rename to ui/desktop/electron-app/src/utils/sanitizer.js index 0c844b790..bc48fe0cf 100644 --- a/ui/desktop/electron-app/src/utils/base62.js +++ b/ui/desktop/electron-app/src/utils/sanitizer.js @@ -1,9 +1,11 @@ +const { quote } = require('shell-quote'); + module.exports = { /** * Super paranoid shell quote/escape and validation. Input must be base62. * @param {string} str */ - escapeAndValidate: (str) => { + base62EscapeAndValidate: (str) => { const candidate = str.toString(); if (candidate.match(/^[A-Za-z0-9_]*$/)) return candidate; throw new Error(` @@ -11,4 +13,10 @@ module.exports = { input contained unsafe characters. `); }, + /** + * Return a quoted string suitable for using in shell commands. + * @param {string} str + * @returns {string} + */ + shellEscape: (str) => quote([str]), }; From 1397df807f83d144ee563822f74fbde525b94073 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Fri, 14 May 2021 10:40:50 -0400 Subject: [PATCH 4/6] fix: Add url validation to origin addr in desktop --- ui/desktop/electron-app/src/models/session.js | 3 ++- ui/desktop/electron-app/src/utils/sanitizer.js | 15 +++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/ui/desktop/electron-app/src/models/session.js b/ui/desktop/electron-app/src/models/session.js index 20d5b37a4..6ec9d3ce8 100644 --- a/ui/desktop/electron-app/src/models/session.js +++ b/ui/desktop/electron-app/src/models/session.js @@ -79,13 +79,14 @@ class Session { const sanitized = { target_id: sanitizer.base62EscapeAndValidate(this.#targetId), token: sanitizer.base62EscapeAndValidate(this.#token), + addr: sanitizer.urlValidate(this.#addr), }; const command = [ 'connect', `-target-id=${sanitized.target_id}`, `-token=${sanitized.token}`, - `-addr=${this.#addr}`, + `-addr=${sanitized.addr}`, '-format=json', ]; diff --git a/ui/desktop/electron-app/src/utils/sanitizer.js b/ui/desktop/electron-app/src/utils/sanitizer.js index bc48fe0cf..582f2532c 100644 --- a/ui/desktop/electron-app/src/utils/sanitizer.js +++ b/ui/desktop/electron-app/src/utils/sanitizer.js @@ -1,4 +1,4 @@ -const { quote } = require('shell-quote'); +const { URL } = require('url'); module.exports = { /** @@ -18,5 +18,16 @@ module.exports = { * @param {string} str * @returns {string} */ - shellEscape: (str) => quote([str]), + urlValidate: (str) => { + try { + const url = new URL(str); + return url.href; + } catch (e) { + // Parse error + throw new Error(` + Could not invoke command: + input contained unsafe url. + `); + } + }, }; From 82a0c7c17cb912bc19ef63fc11ce80eaa01def86 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Fri, 14 May 2021 14:29:26 -0400 Subject: [PATCH 5/6] refactor: Rename origin service to runtime-settings --- .../src/config/content-security-policy.js | 6 +++--- ui/desktop/electron-app/src/index.js | 4 ++-- ui/desktop/electron-app/src/ipc/handlers.js | 12 ++++++------ .../src/services/{origin.js => runtime-settings.js} | 0 4 files changed, 11 insertions(+), 11 deletions(-) rename ui/desktop/electron-app/src/services/{origin.js => runtime-settings.js} (100%) diff --git a/ui/desktop/electron-app/src/config/content-security-policy.js b/ui/desktop/electron-app/src/config/content-security-policy.js index 1de13924e..19ccafc93 100644 --- a/ui/desktop/electron-app/src/config/content-security-policy.js +++ b/ui/desktop/electron-app/src/config/content-security-policy.js @@ -1,4 +1,4 @@ -const origin = require('../services/origin.js'); +const runtimeSettings = require('../services/runtime-settings.js'); const isDev = require('electron-is-dev'); const csp = { @@ -28,9 +28,9 @@ const generateCSPHeader = () => { const policy = Object.assign({}, csp); // If an origin is specified, add it to the connect-src directive - if (origin.origin) { + if (runtimeSettings.origin) { policy['connect-src'] = policy['connect-src'].slice(); - policy['connect-src'].push(origin.origin); + policy['connect-src'].push(runtimeSettings.origin); } return Object.keys(policy) diff --git a/ui/desktop/electron-app/src/index.js b/ui/desktop/electron-app/src/index.js index 8e61bea48..93c9c3024 100644 --- a/ui/desktop/electron-app/src/index.js +++ b/ui/desktop/electron-app/src/index.js @@ -15,7 +15,7 @@ const { } = require('electron'); require('./ipc/handlers.js'); -const origin = require('./services/origin.js'); +const runtimeSettings = require('./services/runtime-settings.js'); const { generateCSPHeader } = require('./config/content-security-policy.js'); const menu = require('./config/menu.js'); @@ -131,7 +131,7 @@ app.on('ready', async () => { // If the user-specified origin changes, reload the page so that // the CSP can be refreshed with the this source allowed - origin.onOriginChange(() => mainWindow.loadURL(emberAppURL)); + runtimeSettings.onOriginChange(() => mainWindow.loadURL(emberAppURL)); // If you want to open up dev tools programmatically, call // mainWindow.openDevTools(); diff --git a/ui/desktop/electron-app/src/ipc/handlers.js b/ui/desktop/electron-app/src/ipc/handlers.js index f32a00e28..e3bdcbabd 100644 --- a/ui/desktop/electron-app/src/ipc/handlers.js +++ b/ui/desktop/electron-app/src/ipc/handlers.js @@ -2,28 +2,28 @@ const { shell } = require('electron'); const isDev = require('electron-is-dev'); const handle = require('./ipc-handler.js'); const boundaryCli = require('../cli/index.js'); -const origin = require('../services/origin.js'); const sessionManager = require('../services/session-manager.js'); +const runtimeSettings = require('../services/runtime-settings.js'); /** * Returns the current runtime origin, which is used by the main thread to * rewrite the CSP to allow requests. */ -handle('getOrigin', () => origin.origin); +handle('getOrigin', () => runtimeSettings.origin); /** * Sets the origin to be used in the content security policy and triggers * a main window reload. */ handle('setOrigin', async (requestOrigin) => { - await origin.validateOrigin(requestOrigin); - origin.origin = requestOrigin; + await runtimeSettings.validateOrigin(requestOrigin); + runtimeSettings.origin = requestOrigin; }); /** * Resets the origin. */ -handle('resetOrigin', async () => origin.resetOrigin()); +handle('resetOrigin', async () => runtimeSettings.resetOrigin()); /** * Opens the specified URL in an external browser. Only secure HTTPs URLs are @@ -55,7 +55,7 @@ handle('cliExists', () => boundaryCli.exists()); * Establishes a boundary session and returns session details. */ handle('connect', ({ target_id, token, host_id }) => - sessionManager.start(origin.origin, target_id, token, host_id) + sessionManager.start(runtimeSettings.origin, target_id, token, host_id) ); /** diff --git a/ui/desktop/electron-app/src/services/origin.js b/ui/desktop/electron-app/src/services/runtime-settings.js similarity index 100% rename from ui/desktop/electron-app/src/services/origin.js rename to ui/desktop/electron-app/src/services/runtime-settings.js From 1a0650ff7cee6bc9db6c6a8a381a0de946d43130 Mon Sep 17 00:00:00 2001 From: Susmitha Girumala Date: Fri, 14 May 2021 15:00:24 -0400 Subject: [PATCH 6/6] fix: Add url validation to origin validation in desktop --- ui/desktop/electron-app/src/ipc/handlers.js | 6 ++++-- ui/desktop/electron-app/src/utils/sanitizer.js | 7 ++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/ui/desktop/electron-app/src/ipc/handlers.js b/ui/desktop/electron-app/src/ipc/handlers.js index e3bdcbabd..529fc34a5 100644 --- a/ui/desktop/electron-app/src/ipc/handlers.js +++ b/ui/desktop/electron-app/src/ipc/handlers.js @@ -4,6 +4,7 @@ const handle = require('./ipc-handler.js'); const boundaryCli = require('../cli/index.js'); const sessionManager = require('../services/session-manager.js'); const runtimeSettings = require('../services/runtime-settings.js'); +const sanitizer = require('../utils/sanitizer.js'); /** * Returns the current runtime origin, which is used by the main thread to @@ -16,8 +17,9 @@ handle('getOrigin', () => runtimeSettings.origin); * a main window reload. */ handle('setOrigin', async (requestOrigin) => { - await runtimeSettings.validateOrigin(requestOrigin); - runtimeSettings.origin = requestOrigin; + const origin = sanitizer.urlValidate(requestOrigin); + await runtimeSettings.validateOrigin(origin); + runtimeSettings.origin = origin; }); /** diff --git a/ui/desktop/electron-app/src/utils/sanitizer.js b/ui/desktop/electron-app/src/utils/sanitizer.js index 582f2532c..9e635b0d5 100644 --- a/ui/desktop/electron-app/src/utils/sanitizer.js +++ b/ui/desktop/electron-app/src/utils/sanitizer.js @@ -21,13 +21,10 @@ module.exports = { urlValidate: (str) => { try { const url = new URL(str); - return url.href; + return `${url.protocol}://${url.host}`; } catch (e) { // Parse error - throw new Error(` - Could not invoke command: - input contained unsafe url. - `); + throw new Error(`URL ${str} could not be validated.`); } }, };