GET /api/v1/peers/{did}/ping is mounted in peer_read_routes (crates/gitlawb-node/src/server.rs:290-292) with no auth layer and no per-IP brake. The only layer on the merged router is TraceLayer (server.rs:482-493), so nothing outer covers it. The two comparable outbound-fan-out routes in the same file both carry a brake: sync_trigger_routes at server.rs:300-308 and peer_write_routes at server.rs:323-328, the latter with a comment explaining why.
So any anonymous caller can make this node issue an outbound HTTP request to a peer-controlled URL, on demand, at whatever rate they like.
What #248 changed, measured rather than assumed
I checked the pre-PR handler before writing this. At 111cff7e ping_peer made exactly one outbound GET {peer}/health and additionally wrote mark_peer_ping, and the shared client already carried .timeout(Duration::from_secs(10)). So two things people might expect are not true: total connection hold time is unchanged, and this node's own per-request database work went down, because #248 deleted that write.
Two things did change:
- A peer answering
/ready with 404 now draws a second outbound leg to /health. I confirmed this by execution with a throwaway #[sqlx::test] driving the mounted handler against a mockito peer with /ready 404 .expect(1) and /health 200 .expect(1); both mock assertions passed, so one inbound anonymous request produced two outbound requests.
- The probe now lands on
/ready (server.rs:505), which runs state.db.ping(), a pool acquire plus a query, on the target node. /health (server.rs:497) is constant JSON and touches nothing.
Combined with the fact that a peer's http_url is attacker-registrable (announce accepts unsigned bodies while GITLAWB_REQUIRE_SIGNED_PEER_WRITES defaults false, config.rs:48), the endpoint is a reflector: the attacker picks the destination, the destination sees this node's IP, and the same peer DID registered across M nodes fans the same victim through M relays.
Fix direction
Either put peer_read_routes behind the same per-IP brake peer_write_routes already uses, with its own bucket, or serve the last gossip-observed last_ping_ok/last_seen straight from the peers row and make no outbound request at all. The second option fits the "read-only diagnostic" framing the handler comment now carries, and costs nothing.
Scope
Pre-existing surface, not a #248 regression, which is why I approved that PR rather than growing it. #196's brake is write-surface only and does not reach this route. #207 (TrustedProxy header rotation) is upstream of any per-IP keying here and would need settling first if the brake route is chosen.
GET /api/v1/peers/{did}/pingis mounted inpeer_read_routes(crates/gitlawb-node/src/server.rs:290-292) with no auth layer and no per-IP brake. The only layer on the merged router isTraceLayer(server.rs:482-493), so nothing outer covers it. The two comparable outbound-fan-out routes in the same file both carry a brake:sync_trigger_routesatserver.rs:300-308andpeer_write_routesatserver.rs:323-328, the latter with a comment explaining why.So any anonymous caller can make this node issue an outbound HTTP request to a peer-controlled URL, on demand, at whatever rate they like.
What #248 changed, measured rather than assumed
I checked the pre-PR handler before writing this. At
111cff7eping_peermade exactly one outboundGET {peer}/healthand additionally wrotemark_peer_ping, and the shared client already carried.timeout(Duration::from_secs(10)). So two things people might expect are not true: total connection hold time is unchanged, and this node's own per-request database work went down, because #248 deleted that write.Two things did change:
/readywith 404 now draws a second outbound leg to/health. I confirmed this by execution with a throwaway#[sqlx::test]driving the mounted handler against a mockito peer with/ready404.expect(1)and/health200.expect(1); both mock assertions passed, so one inbound anonymous request produced two outbound requests./ready(server.rs:505), which runsstate.db.ping(), a pool acquire plus a query, on the target node./health(server.rs:497) is constant JSON and touches nothing.Combined with the fact that a peer's
http_urlis attacker-registrable (announce accepts unsigned bodies whileGITLAWB_REQUIRE_SIGNED_PEER_WRITESdefaults false,config.rs:48), the endpoint is a reflector: the attacker picks the destination, the destination sees this node's IP, and the same peer DID registered across M nodes fans the same victim through M relays.Fix direction
Either put
peer_read_routesbehind the same per-IP brakepeer_write_routesalready uses, with its own bucket, or serve the last gossip-observedlast_ping_ok/last_seenstraight from the peers row and make no outbound request at all. The second option fits the "read-only diagnostic" framing the handler comment now carries, and costs nothing.Scope
Pre-existing surface, not a #248 regression, which is why I approved that PR rather than growing it. #196's brake is write-surface only and does not reach this route. #207 (TrustedProxy header rotation) is upstream of any per-IP keying here and would need settling first if the brake route is chosen.