Skip to content

fix(security): dial-time SSRF guard for participant InferenceUrl (#1470) - #1491

Open
Ryanchen911 wants to merge 1 commit into
gonka-ai:devshard-0.2.15-v5from
Ryanchen911:fix/1470-ssrf-inference-url
Open

fix(security): dial-time SSRF guard for participant InferenceUrl (#1470)#1491
Ryanchen911 wants to merge 1 commit into
gonka-ai:devshard-0.2.15-v5from
Ryanchen911:fix/1470-ssrf-inference-url

Conversation

@Ryanchen911

@Ryanchen911 Ryanchen911 commented Jul 20, 2026

Copy link
Copy Markdown

Fixes #1470.

Summary

Closes the residual SSRF left after #505/#534. The registration gate only rejects literal private IPs and does no DNS resolution, so any participant-controlled InferenceUrl hostname passes — and DNS rebinding / attacker-controlled DNS can force validator/TA DAPIs to connect to loopback, cloud metadata (169.254.169.254), or RFC1918 during mandatory payload retrieval, PoC proof fetch, and TA→executor forwarding.

The real fix is at dial time: a net.Dialer.Control hook re-checks the resolved IP on every connection (including each redirect hop and dual-stack candidate), which is what defeats rebinding. A registration-time resolve alone cannot.

What changed

Dial-time guard applied to all sinks that dial a participant-controlled InferenceUrl:

# Sink Location
1 Validator payload retrieval internal/validation/payload_retrieval.go
2 PoC proof fetch poc/proof_client.go
3 TA → executor forward internal/server/public/post_chat_handler.go
4 devshard payload fetch internal/devshard, cmd/devshardd
5 devshard transport dialer devshard/transport

New helpers: decentralized-api/internal/httpguard (reuses the chain's utils.IsPrivateIP) and devshard/transport/ssrf.go (a deliberate mirror, since the devshard module can't import the chain utils).

Redirects: the payload/proof clients refuse redirects (http.ErrUseLastResponse), so a public InferenceUrl cannot 302 a validator to a private target (attack chain C).

Correctness: shared clients

The server and devshard HTTP clients are shared between the operator's own trusted ML nodes (node.InferenceUrl(), routinely on localhost/private ranges) and untrusted participant URLs. Blanket-guarding them would break local ML calls. The guard is therefore applied via separate clients used only for the participant path — the trusted local-node path is never guarded.

Deployment switch (secure by default)

Local dev / docker-compose / testermint register docker-internal hostnames (e.g. http://genesis-api:9000) that resolve to private IPs, so a hard dial-time block would break them. A new switch — DAPI_API__ALLOW_PRIVATE_INFERENCE_URLS (and DEVSHARD_ALLOW_PRIVATE_ADDRESSES for standalone devshardd), default false = blocked — is enabled only in those environments (docker-compose-base.yml, DockerGroup.kt). Production templates leave it unset, so real validators stay protected.

Registration gate

Intentionally left DNS-free: ValidateBasic is stateless and must be deterministic across validators — network I/O there would break consensus, and a registration-time lookup can't stop rebinding regardless. A code comment documents this. Also fixes an IPv4-mapped IPv6 gap in isPrivateIP so ::ffff:169.254.169.254 is caught.

Tests

  • Dial guard blocks: hostname→127.0.0.1, →169.254.169.254, public→302→private, IPv4-mapped IPv6, decimal/hex host encodings, RFC1918/ULA/link-local.
  • allowPrivate=true permits all of the above (dev/test regression).
  • The trusted local ML path is never guarded.
  • Redirects are refused by the payload/proof clients.

Verified with go build ./... and go test ./... across decentralized-api, devshard, and inference-chain (only pre-existing Docker-dependent testcontainers tests are skipped locally for lack of a Docker host).

@a-kuprin

Copy link
Copy Markdown
Collaborator

Registration only blocks literal private IPs / localhost on InferenceUrl. A hostname passes without DNS. Later, gateways/validators may dial that URL, so DNS can make the dial land on a private IP.

Informational (not High/Medium):

  1. Attacker chooses host/IP via DNS, not the path. Your client always hits fixed routes like /v1/inference/payloads/... or /devshard/{version}/.... On IMDS or random internal services that is usually 404 / wrong handler, not a chosen sensitive path.

  2. Data is not stolen. The response stays on the dialing node. There is no solid exfil back to the attacker.

  3. No economic attack. No escrow drain, no consensus break, no profitable protocol abuse.

  4. Self-limiting. If the URL always points private, the “attacker” cannot really serve as a host/executor and gets missed/penalized. They are not a clean remote attacker with upside.

  5. Max impact is roughly: “did something on this IP:port answer HTTP?” — noise/probe, not takeover.

Disposition: Treat as incomplete defense-in-depth on egress. Optional hardening (dial-time private-IP block) is fine; severity Informational.

For an Informational / constrained-path issue, this PR is larger and more invasive than it needs to be — and it still misses a live sink.

~21 files / +622 for wiring + duplication, not because the idea needs that surface

Verdict on #1491

Technically valid approach, overscoped for the real risk. CI is green (API /devshard / chain / lint). Dial-time Control + no-redirect on participant clients is the right kind of fix. Leaving registration DNS-free is correct.

But for an Informational / constrained-path issue, this PR is larger and more invasive than it needs to be — and it still misses a live sink.

Smarter / smaller design

Two choke points cover almost everything that matters:

Place Covers
common/validation.PayloadRetrievalClient (guarded dial + no redirect) All FetchPayloadsHTTP callers, including standalone devshardd
devshard/transport.getTransport Control Gateway/host peer dials

Optional: one env (ALLOW_PRIVATE_…) read at those two sites.

Skip or defer: classic TA/post_chat, PoC client churn, chain ValidateBasic essay (short comment enough), duplicated isPrivateIP (reuse one helper via common if both modules can import it).

That is roughly 3–6 files, not 21.

Bottom line

@a-kuprin a-kuprin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Oversized PR

Ryanchen911 added a commit to Ryanchen911/gonka that referenced this pull request Jul 20, 2026
Address @a-kuprin review (gonka-ai#1491): for an Informational, constrained-path
finding the guard does not need to touch every sink. Keep the two choke
points that cover the mandatory participant-URL dials and revert the rest.

Kept:
  - payload-retrieval client (guarded dial + no redirect) — covers all
    FetchPayloadsHTTP callers incl. standalone devshardd
  - devshard/transport.getTransport Control — gateway/host peer dials

Reverted (defense-in-depth churn, not needed for the real risk):
  - TA -> executor forward client split (post_chat_handler / server)
  - PoC proof client AllowPrivate plumbing
  - long ValidateBasic essay -> short comment
  - unused exported IsLocalhost helper

The duplicated isPrivateOrLocalAddr in devshard/transport stays: the
devshard module has no dependency on github.com/productscience/inference,
so the chain util cannot be imported there (documented in the mirror).

Co-Authored-By: Claude <noreply@anthropic.com>
@Ryanchen911

Copy link
Copy Markdown
Author

Thanks for the review — agreed on scoping to the real risk. Pushed 489b80f narrowing the PR to the two choke points you identified.

Kept

  • Payload-retrieval client (guarded dial + no-redirect) — covers every FetchPayloadsHTTP caller, including standalone devshardd.
  • devshard/transport.getTransport Control — gateway/host peer dials.

Reverted

  • TA→executor forward client split (post_chat_handler / server)
  • PoC proof-client AllowPrivate plumbing
  • Long ValidateBasic comment → one-liner
  • Unused exported IsLocalhost helper

Net is now ~11 source files (config + one env switch + the two guards + tests + deploy toggles), down from 21.

On "standalone payload default client still unguarded": it is guarded in this PR — the standalone devshardd validator dials payloads through v.payloadHttpClient, built via httpguard.NewClient(...) in cmd/devshardd/main.go and passed into ValidateInferenceWithExecutor (cmd/devshardd/validation.go). I went with constructor injection rather than a shared package-level default client because the two binaries source the toggle from different env vars (DAPI_API__ALLOW_PRIVATE_INFERENCE_URLS vs DEVSHARD_ALLOW_PRIVATE_ADDRESSES) and the ML-node path on the same struct must stay unguarded — so a single global default couldn't serve both. Happy to switch to a shared default client if you'd prefer that shape.

On the duplicated isPrivateOrLocalAddr: the devshard module has no dependency on github.com/productscience/inference, so the chain util can't be imported there. Left it as a documented mirror rather than adding a module dependency just for one predicate — let me know if you'd rather I add the dep or a small shared module.

@Ryanchen911

Copy link
Copy Markdown
Author

hi @a-kuprin , can you please check this PR again, thanks a lot!

@a-kuprin a-kuprin added this to the v0.2.x-devshard5 milestone Aug 6, 2026
@a-kuprin

a-kuprin commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

@Ryanchen911 I've added it to v5 milestone. I believe it will be merged next week

@a-kuprin
a-kuprin changed the base branch from main to devshard-0.2.15-v5 August 13, 2026 08:48
@a-kuprin

Copy link
Copy Markdown
Collaborator

@Ryanchen911 Please rebase. It should be targeted to devshard-0.2.15-v5

…onka-ai#1470)

Registration-time validation of participant InferenceUrl only rejects
literal private IPs and localhost strings; it never resolves DNS, because
ValidateBasic must stay deterministic for consensus. A hostname therefore
always passes, and DNS rebinding needs no new on-chain tx. Honest nodes
then dial that URL during mandatory payload retrieval and devshard peer
communication, so the dial can land on loopback, link-local metadata
(169.254.169.254), or RFC1918.

Fix at dial time, where the resolved IP is known. net.Dialer.Control runs
after resolution and once per candidate IP, with the address already
"ip:port", so it vets every real dial target: each dual-stack candidate
and each redirect hop. That is what defeats rebinding, which a
resolve-then-connect check cannot.

One guard (common/httpguard), reusing the registration gate's predicate
(utils.IsPrivateIP) so both agree on what "private" means. Two choke
points:

- common/validation.PayloadRetrievalClient: the default client for
  executor payload fetches, which also refuses redirects. Covers every
  FetchPayloadsHTTP caller.
- devshard/transport.getTransport: gateway/host peer dials, whose baseURL
  is the on-chain Participant.InferenceUrl.

Clients that dial our own infrastructure are deliberately left unguarded:
the devshardd inference engine's ML-node client, and devshardctl's chain
RPC / public-API clients. Those targets come from local config and
legitimately live on localhost/private ranges.

Local dev, docker-compose, and e2e register docker hostnames that resolve
to private IPs, so an opt-out is required. Secure by default: the guard is
active unless DEVSHARD_ALLOW_PRIVATE_ADDRESSES is set, which is enabled
only in local-test-net, testenv (gencompose), and testermint. Production
templates leave it unset.

Also fixes an IPv4-mapped IPv6 gap in the existing predicate, so
::ffff:169.254.169.254 is now classified private.
@Ryanchen911
Ryanchen911 force-pushed the fix/1470-ssrf-inference-url branch from 489b80f to af78d88 Compare August 13, 2026 10:31
@Ryanchen911

Copy link
Copy Markdown
Author

Rebased onto devshard-0.2.15-v5. The v5 refactor let me shrink this further and drop the duplication you flagged: 21 files → 14.

Both choke points are now exactly the two you named:

  • common/validation.PayloadRetrievalClient — now guarded + no-redirect. The single caller (devshard/cmd/devshardd/inference/validate.go:107) passes nil, so it lands on this default client.
  • devshard/transport.getTransport — guard on the dialer.

The duplicated isPrivateIP is gone. On v5 the dependency graph changed: devshardcommoninference-chain. Previously the devshard module couldn't import the chain util, which is why I'd mirrored the predicate. Now the guard lives once in common/httpguard and both choke points share the same utils.IsPrivateIP as the registration gate.

All decentralized-api changes are gone. v5 moved payload retrieval into common, and dapi no longer imports common/validation or devshard/transport — so the config field, the main.go wiring, the internal/httpguard package and the DAPI_API__ALLOW_PRIVATE_INFERENCE_URLS flag were all dead code. apiconfig/config.go and main.go are now byte-identical to the target branch.

The flag is a single DEVSHARD_ALLOW_PRIVATE_ADDRESSES (default false = secure), wired into the three processes that actually dial: devshardd, devshardctl, and the Testermint in-container devshardctl exec. Production templates under deploy/join/ leave it unset.

One note on scope, since you'd asked about the standalone path earlier: devshardctl also dials through transport, so it needs the same opt-out in citest/Testermint — without it the e2e stacks would be blocked by the guard.

I kept own-ML-node and self-hosted-endpoint dials unguarded, and verified each: the ML engine uses its own client (inference/engine.go:68), and chain RPC / public API go through common/chain and NewChainPhaseGate. Those legitimately point at private addresses in production (node:9090, http://api:9000). Only HostInfo.URL, which comes from the on-chain Participant.InferenceUrl (bridge/grpc.go:117), is guarded.

Tests: four modules build; common/httpguard, common/validation, devshard/transport, devshard/protocol, devshard/cmd/devshardd/..., devshard/cmd/devshardctl, the gencompose golden test, and the chain utils tests all pass. devshard/transport and devshard/protocol needed a TestMain opt-out — they run httptest on loopback through the real dialer. The storage packages fail locally only on rootless Docker not found (testcontainers), unrelated to this change.

Still happy to trim further if you'd prefer the devshardctl/Testermint wiring split into a separate PR.

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.

Security: Residual SSRF on InferenceUrl — DNS/rebind + validator redirect (incomplete fix after #505/#534)

2 participants