From b760cf0b4093d99d16aac81fe35f6b6123288dc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Wed, 19 Jun 2024 17:19:06 +0200 Subject: [PATCH 1/3] Disable debug service in Connect My Computer --- .../mainProcess/createAgentConfigFile.test.ts | 17 ++++++-- .../src/mainProcess/createAgentConfigFile.ts | 42 ++++++++++++++++++- 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/web/packages/teleterm/src/mainProcess/createAgentConfigFile.test.ts b/web/packages/teleterm/src/mainProcess/createAgentConfigFile.test.ts index a11f29d1eec4a..c30830c5b3e5d 100644 --- a/web/packages/teleterm/src/mainProcess/createAgentConfigFile.test.ts +++ b/web/packages/teleterm/src/mainProcess/createAgentConfigFile.test.ts @@ -24,6 +24,7 @@ import { makeRuntimeSettings } from 'teleterm/mainProcess/fixtures/mocks'; import { createAgentConfigFile, + disableDebugServiceStanza, generateAgentConfigPaths, } from './createAgentConfigFile'; @@ -34,10 +35,12 @@ beforeEach(() => { jest .spyOn(childProcess, 'execFile') .mockImplementation((command, args, options, callback) => { - callback(undefined, '', ''); - return this; + callback(null, '', ''); + return undefined; }); jest.spyOn(fs, 'rm').mockImplementation(() => Promise.resolve()); + jest.spyOn(fs, 'mkdir').mockImplementation(() => Promise.resolve(undefined)); + jest.spyOn(fs, 'writeFile').mockImplementation(() => Promise.resolve()); }); test('teleport configure is called with proper arguments', async () => { @@ -69,7 +72,7 @@ test('teleport configure is called with proper arguments', async () => { [ 'node', 'configure', - `--output=${userDataDir}/agents/cluster.local/config.yaml`, + `--output=stdout`, `--data-dir=${userDataDir}/agents/cluster.local/data`, `--proxy=${proxy}`, `--token=${token}`, @@ -80,6 +83,14 @@ test('teleport configure is called with proper arguments', async () => { }, expect.anything() ); + expect(fs.writeFile).toHaveBeenCalledWith( + `${userDataDir}/agents/cluster.local/config.yaml`, + // It'd be nice to make childProcess.execFile return certain output and then verify that this + // argument includes that output + disableDebugServiceStanza. Alas, the promisified version of + // execFile isn't easily mockable – stdout in tests is just "undefined" for some reason. + expect.stringContaining(disableDebugServiceStanza), + expect.any(Object) + ); }); test('previous config file is removed before calling teleport configure', async () => { diff --git a/web/packages/teleterm/src/mainProcess/createAgentConfigFile.ts b/web/packages/teleterm/src/mainProcess/createAgentConfigFile.ts index f5aaf05a5289b..bfe885a870fb2 100644 --- a/web/packages/teleterm/src/mainProcess/createAgentConfigFile.ts +++ b/web/packages/teleterm/src/mainProcess/createAgentConfigFile.ts @@ -52,12 +52,12 @@ export async function createAgentConfigFile( .map(keyAndValue => keyAndValue.join('=')) .join(','); - await asyncExecFile( + const { stdout } = await asyncExecFile( runtimeSettings.agentBinaryPath, [ 'node', 'configure', - `--output=${configFile}`, + '--output=stdout', `--data-dir=${dataDirectory}`, `--proxy=${args.proxy}`, `--token=${args.token}`, @@ -67,8 +67,46 @@ export async function createAgentConfigFile( timeout: 10_000, // 10 seconds } ); + + try { + await fs.mkdir(path.dirname(configFile), { + // Directories must be rwx, not just rw-. + mode: 0o744, + // Create the agents dir too if it doesn't already exist. + recursive: true, + }); + } catch (error) { + // Ignore error if directory already exists. + if (error['code'] !== 'EEXIST') { + throw error; + } + } + + await fs.writeFile(configFile, stdout + disableDebugServiceStanza, { + mode: 0o644, + }); } +// The debug service is enabled by default. It starts when the teleport agent is launched and it +// creates a debug.sock file in the data directory. Unfortunately, there's a length limit on the +// socket path – 107 characters on Linux and 104 characters on macOS [1]. If exceeded, creating a +// new listener fails with "bind: invalid argument". +// +// The default path for debug.sock for Connect My Computer on macOS is +// /Users//Library/Application Support/Teleport Connect/agents//data/debug.sock +// The constant part is 76 characters which leaves just 28 characters for the hostname and user. +// +// As a workaround, we disable the debug service. This is going to work until someone adds another +// socket which is crucial to run a Teleport agent. +// +// See the GitHub issue for more details: https://github.com/gravitational/teleport/issues/43250 +// +// [1] https://unix.stackexchange.com/questions/367008/why-is-socket-path-length-limited-to-a-hundred-chars +export const disableDebugServiceStanza = ` +debug_service: + enabled: false +`; + export async function removeAgentDirectory( runtimeSettings: RuntimeSettings, rootClusterUri: RootClusterUri From 4c7c0ec65b9014e063a6cc808101b36eae9b9c7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Wed, 19 Jun 2024 17:22:55 +0200 Subject: [PATCH 2/3] Update jest-environment of Node.js tests --- .../teleterm/src/agentCleanupDaemon/agentCleanupDaemon.test.ts | 3 +++ .../src/mainProcess/agentDownloader/fileDownloader.test.ts | 2 +- .../teleterm/src/mainProcess/createAgentConfigFile.test.ts | 3 +++ .../terminateWithTimeout/terminateWithTimeout.test.ts | 3 +++ .../src/services/grpcCredentials/makeCert/makeCert.test.ts | 3 +++ 5 files changed, 13 insertions(+), 1 deletion(-) diff --git a/web/packages/teleterm/src/agentCleanupDaemon/agentCleanupDaemon.test.ts b/web/packages/teleterm/src/agentCleanupDaemon/agentCleanupDaemon.test.ts index 997abd394c699..4a9d7001a61f1 100644 --- a/web/packages/teleterm/src/agentCleanupDaemon/agentCleanupDaemon.test.ts +++ b/web/packages/teleterm/src/agentCleanupDaemon/agentCleanupDaemon.test.ts @@ -1,3 +1,6 @@ +/** + * @jest-environment node + */ /** * Teleport * Copyright (C) 2023 Gravitational, Inc. diff --git a/web/packages/teleterm/src/mainProcess/agentDownloader/fileDownloader.test.ts b/web/packages/teleterm/src/mainProcess/agentDownloader/fileDownloader.test.ts index e1171c33956d8..7f9af2d97c133 100644 --- a/web/packages/teleterm/src/mainProcess/agentDownloader/fileDownloader.test.ts +++ b/web/packages/teleterm/src/mainProcess/agentDownloader/fileDownloader.test.ts @@ -1,5 +1,5 @@ /** - * @jest-environment jsdom + * @jest-environment node */ /** * Teleport diff --git a/web/packages/teleterm/src/mainProcess/createAgentConfigFile.test.ts b/web/packages/teleterm/src/mainProcess/createAgentConfigFile.test.ts index c30830c5b3e5d..fc5ae229f45d6 100644 --- a/web/packages/teleterm/src/mainProcess/createAgentConfigFile.test.ts +++ b/web/packages/teleterm/src/mainProcess/createAgentConfigFile.test.ts @@ -1,3 +1,6 @@ +/** + * @jest-environment node + */ /** * Teleport * Copyright (C) 2023 Gravitational, Inc. diff --git a/web/packages/teleterm/src/mainProcess/terminateWithTimeout/terminateWithTimeout.test.ts b/web/packages/teleterm/src/mainProcess/terminateWithTimeout/terminateWithTimeout.test.ts index 6138dc074d95d..e26a7fd20e5c2 100644 --- a/web/packages/teleterm/src/mainProcess/terminateWithTimeout/terminateWithTimeout.test.ts +++ b/web/packages/teleterm/src/mainProcess/terminateWithTimeout/terminateWithTimeout.test.ts @@ -1,3 +1,6 @@ +/** + * @jest-environment node + */ /** * Teleport * Copyright (C) 2023 Gravitational, Inc. diff --git a/web/packages/teleterm/src/services/grpcCredentials/makeCert/makeCert.test.ts b/web/packages/teleterm/src/services/grpcCredentials/makeCert/makeCert.test.ts index b7aa3e437297f..2a7c6358a4ba5 100644 --- a/web/packages/teleterm/src/services/grpcCredentials/makeCert/makeCert.test.ts +++ b/web/packages/teleterm/src/services/grpcCredentials/makeCert/makeCert.test.ts @@ -1,3 +1,6 @@ +/** + * @jest-environment node + */ /** * (The MIT License) * From 5e39fd96de675a355f2a293e0bec9f817ed60b51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Thu, 20 Jun 2024 14:13:16 +0200 Subject: [PATCH 3/3] Remove modes from mkdir and writeFile --- .../teleterm/src/mainProcess/createAgentConfigFile.test.ts | 3 +-- .../teleterm/src/mainProcess/createAgentConfigFile.ts | 6 +----- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/web/packages/teleterm/src/mainProcess/createAgentConfigFile.test.ts b/web/packages/teleterm/src/mainProcess/createAgentConfigFile.test.ts index fc5ae229f45d6..30ecda05521bc 100644 --- a/web/packages/teleterm/src/mainProcess/createAgentConfigFile.test.ts +++ b/web/packages/teleterm/src/mainProcess/createAgentConfigFile.test.ts @@ -91,8 +91,7 @@ test('teleport configure is called with proper arguments', async () => { // It'd be nice to make childProcess.execFile return certain output and then verify that this // argument includes that output + disableDebugServiceStanza. Alas, the promisified version of // execFile isn't easily mockable – stdout in tests is just "undefined" for some reason. - expect.stringContaining(disableDebugServiceStanza), - expect.any(Object) + expect.stringContaining(disableDebugServiceStanza) ); }); diff --git a/web/packages/teleterm/src/mainProcess/createAgentConfigFile.ts b/web/packages/teleterm/src/mainProcess/createAgentConfigFile.ts index bfe885a870fb2..4345563e3ee7f 100644 --- a/web/packages/teleterm/src/mainProcess/createAgentConfigFile.ts +++ b/web/packages/teleterm/src/mainProcess/createAgentConfigFile.ts @@ -70,8 +70,6 @@ export async function createAgentConfigFile( try { await fs.mkdir(path.dirname(configFile), { - // Directories must be rwx, not just rw-. - mode: 0o744, // Create the agents dir too if it doesn't already exist. recursive: true, }); @@ -82,9 +80,7 @@ export async function createAgentConfigFile( } } - await fs.writeFile(configFile, stdout + disableDebugServiceStanza, { - mode: 0o644, - }); + await fs.writeFile(configFile, stdout + disableDebugServiceStanza); } // The debug service is enabled by default. It starts when the teleport agent is launched and it