Skip to content

enh: add disableClientMask option for WebSocket payload masking and optimize mask calculation #985

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

huskar-t
Copy link

@huskar-t huskar-t commented May 17, 2025

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update
  • Go Version Update
  • Dependency Update

Description

Optimized WebSocket masking by:

  1. Implementing fast-path bypass when mask key equals [4]byte{0,0,0,0}
  2. Adding configurable masking disablement for secure contexts (e.g., TLS)
goos: linux
goarch: amd64
pkg: github.com/gorilla/websocket
cpu: Intel(R) Core(TM) i7-10700 CPU @ 2.90GHz
BenchmarkMaskBytes
BenchmarkMaskBytes/size-2
BenchmarkMaskBytes/size-2/align-4
BenchmarkMaskBytes/size-2/align-4/byte
BenchmarkMaskBytes/size-2/align-4/byte-16         	455632444	         2.629 ns/op	 760.76 MB/s
BenchmarkMaskBytes/size-2/align-4/wordV1
BenchmarkMaskBytes/size-2/align-4/wordV1-16       	421480846	         2.849 ns/op	 702.07 MB/s
BenchmarkMaskBytes/size-2/align-4/word
BenchmarkMaskBytes/size-2/align-4/word-16         	423765326	         2.828 ns/op	 707.18 MB/s
BenchmarkMaskBytes/size-4
BenchmarkMaskBytes/size-4/align-4
BenchmarkMaskBytes/size-4/align-4/byte
BenchmarkMaskBytes/size-4/align-4/byte-16         	290015848	         4.140 ns/op	 966.14 MB/s
BenchmarkMaskBytes/size-4/align-4/wordV1
BenchmarkMaskBytes/size-4/align-4/wordV1-16       	323833113	         3.735 ns/op	1071.01 MB/s
BenchmarkMaskBytes/size-4/align-4/word
BenchmarkMaskBytes/size-4/align-4/word-16         	323727016	         3.692 ns/op	1083.36 MB/s
BenchmarkMaskBytes/size-8
BenchmarkMaskBytes/size-8/align-4
BenchmarkMaskBytes/size-8/align-4/byte
BenchmarkMaskBytes/size-8/align-4/byte-16         	220623301	         5.432 ns/op	1472.64 MB/s
BenchmarkMaskBytes/size-8/align-4/wordV1
BenchmarkMaskBytes/size-8/align-4/wordV1-16       	212184852	         5.671 ns/op	1410.73 MB/s
BenchmarkMaskBytes/size-8/align-4/word
BenchmarkMaskBytes/size-8/align-4/word-16         	211640461	         5.642 ns/op	1417.93 MB/s
BenchmarkMaskBytes/size-16
BenchmarkMaskBytes/size-16/align-4
BenchmarkMaskBytes/size-16/align-4/byte
BenchmarkMaskBytes/size-16/align-4/byte-16        	128340018	         9.387 ns/op	1704.47 MB/s
BenchmarkMaskBytes/size-16/align-4/wordV1
BenchmarkMaskBytes/size-16/align-4/wordV1-16      	85413890	        14.01 ns/op	1142.38 MB/s
BenchmarkMaskBytes/size-16/align-4/word
BenchmarkMaskBytes/size-16/align-4/word-16        	125297701	         9.564 ns/op	1673.03 MB/s
BenchmarkMaskBytes/size-32
BenchmarkMaskBytes/size-32/align-4
BenchmarkMaskBytes/size-32/align-4/byte
BenchmarkMaskBytes/size-32/align-4/byte-16        	70067914	        17.31 ns/op	1848.34 MB/s
BenchmarkMaskBytes/size-32/align-4/wordV1
BenchmarkMaskBytes/size-32/align-4/wordV1-16      	81590757	        14.68 ns/op	2180.16 MB/s
BenchmarkMaskBytes/size-32/align-4/word
BenchmarkMaskBytes/size-32/align-4/word-16        	90093694	        13.32 ns/op	2402.45 MB/s
BenchmarkMaskBytes/size-512
BenchmarkMaskBytes/size-512/align-4
BenchmarkMaskBytes/size-512/align-4/byte
BenchmarkMaskBytes/size-512/align-4/byte-16       	 4654480	       256.3 ns/op	1997.71 MB/s
BenchmarkMaskBytes/size-512/align-4/wordV1
BenchmarkMaskBytes/size-512/align-4/wordV1-16     	30228372	        39.81 ns/op	12862.16 MB/s
BenchmarkMaskBytes/size-512/align-4/word
BenchmarkMaskBytes/size-512/align-4/word-16       	38939992	        30.81 ns/op	16616.76 MB/s
BenchmarkMaskBytes/size-1024
BenchmarkMaskBytes/size-1024/align-4
BenchmarkMaskBytes/size-1024/align-4/byte
BenchmarkMaskBytes/size-1024/align-4/byte-16      	 2361459	       506.7 ns/op	2020.90 MB/s
BenchmarkMaskBytes/size-1024/align-4/wordV1
BenchmarkMaskBytes/size-1024/align-4/wordV1-16    	18674653	        64.18 ns/op	15955.39 MB/s
BenchmarkMaskBytes/size-1024/align-4/word
BenchmarkMaskBytes/size-1024/align-4/word-16      	22626250	        52.46 ns/op	19518.92 MB/s
BenchmarkMaskBytes/size-1048576
BenchmarkMaskBytes/size-1048576/align-4
BenchmarkMaskBytes/size-1048576/align-4/byte
BenchmarkMaskBytes/size-1048576/align-4/byte-16   	    2314	    517830 ns/op	2024.94 MB/s
BenchmarkMaskBytes/size-1048576/align-4/wordV1
BenchmarkMaskBytes/size-1048576/align-4/wordV1-16 	   23004	     53214 ns/op	19704.72 MB/s
BenchmarkMaskBytes/size-1048576/align-4/word
BenchmarkMaskBytes/size-1048576/align-4/word-16   	   25855	     46638 ns/op	22483.08 MB/s

