From 17aed4f47140d0a08bbd4398c601fa726f58acdb Mon Sep 17 00:00:00 2001 From: Shreemaan Abhishek Date: Tue, 22 Sep 2026 19:02:47 +0545 Subject: [PATCH] bugfix: client: verify the handshake response headers The client accepted any 101 response as a successful handshake, so anything that answers 101 passed for a websocket server. RFC 6455 section 4.1 requires the client to fail the connection unless the server proves it understood the handshake. Verify Upgrade, Connection, Sec-WebSocket-Accept, the selected subprotocol, and the absence of extensions that were never offered. On failure close the socket and mark the object fatal so no frames can be sent on a connection that is not a websocket. --- README.markdown | 2 + lib/resty/websocket/client.lua | 96 +++++++- t/handshake_verify.t | 392 +++++++++++++++++++++++++++++++++ 3 files changed, 488 insertions(+), 2 deletions(-) create mode 100644 t/handshake_verify.t diff --git a/README.markdown b/README.markdown index d899a28..0de4df6 100644 --- a/README.markdown +++ b/README.markdown @@ -386,6 +386,8 @@ Connects to the remote WebSocket service port and performs the websocket handsha Before actually resolving the host name and connecting to the remote backend, this method will always look up the connection pool for matched idle connections created by previous calls of this method. +The handshake response is validated per RFC 6455 section 4.1: the status must be `101`, `Upgrade` must be `websocket`, `Connection` must carry the `upgrade` token, `Sec-WebSocket-Accept` must match the key that was sent, any `Sec-WebSocket-Protocol` must be one of the offered subprotocols, and `Sec-WebSocket-Extensions` must be absent since no extension is ever offered. When validation fails the method returns `nil` plus an error message, the underlying socket is closed, and the object is marked fatal. + The third return value of this method contains the raw, plain-text response (status line and headers) to the handshake request. This allows the caller to perform additional validation and/or extract the response headers. When the connection is reused and no handshake request is sent, the string `"connection reused"` is returned in lieu of the response. An optional Lua table can be specified as the last argument to this method to specify various connect options: diff --git a/lib/resty/websocket/client.lua b/lib/resty/websocket/client.lua index bb95958..4ac4383 100644 --- a/lib/resty/websocket/client.lua +++ b/lib/resty/websocket/client.lua @@ -17,15 +17,18 @@ local re_match = ngx.re.match local re_find = ngx.re.find local re_gmatch = ngx.re.gmatch local encode_base64 = ngx.encode_base64 +local sha1_bin = ngx.sha1_bin local concat = table.concat local insert = table.insert local char = string.char local str_find = string.find +local str_lower = string.lower local str_sub = string.sub local rand = math.random local rshift = bit.rshift local band = bit.band local setmetatable = setmetatable +local ipairs = ipairs local type = type local debug = ngx.config.debug local ngx_log = ngx.log @@ -48,6 +51,71 @@ _M._VERSION = '0.13' local mt = { __index = _M } +local WS_GUID = "258EAFA5-E914-47DA-95CA-C5AB0DC85B11" + + +-- true if a comma-separated header value carries the given token +local function has_token(value, token) + local iter, err = re_gmatch(value, [[[^,\s]+]], "jo") + if not iter then + ngx_log(ngx_DEBUG, "failed to parse header value: ", err) + return false + end + + while true do + local m = iter() + if not m then + return false + end + + if str_lower(m[0]) == token then + return true + end + end +end + + +-- RFC 6455 section 4.1: the client must fail the connection unless the server +-- proves it understood the handshake. Without these checks anything that +-- answers 101 passes for a websocket server, and a duplicated header (parsed +-- into a table) is a protocol error in its own right. +local function verify_handshake(resp_headers, key, protocols) + local upgrade = resp_headers.upgrade + if type(upgrade) ~= "string" or str_lower(upgrade) ~= "websocket" then + return nil, "invalid \"Upgrade\" response header" + end + + local connection = resp_headers.connection + if type(connection) ~= "string" or not has_token(connection, "upgrade") then + return nil, "invalid \"Connection\" response header" + end + + local accept = resp_headers.sec_websocket_accept + if type(accept) ~= "string" then + return nil, "missing \"Sec-WebSocket-Accept\" response header" + end + + if accept ~= encode_base64(sha1_bin(key .. WS_GUID)) then + return nil, "invalid \"Sec-WebSocket-Accept\" response header" + end + + -- the server may decline the subprotocol, but it may not invent one + local proto = resp_headers.sec_websocket_protocol + if proto ~= nil + and (type(proto) ~= "string" or not protocols[str_lower(proto)]) + then + return nil, "invalid \"Sec-WebSocket-Protocol\" response header" + end + + -- no extension is ever offered, so none may be accepted + if resp_headers.sec_websocket_extensions ~= nil then + return nil, "unexpected \"Sec-WebSocket-Extensions\" response header" + end + + return true +end + + function _M.new(self, opts) local sock, err = tcp() if not sock then @@ -135,6 +203,7 @@ function _M.connect(self, uri, opts) end local ssl_verify, server_name, headers, proto_header, origin_header + local offered_protocols = {} local sock_opts = {} local client_cert, client_priv_key local header_host @@ -147,8 +216,13 @@ function _M.connect(self, uri, opts) proto_header = "\r\nSec-WebSocket-Protocol: " .. concat(protos, ",") + for _, proto in ipairs(protos) do + offered_protocols[str_lower(proto)] = true + end + else proto_header = "\r\nSec-WebSocket-Protocol: " .. protos + offered_protocols[str_lower(protos)] = true end end @@ -369,8 +443,6 @@ function _M.connect(self, uri, opts) -- error("header: " .. header) - -- FIXME: verify the response headers - m, err = re_match(header, [[^\s*HTTP/1\.1\s+(\d+)]], "jo") if not m then return nil, "bad HTTP response status line: " .. header @@ -395,6 +467,26 @@ function _M.connect(self, uri, opts) return nil, "unexpected HTTP response code: " .. m[1], header end + local resp_headers + resp_headers, err = self:get_resp_headers() + if not resp_headers then + err = "failed to parse response headers: " .. err + + else + ok, err = verify_handshake(resp_headers, key, offered_protocols) + end + + if err then + local closing_ok, closing_err = sock:close() + if not closing_ok then + ngx_log(ngx_DEBUG, "failed to close the underlying socket: ", + closing_err, " when handling a failed handshake") + end + + self.fatal = true + return nil, "failed websocket handshake: " .. err, header + end + return 1, nil, header end diff --git a/t/handshake_verify.t b/t/handshake_verify.t new file mode 100644 index 0000000..055439f --- /dev/null +++ b/t/handshake_verify.t @@ -0,0 +1,392 @@ +# vim:set ft= ts=4 sw=4 et: + +use Test::Nginx::Socket::Lua; +use Cwd qw(cwd); + +repeat_each(2); + +plan tests => repeat_each() * (3 * blocks()); + +my $pwd = cwd(); + +our $HttpConfig = qq{ + lua_package_path "$pwd/lib/?.lua;;"; + lua_package_cpath "/usr/local/openresty-debug/lualib/?.so;/usr/local/openresty/lualib/?.so;;"; +}; + +# all the mock replies below answer the fixed key "dGhlIHNhbXBsZSBub25jZQ==" +# whose accept value is "s3pPLMBiTxaQ9kYGzzhZRbK+xOo=" (RFC 6455 section 1.3) + +no_long_string(); + +run_tests(); + +__DATA__ + +=== TEST 1: a well formed handshake response is accepted +--- http_config eval: $::HttpConfig +--- config + location = /t { + content_by_lua_block { + local client = require "resty.websocket.client" + local wb = client:new() + + local ok, err = wb:connect("ws://127.0.0.1:7986/", + { key = "dGhlIHNhbXBsZSBub25jZQ==" }) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("connected") + } + } +--- request +GET /t +--- tcp_listen: 7986 +--- tcp_reply eval +"HTTP/1.1 101 Switching Protocols\r +Upgrade: websocket\r +Connection: Upgrade\r +Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r +\r +" +--- response_body +connected +--- no_error_log +[error] + + + +=== TEST 2: a wrong Sec-WebSocket-Accept is rejected +--- http_config eval: $::HttpConfig +--- config + location = /t { + content_by_lua_block { + local client = require "resty.websocket.client" + local wb = client:new() + + local ok, err = wb:connect("ws://127.0.0.1:7986/", + { key = "dGhlIHNhbXBsZSBub25jZQ==" }) + if not ok then + ngx.say("failed to connect: ", err) + ngx.say("fatal: ", wb.fatal) + return + end + + ngx.say("connected") + } + } +--- request +GET /t +--- tcp_listen: 7986 +--- tcp_reply eval +"HTTP/1.1 101 Switching Protocols\r +Upgrade: websocket\r +Connection: Upgrade\r +Sec-WebSocket-Accept: 0000000000000000000000000000=\r +\r +" +--- response_body +failed to connect: failed websocket handshake: invalid "Sec-WebSocket-Accept" response header +fatal: true +--- no_error_log +[error] + + + +=== TEST 3: a missing Sec-WebSocket-Accept is rejected +--- http_config eval: $::HttpConfig +--- config + location = /t { + content_by_lua_block { + local client = require "resty.websocket.client" + local wb = client:new() + + local ok, err = wb:connect("ws://127.0.0.1:7986/", + { key = "dGhlIHNhbXBsZSBub25jZQ==" }) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("connected") + } + } +--- request +GET /t +--- tcp_listen: 7986 +--- tcp_reply eval +"HTTP/1.1 101 Switching Protocols\r +Upgrade: websocket\r +Connection: Upgrade\r +\r +" +--- response_body +failed to connect: failed websocket handshake: missing "Sec-WebSocket-Accept" response header +--- no_error_log +[error] + + + +=== TEST 4: a non-websocket Upgrade is rejected +--- http_config eval: $::HttpConfig +--- config + location = /t { + content_by_lua_block { + local client = require "resty.websocket.client" + local wb = client:new() + + local ok, err = wb:connect("ws://127.0.0.1:7986/", + { key = "dGhlIHNhbXBsZSBub25jZQ==" }) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("connected") + } + } +--- request +GET /t +--- tcp_listen: 7986 +--- tcp_reply eval +"HTTP/1.1 101 Switching Protocols\r +Upgrade: h2c\r +Connection: Upgrade\r +Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r +\r +" +--- response_body +failed to connect: failed websocket handshake: invalid "Upgrade" response header +--- no_error_log +[error] + + + +=== TEST 5: a Connection header without the upgrade token is rejected +--- http_config eval: $::HttpConfig +--- config + location = /t { + content_by_lua_block { + local client = require "resty.websocket.client" + local wb = client:new() + + local ok, err = wb:connect("ws://127.0.0.1:7986/", + { key = "dGhlIHNhbXBsZSBub25jZQ==" }) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("connected") + } + } +--- request +GET /t +--- tcp_listen: 7986 +--- tcp_reply eval +"HTTP/1.1 101 Switching Protocols\r +Upgrade: websocket\r +Connection: close\r +Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r +\r +" +--- response_body +failed to connect: failed websocket handshake: invalid "Connection" response header +--- no_error_log +[error] + + + +=== TEST 6: the upgrade token is found in a Connection token list +--- http_config eval: $::HttpConfig +--- config + location = /t { + content_by_lua_block { + local client = require "resty.websocket.client" + local wb = client:new() + + local ok, err = wb:connect("ws://127.0.0.1:7986/", + { key = "dGhlIHNhbXBsZSBub25jZQ==" }) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("connected") + } + } +--- request +GET /t +--- tcp_listen: 7986 +--- tcp_reply eval +"HTTP/1.1 101 Switching Protocols\r +Upgrade: WebSocket\r +Connection: keep-alive, Upgrade\r +Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r +\r +" +--- response_body +connected +--- no_error_log +[error] + + + +=== TEST 7: a subprotocol that was never offered is rejected +--- http_config eval: $::HttpConfig +--- config + location = /t { + content_by_lua_block { + local client = require "resty.websocket.client" + local wb = client:new() + + local ok, err = wb:connect("ws://127.0.0.1:7986/", + { key = "dGhlIHNhbXBsZSBub25jZQ==" }) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("connected") + } + } +--- request +GET /t +--- tcp_listen: 7986 +--- tcp_reply eval +"HTTP/1.1 101 Switching Protocols\r +Upgrade: websocket\r +Connection: Upgrade\r +Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r +Sec-WebSocket-Protocol: json\r +\r +" +--- response_body +failed to connect: failed websocket handshake: invalid "Sec-WebSocket-Protocol" response header +--- no_error_log +[error] + + + +=== TEST 8: an offered subprotocol is accepted +--- http_config eval: $::HttpConfig +--- config + location = /t { + content_by_lua_block { + local client = require "resty.websocket.client" + local wb = client:new() + + local ok, err = wb:connect("ws://127.0.0.1:7986/", + { key = "dGhlIHNhbXBsZSBub25jZQ==", + protocols = { "xml", "json" } }) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("connected") + } + } +--- request +GET /t +--- tcp_listen: 7986 +--- tcp_reply eval +"HTTP/1.1 101 Switching Protocols\r +Upgrade: websocket\r +Connection: Upgrade\r +Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r +Sec-WebSocket-Protocol: json\r +\r +" +--- response_body +connected +--- no_error_log +[error] + + + +=== TEST 9: an extension that was never offered is rejected +--- http_config eval: $::HttpConfig +--- config + location = /t { + content_by_lua_block { + local client = require "resty.websocket.client" + local wb = client:new() + + local ok, err = wb:connect("ws://127.0.0.1:7986/", + { key = "dGhlIHNhbXBsZSBub25jZQ==" }) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("connected") + } + } +--- request +GET /t +--- tcp_listen: 7986 +--- tcp_reply eval +"HTTP/1.1 101 Switching Protocols\r +Upgrade: websocket\r +Connection: Upgrade\r +Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r +Sec-WebSocket-Extensions: permessage-deflate\r +\r +" +--- response_body +failed to connect: failed websocket handshake: unexpected "Sec-WebSocket-Extensions" response header +--- no_error_log +[error] + + + +=== TEST 10: a real handshake still succeeds +--- http_config eval: $::HttpConfig +--- config + location = /ws { + content_by_lua_block { + local server = require "resty.websocket.server" + local wb, err = server:new() + if not wb then + ngx.log(ngx.ERR, "failed to new websocket: ", err) + return ngx.exit(444) + end + + local data, typ = wb:recv_frame() + wb:send_text(data .. " [" .. typ .. "]") + } + } + + location = /t { + content_by_lua_block { + local client = require "resty.websocket.client" + local wb = client:new() + + local uri = "ws://127.0.0.1:" .. ngx.var.server_port .. "/ws" + local ok, err = wb:connect(uri) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + wb:send_text("hello") + + local data, typ, err = wb:recv_frame() + if not data then + ngx.say("failed to receive frame: ", err) + return + end + + ngx.say("received: ", data, " (", typ, ")") + wb:close() + } + } +--- request +GET /t +--- response_body +received: hello [text] (text) +--- no_error_log +[error]