fix(security): block SSRF in remote connection endpoints#27
Conversation
The testSignalKConnection, requestAccess and checkAccessRequest endpoints passed an attacker-controlled host straight to http(s).request, letting the server be driven to loopback, the cloud metadata service (169.254.169.254) and other non-routable destinations (GHSA-q59x-jc9f-gfqf). Validate the destination before connecting: reject special-use addresses (loopback, link-local/metadata, unspecified, multicast, reserved) for both IP literals and resolved hostnames, pinning the connection to a checked address to prevent DNS rebinding. Private LAN ranges stay reachable so the intended use - connecting to another Signal K server on the boat network - keeps working. Also require requestId to be a UUID to stop path traversal in checkAccessRequest.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR introduces SSRF (Server-Side Request Forgery) protection and request validation for remote-connection admin endpoints. It adds a new SSRF guard module that blocks outbound connections to special-use IP ranges, defines TypeBox schemas for validating remote requests, and integrates both into existing endpoints with standardized error handling and safe DNS resolution. ChangesRemote Connection SSRF Protection and Validation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Follow-up hardening of the SSRF fix for the remote-connection endpoints, addressing maintainer review: - Validate request bodies with TypeBox (the existing serverroutes pattern) instead of ad-hoc checks: host must be a non-empty string, port a number or numeric string in range, and requestId a UUID - which subsumes the earlier requestId path-traversal guard. - Return a single generic connection error instead of echoing the raw transport error, so the endpoints can no longer be used to map internal network topology by error differentiation. The real cause is still logged server-side via debug; post-authentication states stay distinct. - Reflect only the fields the admin UI consumes from remote responses (server id/version, access-request state/requestId/permission/token) rather than passing the remote body through verbatim. - Block the NAT64 well-known prefix (64:ff9b::/96) and deprecated IPv4-compatible (::/96) IPv6 forms, which otherwise encode loopback and metadata addresses past the guard on translating networks.
|
@coderabbitai review |
✅ Action performedReview finished.
|
Motivation
The
testSignalKConnection,requestAccessandcheckAccessRequestadmin endpoints passed an attacker-controlledhoststraight intohttp(s).request, letting the server be driven to make requests to loopback, the cloud metadata service (169.254.169.254) and other non-routable destinations (SSRF, GHSA-q59x-jc9f-gfqf).requestIdwas also interpolated into the remote request path, enabling traversal.Approach
Validate the destination before connecting, in a new
src/ssrfGuard.ts:lookupfor literal hosts) and resolved hostnames (via a validatinglookupthat pins the socket to the checked address, preventing DNS rebinding).requestIdto be a UUID, stopping path traversal incheckAccessRequest.The
selfsignedcerttoggle is intentionally retained (self-signed certs are common on boat LANs) and is now only reachable for non-special-use hosts. The auth model is unchanged: these endpoints must work during initial setup before security is enabled, andaddAdminMiddlewarealready gates them once security is on.Tested
npm run build,eslint, andprettier --checkall passtest/ssrfGuard.tscovering address classification, the validating lookup, the literal-host guard, andrequestIdvalidation127.0.0.1,localhost,169.254.169.254(HTTP and HTTPS) and the::ffff:IPv4-mapped form are blocked, while a192.168.xLAN host still reaches the network layerSSRF Protection for Remote Connection Endpoints
This PR adds server-side request forgery (SSRF) protection to the
testSignalKConnection,requestAccess, andcheckAccessRequestadmin endpoints, which previously accepted attacker-controlled hosts and passed them directly to HTTP(S) request logic.Core SSRF Guard Module
A new
src/ssrfGuard.tsmodule provides SSRF protection utilities:isBlockedAddress()andassertAllowedHost()reject special-use addresses including loopback (127.0.0.0/8, ::1), link-local/cloud metadata (169.254.0.0/16, fe80::/10), multicast, unspecified, and reserved rangesssrfSafeLookupreplaces the standard DNS lookup during HTTP requests, validating all resolved addresses before connection and pinning the socket to the checked addressRequest Validation & Response Filtering
A new
src/remoteConnectionSchemas.tsdefines TypeBox validation schemas for request bodies:hostas non-empty string,portas numeric value in range 1–65535requestIdto be a valid UUID (preventing path traversal)Updated
src/serverroutes.tsendpoints integrate validation and implement safer response handling:pickServerInfo()to expose onlyserver.idandserver.versionpickAccessRequestReply()to expose only relevant fields (state, optionalrequestId, and access permission/token data)CONNECTION_FAILEDerror (detailed errors logged server-side)ssrfSafeLookupandassertAllowedHost()checks to remote HTTP fetching logicTesting
Comprehensive test coverage validates the security improvements:
test/ssrfGuard.tscovering address classification, validating lookup behavior, literal-host guards, and requestId validationtest/remoteConnectionSchemas.tsverify schema validation for request bodies, UUID patterns, and port ranges