fix(didcomm): refuse mediator and VTA endpoints that name a non-public host - #241
Merged
Merged
Conversation
…c host A mediator's REST, auth and WebSocket URLs come out of its DID document, and that document is not ours. A mediator DID arriving by QR, or one whose document changed after onboarding, decided where this wallet sent its auth handshake and the mediator JWT it gets back, and the only check was the scheme. An extension fetch is not CORS-bound and the wallet runs in the user's browser, so `https://127.0.0.1`, `https://169.254.169.254` or `https://printer.local` in a mediator document was a request from inside the user's own network with the wallet's credentials attached. A VTA's REST base URL is the same shape of input, written from a document (or a QR code) at onboarding, and it is the one channel that carries a bearer token. `@openvtc/vti-didcomm-js` 0.8 adds the guard (`net-guard.js`) and threads a `netPolicy` through `resolveMediator`, `authenticateToMediator`, `MediatorSession` and VTA REST auth. This adopts it. ## What decides, and where it is passed `walletNetPolicy` (`extension/src/net-policy.ts`) is the only place the policy is chosen, and it keys off the build: `npm run dev` builds with `--mode development`, so `import.meta.env.DEV` is set and the policy carries `allowInsecure` **and** `allowPrivate`. Every packaged build — `npm run build`, which is what CI and the Web Store zip run — gets neither. Outside a vite build `import.meta.env` is absent and that reads as production, so a test has to ask for the dev policy rather than inherit it. It reaches every call site that reaches the library: - `resolveMediatorEndpoint` — the transport diagnosis and the connection self-test, so the self-test resolves under the policy the real path honours rather than passing a check the wallet will not. - `connectMediatorSession` — onboarding (twice), the warm holder session and the approver session. It hands the policy to `authenticateToMediator` *and* to `MediatorSession`, so the socket that carries the JWT is held to the same terms as the handshake; the session re-checks the endpoint before every open. - `RestChannel` — its `baseUrl` reaches `getVtaBearer` and the dispatcher POST through `guardedFetch`, so every URL built from it is vetted before it is dialed and no request follows a redirect. One exported `vtaRestEndpointPolicy` serves both, rather than two policies that happen to match. ## Breaking for a local stack `allowInsecure: true` no longer implies that private hosts are allowed, and the option is gone from `@openvtc/pnm-core` rather than kept as an alias — nothing is deployed, so there is no fold to write. `connectMediatorSession`, `resolveMediatorEndpoint` and `WalletSession.fromDids` take `netPolicy`. A mediator or VTA on loopback now needs both flags: netPolicy: { allowInsecure: true, allowPrivate: true } In the extension that is automatic with `npm run dev`. A wallet built with `npm run build` refuses a local mediator, which is the point of tying it to the build rather than to a setting. ## Surfacing a refusal A refusal carries `code: "E_BLOCKED_ENDPOINT"`. `isBlockedEndpointError` (`core/src/did/egress-guard.ts`) matches the code, not the class: there are now two guards carrying one code — the library's, and this package's did:webvh one — and an `instanceof` would silently miss whichever it was not written against (R3.7). `transport-diagnosis.ts` classifies it first, as `mediator/blocked-endpoint`, and deliberately ignores the reachability probe. Nothing was contacted, so "the mediator is up and refused this request" would be a claim about a request that was never made, and it would send the reader to a mediator's CORS config instead of to the address in its document. The network pane and the pasted self-test report render that line as-is. 0.8 also stops copying a response body into the error message — from a hostile endpoint that is attacker-chosen text, and these strings end up in logs, a pasted report and the UI — and puts it on `err.body` with the HTTP status on `err.status`. The `rejected` branch reads both from the fields and bounds the excerpt. Nothing else in this repo parsed those messages. ## Tests - `core/tests/didcomm.net-policy.mjs`: a mediator document advertising loopback is refused by default and accepted under the dev policy; `allowInsecure` alone is not enough, which is the 0.8 semantics the wallet now depends on; a `http://localhost` stack needs both flags and each alone is refused. The refusal reaches neither the injected `fetch` nor the injected WebSocket — the difference between a control and an error message. The positive control drives the whole path, handshake then socket. - `core/tests/vta.rest-net-policy.mjs`: the same for a VTA REST base URL — loopback, a local-only name and plaintext on a public host, each with zero requests, then the dev policy completing the handshake. - `extension/tests/transport-diagnosis.test.mts`: the blocked-endpoint case as the pane renders it, that a probe result cannot re-classify it, and that the status and body are read from the fields. ## What it cannot do An extension has no DNS API, so a public name that *resolves* to a private address (`127.0.0.1.nip.io`, a rebinding domain) passes every check here. `allowHosts` is the answer to that and needs a pinned list of hosts; this wallet records its mediators and agents as DIDs, so it has no such list, and narrowing to one derived from the same document would check a value against itself. The parameter is plumbed through and left unset; pinning it belongs beside the inbox in `config.ts`, where an operator can state it. Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A mediator's REST, auth and WebSocket URLs come out of its DID document, and
that document is not ours. A mediator DID arriving by QR, or one whose document
changed after onboarding, decided where this wallet sent its auth handshake and
the mediator JWT it gets back, and the only check was the scheme. An extension
fetch is not CORS-bound and the wallet runs in the user's browser, so
https://127.0.0.1,https://169.254.169.254orhttps://printer.localin amediator document was a request from inside the user's own network with the
wallet's credentials attached. A VTA's REST base URL is the same shape of input,
written from a document (or a QR code) at onboarding, and it is the one channel
that carries a bearer token.
@openvtc/vti-didcomm-js0.8 adds the guard (net-guard.js) and threads anetPolicythroughresolveMediator,authenticateToMediator,MediatorSessionand VTA REST auth. This bumpspackages/coreto^0.8.0(the only package that depends on it —
packages/tsp-jsdoes not) and adoptsit.
What decides, and where it is passed
walletNetPolicy(extension/src/net-policy.ts) is the only place the policyis chosen, and it keys off the build:
npm run devbuilds with--mode development, soimport.meta.env.DEVis set and the policy carriesallowInsecureandallowPrivate. Every packaged build —npm run build,which is what CI and the Web Store zip run — gets neither. Outside a vite build
import.meta.envis absent and that reads as production, so a test has to askfor the dev policy rather than inherit it.
It reaches every call site that reaches the library:
resolveMediatorEndpoint— the transport diagnosis and the connectionself-test, so the self-test resolves under the policy the real path honours
rather than passing a check the wallet will not.
connectMediatorSession— onboarding (twice), the warm holder session and theapprover session. It hands the policy to
authenticateToMediatorand toMediatorSession, so the socket that carries the JWT is held to the sameterms as the handshake; the session re-checks the endpoint before every open.
RestChannel— itsbaseUrlreachesgetVtaBearerand the dispatcher POSTthrough
guardedFetch, so every URL built from it is vetted before it isdialed and no request follows a redirect. One exported
vtaRestEndpointPolicyserves both, rather than two policies that happen tomatch.
swapAclResttakes the policy too, so the deprecated REST swap pathis not the one place that cannot reach a dev VTA.
What developers must change locally
A local stack now needs two flags, not one.
allowInsecure: trueno longerimplies that private hosts are allowed, so a dev setup pointed at a mediator or
VTA on
localhostor127.0.0.1must pass:with
npm run dev(ornpm run dev:extension). A wallet built withnpm run buildwill refuse a mediator or VTA on loopback, and the refusal isdeliberate: tying the opt-outs to the build is what stops a packaged wallet
carrying them.
@openvtc/pnm-coredirectly (a script, the PWA, a test): theallowInsecureoption is gone, not deprecated — nothing is deployed, sothere is no compatibility fold. Replace
allowInsecure: truewith thenetPolicyabove onconnectMediatorSession,resolveMediatorEndpointandWalletSession.fromDids, and pass the same toRestChannel/getVtaBearer(and anything built on them, e.g.vaultList) when the VTA islocal. Passing nothing is the strict, production policy.
https://127.0.0.1is refusedby
allowInsecurealone. If a dev build suddenly cannot reach a stack it usedto, that combination is why.
Surfacing a refusal
A refusal carries
code: "E_BLOCKED_ENDPOINT".isBlockedEndpointError(
core/src/did/egress-guard.ts) matches the code, not the class: there are nowtwo guards carrying one code — the library's, and this package's did:webvh one —
and an
instanceofwould silently miss whichever it was not written against(R3.7).
transport-diagnosis.tsclassifies it first, asmediator/blocked-endpoint,and deliberately ignores the reachability probe. Nothing was contacted, so "the
mediator is up and refused this request" would be a claim about a request that
was never made, and it would send the reader to a mediator's CORS config instead
of to the address in its document. The network pane and the pasted self-test
report render that line as-is; the mediator-resolve check in the self-test
reports it too, since it now resolves under the same policy.
0.8 also stops copying a response body into the error message — from a hostile
endpoint that is attacker-chosen text, and these strings end up in logs, a
pasted report and the UI — and puts it on
err.bodywith the HTTP status onerr.status. Therejectedbranch reads both from the fields and bounds theexcerpt. Nothing else in this repo parsed those messages: the only
message-matching left is in tests.
Tests
core/tests/didcomm.net-policy.mjs: a mediator document advertising loopbackis refused by default and accepted under the dev policy;
allowInsecurealoneis not enough, which is the 0.8 semantics the wallet now depends on; a
http://localhoststack needs both flags and each alone is refused. Therefusal reaches neither the injected
fetchnor the injected WebSocket — thedifference between a control and an error message. The positive control drives
the whole path, handshake then socket, through a fake WebSocket.
core/tests/vta.rest-net-policy.mjs: the same for a VTA REST base URL —loopback, a local-only name and plaintext on a public host, each with zero
requests, then the dev policy completing the handshake.
extension/tests/transport-diagnosis.test.mts: the blocked-endpoint case asthe pane renders it, that a probe result cannot re-classify it, and that the
status and body are read from the fields.
Full CI sequence run locally on Node 24:
npm run lint,npm run build,npm test(core 621, extension 594, tsp-js, demos — all passing), the MV3single-bundle and dynamic-
import()assertion, the admin / key-material /persona bundle guards in both directions, the console single-bundle assertion,
and
scripts/package.mjswith the Web Store zip checks.What it cannot do
An extension has no DNS API, so a public name that resolves to a private
address (
127.0.0.1.nip.io, a rebinding domain) passes every check here.allowHostsis the answer to that and needs a pinned list of hosts; this walletrecords its mediators and agents as DIDs, so it has no such list, and narrowing
to one derived from the same document would check a value against itself. The
parameter is plumbed through and left unset; pinning it belongs beside the inbox
in
config.ts, where an operator can state it. The did:webvh resolution guardthis package already has (
did/egress-guard.ts) is left in place — replacing itwith the library's now-published
net-guardis a separate change, and the twoagree on ranges, names, code and reasons.
Overlap with my other open branches
Branched from
origin/main. None of the filessec-4045/egress-and-input-boundsadds or changes are touched (
core/src/http/public-endpoint.ts,proxyFetch,registerPushChannel,content.ts,armor.ts). Two shared files will conflicttextually if another branch lands first:
package-lock.json(the dependencybump) and
CLAUDE.md(one new bullet in the networking-rules list, after R3.7).