Reject invalid "Sec-WebSocket-Key" headers from clients

Client "Sec-WebSocket-Key" should be a valid 16 byte base64 encoded nonce. If the header is not valid, the server should reject the client.
nhooyr · Oct 19, 2023 · 1a344a4 · 1a344a4
1 parent 64ce009
commit 1a344a4
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 8 deletions.
diff --git a/accept.go b/accept.go
@@ -185,10 +185,15 @@ func verifyClientRequest(w http.ResponseWriter, r *http.Request) (errCode int, _
 		return http.StatusBadRequest, fmt.Errorf("unsupported WebSocket protocol version (only 13 is supported): %q", r.Header.Get("Sec-WebSocket-Version"))
 	}
 
-	if r.Header.Get("Sec-WebSocket-Key") == "" {
+	websocketSecKey := r.Header.Get("Sec-WebSocket-Key")
+	if websocketSecKey == "" {
 		return http.StatusBadRequest, errors.New("WebSocket protocol violation: missing Sec-WebSocket-Key")
 	}
 
+	if v, err := base64.StdEncoding.DecodeString(websocketSecKey); err != nil || len(v) != 16 {
+		return http.StatusBadRequest, fmt.Errorf("WebSocket protocol violation: invalid Sec-WebSocket-Key %q, must be a 16 byte base64 encoded string", websocketSecKey)
+	}
+
 	return 0, nil
 }
 

diff --git a/accept_test.go b/accept_test.go
@@ -9,6 +9,7 @@ import (
 	"net"
 	"net/http"
 	"net/http/httptest"
+	"nhooyr.io/websocket/internal/test/xrand"
 	"strings"
 	"testing"
 
@@ -36,7 +37,7 @@ func TestAccept(t *testing.T) {
 		r.Header.Set("Connection", "Upgrade")
 		r.Header.Set("Upgrade", "websocket")
 		r.Header.Set("Sec-WebSocket-Version", "13")
-		r.Header.Set("Sec-WebSocket-Key", "meow123")
+		r.Header.Set("Sec-WebSocket-Key", xrand.Base64(16))
 		r.Header.Set("Origin", "harhar.com")
 
 		_, err := Accept(w, r, nil)
@@ -52,7 +53,7 @@ func TestAccept(t *testing.T) {
 		r.Header.Set("Connection", "Upgrade")
 		r.Header.Set("Upgrade", "websocket")
 		r.Header.Set("Sec-WebSocket-Version", "13")
-		r.Header.Set("Sec-WebSocket-Key", "meow123")
+		r.Header.Set("Sec-WebSocket-Key", xrand.Base64(16))
 		r.Header.Set("Origin", "https://harhar.com")
 
 		_, err := Accept(w, r, nil)
@@ -116,7 +117,7 @@ func TestAccept(t *testing.T) {
 		r.Header.Set("Connection", "Upgrade")
 		r.Header.Set("Upgrade", "websocket")
 		r.Header.Set("Sec-WebSocket-Version", "13")
-		r.Header.Set("Sec-WebSocket-Key", "meow123")
+		r.Header.Set("Sec-WebSocket-Key", xrand.Base64(16))
 
 		_, err := Accept(w, r, nil)
 		assert.Contains(t, err, `http.ResponseWriter does not implement http.Hijacker`)
@@ -136,7 +137,7 @@ func TestAccept(t *testing.T) {
 		r.Header.Set("Connection", "Upgrade")
 		r.Header.Set("Upgrade", "websocket")
 		r.Header.Set("Sec-WebSocket-Version", "13")
-		r.Header.Set("Sec-WebSocket-Key", "meow123")
+		r.Header.Set("Sec-WebSocket-Key", xrand.Base64(16))
 
 		_, err := Accept(w, r, nil)
 		assert.Contains(t, err, `failed to hijack connection`)
@@ -183,21 +184,39 @@ func Test_verifyClientHandshake(t *testing.T) {
 			},
 		},
 		{
-			name: "badWebSocketKey",
+			name: "missingWebSocketKey",
 			h: map[string]string{
 				"Connection":            "Upgrade",
 				"Upgrade":               "websocket",
 				"Sec-WebSocket-Version": "13",
 				"Sec-WebSocket-Key":     "",
 			},
 		},
+		{
+			name: "shortWebSocketKey",
+			h: map[string]string{
+				"Connection":            "Upgrade",
+				"Upgrade":               "websocket",
+				"Sec-WebSocket-Version": "13",
+				"Sec-WebSocket-Key":     xrand.Base64(15),
+			},
+		},
+		{
+			name: "invalidWebSocketKey",
+			h: map[string]string{
+				"Connection":            "Upgrade",
+				"Upgrade":               "websocket",
+				"Sec-WebSocket-Version": "13",
+				"Sec-WebSocket-Key":     "notbase64",
+			},
+		},
 		{
 			name: "badHTTPVersion",
 			h: map[string]string{
 				"Connection":            "Upgrade",
 				"Upgrade":               "websocket",
 				"Sec-WebSocket-Version": "13",
-				"Sec-WebSocket-Key":     "meow123",
+				"Sec-WebSocket-Key":     xrand.Base64(16),
 			},
 			http1: true,
 		},
@@ -207,7 +226,7 @@ func Test_verifyClientHandshake(t *testing.T) {
 				"Connection":            "keep-alive, Upgrade",
 				"Upgrade":               "websocket",
 				"Sec-WebSocket-Version": "13",
-				"Sec-WebSocket-Key":     "meow123",
+				"Sec-WebSocket-Key":     xrand.Base64(16),
 			},
 			success: true,
 		},

diff --git a/internal/test/xrand/xrand.go b/internal/test/xrand/xrand.go
@@ -2,6 +2,7 @@ package xrand
 
 import (
 	"crypto/rand"
+	"encoding/base64"
 	"fmt"
 	"math/big"
 	"strings"
@@ -45,3 +46,7 @@ func Int(max int) int {
 	}
 	return int(x.Int64())
 }
+
+func Base64(n int) string {
+	return base64.StdEncoding.EncodeToString(Bytes(n))
+}