Repository navigation
fix(sync): validate trusted WebSocket master redirects before forwarding credentials (#8222) - #8242
Conversation
Aias00
left a comment
There was a problem hiding this comment.
Found one IPv6 allowlist matching issue in the new master-target policy. A standalone probe compiled the production policy unchanged and demonstrated that valid bracketed IPv6 host and host:port entries are rejected, while the equivalent URI entry is accepted.
|
Thanks — the IPv6 mismatch is real. Fixed by normalizing the host representation in one place: Added Verification: |
Aias00
left a comment
There was a problem hiding this comment.
The IPv6 allowlist finding is fixed: origin hosts and bracketed configured entries now use the same normalized representation. Reviewed trust boundaries, ports and transport downgrade handling; no remaining actionable finding.
Validation: 37 WebSocket policy/configuration/redirect/service tests passed, including bracketed IPv6 host, host:port and URI cases. CI was still pending at the initial review snapshot. Apache RAT passed locally. Scope: source review and targeted verification; full integration/e2e suites were not run locally for this review.
Fixes #8222.
Problem
An advertised cluster master is currently trusted whenever it uses
ws/wssand has a host. A trustedwss://seed can therefore redirect the gateway to another host and even to plaintextws://, and the sameX-Shenyu-Sync-Tokenis attached to the new handshake. Becauseshenyu.cluster.schemadefaults tohttp, a TLS-terminated Admin also advertises a plaintext master by default.Change
MasterTargetPolicyvalidates every advertised master before a client — and therefore its credential headers — is constructed:shenyu.sync.websocket.trusted-master-hostslist (host,host:port, orws(s)://host:port).wssconnection never follows a plaintextwsadvertisement unless the operator setsshenyu.sync.websocket.allow-insecure-master-redirect=true.ws→wssupgrades remain allowed.shenyu.cluster.schema=httpsso the advertised master iswss; an explicit insecure opt-in is required for plaintext.Tests
host:portentries, different ports,ws↔wsstransitions, the insecure opt-in, seeds without a scheme, and invalid schemes/missing current context.wss→wsdowngrade construct no client and do not close the trusted follower; an allowlisted master and the explicit insecure opt-in construct a client whose headers still carry the sync token.Verification
The check runs only when a master advertisement is evaluated, not on request or sync-data paths; no DNS, remote authorization or per-frame work is added.