diff --git a/.eslintignore b/.eslintignore index 0b3ae6f5df..27c673cf51 100644 --- a/.eslintignore +++ b/.eslintignore @@ -1073,6 +1073,8 @@ packages/lib/fs-driver-base.js packages/lib/fs-driver-node.js packages/lib/fsDriver.test.js packages/lib/geolocation-node.js +packages/lib/getAppName.test.js +packages/lib/getAppName.js packages/lib/hooks/useAsyncEffect.js packages/lib/hooks/useElementSize.js packages/lib/hooks/useEventListener.js diff --git a/.gitignore b/.gitignore index 465a5a078d..35416946db 100644 --- a/.gitignore +++ b/.gitignore @@ -1048,6 +1048,8 @@ packages/lib/fs-driver-base.js packages/lib/fs-driver-node.js packages/lib/fsDriver.test.js packages/lib/geolocation-node.js +packages/lib/getAppName.test.js +packages/lib/getAppName.js packages/lib/hooks/useAsyncEffect.js packages/lib/hooks/useElementSize.js packages/lib/hooks/useEventListener.js diff --git a/packages/app-desktop/ElectronAppWrapper.ts b/packages/app-desktop/ElectronAppWrapper.ts index ba76b492bc..53c6aa7bf6 100644 --- a/packages/app-desktop/ElectronAppWrapper.ts +++ b/packages/app-desktop/ElectronAppWrapper.ts @@ -21,6 +21,8 @@ import { clearTimeout, setTimeout } from 'timers'; import { resolve } from 'path'; import { defaultWindowId } from '@joplin/lib/reducer'; import { msleep, Second } from '@joplin/utils/time'; +import determineBaseAppDirs from '@joplin/lib/determineBaseAppDirs'; +import getAppName from '@joplin/lib/getAppName'; interface RendererProcessQuitReply { canClose: boolean; @@ -579,7 +581,11 @@ export default class ElectronAppWrapper { if (port === null) port = this.ipcStartPort_; - return await sendMessage(port, { ...message, sourcePort: this.ipcServer_.port }, { + return await sendMessage(port, { + ...message, + sourcePort: this.ipcServer_.port, + secretKey: this.ipcServer_.secretKey, + }, { logger: this.ipcLogger_, ...options, }); @@ -631,6 +637,7 @@ export default class ElectronAppWrapper { const response = await this.sendCrossAppIpcMessage({ action: 'ping', data: null, + secretKey: this.ipcServer_.secretKey, }, message.sourcePort, { sendToSpecificPortOnly: true, }); @@ -662,7 +669,12 @@ export default class ElectronAppWrapper { }, }; - this.ipcServer_ = await startServer(this.ipcStartPort_, async (message) => { + const defaultProfileDir = determineBaseAppDirs('', getAppName(true, this.env() === 'dev'), '').rootProfileDir; + const secretKeyFilePath = `${defaultProfileDir}/ipc_secret_key.txt`; + + this.ipcLogger_.info('Starting server using secret key:', secretKeyFilePath); + + this.ipcServer_ = await startServer(this.ipcStartPort_, secretKeyFilePath, async (message) => { if (messageHandlers[message.action]) { this.ipcLogger_.info('Got message:', message); return messageHandlers[message.action](message); @@ -684,6 +696,7 @@ export default class ElectronAppWrapper { profilePath: this.profilePath_, argv: process.argv, }, + secretKey: this.ipcServer_.secretKey, }; await this.sendCrossAppIpcMessage(message); diff --git a/packages/lib/BaseApplication.ts b/packages/lib/BaseApplication.ts index 1358117001..bc00a6f526 100644 --- a/packages/lib/BaseApplication.ts +++ b/packages/lib/BaseApplication.ts @@ -65,6 +65,7 @@ import processStartFlags from './utils/processStartFlags'; import { setupAutoDeletion } from './services/trash/permanentlyDeleteOldItems'; import determineProfileAndBaseDir from './determineBaseAppDirs'; import NavService from './services/NavService'; +import getAppName from './getAppName'; const appLogger: LoggerWrapper = Logger.create('App'); @@ -679,8 +680,7 @@ export default class BaseApplication { let appName = options.appName; if (!appName) { - appName = initArgs.env === 'dev' ? 'joplindev' : 'joplin'; - if (Setting.value('appId').indexOf('-desktop') >= 0) appName += '-desktop'; + appName = getAppName(Setting.value('appId').indexOf('-desktop') >= 0, initArgs.env === 'dev'); } Setting.setConstant('appName', appName); diff --git a/packages/lib/getAppName.test.ts b/packages/lib/getAppName.test.ts new file mode 100644 index 0000000000..ae1c0280da --- /dev/null +++ b/packages/lib/getAppName.test.ts @@ -0,0 +1,12 @@ +import getAppName from './getAppName'; + +describe('getAppName', () => { + + it('should get the app name', () => { + expect(getAppName(true, true)).toBe('joplindev-desktop'); + expect(getAppName(true, false)).toBe('joplin-desktop'); + expect(getAppName(false, false)).toBe('joplin'); + expect(getAppName(false, true)).toBe('joplindev'); + }); + +}); diff --git a/packages/lib/getAppName.ts b/packages/lib/getAppName.ts new file mode 100644 index 0000000000..0b7a865fbe --- /dev/null +++ b/packages/lib/getAppName.ts @@ -0,0 +1,5 @@ +export default (isDesktop: boolean, isDev: boolean) => { + let appName = isDev ? 'joplindev' : 'joplin'; + if (isDesktop) appName += '-desktop'; + return appName; +}; diff --git a/packages/utils/crypto.ts b/packages/utils/crypto.ts new file mode 100644 index 0000000000..6c043926e0 --- /dev/null +++ b/packages/utils/crypto.ts @@ -0,0 +1,9 @@ +/* eslint-disable import/prefer-default-export */ + +import { randomBytes } from 'crypto'; + +export const getSecureRandomString = (length: number): string => { + const bytes = randomBytes(Math.ceil(length * 2)); + const randomString = bytes.toString('base64').replace(/[^a-zA-Z0-9]/g, ''); + return randomString.slice(0, length); +}; diff --git a/packages/utils/fs.test.ts b/packages/utils/fs.test.ts index e24553c9d6..b0e44742b4 100644 --- a/packages/utils/fs.test.ts +++ b/packages/utils/fs.test.ts @@ -1,10 +1,12 @@ +/* eslint-disable import/prefer-default-export */ + import { mkdirp } from 'fs-extra'; import { FileLocker } from './fs'; import { msleep, Second } from './time'; const baseTempDir = `${__dirname}/../app-cli/tests/tmp`; -const createTempDir = async () => { +export const createTempDir = async () => { const p = `${baseTempDir}/${Date.now()}`; await mkdirp(p); return p; diff --git a/packages/utils/ipc.test.ts b/packages/utils/ipc.test.ts index 0c4c56cc03..0fc572f926 100644 --- a/packages/utils/ipc.test.ts +++ b/packages/utils/ipc.test.ts @@ -1,11 +1,15 @@ +import { readFile } from 'fs/promises'; +import { createTempDir } from './fs.test'; import { newHttpError, sendMessage, startServer, stopServer } from './ipc'; describe('ipc', () => { it('should send and receive messages', async () => { + const tempDir = await createTempDir(); + const secretFilePath = `${tempDir}/secret.txt`; const startPort = 41168; - const server1 = await startServer(startPort, async (request) => { + const server1 = await startServer(startPort, secretFilePath, async (request) => { if (request.action === 'testing') { return { text: 'hello1', @@ -15,7 +19,7 @@ describe('ipc', () => { throw newHttpError(404); }); - const server2 = await startServer(startPort, async (request) => { + const server2 = await startServer(startPort, secretFilePath, async (request) => { if (request.action === 'testing') { return { text: 'hello2', @@ -31,12 +35,15 @@ describe('ipc', () => { throw newHttpError(404); }); + const secretKey = await readFile(secretFilePath, 'utf-8'); + { const responses = await sendMessage(startPort, { action: 'testing', data: { test: 1234, }, + secretKey, }); expect(responses).toEqual([ @@ -49,6 +56,7 @@ describe('ipc', () => { const responses = await sendMessage(startPort, { action: 'ping', data: null, + secretKey, }); expect(responses).toEqual([ @@ -63,6 +71,7 @@ describe('ipc', () => { test: 1234, }, sourcePort: 41168, + secretKey, }); expect(responses).toEqual([ @@ -74,4 +83,44 @@ describe('ipc', () => { await stopServer(server2); }); + it('should not process message if secret is invalid', async () => { + const tempDir = await createTempDir(); + const secretFilePath = `${tempDir}/secret.txt`; + const startPort = 41168; + + const server = await startServer(startPort, secretFilePath, async (request) => { + if (request.action === 'testing') { + return { + text: 'hello1', + }; + } + + throw newHttpError(404); + }); + + const secretKey = await readFile(secretFilePath, 'utf-8'); + + { + const responses = await sendMessage(startPort, { + action: 'testing', + data: null, + secretKey: 'wrong_key', + }); + + expect(responses.length).toBe(0); + } + + { + const responses = await sendMessage(startPort, { + action: 'testing', + data: null, + secretKey, + }); + + expect(responses.length).toBe(1); + } + + await stopServer(server); + }); + }); diff --git a/packages/utils/ipc.ts b/packages/utils/ipc.ts index 480428db60..344d669681 100644 --- a/packages/utils/ipc.ts +++ b/packages/utils/ipc.ts @@ -2,6 +2,9 @@ import { createServer, IncomingMessage, ServerResponse } from 'http'; import fetch from 'node-fetch'; import { Server } from 'http'; import Logger from './Logger'; +import { pathExists } from 'fs-extra'; +import { readFile, writeFile } from 'fs/promises'; +import { getSecureRandomString } from './crypto'; const tcpPortUsed = require('tcp-port-used'); const maxPorts = 10; @@ -51,6 +54,7 @@ export interface Message { action: string; data: object|number|string|null; sourcePort?: number; + secretKey?: string; } type Response = string|number|object|boolean; @@ -66,21 +70,52 @@ export type IpcMessageHandler = (message: Message)=> Promise; export interface IpcServer { port: number; httpServer: Server; + secretKey: string; } interface StartServerOptions { logger?: Logger; } -export const startServer = async (startPort: number, messageHandler: IpcMessageHandler, options: StartServerOptions|null = null): Promise => { +const getSecretKey = async (filePath: string) => { + try { + const keyLength = 64; + + const writeKeyToFile = async () => { + const key = getSecureRandomString(keyLength); + await writeFile(filePath, key, 'utf-8'); + return key; + }; + + if (!(await pathExists(filePath))) { + return await writeKeyToFile(); + } + + const key = await readFile(filePath, 'utf-8'); + if (key.length !== keyLength) return await writeKeyToFile(); + + return key; + } catch (error) { + const e = error as NodeJS.ErrnoException; + e.message = `Could not get secret key from file: ${filePath}`; + throw e; + } +}; + +// `secretKeyFilePath` must be the same for all the instances that can communicate with each others +export const startServer = async (startPort: number, secretKeyFilePath: string, messageHandler: IpcMessageHandler, options: StartServerOptions|null = null): Promise => { const port = await findAvailablePort(startPort); const logger = options && options.logger ? options.logger : new Logger(); + const secretKey = await getSecretKey(secretKeyFilePath); + return new Promise((resolve, reject) => { try { const server = createServer(async (req: IncomingMessage, res: ServerResponse) => { + let message: Message|null = null; try { - const message = await parseJson(req) as Message; + message = await parseJson(req) as Message; + if (message.secretKey !== secretKey) throw newHttpError(401, 'Invalid secret key'); if (!message.action) throw newHttpError(400, 'Missing "action" property in message'); const response = await messageHandler(message); res.writeHead(200, { 'Content-Type': 'application/json' }); @@ -88,6 +123,7 @@ export const startServer = async (startPort: number, messageHandler: IpcMessageH } catch (error) { const httpError = error as HttpError; const httpCode = httpError.httpCode || 500; + logger.error('Could not response to request:', message, 'Error', httpCode, httpError.message); res.writeHead(httpCode, { 'Content-Type': 'text/plain' }); res.end(`Error ${httpCode}: ${httpError.message}`); } @@ -101,6 +137,7 @@ export const startServer = async (startPort: number, messageHandler: IpcMessageH resolve({ httpServer: server, port, + secretKey, }); }); } catch (error) { diff --git a/readme/dev/spec/multiple_instances.md b/readme/dev/spec/multiple_instances.md index 4facfc3797..5dd4096ab5 100644 --- a/readme/dev/spec/multiple_instances.md +++ b/readme/dev/spec/multiple_instances.md @@ -14,6 +14,8 @@ Joplin Desktop supports multiple instances through **profile locking** and **IPC - When a message is sent, the implementation automatically discovers running IPC servers. +- Messages are secured using a secret that is shared by all applications. That secret is read from the profile directory of the main instance. It is created by the server if it doesn't exist. This ensures that, for example, a browser cannot send a valid message to the apps. + ### Instance Differentiation - The `--alt-instance-id` flag must be used to launch an alternative instance. This disables services like the Web Clipper.