Commit b822448
committed
fix(net-websockets): the WebSocket nonce and masking key come from a CSPRNG (#2401)
Found by asking #2398's own question once more rather than by opening another
module: #2228 put a real CSPRNG behind Guid::NewGuid, so WHAT ELSE IN THIS
RUNTIME NEEDS UNPREDICTABLE BYTES, AND WHERE DOES IT GET THEM? ClientWebSocket
drew BOTH its Sec-WebSocket-Key nonce and its PER-FRAME MASKING KEY from
std::random_device.
THAT IS A DEFECT RATHER THAN A STYLE POINT, AND THIS REPOSITORY HAD ALREADY
MEASURED WHY. The standard explicitly permits a deterministic
std::random_device, and Random.cpp:69-70 records in its own comment "on a
platform whose random_device is deterministic (MinGW-w64's historically was)" --
MinGW-w64 being a supported compile target under CLAUDE.md's platform policy. On
such a platform every connection sends the SAME Sec-WebSocket-Key and every
frame's masking key is PREDICTABLE.
RFC 6455 does not leave this to taste. Section 5.3: "The masking key needs to be
unpredictable; thus, the masking key MUST be derived from a strong source of
entropy, and the masking key for a given frame MUST NOT make it simple for a
server/proxy to predict the masking key for a subsequent frame." Masking exists
to stop cache-poisoning of intermediaries (section 10.3), so a predictable key
defeats the one attack it was introduced for.
.NET'S TWO ROUTES ARE DIFFERENT AND ARE TRANSCRIBED SEPARATELY RATHER THAN
HARMONISED INTO ONE: the nonce is Guid.NewGuid().TryWriteBytes base64-encoded
(WebSocketHandle.Managed.cs:490-494) and the mask is RandomNumberGenerator.Fill
(ManagedWebSocket.cs:762-763). THE NONCE COST NO COMPONENT EDGE AT ALL, because
#2228 already put the CSPRNG behind Guid::NewGuid and Core.Base was already a
public dependency here.
Module graph 41/93 -> 41/94: Security.Cryptography.Random becomes a PRIVATE
dependency of Net.WebSockets, private because the only caller is a file-local
helper so no public header names the type. Catalogue regenerated;
scripts/check_selective_components.sh Net.WebSockets net_websockets.cpp passes
with its 107 tests. CALLING getentropy() DIRECTLY FROM net-websockets WAS
REJECTED ON A RECORDED PRECEDENT: it would be a third copy of the platform
entropy call, the duplication #2354 spent a ticket removing.
WHAT CANNOT BE OBSERVED HERE IS STATED RATHER THAN IMPLIED. On glibc,
std::random_device reads /dev/urandom, so the SOURCE change is not behaviourally
visible on this platform. The evidence is the reference, the RFC, this
repository's own MinGW-w64 measurement, and SYMBOL INSPECTION, which is
precisely discriminating (the #1983 idiom):
after the repair random_device=0 NewGuid=1 Fill=1
nonce reverted random_device=16 NewGuid=0 Fill=1
mask reverted random_device=16 NewGuid=1 Fill=0
Five mutations. THE TWO SOURCE REVERSIONS ARE NOT CAUGHT BY TESTS AND ARE CAUGHT
BY THE SYMBOL TABLE, reported as such rather than as passes. The three
RFC-property mutations -- one mask cached per connection, a constant mask, and a
nonce cached for the process lifetime -- are ALL CAUGHT, and each is the
PLAUSIBLE OPTIMISATION rather than a contrived defect: a cached mask still
unmasks correctly at the server, because both ends agree on whatever key was
sent, so it passes every other assertion in the file. One mutation was INVALID
AS FIRST WRITTEN and was reformulated rather than counted: zeroing the mask left
randomMaskingKey() unreferenced and -Werror=unused-function rejected it.
WHAT IS NEWLY PINNED ARE THE RFC PROPERTIES THEMSELVES, which nothing pinned
before: a 16-byte nonce differing between connections (section 4.1) and a fresh
masking key per frame (section 5.3), plus a separate assertion that no mask is
all zeroes, which the freshness check alone would not catch for a single frame.
Both are reachable from the existing mock-server harness.
Landed under SA-5. No public signature, layout, vtable or noexcept change and no
outlawed spelling -- both helpers are file-local to the .cpp -- so no negative
fixture was owed; the set stays 49/248.
Gate 17,629 / 38 executables: 17,629 run, 17,629 passed, 0 failed, 0 skipped,
recounted from the per-executable logs with every executable run separately and
continuing past failures. +2 on 17,627, in SharpRuntimeTests_Net_WebSockets
(105 -> 107); no other executable moved. Zero build warnings at --parallel 2;
module boundaries green at 41/94.1 parent 17917fa commit b822448
9 files changed
Lines changed: 335 additions & 30 deletions
File tree
- docs
- modules/net-websockets
- src/System/Net/WebSockets
- tests/System/Net/WebSockets
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
6 | | - | |
7 | | - | |
| 6 | + | |
| 7 | + | |
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| |||
28 | 28 | | |
29 | 29 | | |
30 | 30 | | |
31 | | - | |
32 | | - | |
33 | | - | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
34 | 38 | | |
35 | | - | |
36 | | - | |
37 | | - | |
38 | | - | |
39 | | - | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
40 | 49 | | |
41 | 50 | | |
42 | 51 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
11 | | - | |
| 11 | + | |
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| |||
63 | 63 | | |
64 | 64 | | |
65 | 65 | | |
66 | | - | |
| 66 | + | |
67 | 67 | | |
68 | 68 | | |
69 | 69 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
22 | 31 | | |
Lines changed: 36 additions & 10 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
12 | | - | |
13 | 12 | | |
14 | 13 | | |
15 | 14 | | |
| 15 | + | |
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
20 | 20 | | |
| 21 | + | |
21 | 22 | | |
22 | 23 | | |
23 | 24 | | |
| |||
101 | 102 | | |
102 | 103 | | |
103 | 104 | | |
104 | | - | |
105 | | - | |
106 | | - | |
107 | | - | |
108 | | - | |
109 | | - | |
110 | | - | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
111 | 135 | | |
112 | | - | |
113 | | - | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
114 | 140 | | |
115 | 141 | | |
116 | 142 | | |
| |||
0 commit comments