Skip to content

fix(ocap-kernel): restrict plain ws:// relay dialing to private/allowed addresses#857

Merged
rekmarks merged 5 commits intomainfrom
rekm/restrict-ws-connections
Feb 27, 2026
Merged

fix(ocap-kernel): restrict plain ws:// relay dialing to private/allowed addresses#857
rekmarks merged 5 commits intomainfrom
rekm/restrict-ws-connections

Conversation

@rekmarks
Copy link
Copy Markdown
Member

@rekmarks rekmarks commented Feb 27, 2026

PR #855 switched webSockets() to use wsFilters.all to enable plain ws:// connections for private-network and reverse-proxy relay scenarios. As written, this also allowed ws:// to any public internet address, sending unencrypted traffic over the open internet.

This PR keeps the transport-level change (wsFilters.all must stay so the transport handles ws:// at all) and adds enforcement in connectionGater.denyDialMultiaddr.

Changes

  • types.ts: Adds allowedWsHosts?: string[] to both RemoteCommsOptions and ConnectionFactoryOptions — an explicit per-instance list of hostnames/IPs that are trusted for plain ws:// beyond the always-allowed private ranges.
  • transport.ts: Threads allowedWsHosts through to ConnectionFactory.make().
  • connection-factory.ts: Replaces the async () => false stub with real gating logic via two new module-level helpers:
    • isPlainWs(ma) — detects plain ws:// using ma.protoNames() (true only when ws is present and neither wss nor tls is).
    • isPrivateAddress(host) — checks IPv4 loopback (127.0.0.0/8), RFC 1918 ranges, IPv6 loopback/ULA/link-local by direct octet comparison (no bitwise operators).
    • denyDialMultiaddr allows wss://, WebRTC, and circuit-relay addresses unconditionally; allows ws:// only to private/loopback IPs or hosts on allowedWsHosts; denies everything else.
  • connection-factory.test.ts: Replaces the single stub test with a 9-case it.each suite covering secure transports, all four private IPv4 ranges, public IP denial, allowlisted/non-allowlisted hostnames, and non-WebSocket multiaddrs.

Testing

The new connectionGater.denyDialMultiaddr describe block exercises the gating function directly by passing minimal Multiaddr-shaped objects (with protoNames() and toOptions()) extracted from the createLibp2p mock call args. This avoids any libp2p networking and keeps the tests fast and deterministic.


Note

Medium Risk
Changes libp2p connection gating for ws:// dials, which can impact connectivity if deployments rely on public plaintext WebSocket relays without configuring allowedWsHosts. Logic is security-sensitive (transport encryption policy) but narrowly scoped and covered by new table-driven tests.

Overview
Tightens relay dialing security by adding connectionGater.denyDialMultiaddr logic that denies plaintext ws:// connections to public/unrecognised hosts while still allowing wss:// and non-WebSocket transports.

Introduces a new allowedWsHosts option (plumbed from RemoteCommsOptions through transport.ts into ConnectionFactory) to permit specific non-private ws:// relay hosts when needed, and replaces the prior “allow all” gater stub with RFC-based private/loopback detection plus an allowlist.

Updates connection-factory.test.ts to validate the new gating behavior across private IPv4 ranges, public IP denial, allowlisted hostnames, and non-WebSocket multiaddrs.

Written by Cursor Bugbot for commit 0b7674f. This will update automatically on new commits. Configure here.

…ed addresses

Adds a connectionGater.denyDialMultiaddr implementation that blocks
plain ws:// connections to public internet addresses. Private/loopback
IPs (RFC 1918, 127.0.0.0/8, IPv6 ULA/link-local) and an explicit
per-instance allowedWsHosts list are always permitted; everything else
over plain ws:// is denied. wss://, WebRTC, and circuit-relay addresses
are unaffected.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@rekmarks rekmarks requested a review from a team as a code owner February 27, 2026 01:35
@rekmarks rekmarks requested a review from sirtimid February 27, 2026 01:36
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread packages/ocap-kernel/src/remotes/platform/connection-factory.ts
… positives

The startsWith('fc'/'fd'/'fe80:') checks in isPrivateAddress could
match DNS hostnames like 'fcevil.attacker.com', allowing plain ws://
connections to attacker-controlled hosts. Fix by validating the host is
an actual IPv6 address using the URL parser (new URL('http://[<host>]')
throws for non-IPv6 strings) before applying the prefix checks. Adds a
regression test for the bypass.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 27, 2026

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 76.05%
⬆️ +0.03%
6588 / 8662
🔵 Statements 75.94%
⬆️ +0.03%
6694 / 8814
🔵 Functions 73.88%
⬆️ +0.05%
1647 / 2229
🔵 Branches 75.33%
⬆️ +0.06%
2452 / 3255
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/ocap-kernel/src/remotes/types.ts 100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
100%
🟰 ±0%
packages/ocap-kernel/src/remotes/platform/connection-factory.ts 95.55%
⬇️ -1.72%
86.11%
⬇️ -3.89%
96.87%
⬆️ +0.58%
95.45%
⬇️ -1.77%
66, 75, 82, 340, 485, 519
packages/ocap-kernel/src/remotes/platform/transport.ts 82.37%
🟰 ±0%
80.72%
🟰 ±0%
74.19%
🟰 ±0%
82.37%
🟰 ±0%
117, 159-160, 165-169, 211-220, 253, 287-305, 329, 413, 457-460, 484, 508-513, 516-517, 521-524, 565, 595, 614-616, 625, 736
Generated in workflow #3836 for commit 0b7674f by the Vitest Coverage Report Action

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@rekmarks rekmarks force-pushed the rekm/restrict-ws-connections branch from 2c812fe to cf49c01 Compare February 27, 2026 04:04
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Comment thread packages/ocap-kernel/src/remotes/platform/connection-factory.ts
…ostname bypass

An all-hex hostname like 'fdcafe' passed the previous /^[0-9a-f:]+$/
regex and the startsWith('fd') check, causing isPrivateAddress to return
true and allowing plain ws:// to attacker-controlled hosts. Every valid
IPv6 address contains at least one colon; requiring one in the regex
closes the gap. Adds a regression test for the bypass.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@rekmarks rekmarks enabled auto-merge February 27, 2026 04:39
@rekmarks rekmarks added this pull request to the merge queue Feb 27, 2026
Merged via the queue into main with commit 3f4caa7 Feb 27, 2026
29 checks passed
@rekmarks rekmarks deleted the rekm/restrict-ws-connections branch February 27, 2026 10:10
sirtimid added a commit that referenced this pull request Feb 27, 2026
After merging PR #857, plain ws:// dials to public IPs are denied by
the connection gater. Extract the relay host from the multiaddr and
pass it as allowedWsHosts to initRemoteComms so the relay dial is
permitted.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sirtimid added a commit that referenced this pull request Feb 27, 2026
PR #857 added allowedWsHosts to RemoteCommsOptions and the transport
layer but not to the RPC handler's superstruct validation. The
daemon exec call rejects allowedWsHosts as an unknown field.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sirtimid added a commit that referenced this pull request Feb 27, 2026
PR #857 added allowedWsHosts to RemoteCommsOptions and the transport
layer but not to the RPC handler's superstruct validation. Daemon
callers passing allowedWsHosts get a validation error because the
params struct rejects the unknown field.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
github-merge-queue Bot pushed a commit that referenced this pull request Feb 27, 2026
)

## Summary

- Add `allowedWsHosts` to the `initRemoteComms` RPC handler's
superstruct params validation
- Thread the value through to `kernel.initRemoteComms(options)`
- PR #857 added `allowedWsHosts` to `RemoteCommsOptions` and the
transport/connection-factory layer but missed the RPC handler, so daemon
callers passing `allowedWsHosts` get a validation error

## Test plan

- [x] `yarn workspace @MetaMask/ocap-kernel test:dev:quiet` — existing
tests pass
- [x] `ocap daemon exec initRemoteComms
'{"relays":["/ip4/1.2.3.4/tcp/9001/ws/p2p/..."],"allowedWsHosts":["1.2.3.4"]}'`
— no longer rejected

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> Low risk: adds a new optional RPC param and passes it through to
existing remote comms initialization, with validation limited to a
string array.
> 
> **Overview**
> Fixes the `initRemoteComms` RPC handler to accept an optional
`allowedWsHosts` array in its superstruct params validation.
> 
> When provided, the handler now threads `allowedWsHosts` into
`RemoteCommsOptions` passed to `kernel.initRemoteComms`, preventing
callers from being rejected due to unknown params.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
4085270. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sirtimid added a commit that referenced this pull request Mar 3, 2026
After merging PR #857, plain ws:// dials to public IPs are denied by
the connection gater. Extract the relay host from the multiaddr and
pass it as allowedWsHosts to initRemoteComms so the relay dial is
permitted.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sirtimid added a commit that referenced this pull request Mar 3, 2026
After merging PR #857, plain ws:// dials to public IPs are denied by
the connection gater. Extract the relay host from the multiaddr and
pass it as allowedWsHosts to initRemoteComms so the relay dial is
permitted.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sirtimid added a commit that referenced this pull request Mar 16, 2026
After merging PR #857, plain ws:// dials to public IPs are denied by
the connection gater. Extract the relay host from the multiaddr and
pass it as allowedWsHosts to initRemoteComms so the relay dial is
permitted.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants