Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v16] Disable debug service in agents ran by Connect My Computer #43287

Merged
merged 3 commits into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
/**
* @jest-environment node
*/
/**
* Teleport
* Copyright (C) 2023 Gravitational, Inc.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* @jest-environment jsdom
* @jest-environment node
*/
/**
* Teleport
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
/**
* @jest-environment node
*/
/**
* Teleport
* Copyright (C) 2023 Gravitational, Inc.
Expand All @@ -24,6 +27,7 @@ import { makeRuntimeSettings } from 'teleterm/mainProcess/fixtures/mocks';

import {
createAgentConfigFile,
disableDebugServiceStanza,
generateAgentConfigPaths,
} from './createAgentConfigFile';

Expand All @@ -34,10 +38,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 () => {
Expand Down Expand Up @@ -69,7 +75,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}`,
Expand All @@ -80,6 +86,13 @@ 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)
);
});

test('previous config file is removed before calling teleport configure', async () => {
Expand Down
38 changes: 36 additions & 2 deletions web/packages/teleterm/src/mainProcess/createAgentConfigFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`,
Expand All @@ -67,8 +67,42 @@ export async function createAgentConfigFile(
timeout: 10_000, // 10 seconds
}
);

try {
await fs.mkdir(path.dirname(configFile), {
// 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);
}

// 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/<user>/Library/Application Support/Teleport Connect/agents/<proxy hostname>/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
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
/**
* @jest-environment node
*/
/**
* Teleport
* Copyright (C) 2023 Gravitational, Inc.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
/**
* @jest-environment node
*/
/**
* (The MIT License)
*
Expand Down
Loading