mirror of
https://github.com/openclaw/openclaw.git
synced 2026-02-19 18:39:20 -05:00
fix(security): block plaintext WebSocket connections to non-loopback addresses (#20803)
* fix(security): block plaintext WebSocket connections to non-loopback addresses Addresses CWE-319 (Cleartext Transmission of Sensitive Information). Previously, ws:// connections to remote hosts were allowed, exposing both credentials and chat data to network interception. This change blocks ALL plaintext ws:// connections to non-loopback addresses, regardless of whether explicit credentials are configured (device tokens may be loaded dynamically). Security policy: - wss:// allowed to any host - ws:// allowed only to loopback (127.x.x.x, localhost, ::1) - ws:// to LAN/tailnet/remote hosts now requires TLS Changes: - Add isSecureWebSocketUrl() validation in net.ts - Block insecure connections in GatewayClient.start() - Block insecure URLs in buildGatewayConnectionDetails() - Handle malformed URLs gracefully without crashing - Update tests to use wss:// for non-loopback URLs Fixes #12519 * fix(test): update gateway-chat mock to preserve net.js exports Use importOriginal to spread actual module exports and mock only the functions needed for testing. This ensures isSecureWebSocketUrl and other exports remain available to the code under test.
This commit is contained in:
@@ -3,7 +3,7 @@ import { describe, expect, it, vi } from "vitest";
|
||||
const loadConfig = vi.fn(() => ({
|
||||
gateway: {
|
||||
mode: "remote",
|
||||
remote: { url: "ws://remote.example:18789", token: "rtok" },
|
||||
remote: { url: "wss://remote.example:18789", token: "rtok" },
|
||||
auth: { token: "ltok" },
|
||||
},
|
||||
}));
|
||||
@@ -272,7 +272,7 @@ describe("gateway-status command", () => {
|
||||
loadConfig.mockReturnValueOnce({
|
||||
gateway: {
|
||||
mode: "remote",
|
||||
remote: { url: "ws://studio.example:18789", token: "rtok" },
|
||||
remote: { url: "wss://studio.example:18789", token: "rtok" },
|
||||
auth: { token: "ltok" },
|
||||
},
|
||||
});
|
||||
@@ -298,7 +298,7 @@ describe("gateway-status command", () => {
|
||||
loadConfig.mockReturnValueOnce({
|
||||
gateway: {
|
||||
mode: "remote",
|
||||
remote: { url: "ws://studio.example:18789", token: "rtok" },
|
||||
remote: { url: "wss://studio.example:18789", token: "rtok" },
|
||||
auth: { token: "ltok" },
|
||||
},
|
||||
});
|
||||
|
||||
@@ -31,9 +31,13 @@ vi.mock("../infra/tailnet.js", () => ({
|
||||
pickPrimaryTailnetIPv4,
|
||||
}));
|
||||
|
||||
vi.mock("./net.js", () => ({
|
||||
pickPrimaryLanIPv4,
|
||||
}));
|
||||
vi.mock("./net.js", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("./net.js")>();
|
||||
return {
|
||||
...actual,
|
||||
pickPrimaryLanIPv4,
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("./client.js", () => ({
|
||||
describeGatewayCloseCode: (code: number) => {
|
||||
@@ -91,7 +95,7 @@ function makeRemotePasswordGatewayConfig(remotePassword: string, localPassword =
|
||||
return {
|
||||
gateway: {
|
||||
mode: "remote",
|
||||
remote: { url: "ws://remote.example:18789", password: remotePassword },
|
||||
remote: { url: "wss://remote.example:18789", password: remotePassword },
|
||||
auth: { password: localPassword },
|
||||
},
|
||||
};
|
||||
@@ -122,25 +126,46 @@ describe("callGateway url resolution", () => {
|
||||
expect(lastClientOptions?.url).toBe("ws://127.0.0.1:18800");
|
||||
});
|
||||
|
||||
it("uses tailnet IP when local bind is tailnet and tailnet is present", async () => {
|
||||
loadConfig.mockReturnValue({ gateway: { mode: "local", bind: "tailnet" } });
|
||||
it("uses tailnet IP with TLS when local bind is tailnet", async () => {
|
||||
loadConfig.mockReturnValue({
|
||||
gateway: { mode: "local", bind: "tailnet", tls: { enabled: true } },
|
||||
});
|
||||
resolveGatewayPort.mockReturnValue(18800);
|
||||
pickPrimaryTailnetIPv4.mockReturnValue("100.64.0.1");
|
||||
|
||||
await callGateway({ method: "health" });
|
||||
|
||||
expect(lastClientOptions?.url).toBe("ws://100.64.0.1:18800");
|
||||
expect(lastClientOptions?.url).toBe("wss://100.64.0.1:18800");
|
||||
});
|
||||
|
||||
it("uses LAN IP when bind is lan and LAN IP is available", async () => {
|
||||
loadConfig.mockReturnValue({ gateway: { mode: "local", bind: "lan" } });
|
||||
it("blocks ws:// to tailnet IP without TLS (CWE-319)", async () => {
|
||||
loadConfig.mockReturnValue({ gateway: { mode: "local", bind: "tailnet" } });
|
||||
resolveGatewayPort.mockReturnValue(18800);
|
||||
pickPrimaryTailnetIPv4.mockReturnValue("100.64.0.1");
|
||||
|
||||
await expect(callGateway({ method: "health" })).rejects.toThrow("SECURITY ERROR");
|
||||
});
|
||||
|
||||
it("uses LAN IP with TLS when bind is lan", async () => {
|
||||
loadConfig.mockReturnValue({
|
||||
gateway: { mode: "local", bind: "lan", tls: { enabled: true } },
|
||||
});
|
||||
resolveGatewayPort.mockReturnValue(18800);
|
||||
pickPrimaryTailnetIPv4.mockReturnValue(undefined);
|
||||
pickPrimaryLanIPv4.mockReturnValue("192.168.1.42");
|
||||
|
||||
await callGateway({ method: "health" });
|
||||
|
||||
expect(lastClientOptions?.url).toBe("ws://192.168.1.42:18800");
|
||||
expect(lastClientOptions?.url).toBe("wss://192.168.1.42:18800");
|
||||
});
|
||||
|
||||
it("blocks ws:// to LAN IP without TLS (CWE-319)", async () => {
|
||||
loadConfig.mockReturnValue({ gateway: { mode: "local", bind: "lan" } });
|
||||
resolveGatewayPort.mockReturnValue(18800);
|
||||
pickPrimaryTailnetIPv4.mockReturnValue(undefined);
|
||||
pickPrimaryLanIPv4.mockReturnValue("192.168.1.42");
|
||||
|
||||
await expect(callGateway({ method: "health" })).rejects.toThrow("SECURITY ERROR");
|
||||
});
|
||||
|
||||
it("falls back to loopback when bind is lan but no LAN IP found", async () => {
|
||||
@@ -214,9 +239,9 @@ describe("buildGatewayConnectionDetails", () => {
|
||||
expect(details.message).toContain("Gateway target: ws://127.0.0.1:18789");
|
||||
});
|
||||
|
||||
it("uses LAN IP and reports lan source when bind is lan", () => {
|
||||
it("uses LAN IP with TLS and reports lan source when bind is lan", () => {
|
||||
loadConfig.mockReturnValue({
|
||||
gateway: { mode: "local", bind: "lan" },
|
||||
gateway: { mode: "local", bind: "lan", tls: { enabled: true } },
|
||||
});
|
||||
resolveGatewayPort.mockReturnValue(18800);
|
||||
pickPrimaryTailnetIPv4.mockReturnValue(undefined);
|
||||
@@ -224,11 +249,22 @@ describe("buildGatewayConnectionDetails", () => {
|
||||
|
||||
const details = buildGatewayConnectionDetails();
|
||||
|
||||
expect(details.url).toBe("ws://10.0.0.5:18800");
|
||||
expect(details.url).toBe("wss://10.0.0.5:18800");
|
||||
expect(details.urlSource).toBe("local lan 10.0.0.5");
|
||||
expect(details.bindDetail).toBe("Bind: lan");
|
||||
});
|
||||
|
||||
it("throws for ws:// to LAN IP without TLS (CWE-319)", () => {
|
||||
loadConfig.mockReturnValue({
|
||||
gateway: { mode: "local", bind: "lan" },
|
||||
});
|
||||
resolveGatewayPort.mockReturnValue(18800);
|
||||
pickPrimaryTailnetIPv4.mockReturnValue(undefined);
|
||||
pickPrimaryLanIPv4.mockReturnValue("10.0.0.5");
|
||||
|
||||
expect(() => buildGatewayConnectionDetails()).toThrow("SECURITY ERROR");
|
||||
});
|
||||
|
||||
it("prefers remote url when configured", () => {
|
||||
loadConfig.mockReturnValue({
|
||||
gateway: {
|
||||
@@ -247,6 +283,34 @@ describe("buildGatewayConnectionDetails", () => {
|
||||
expect(details.bindDetail).toBeUndefined();
|
||||
expect(details.remoteFallbackNote).toBeUndefined();
|
||||
});
|
||||
|
||||
it("throws for insecure ws:// remote URLs (CWE-319)", () => {
|
||||
loadConfig.mockReturnValue({
|
||||
gateway: {
|
||||
mode: "remote",
|
||||
bind: "loopback",
|
||||
remote: { url: "ws://remote.example.com:18789" },
|
||||
},
|
||||
});
|
||||
resolveGatewayPort.mockReturnValue(18789);
|
||||
pickPrimaryTailnetIPv4.mockReturnValue(undefined);
|
||||
|
||||
expect(() => buildGatewayConnectionDetails()).toThrow("SECURITY ERROR");
|
||||
expect(() => buildGatewayConnectionDetails()).toThrow("plaintext ws://");
|
||||
expect(() => buildGatewayConnectionDetails()).toThrow("wss://");
|
||||
});
|
||||
|
||||
it("allows ws:// for loopback addresses in local mode", () => {
|
||||
loadConfig.mockReturnValue({
|
||||
gateway: { mode: "local", bind: "loopback" },
|
||||
});
|
||||
resolveGatewayPort.mockReturnValue(18789);
|
||||
pickPrimaryTailnetIPv4.mockReturnValue(undefined);
|
||||
|
||||
const details = buildGatewayConnectionDetails();
|
||||
|
||||
expect(details.url).toBe("ws://127.0.0.1:18789");
|
||||
});
|
||||
});
|
||||
|
||||
describe("callGateway error details", () => {
|
||||
|
||||
@@ -16,7 +16,7 @@ import {
|
||||
type GatewayClientName,
|
||||
} from "../utils/message-channel.js";
|
||||
import { GatewayClient } from "./client.js";
|
||||
import { pickPrimaryLanIPv4 } from "./net.js";
|
||||
import { isSecureWebSocketUrl, pickPrimaryLanIPv4 } from "./net.js";
|
||||
import { PROTOCOL_VERSION } from "./protocol/index.js";
|
||||
|
||||
export type CallGatewayOptions = {
|
||||
@@ -134,6 +134,22 @@ export function buildGatewayConnectionDetails(
|
||||
? "Warn: gateway.mode=remote but gateway.remote.url is missing; set gateway.remote.url or switch gateway.mode=local."
|
||||
: undefined;
|
||||
const bindDetail = !urlOverride && !remoteUrl ? `Bind: ${bindMode}` : undefined;
|
||||
|
||||
// Security check: block ALL insecure ws:// to non-loopback addresses (CWE-319, CVSS 9.8)
|
||||
// This applies to the FINAL resolved URL, regardless of source (config, CLI override, etc).
|
||||
// Both credentials and chat/conversation data must not be transmitted over plaintext to remote hosts.
|
||||
if (!isSecureWebSocketUrl(url)) {
|
||||
throw new Error(
|
||||
[
|
||||
`SECURITY ERROR: Gateway URL "${url}" uses plaintext ws:// to a non-loopback address.`,
|
||||
"Both credentials and chat data would be exposed to network interception.",
|
||||
`Source: ${urlSource}`,
|
||||
`Config: ${configPath}`,
|
||||
"Fix: Use wss:// for the gateway URL, or connect via SSH tunnel to localhost.",
|
||||
].join("\n"),
|
||||
);
|
||||
}
|
||||
|
||||
const message = [
|
||||
`Gateway target: ${url}`,
|
||||
`Source: ${urlSource}`,
|
||||
|
||||
@@ -86,6 +86,77 @@ function getLatestWs(): MockWebSocket {
|
||||
return ws;
|
||||
}
|
||||
|
||||
describe("GatewayClient security checks", () => {
|
||||
beforeEach(() => {
|
||||
wsInstances.length = 0;
|
||||
});
|
||||
|
||||
it("blocks ws:// to non-loopback addresses (CWE-319)", () => {
|
||||
const onConnectError = vi.fn();
|
||||
const client = new GatewayClient({
|
||||
url: "ws://remote.example.com:18789",
|
||||
onConnectError,
|
||||
});
|
||||
|
||||
client.start();
|
||||
|
||||
expect(onConnectError).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
message: expect.stringContaining("SECURITY ERROR"),
|
||||
}),
|
||||
);
|
||||
expect(wsInstances.length).toBe(0); // No WebSocket created
|
||||
client.stop();
|
||||
});
|
||||
|
||||
it("handles malformed URLs gracefully without crashing", () => {
|
||||
const onConnectError = vi.fn();
|
||||
const client = new GatewayClient({
|
||||
url: "not-a-valid-url",
|
||||
onConnectError,
|
||||
});
|
||||
|
||||
// Should not throw
|
||||
expect(() => client.start()).not.toThrow();
|
||||
|
||||
expect(onConnectError).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
message: expect.stringContaining("SECURITY ERROR"),
|
||||
}),
|
||||
);
|
||||
expect(wsInstances.length).toBe(0); // No WebSocket created
|
||||
client.stop();
|
||||
});
|
||||
|
||||
it("allows ws:// to loopback addresses", () => {
|
||||
const onConnectError = vi.fn();
|
||||
const client = new GatewayClient({
|
||||
url: "ws://127.0.0.1:18789",
|
||||
onConnectError,
|
||||
});
|
||||
|
||||
client.start();
|
||||
|
||||
expect(onConnectError).not.toHaveBeenCalled();
|
||||
expect(wsInstances.length).toBe(1); // WebSocket created
|
||||
client.stop();
|
||||
});
|
||||
|
||||
it("allows wss:// to any address", () => {
|
||||
const onConnectError = vi.fn();
|
||||
const client = new GatewayClient({
|
||||
url: "wss://remote.example.com:18789",
|
||||
onConnectError,
|
||||
});
|
||||
|
||||
client.start();
|
||||
|
||||
expect(onConnectError).not.toHaveBeenCalled();
|
||||
expect(wsInstances.length).toBe(1); // WebSocket created
|
||||
client.stop();
|
||||
});
|
||||
});
|
||||
|
||||
describe("GatewayClient close handling", () => {
|
||||
beforeEach(() => {
|
||||
wsInstances.length = 0;
|
||||
|
||||
@@ -21,6 +21,7 @@ import {
|
||||
type GatewayClientName,
|
||||
} from "../utils/message-channel.js";
|
||||
import { buildDeviceAuthPayload } from "./device-auth.js";
|
||||
import { isSecureWebSocketUrl } from "./net.js";
|
||||
import {
|
||||
type ConnectParams,
|
||||
type EventFrame,
|
||||
@@ -109,6 +110,26 @@ export class GatewayClient {
|
||||
this.opts.onConnectError?.(new Error("gateway tls fingerprint requires wss:// gateway url"));
|
||||
return;
|
||||
}
|
||||
|
||||
// Security check: block ALL plaintext ws:// to non-loopback addresses (CWE-319, CVSS 9.8)
|
||||
// This protects both credentials AND chat/conversation data from MITM attacks.
|
||||
// Device tokens may be loaded later in sendConnect(), so we block regardless of hasCredentials.
|
||||
if (!isSecureWebSocketUrl(url)) {
|
||||
// Safe hostname extraction - avoid throwing on malformed URLs in error path
|
||||
let displayHost = url;
|
||||
try {
|
||||
displayHost = new URL(url).hostname || url;
|
||||
} catch {
|
||||
// Use raw URL if parsing fails
|
||||
}
|
||||
const error = new Error(
|
||||
`SECURITY ERROR: Cannot connect to "${displayHost}" over plaintext ws://. ` +
|
||||
"Both credentials and chat data would be exposed to network interception. " +
|
||||
"Use wss:// for the gateway URL, or connect via SSH tunnel to localhost.",
|
||||
);
|
||||
this.opts.onConnectError?.(error);
|
||||
return;
|
||||
}
|
||||
// Allow node screen snapshots and other large responses.
|
||||
const wsOptions: ClientOptions = {
|
||||
maxPayload: 25 * 1024 * 1024,
|
||||
|
||||
@@ -2,6 +2,7 @@ import os from "node:os";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
isPrivateOrLoopbackAddress,
|
||||
isSecureWebSocketUrl,
|
||||
isTrustedProxyAddress,
|
||||
pickPrimaryLanIPv4,
|
||||
resolveGatewayListenHosts,
|
||||
@@ -237,3 +238,42 @@ describe("isPrivateOrLoopbackAddress", () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("isSecureWebSocketUrl", () => {
|
||||
describe("wss:// (TLS) URLs", () => {
|
||||
it("returns true for wss:// regardless of host", () => {
|
||||
expect(isSecureWebSocketUrl("wss://127.0.0.1:18789")).toBe(true);
|
||||
expect(isSecureWebSocketUrl("wss://localhost:18789")).toBe(true);
|
||||
expect(isSecureWebSocketUrl("wss://remote.example.com:18789")).toBe(true);
|
||||
expect(isSecureWebSocketUrl("wss://192.168.1.100:18789")).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("ws:// (plaintext) URLs", () => {
|
||||
it("returns true for ws:// to loopback addresses", () => {
|
||||
expect(isSecureWebSocketUrl("ws://127.0.0.1:18789")).toBe(true);
|
||||
expect(isSecureWebSocketUrl("ws://localhost:18789")).toBe(true);
|
||||
expect(isSecureWebSocketUrl("ws://[::1]:18789")).toBe(true);
|
||||
expect(isSecureWebSocketUrl("ws://127.0.0.42:18789")).toBe(true);
|
||||
});
|
||||
|
||||
it("returns false for ws:// to non-loopback addresses (CWE-319)", () => {
|
||||
expect(isSecureWebSocketUrl("ws://remote.example.com:18789")).toBe(false);
|
||||
expect(isSecureWebSocketUrl("ws://192.168.1.100:18789")).toBe(false);
|
||||
expect(isSecureWebSocketUrl("ws://10.0.0.5:18789")).toBe(false);
|
||||
expect(isSecureWebSocketUrl("ws://100.64.0.1:18789")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("invalid URLs", () => {
|
||||
it("returns false for invalid URLs", () => {
|
||||
expect(isSecureWebSocketUrl("not-a-url")).toBe(false);
|
||||
expect(isSecureWebSocketUrl("")).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false for non-WebSocket protocols", () => {
|
||||
expect(isSecureWebSocketUrl("http://127.0.0.1:18789")).toBe(false);
|
||||
expect(isSecureWebSocketUrl("https://127.0.0.1:18789")).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -396,3 +396,33 @@ export function isLoopbackHost(host: string): boolean {
|
||||
const unbracket = h.startsWith("[") && h.endsWith("]") ? h.slice(1, -1) : h;
|
||||
return isLoopbackAddress(unbracket);
|
||||
}
|
||||
|
||||
/**
|
||||
* Security check for WebSocket URLs (CWE-319: Cleartext Transmission of Sensitive Information).
|
||||
*
|
||||
* Returns true if the URL is secure for transmitting data:
|
||||
* - wss:// (TLS) is always secure
|
||||
* - ws:// is only secure for loopback addresses (localhost, 127.x.x.x, ::1)
|
||||
*
|
||||
* All other ws:// URLs are considered insecure because both credentials
|
||||
* AND chat/conversation data would be exposed to network interception.
|
||||
*/
|
||||
export function isSecureWebSocketUrl(url: string): boolean {
|
||||
let parsed: URL;
|
||||
try {
|
||||
parsed = new URL(url);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (parsed.protocol === "wss:") {
|
||||
return true;
|
||||
}
|
||||
|
||||
if (parsed.protocol !== "ws:") {
|
||||
return false;
|
||||
}
|
||||
|
||||
// ws:// is only secure for loopback addresses
|
||||
return isLoopbackHost(parsed.hostname);
|
||||
}
|
||||
|
||||
@@ -2018,7 +2018,7 @@ description: test skill
|
||||
mode: "remote",
|
||||
auth: { token: "local-token-should-not-use" },
|
||||
remote: {
|
||||
url: "ws://remote.example.com:18789",
|
||||
url: "wss://remote.example.com:18789",
|
||||
token: "remote-token-xyz789",
|
||||
},
|
||||
},
|
||||
@@ -2057,7 +2057,7 @@ description: test skill
|
||||
mode: "remote",
|
||||
auth: { token: "local-token-should-not-use" },
|
||||
remote: {
|
||||
url: "ws://remote.example.com:18789",
|
||||
url: "wss://remote.example.com:18789",
|
||||
token: "remote-token",
|
||||
},
|
||||
},
|
||||
@@ -2094,7 +2094,7 @@ description: test skill
|
||||
gateway: {
|
||||
mode: "remote",
|
||||
remote: {
|
||||
url: "ws://remote.example.com:18789",
|
||||
url: "wss://remote.example.com:18789",
|
||||
password: "remote-pass",
|
||||
},
|
||||
},
|
||||
@@ -2132,7 +2132,7 @@ description: test skill
|
||||
gateway: {
|
||||
mode: "remote",
|
||||
remote: {
|
||||
url: "ws://remote.example.com:18789",
|
||||
url: "wss://remote.example.com:18789",
|
||||
password: "remote-pass",
|
||||
},
|
||||
},
|
||||
|
||||
@@ -21,9 +21,15 @@ vi.mock("../infra/tailnet.js", () => ({
|
||||
pickPrimaryTailnetIPv4,
|
||||
}));
|
||||
|
||||
vi.mock("../gateway/net.js", () => ({
|
||||
pickPrimaryLanIPv4,
|
||||
}));
|
||||
vi.mock("../gateway/net.js", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("../gateway/net.js")>();
|
||||
return {
|
||||
...actual,
|
||||
pickPrimaryLanIPv4,
|
||||
// Allow all URLs in tests - security validation is tested separately
|
||||
isSecureWebSocketUrl: () => true,
|
||||
};
|
||||
});
|
||||
|
||||
const { resolveGatewayConnection } = await import("./gateway-chat.js");
|
||||
|
||||
|
||||
Reference in New Issue
Block a user