Skip to content

Commit

Permalink
Use cryptographically secure rand for Sec-WebSocket-Key and client ma…
Browse files Browse the repository at this point in the history
…sk (#844)

* Use cryptographically secure rand for Sec-WebSocket-Key and client mask

* Ensure websocket streams look properly _complete_ to the rest of the HTTP request stack once finished by setting stream.ntoread to 0

* try again 1

* try again 2
  • Loading branch information
quinnj committed Jun 13, 2022
1 parent 64c980e commit b71aded
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 11 deletions.
1 change: 1 addition & 0 deletions Project.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ Logging = "56ddb016-857b-54e1-b83d-db4d58db5568"
LoggingExtras = "e6f89c97-d47a-5376-807f-9c37f3926c36"
MbedTLS = "739be429-bea8-5141-9913-cc70e7f3736d"
NetworkOptions = "ca575930-c2e3-43a9-ace4-1e988b2c1908"
Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
Sockets = "6462fe0b-24de-5631-8697-dd941f90decc"
URIs = "5c2747f8-b7ea-4ff2-ba2e-563bfd36b1d4"
UUIDs = "cf7118a7-6976-5b1a-9a39-7adc72f591a4"
Expand Down
23 changes: 14 additions & 9 deletions src/WebSockets.jl
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module WebSockets

using Base64, LoggingExtras, UUIDs, Sockets
using Base64, LoggingExtras, UUIDs, Sockets, Random
using MbedTLS: digest, MD_SHA1, SSLContext
using ..IOExtras, ..Streams, ..ConnectionPool, ..Messages, ..Conditions, ..Servers
import ..open
Expand Down Expand Up @@ -60,7 +60,7 @@ primitive type Mask 32 end
Base.UInt32(x::Mask) = Base.bitcast(UInt32, x)
Mask(x::UInt32) = Base.bitcast(Mask, x)
Base.getindex(x::Mask, i::Int) = (UInt32(x) >> (8 * ((i - 1) % 4))) % UInt8
Base.rand(::Type{Mask}) = Mask(rand(UInt32))
mask() = Mask(rand(Random.RandomDevice(), UInt32))
const EMPTY_MASK = Mask(UInt32(0))

# representation of a single websocket frame
Expand Down Expand Up @@ -94,12 +94,12 @@ end
function Frame(final::Bool, opcode::OpCode, client::Bool, payload::AbstractVector{UInt8}; rsv1::Bool=false, rsv2::Bool=false, rsv3::Bool=false)
len, extlen = wslength(length(payload))
if client
mask = Base.rand(Mask)
mask!(payload, mask)
msk = mask()
mask!(payload, msk)
else
mask = EMPTY_MASK
msk = EMPTY_MASK
end
return Frame(FrameFlags(final, opcode, client, len; rsv1, rsv2, rsv3), extlen, mask, payload)
return Frame(FrameFlags(final, opcode, client, len; rsv1, rsv2, rsv3), extlen, msk, payload)
end

Base.show(io::IO, x::Frame) =
Expand Down Expand Up @@ -344,7 +344,7 @@ end
```
"""
function open(f::Function, url; suppress_close_error::Bool=false, verbose=false, headers=[], maxframesize::Integer=typemax(Int), maxfragmentation::Integer=DEFAULT_MAX_FRAG, kw...)
key = base64encode(rand(UInt8, 16))
key = base64encode(rand(Random.RandomDevice(), UInt8, 16))
headers = [
"Upgrade" => "websocket",
"Connection" => "Upgrade",
Expand All @@ -359,6 +359,11 @@ function open(f::Function, url; suppress_close_error::Bool=false, verbose=false,
if header(http, "Sec-WebSocket-Accept") != hashedkey(key)
throw(WebSocketError("Invalid Sec-WebSocket-Accept\n" * "$(http.message)"))
end
# later stream logic checks to see if the HTTP message is "complete"
# by seeing if ntoread is 0, which is typemax(Int) for websockets by default
# so set it to 0 so it's correctly viewed as "complete" once we're done
# doing websocket things
http.ntoread = 0
io = http.stream
ws = WebSocket(io, http.message.request, http.message; maxframesize, maxfragmentation)
@debugv 2 "$(ws.id): WebSocket opened"
Expand Down Expand Up @@ -401,9 +406,9 @@ WebSockets.listen(host, port) do ws
end
"""
function listen(f::Function, host="localhost", port::Integer=UInt16(8081); verbose=false, kw...)
function listen(f::Function, host="localhost", port::Integer=UInt16(8081); verbose=false, suppress_close_error::Bool=false, kw...)
Servers.listen(host, port; verbose=verbose, kw...) do http
upgrade(f, http; kw...)
upgrade(f, http; suppress_close_error, kw...)
end
end

Expand Down
76 changes: 76 additions & 0 deletions test/websockets/autobahn.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
using Test, Sockets, HTTP, HTTP.WebSockets, JSON

const DIR = abspath(joinpath(dirname(pathof(HTTP)), "../test/websockets"))

# 32-bit not supported by autobahn
if Int === Int64 && !Sys.iswindows()

@testset "Autobahn WebSocket Tests" begin

@testset "Client" begin
serverproc = run(Cmd(`wstest -u 0 -m fuzzingserver -s config/fuzzingserver.json`; dir=DIR), stdin, stdout, stdout; wait=false)
sleep(5) # give time for server to get setup
cases = Ref(0)
WebSockets.open("ws://127.0.0.1:9001/getCaseCount") do ws
for msg in ws
cases[] = parse(Int, msg)
end
end

for i = 1:cases[]
println("Running test case = $i")
verbose = false
try
WebSockets.open("ws://127.0.0.1:9001/runCase?case=$(i)&agent=main"; verbose, suppress_close_error=true) do ws
for msg in ws
send(ws, msg)
end
end
catch
# ignore errors here since we want to run all cases + some are expected to throw
end
end

rm(joinpath(DIR, "reports/clients/index.json"); force=true)
sleep(1)
try
WebSockets.open("ws://127.0.0.1:9001/updateReports?agent=main") do ws
receive(ws)
end
catch
WebSockets.open("ws://127.0.0.1:9001/updateReports?agent=main") do ws
receive(ws)
end
end

report = JSON.parsefile(joinpath(DIR, "reports/clients/index.json"))
for (k, v) in pairs(report["main"])
@test v["behavior"] in ("OK", "NON-STRICT", "INFORMATIONAL")
end
# stop/remove server process
kill(serverproc)
end # @testset "Autobahn testsuite"

@testset "Server" begin
server = Sockets.listen(Sockets.localhost, 9002)
ready_to_accept = Ref(false)
@async WebSockets.listen(Sockets.localhost, 9002; server, ready_to_accept, suppress_close_error=true) do ws
for msg in ws
send(ws, msg)
end
end
while !ready_to_accept[]
sleep(0.5)
end
rm(joinpath(DIR, "reports/server/index.json"); force=true)
@test success(run(Cmd(`wstest -u 0 -m fuzzingclient -s config/fuzzingclient.json`; dir=DIR)))
close(server)
report = JSON.parsefile(joinpath(DIR, "reports/server/index.json"))
for (k, v) in pairs(report["main"])
@test v["behavior"] in ("OK", "NON-STRICT", "INFORMATIONAL", "UNIMPLEMENTED")
end
end

end # @testset "WebSockets"

end # 64-bit only
4 changes: 2 additions & 2 deletions test/websockets/config/fuzzingclient.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
"servers": [
{
"agent": "main",
"url": "ws://host.docker.internal:9002"
"url": "ws://127.0.0.1:9002"
}
],
"cases": ["*"],
"exclude-cases": [],
"exclude-cases": ["9.*"],
"exclude-agent-cases": {}
}
1 change: 1 addition & 0 deletions test/websockets/config/fuzzingserver.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"outdir": "./reports/clients",
"cases": ["*"],
"exclude-cases": [
"9.*",
"12.*",
"13.*"
],
Expand Down

0 comments on commit b71aded

Please sign in to comment.