Enforce RFC 6455 (§5.1) frame-masking rules in NIOWebSocket#3653
Open
rafaelcepeda wants to merge 1 commit into
Open
Enforce RFC 6455 (§5.1) frame-masking rules in NIOWebSocket#3653rafaelcepeda wants to merge 1 commit into
NIOWebSocket#3653rafaelcepeda wants to merge 1 commit into
Conversation
WebSocketFrameDecoder was direction-agnostic and never inspected the mask bit, so a server would silently accept unmasked client frames (and a client would accept masked server frames), violating RFC 6455 (§5.1). - Add WebSocketMaskingVerification (disabled / serverExpectsMaskedFrames / clientExpectsUnmaskedFrames) and enforce the selected rule in WSParser.validateState, rejecting the frame before its body is buffered. - Surface violations via a dedicated NIOWebSocketMaskingError (maskedFrame / unmaskedFrame) rather than new NIOWebSocketError cases, keeping the change purely additive. WebSocketProtocolErrorHandler maps it to the 1002 protocol-error close code. - Enable enforcement by default in the server and client upgraders via an enforceMaskingRules opt-out, added as new initializer overloads so the existing initializers keep their signatures (no API breakage). - WebSocketFrameDecoder gains a maskingVerification initializer overload, defaulting behaviour to no enforcement for direct decoder users. - Add decoder tests for both directions plus a default no-enforcement test; opt the raw-client end-to-end test out of enforcement. Verified: 'swift package diagnose-api-breaking-changes main' reports no breaking changes in NIOWebSocket.
rafaelcepeda
force-pushed
the
rc-websocket-masking-enforcement
branch
from
1c13339 to
c25cb91
Compare
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
1 participant
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.
Motivation:
WebSocketFrameDecoderis direction-agnostic: it only tracksmaxFrameSizeand never inspects the mask bit.validateState()checks frame length, fragmented control frames, and control-frame length, but nothing about masking.RFC 6455 (§5.1) is directional: a client must mask every frame it sends to a server, and a server must not mask frames it sends to a client. Because the decoder never enforced this, a server would silently accept unmasked client frames (and a client would accept masked server frames), rather than failing the connection as the spec requires. The masking requirement exists to prevent crafted frames from being misinterpreted by caching intermediaries, so this is an RFC-compliance gap.
Modifications:
WebSocketMaskingVerification(.disabled/.serverExpectsMaskedFrames/.clientExpectsUnmaskedFrames) and amaskingVerificationparameter onWebSocketFrameDecoder. It defaults to.disabled, so direct decoder users see no behavioural change.WSParser.validateState, rejecting the offending frame before its body is buffered. Two newNIOWebSocketErrorcases —.maskedFrameand.unmaskedFrame— surface the violation and map to the existing1002(protocol error) close code.NIOWebSocketServerUpgrader,NIOTypedWebSocketServerUpgrader,NIOWebSocketClientUpgrader,NIOTypedWebSocketClientUpgrader) via a newenforceMaskingRules: Bool = trueparameter, which can be set tofalsefor peers known to be non-compliant.Result:
Servers and clients configured through the WebSocket upgraders now reject frames that violate RFC 6455's masking rules by default, closing the connection with a
1002protocol-error close frame.This is a behavioural change for the upgrader paths: a peer that previously accepted non-compliant (un)masked frames will now close those connections. Users who need the old behaviour can pass
enforceMaskingRules: false.The raw
WebSocketFrameDecoderdefault (.disabled) is unchanged, so direct decoder users are unaffected. The change is API-additive (no source break).