Related Tickets & Documents

  • Related Issue #
  • Closes #

Added/updated tests?

  • Yes
  • No, and this is why: please replace this line with details on why tests
    have not been included
  • I need help with writing tests

Run verifications and test

  • make verify is passing
  • make test is passing

@huskar-t huskar-t marked this pull request as ready for review May 17, 2025 06:39
@ghost
Copy link

ghost commented Jun 2, 2025

Adding configurable masking disablement for secure contexts (e.g., TLS)

Masking prevents a client application from sending a specific stream of bytes to a server. TLS does not eliminate the need for this feature. See Attacks On Infrastructure for more information.

@huskar-t
Copy link
Author

huskar-t commented Jun 4, 2025

Masking prevents a client application from sending a specific stream of bytes to a server. TLS does not eliminate the need for this feature.

I believe TLS can prevent data interception and tampering, making the protocol's built-in masking mechanism redundant under TLS protection.

@ghost
Copy link

ghost commented Jun 4, 2025

The purpose of masking is to prevent a client application running in a browser from controlling the sequence of bytes sent to a server.

Masking is useless for preventing data interception or tampering.

If the feature is added, the feature should be enabled with a Dialer field. There are two reasons for this:

  • If it's safe to disable masking for one message, then it's safe to disable masking for all messages. There's no need for an API to turn masking on and off.
  • The feature only applies to clients.

To keep the connection method as in the current PR, OP should give a use case for turning the flag on and off. Also, OP should given a justification for not adding the setting with other client specific settings in Dialer.

@huskar-t
Copy link
Author

huskar-t commented Jun 5, 2025

The purpose of masking is to prevent a client application running in a browser from controlling the sequence of bytes sent to a server.

Masking is useless for preventing data interception or tampering.

If the feature is added, the feature should be enabled with a Dialer field. There are two reasons for this:

  • If it's safe to disable masking for one message, then it's safe to disable masking for all messages. There's no need for an API to turn masking on and off.
  • The feature only applies to clients.

I didn't modify the protocol itself, but mathematically bypassed the masking computation. This serves as a performance optimization in specific scenarios, with precise control through parameters.

// by generating zero-value mask keys ([4]byte{0,0,0,0}), effectively omitting XOR operations
// while maintaining formal protocol compliance.
//
// Security Advisory:
Copy link

@ghost ghost Jun 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This description of the security considerations is wrong. Secure transport does not obviate the need for masking.

Refer readers to the RFC sections 5.3 and 10.3 instead of what's written in the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant