Correlate WebSocket auth with the originating TCP socket#1394
Open
TristanInSec wants to merge 1 commit into
Open
Correlate WebSocket auth with the originating TCP socket#1394TristanInSec wants to merge 1 commit into
TristanInSec wants to merge 1 commit into
Conversation
The HTTP upgrade handler and the SockJS connection handler previously passed proxy auth credentials to each other through shared closure variables. Those variables were last-writer-wins across all in-flight upgrades, so under concurrent load a connection could observe the credentials written by a different upgrade. Key the pending auth entries on the TCP socket's remoteAddress:remotePort pair, which is unique per live socket, so each SockJS connection deterministically picks up the credentials from its own upgrade. The entry is deleted on connection, socket close, or socket error to keep the map bounded.
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.
Summary
The HTTP
upgradehandler and the SockJSconnectionhandler inserver.jspreviously passed the proxy auth token and user id between themselves via two shared closure variables (authToken,userid). Those variables are module-scoped to the promise callback, so they are last-writer-wins across every in-flight upgrade. Under concurrent load a connection could observe the credentials written by a different upgrade that happened to interleave between its ownupgradeevent and its ownconnectionevent.This PR keys the pending auth entries on the TCP socket's
remoteAddress:remotePortpair, which is guaranteed unique per live socket. The SockJSconnectionevent retrieves the entry using the same key, so each connection deterministically picks up the credentials from its own upgrade. The entry is removed on connection pickup, socketclose, or socketerrorto keep the map bounded.Changes
pendingAuthMap keyed by${remoteAddress}:${remotePort}.upgradehandler into the map after the origin check.connectionhandler before creating the Aggregator.close/errorso unmatched upgrades do not accumulate.Test plan
authToken/userid.