fix(adapter-utils): validate SSH env-lab fixture state before signals
Any local process running as the same user can write a fixture state file. A forged pid or an empty sshdConfigPath weakened the identity check that gates SIGTERM/SIGKILL: an empty string is a substring of every command line, so it matched any running process. readSshEnvLabFixtureState now rejects a state file unless the pid is a positive safe integer, every path field is absolute and rooted at the fixture directory the state was read from, and sshdConfigPath is non-empty and equals rootDir/sshd_config exactly. Rejection happens before any identity check or signal. Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
parent
45104692db
commit
c6dea6b3f9
|
|
@ -1,4 +1,4 @@
|
|||
import { execFile } from "node:child_process";
|
||||
import { execFile, spawn } from "node:child_process";
|
||||
import { mkdir, mkdtemp, readFile, rm, stat, symlink, writeFile } from "node:fs/promises";
|
||||
import net from "node:net";
|
||||
import os from "node:os";
|
||||
|
|
@ -247,6 +247,73 @@ describe("ssh env-lab fixture", () => {
|
|||
await stopSshEnvLabFixture(restarted);
|
||||
}, SSH_FIXTURE_TEST_TIMEOUT_MS);
|
||||
|
||||
it("rejects a forged state file and cannot signal an unrelated local process", async () => {
|
||||
const rootDir = await createFixtureRootDir();
|
||||
const statePath = path.join(rootDir, "state.json");
|
||||
|
||||
// A process this test does not own. A forged state must never be able
|
||||
// to target it for SIGTERM or SIGKILL.
|
||||
const bystander = spawn("sleep", ["30"], { stdio: "ignore" });
|
||||
const bystanderPid = bystander.pid;
|
||||
if (!bystanderPid) {
|
||||
throw new Error("Failed to spawn the bystander process for this regression test.");
|
||||
}
|
||||
|
||||
try {
|
||||
const baseState = {
|
||||
kind: "ssh_openbsd" as const,
|
||||
bindHost: "127.0.0.1",
|
||||
host: "127.0.0.1",
|
||||
port: 0,
|
||||
username: os.userInfo().username,
|
||||
rootDir,
|
||||
workspaceDir: path.join(rootDir, "workspace"),
|
||||
statePath,
|
||||
createdAt: new Date().toISOString(),
|
||||
clientPrivateKeyPath: path.join(rootDir, "client_key"),
|
||||
clientPublicKeyPath: path.join(rootDir, "client_key.pub"),
|
||||
hostPrivateKeyPath: path.join(rootDir, "host_key"),
|
||||
hostPublicKeyPath: path.join(rootDir, "host_key.pub"),
|
||||
authorizedKeysPath: path.join(rootDir, "authorized_keys"),
|
||||
knownHostsPath: path.join(rootDir, "known_hosts"),
|
||||
sshdConfigPath: path.join(rootDir, "sshd_config"),
|
||||
sshdLogPath: path.join(rootDir, "sshd.log"),
|
||||
};
|
||||
|
||||
const forgedVariants = [
|
||||
// An empty sshdConfigPath used to defeat the identity check: an
|
||||
// empty string is a substring of every command line.
|
||||
{ ...baseState, pid: bystanderPid, sshdConfigPath: "" },
|
||||
// A sshdConfigPath outside the fixture root.
|
||||
{ ...baseState, pid: bystanderPid, sshdConfigPath: "/etc/ssh/sshd_config" },
|
||||
// A non-positive pid.
|
||||
{ ...baseState, pid: 0 },
|
||||
{ ...baseState, pid: -1 },
|
||||
];
|
||||
|
||||
for (const forged of forgedVariants) {
|
||||
await writeFile(statePath, JSON.stringify(forged, null, 2), { mode: 0o600 });
|
||||
|
||||
const status = await readSshEnvLabFixtureStatus(statePath);
|
||||
expect(status.running).toBe(false);
|
||||
expect(status.state).toBeNull();
|
||||
|
||||
const stopped = await stopSshEnvLabFixture(statePath);
|
||||
expect(stopped).toBe(false);
|
||||
}
|
||||
|
||||
// No forged state ever reached the identity check or a signal call,
|
||||
// so the bystander process is still alive.
|
||||
expect(() => process.kill(bystanderPid, 0)).not.toThrow();
|
||||
} finally {
|
||||
try {
|
||||
process.kill(bystanderPid, "SIGKILL");
|
||||
} catch {
|
||||
// Already gone.
|
||||
}
|
||||
}
|
||||
}, SSH_FIXTURE_TEST_TIMEOUT_MS);
|
||||
|
||||
it("stops the fixture listener and frees its loopback port", async () => {
|
||||
const rootDir = await createFixtureRootDir();
|
||||
const statePath = path.join(rootDir, "state.json");
|
||||
|
|
|
|||
|
|
@ -1704,12 +1704,70 @@ export async function ensureSshWorkspaceReady(
|
|||
};
|
||||
}
|
||||
|
||||
const SSH_ENV_LAB_FIXTURE_PATH_FIELDS = [
|
||||
"rootDir",
|
||||
"workspaceDir",
|
||||
"statePath",
|
||||
"clientPrivateKeyPath",
|
||||
"clientPublicKeyPath",
|
||||
"hostPrivateKeyPath",
|
||||
"hostPublicKeyPath",
|
||||
"authorizedKeysPath",
|
||||
"knownHostsPath",
|
||||
"sshdConfigPath",
|
||||
"sshdLogPath",
|
||||
] as const satisfies readonly (keyof SshEnvLabFixtureState)[];
|
||||
|
||||
// True when candidate is an absolute path equal to rootDir or nested under
|
||||
// it. Used to reject a state file whose paths point outside the fixture
|
||||
// root it was read from.
|
||||
function isPathRootedAt(candidate: string, rootDir: string): boolean {
|
||||
if (!path.isAbsolute(candidate)) return false;
|
||||
if (candidate === rootDir) return true;
|
||||
const relative = path.relative(rootDir, candidate);
|
||||
return (
|
||||
relative.length > 0 &&
|
||||
relative !== ".." &&
|
||||
!relative.startsWith(`..${path.sep}`) &&
|
||||
!path.isAbsolute(relative)
|
||||
);
|
||||
}
|
||||
|
||||
// The state file is untrusted input: any local process running as the same
|
||||
// user can write one. A forged pid or an empty sshdConfigPath would weaken
|
||||
// isSshEnvLabFixtureProcess's identity check — an empty string is a
|
||||
// substring of every command line, so it would match any running process.
|
||||
// Reject a state file that fails this check before any identity check or
|
||||
// signal runs against it.
|
||||
function isValidSshEnvLabFixtureState(
|
||||
raw: SshEnvLabFixtureState,
|
||||
expectedRootDir: string,
|
||||
): boolean {
|
||||
if (!Number.isSafeInteger(raw.pid) || raw.pid <= 0) return false;
|
||||
if (raw.rootDir !== expectedRootDir) return false;
|
||||
|
||||
for (const field of SSH_ENV_LAB_FIXTURE_PATH_FIELDS) {
|
||||
const value = raw[field];
|
||||
if (typeof value !== "string" || !isPathRootedAt(value, expectedRootDir)) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
const expectedSshdConfigPath = path.join(expectedRootDir, "sshd_config");
|
||||
if (raw.sshdConfigPath.length === 0 || raw.sshdConfigPath !== expectedSshdConfigPath) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
export async function readSshEnvLabFixtureState(
|
||||
statePath: string,
|
||||
): Promise<SshEnvLabFixtureState | null> {
|
||||
try {
|
||||
const raw = JSON.parse(await fs.readFile(statePath, "utf8")) as SshEnvLabFixtureState;
|
||||
if (!raw || raw.kind !== "ssh_openbsd") return null;
|
||||
if (!isValidSshEnvLabFixtureState(raw, path.dirname(statePath))) return null;
|
||||
return raw;
|
||||
} catch {
|
||||
return null;
|
||||
|
|
|
|||
Loading…
Reference in New Issue