From 7dec0bd2bc855b8f9d3034496a606bd87c36d76e Mon Sep 17 00:00:00 2001 From: Ben Schwartz Date: Thu, 17 Mar 2022 10:37:14 -0400 Subject: [PATCH] fix: Limit trust anchor scope to a single host --- src/server_manager/electron_app/index.ts | 15 +++++++++++---- src/server_manager/electron_app/preload.ts | 6 +++--- src/server_manager/electron_app/util.ts | 6 ++++++ src/server_manager/types/preload.d.ts | 7 ++++++- src/server_manager/web_app/browser_main.ts | 5 +++-- src/server_manager/web_app/digitalocean_server.ts | 3 ++- src/server_manager/web_app/gcp_server.ts | 9 ++++++++- src/server_manager/web_app/manual_server.ts | 4 +++- 8 files changed, 42 insertions(+), 13 deletions(-) diff --git a/src/server_manager/electron_app/index.ts b/src/server_manager/electron_app/index.ts index 933ed8c7..0ec73f80 100644 --- a/src/server_manager/electron_app/index.ts +++ b/src/server_manager/electron_app/index.ts @@ -20,6 +20,7 @@ import * as path from 'path'; import {URL, URLSearchParams} from 'url'; import * as menu from './menu'; +import {HostAnchor} from './util'; const app = electron.app; const ipcMain = electron.ipcMain; @@ -239,14 +240,20 @@ function main() { }); // Handle request to trust the certificate from the renderer process. - const trustedFingerprints = new Set(); - ipcMain.on('trust-certificate', (event: IpcEvent, fingerprint: string) => { - trustedFingerprints.add(`sha256/${fingerprint}`); + const trustedFingerprints = new Map(); + ipcMain.on('trust-certificate', (event: IpcEvent, anchor: HostAnchor) => { + trustedFingerprints.set(anchor.host, `sha256/${anchor.fingerprint}`); event.returnValue = true; }); app.on('certificate-error', (event, webContents, url, error, certificate, callback) => { event.preventDefault(); - callback(trustedFingerprints.has(certificate.fingerprint)); + try { + const parsed = new URL(url); + callback(trustedFingerprints.get(parsed.host) === certificate.fingerprint); + } catch (e) { + console.error(e); + callback(false); + } }); // Restores the mainWindow if minimized and brings it into focus. diff --git a/src/server_manager/electron_app/preload.ts b/src/server_manager/electron_app/preload.ts index ad70cae6..6dcd03fd 100644 --- a/src/server_manager/electron_app/preload.ts +++ b/src/server_manager/electron_app/preload.ts @@ -18,7 +18,7 @@ import {URL} from 'url'; import * as digitalocean_oauth from './digitalocean_oauth'; import * as gcp_oauth from './gcp_oauth'; -import {redactManagerUrl} from './util'; +import {HostAnchor, redactManagerUrl} from './util'; // This file is run in the renderer process *before* nodeIntegration is disabled. // @@ -47,8 +47,8 @@ if (sentryDsn) { }); } -contextBridge.exposeInMainWorld('trustCertificate', (fingerprint: string) => { - return ipcRenderer.sendSync('trust-certificate', fingerprint); +contextBridge.exposeInMainWorld('trustCertificate', (anchor: HostAnchor) => { + return ipcRenderer.sendSync('trust-certificate', anchor); }); contextBridge.exposeInMainWorld('openImage', (basename: string) => { diff --git a/src/server_manager/electron_app/util.ts b/src/server_manager/electron_app/util.ts index 9f520278..c78798b1 100644 --- a/src/server_manager/electron_app/util.ts +++ b/src/server_manager/electron_app/util.ts @@ -25,3 +25,9 @@ import {URL} from 'url'; export function redactManagerUrl(s: string) { return new URL(s).pathname.split('/').slice(2).join('/'); } + +/** Represents an additional trust anchor for a single host. */ +export interface HostAnchor { + host: string; + fingerprint: string; +} diff --git a/src/server_manager/types/preload.d.ts b/src/server_manager/types/preload.d.ts index 03342ff6..8ea240d7 100644 --- a/src/server_manager/types/preload.d.ts +++ b/src/server_manager/types/preload.d.ts @@ -14,7 +14,12 @@ // Functions made available to the renderer process via preload.ts. -declare function trustCertificate(fingerprint: string): boolean; +interface HostAnchor { + host: string; + fingerprint: string; +} + +declare function trustCertificate(anchor: HostAnchor): boolean; declare function openImage(basename: string): void; declare function onUpdateDownloaded(callback: () => void): void; diff --git a/src/server_manager/web_app/browser_main.ts b/src/server_manager/web_app/browser_main.ts index 46474e23..6d89f7fc 100644 --- a/src/server_manager/web_app/browser_main.ts +++ b/src/server_manager/web_app/browser_main.ts @@ -13,8 +13,8 @@ // limitations under the License. // eslint-disable-next-line @typescript-eslint/no-explicit-any -(window as any).trustCertificate = (fingerprint: string) => { - console.log(`Requested to trust certificate with fingerprint ${fingerprint}`); +(window as any).trustCertificate = (anchor: HostAnchor) => { + console.log(`Requested to trust certificate: ${anchor}`); }; // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -59,4 +59,5 @@ console.info(`Requested bringToFront`); }; +import {HostAnchor} from '../electron_app/util'; import './main'; diff --git a/src/server_manager/web_app/digitalocean_server.ts b/src/server_manager/web_app/digitalocean_server.ts index a8eb787e..46c17784 100644 --- a/src/server_manager/web_app/digitalocean_server.ts +++ b/src/server_manager/web_app/digitalocean_server.ts @@ -185,8 +185,9 @@ export class DigitalOceanServer extends ShadowboxServer implements server.Manage // these methods throw exceptions if the fields are unavailable. const certificateFingerprint = this.getCertificateFingerprint(); const apiAddress = this.getManagementApiAddress(); + const parsed = new URL(apiAddress); // Loaded both the cert and url without exceptions, they can be set. - trustCertificate(certificateFingerprint); + trustCertificate({host: parsed.host, fingerprint: certificateFingerprint}); this.setManagementApiUrl(apiAddress); return true; } catch (e) { diff --git a/src/server_manager/web_app/gcp_server.ts b/src/server_manager/web_app/gcp_server.ts index 79d951de..aac1a844 100644 --- a/src/server_manager/web_app/gcp_server.ts +++ b/src/server_manager/web_app/gcp_server.ts @@ -178,7 +178,14 @@ export class GcpServer extends ShadowboxServer implements server.ManagedServer { if (outlineGuestAttributes.has('apiUrl') && outlineGuestAttributes.has('certSha256')) { const certSha256 = outlineGuestAttributes.get('certSha256'); const apiUrl = outlineGuestAttributes.get('apiUrl'); - trustCertificate(certSha256); + try { + const parsed = new URL(apiUrl); + trustCertificate({host: parsed.host, fingerprint: certSha256}); + } catch (e) { + console.error(e); + this.setInstallState(InstallState.FAILED); + break; + } this.setManagementApiUrl(apiUrl); this.setInstallState(InstallState.COMPLETED); break; diff --git a/src/server_manager/web_app/manual_server.ts b/src/server_manager/web_app/manual_server.ts index 953d087b..9406c1e1 100644 --- a/src/server_manager/web_app/manual_server.ts +++ b/src/server_manager/web_app/manual_server.ts @@ -29,7 +29,9 @@ class ManualServer extends ShadowboxServer implements server.ManualServer { // Electron requires that this be decoded from hex (to unprintable binary), // then encoded as base64. try { - trustCertificate(btoa(hexToString(manualServerConfig.certSha256))); + const parsed = new URL(manualServerConfig.apiUrl); + const fingerprint = btoa(hexToString(manualServerConfig.certSha256)); + trustCertificate({host: parsed.host, fingerprint}); } catch (e) { // Error trusting certificate, may be due to bad user input. console.error('Error trusting certificate');