reverseproxy: Mask the WS close message when we're the client (#5199)

* reverseproxy: Mask the WS close message when we're the client

* weakrand

* Bump golangci-lint version so path ignores work on Windows

* gofmt

* ugh, gofmt everything, I guess

.github/workflows/lint.yml

@@ -34,7 +34,7 @@ jobs:
       - name: golangci-lint
         uses: golangci/golangci-lint-action@v3
         with:
-          version: v1.47
+          version: v1.50
           # Windows times out frequently after about 5m50s if we don't set a longer timeout.
           args: --timeout 10m
           # Optional: show only new issues if it's a pull request. The default value is `false`.

.golangci.yml

@@ -96,3 +96,7 @@ issues:
       text: "G404" # G404: Insecure random number source (rand)
       linters:
         - gosec
+    - path: modules/caddyhttp/reverseproxy/streaming.go
+      text: "G404" # G404: Insecure random number source (rand)
+      linters:
+        - gosec

caddyconfig/httpcaddyfile/pkiapp.go

@@ -26,23 +26,23 @@ func init() {
 
 // parsePKIApp parses the global log option. Syntax:
 //
-//     pki {
-//         ca [<id>] {
-//             name            <name>
-//             root_cn         <name>
-//             intermediate_cn <name>
-//             root {
-//                 cert   <path>
-//                 key    <path>
-//                 format <format>
-//             }
-//             intermediate {
-//                 cert   <path>
-//                 key    <path>
-//                 format <format>
-//             }
-//         }
-//     }
+//	pki {
+//	    ca [<id>] {
+//	        name            <name>
+//	        root_cn         <name>
+//	        intermediate_cn <name>
+//	        root {
+//	            cert   <path>
+//	            key    <path>
+//	            format <format>
+//	        }
+//	        intermediate {
+//	            cert   <path>
+//	            key    <path>
+//	            format <format>
+//	        }
+//	    }
+//	}
 //
 // When the CA ID is unspecified, 'local' is assumed.
 func parsePKIApp(d *caddyfile.Dispenser, existingVal any) (any, error) {

cmd/caddy/main.go

@@ -19,10 +19,10 @@
 // There is no need to modify the Caddy source code to customize your
 // builds. You can easily build a custom Caddy with these simple steps:
 //
-//   1. Copy this file (main.go) into a new folder
-//   2. Edit the imports below to include the modules you want plugged in
-//   3. Run `go mod init caddy`
-//   4. Run `go install` or `go build` - you now have a custom binary!
+//  1. Copy this file (main.go) into a new folder
+//  2. Edit the imports below to include the modules you want plugged in
+//  3. Run `go mod init caddy`
+//  4. Run `go install` or `go build` - you now have a custom binary!
 //
 // Or you can use xcaddy which does it all for you as a command:
 // https://github.com/caddyserver/xcaddy

modules/caddyhttp/caddyauth/caddyfile.go

@@ -27,10 +27,10 @@ func init() {
 
 // parseCaddyfile sets up the handler from Caddyfile tokens. Syntax:
 //
-//     basicauth [<matcher>] [<hash_algorithm> [<realm>]] {
-//         <username> <hashed_password_base64> [<salt_base64>]
-//         ...
-//     }
+//	basicauth [<matcher>] [<hash_algorithm> [<realm>]] {
+//	    <username> <hashed_password_base64> [<salt_base64>]
+//	    ...
+//	}
 //
 // If no hash algorithm is supplied, bcrypt will be assumed.
 func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) {

modules/caddyhttp/encode/caddyfile.go

@@ -39,18 +39,18 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error)
 
 // UnmarshalCaddyfile sets up the handler from Caddyfile tokens. Syntax:
 //
-//     encode [<matcher>] <formats...> {
-//         gzip           [<level>]
-//         zstd
-//         minimum_length <length>
-//         # response matcher block
-//         match {
-//             status <code...>
-//             header <field> [<value>]
-//         }
-//         # or response matcher single line syntax
-//         match [header <field> [<value>]] | [status <code...>]
-//     }
+//	encode [<matcher>] <formats...> {
+//	    gzip           [<level>]
+//	    zstd
+//	    minimum_length <length>
+//	    # response matcher block
+//	    match {
+//	        status <code...>
+//	        header <field> [<value>]
+//	    }
+//	    # or response matcher single line syntax
+//	    match [header <field> [<value>]] | [status <code...>]
+//	}
 //
 // Specifying the formats on the first line will use those formats' defaults.
 func (enc *Encode) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {

modules/caddyhttp/push/caddyfile.go

@@ -26,13 +26,13 @@ func init() {
 
 // parseCaddyfile sets up the push handler. Syntax:
 //
-//     push [<matcher>] [<resource>] {
-//         [GET|HEAD] <resource>
-//         headers {
-//             [+]<field> [<value|regexp> [<replacement>]]
-//             -<field>
-//         }
-//     }
+//	push [<matcher>] [<resource>] {
+//	    [GET|HEAD] <resource>
+//	    headers {
+//	        [+]<field> [<value|regexp> [<replacement>]]
+//	        -<field>
+//	    }
+//	}
 //
 // A single resource can be specified inline without opening a
 // block for the most common/simple case. Or, a block can be

modules/caddyhttp/push/link.go

@@ -29,9 +29,9 @@ type linkResource struct {
 //
 // Accepted formats are:
 //
-//     Link: <resource>; as=script
-//     Link: <resource>; as=script,<resource>; as=style
-//     Link: <resource>;<resource2>
+//	Link: <resource>; as=script
+//	Link: <resource>; as=script,<resource>; as=style
+//	Link: <resource>;<resource2>
 //
 // where <resource> begins with a forward slash (/).
 func parseLinkHeader(header string) []linkResource {

modules/caddyhttp/push/link_test.go

@@ -4,7 +4,7 @@
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
 //
-//     http://www.apache.org/licenses/LICENSE-2.0
+//	http://www.apache.org/licenses/LICENSE-2.0
 //
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,

modules/caddyhttp/responsematchers.go

@@ -58,15 +58,14 @@ func (rm ResponseMatcher) matchStatusCode(statusCode int) bool {
 
 // ParseNamedResponseMatcher parses the tokens of a named response matcher.
 //
-//     @name {
-//         header <field> [<value>]
-//         status <code...>
-//     }
+//	@name {
+//	    header <field> [<value>]
+//	    status <code...>
+//	}
 //
 // Or, single line syntax:
 //
-//     @name [header <field> [<value>]] | [status <code...>]
-//
+//	@name [header <field> [<value>]] | [status <code...>]
 func ParseNamedResponseMatcher(d *caddyfile.Dispenser, matchers map[string]ResponseMatcher) error {
 	for d.Next() {
 		definitionName := d.Val()

modules/caddyhttp/reverseproxy/selectionpolicies.go

@@ -418,7 +418,8 @@ func (s CookieHashSelection) Select(pool UpstreamPool, req *http.Request, w http
 }
 
 // UnmarshalCaddyfile sets up the module from Caddyfile tokens. Syntax:
-//     lb_policy cookie [<name> [<secret>]]
+//
+//	lb_policy cookie [<name> [<secret>]]
 //
 // By default name is `lb`
 func (s *CookieHashSelection) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {

modules/caddyhttp/reverseproxy/streaming.go

@@ -20,12 +20,13 @@ package reverseproxy
 
 import (
 	"context"
-	"encoding/binary"
 	"io"
+	weakrand "math/rand"
 	"mime"
 	"net/http"
 	"sync"
 	"time"
+	"unsafe"
 
 	"go.uber.org/zap"
 	"golang.org/x/net/http/httpguts"
@@ -103,16 +104,19 @@ func (h Handler) handleUpgradeResponse(logger *zap.Logger, rw http.ResponseWrite
 	// with the backend, are both closed in the event of a server shutdown. This
 	// is done by registering them. We also try to gracefully close connections
 	// we recognize as websockets.
-	gracefulClose := func(conn io.ReadWriteCloser) func() error {
+	// We need to make sure the client connection messages (i.e. to upstream)
+	// are masked, so we need to know whether the connection is considered the
+	// server or the client side of the proxy.
+	gracefulClose := func(conn io.ReadWriteCloser, isClient bool) func() error {
 		if isWebsocket(req) {
 			return func() error {
-				return writeCloseControl(conn)
+				return writeCloseControl(conn, isClient)
 			}
 		}
 		return nil
 	}
-	deleteFrontConn := h.registerConnection(conn, gracefulClose(conn))
-	deleteBackConn := h.registerConnection(backConn, gracefulClose(backConn))
+	deleteFrontConn := h.registerConnection(conn, gracefulClose(conn, false))
+	deleteBackConn := h.registerConnection(backConn, gracefulClose(backConn, true))
 	defer deleteFrontConn()
 	defer deleteBackConn()
 
@@ -248,27 +252,108 @@ func (h *Handler) registerConnection(conn io.ReadWriteCloser, gracefulClose func
 // writeCloseControl sends a best-effort Close control message to the given
 // WebSocket connection. Thanks to @pascaldekloe who provided inspiration
 // from his simple implementation of this I was able to learn from at:
-// github.com/pascaldekloe/websocket.
-func writeCloseControl(conn io.Writer) error {
+// github.com/pascaldekloe/websocket. Further work for handling masking
+// taken from github.com/gorilla/websocket.
+func writeCloseControl(conn io.Writer, isClient bool) error {
+	// Sources:
 	// https://github.com/pascaldekloe/websocket/blob/32050af67a5d/websocket.go#L119
+	// https://github.com/gorilla/websocket/blob/v1.5.0/conn.go#L413
 
+	// For now, we're not using a reason. We might later, though.
+	// The code handling the reason is left in
 	var reason string // max 123 bytes (control frame payload limit is 125; status code takes 2)
-	const goingAway uint16 = 1001
 
-	// TODO: we might need to ensure we are the exclusive writer by this point (io.Copy is stopped)?
-	var writeBuf [127]byte
 	const closeMessage = 8
-	const finalBit = 1 << 7
-	writeBuf[0] = closeMessage | finalBit
-	writeBuf[1] = byte(len(reason) + 2)
-	binary.BigEndian.PutUint16(writeBuf[2:4], goingAway)
-	copy(writeBuf[4:], reason)
+	const finalBit = 1 << 7 // Frame header byte 0 bits from Section 5.2 of RFC 6455
+	const maskBit = 1 << 7  // Frame header byte 1 bits from Section 5.2 of RFC 6455
+	const goingAwayUpper uint8 = 1001 >> 8
+	const goingAwayLower uint8 = 1001 & 0xff
+
+	b0 := byte(closeMessage) | finalBit
+	b1 := byte(len(reason) + 2)
+	if isClient {
+		b1 |= maskBit
+	}
+
+	buf := make([]byte, 0, 127)
+	buf = append(buf, b0, b1)
+	msgLength := 4 + len(reason)
+
+	// Both branches below append the "going away" code and reason
+	appendMessage := func(buf []byte) []byte {
+		buf = append(buf, goingAwayUpper, goingAwayLower)
+		buf = append(buf, []byte(reason)...)
+		return buf
+	}
+
+	// When we're the client, we need to mask the message as per
+	// https://www.rfc-editor.org/rfc/rfc6455#section-5.3
+	if isClient {
+		key := newMaskKey()
+		buf = append(buf, key[:]...)
+		msgLength += len(key)
+		buf = appendMessage(buf)
+		maskBytes(key, 0, buf[2+len(key):])
+	} else {
+		buf = appendMessage(buf)
+	}
 
 	// simply best-effort, but return error for logging purposes
-	_, err := conn.Write(writeBuf[:4+len(reason)])
+	// TODO: we might need to ensure we are the exclusive writer by this point (io.Copy is stopped)?
+	_, err := conn.Write(buf[:msgLength])
 	return err
 }
 
+// Copied from https://github.com/gorilla/websocket/blob/v1.5.0/mask.go
+func maskBytes(key [4]byte, pos int, b []byte) int {
+	// Mask one byte at a time for small buffers.
+	if len(b) < 2*wordSize {
+		for i := range b {
+			b[i] ^= key[pos&3]
+			pos++
+		}
+		return pos & 3
+	}
+
+	// Mask one byte at a time to word boundary.
+	if n := int(uintptr(unsafe.Pointer(&b[0]))) % wordSize; n != 0 {
+		n = wordSize - n
+		for i := range b[:n] {
+			b[i] ^= key[pos&3]
+			pos++
+		}
+		b = b[n:]
+	}
+
+	// Create aligned word size key.
+	var k [wordSize]byte
+	for i := range k {
+		k[i] = key[(pos+i)&3]
+	}
+	kw := *(*uintptr)(unsafe.Pointer(&k))
+
+	// Mask one word at a time.
+	n := (len(b) / wordSize) * wordSize
+	for i := 0; i < n; i += wordSize {
+		*(*uintptr)(unsafe.Pointer(uintptr(unsafe.Pointer(&b[0])) + uintptr(i))) ^= kw
+	}
+
+	// Mask one byte at a time for remaining bytes.
+	b = b[n:]
+	for i := range b {
+		b[i] ^= key[pos&3]
+		pos++
+	}
+
+	return pos & 3
+}
+
+// Copied from https://github.com/gorilla/websocket/blob/v1.5.0/conn.go#L184
+func newMaskKey() [4]byte {
+	n := weakrand.Uint32()
+	return [4]byte{byte(n), byte(n >> 8), byte(n >> 16), byte(n >> 24)}
+}
+
 // isWebsocket returns true if r looks to be an upgrade request for WebSockets.
 // It is a fairly naive check.
 func isWebsocket(r *http.Request) bool {
@@ -364,3 +449,4 @@ var streamingBufPool = sync.Pool{
 }
 
 const defaultBufferSize = 32 * 1024
+const wordSize = int(unsafe.Sizeof(uintptr(0)))

modules/caddyhttp/rewrite/caddyfile.go

@@ -34,7 +34,7 @@ func init() {
 
 // parseCaddyfileRewrite sets up a basic rewrite handler from Caddyfile tokens. Syntax:
 //
-//     rewrite [<matcher>] <to>
+//	rewrite [<matcher>] <to>
 //
 // Only URI components which are given in <to> will be set in the resulting URI.
 // See the docs for the rewrite handler for more information.
@@ -54,8 +54,7 @@ func parseCaddyfileRewrite(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler,
 
 // parseCaddyfileMethod sets up a basic method rewrite handler from Caddyfile tokens. Syntax:
 //
-//     method [<matcher>] <method>
-//
+//	method [<matcher>] <method>
 func parseCaddyfileMethod(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) {
 	var rewr Rewrite
 	for h.Next() {
@@ -73,7 +72,7 @@ func parseCaddyfileMethod(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler,
 // parseCaddyfileURI sets up a handler for manipulating (but not "rewriting") the
 // URI from Caddyfile tokens. Syntax:
 //
-//     uri [<matcher>] strip_prefix|strip_suffix|replace|path_regexp <target> [<replacement> [<limit>]]
+//	uri [<matcher>] strip_prefix|strip_suffix|replace|path_regexp <target> [<replacement> [<limit>]]
 //
 // If strip_prefix or strip_suffix are used, then <target> will be stripped
 // only if it is the beginning or the end, respectively, of the URI path. If
@@ -147,9 +146,9 @@ func parseCaddyfileURI(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, err
 
 // parseCaddyfileHandlePath parses the handle_path directive. Syntax:
 //
-//     handle_path [<matcher>] {
-//         <directives...>
-//     }
+//	handle_path [<matcher>] {
+//	    <directives...>
+//	}
 //
 // Only path matchers (with a `/` prefix) are supported as this is a shortcut
 // for the handle directive with a strip_prefix rewrite.

modules/caddyhttp/staticerror.go

@@ -53,9 +53,9 @@ func (StaticError) CaddyModule() caddy.ModuleInfo {
 
 // UnmarshalCaddyfile sets up the handler from Caddyfile tokens. Syntax:
 //
-//     error [<matcher>] <status>|<message> [<status>] {
-//         message <text>
-//     }
+//	error [<matcher>] <status>|<message> [<status>] {
+//	    message <text>
+//	}
 //
 // If there is just one argument (other than the matcher), it is considered
 // to be a status code if it's a valid positive integer of 3 digits.

modules/caddyhttp/templates/caddyfile.go

@@ -25,12 +25,11 @@ func init() {
 
 // parseCaddyfile sets up the handler from Caddyfile tokens. Syntax:
 //
-//     templates [<matcher>] {
-//         mime <types...>
-//         between <open_delim> <close_delim>
-//         root <path>
-//     }
-//
+//	templates [<matcher>] {
+//	    mime <types...>
+//	    between <open_delim> <close_delim>
+//	    root <path>
+//	}
 func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) {
 	t := new(Templates)
 	for h.Next() {

modules/caddyhttp/templates/templates.go

@@ -152,10 +152,10 @@ func init() {
 //
 // Accesses the current HTTP request, which has various fields, including:
 //
-//    - `.Method` - the method
-//    - `.URL` - the URL, which in turn has component fields (Scheme, Host, Path, etc.)
-//    - `.Header` - the header fields
-//    - `.Host` - the Host or :authority header of the request
+//   - `.Method` - the method
+//   - `.URL` - the URL, which in turn has component fields (Scheme, Host, Path, etc.)
+//   - `.Header` - the header fields
+//   - `.Host` - the Host or :authority header of the request
 //
 // ```
 // {{.Req.Header.Get "User-Agent"}}
@@ -221,15 +221,16 @@ func init() {
 // ---
 // ```
 //
-//
 // **JSON** is simply `{` and `}`:
 //
 // ```
-// {
-// 	"template": "blog",
-// 	"title": "Blog Homepage",
-// 	"sitename": "A Caddy site"
-// }
+//
+//	{
+//		"template": "blog",
+//		"title": "Blog Homepage",
+//		"sitename": "A Caddy site"
+//	}
+//
 // ```
 //
 // The resulting front matter will be made available like so:
@@ -237,7 +238,6 @@ func init() {
 // - `.Meta` to access the metadata fields, for example: `{{$parsed.Meta.title}}`
 // - `.Body` to access the body after the front matter, for example: `{{markdown $parsed.Body}}`
 //
-//
 // ##### `stripHTML`
 //
 // Removes HTML from a string.

modules/caddypki/acmeserver/caddyfile.go

@@ -25,10 +25,9 @@ func init() {
 
 // parseACMEServer sets up an ACME server handler from Caddyfile tokens.
 //
-//     acme_server [<matcher>] {
-//         ca <id>
-//     }
-//
+//	acme_server [<matcher>] {
+//	    ca <id>
+//	}
 func parseACMEServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) {
 	if !h.Next() {
 		return nil, h.ArgErr()

modules/logging/filewriter.go

@@ -131,14 +131,14 @@ func (fw FileWriter) OpenWriter() (io.WriteCloser, error) {
 
 // UnmarshalCaddyfile sets up the module from Caddyfile tokens. Syntax:
 //
-//     file <filename> {
-//         roll_disabled
-//         roll_size     <size>
-//         roll_uncompressed
-//         roll_local_time
-//         roll_keep     <num>
-//         roll_keep_for <days>
-//     }
+//	file <filename> {
+//	    roll_disabled
+//	    roll_size     <size>
+//	    roll_uncompressed
+//	    roll_local_time
+//	    roll_keep     <num>
+//	    roll_keep_for <days>
+//	}
 //
 // The roll_size value has megabyte resolution.
 // Fractional values are rounded up to the next whole megabyte (MiB).

modules/logging/filterencoder.go

@@ -98,14 +98,14 @@ func (fe *FilterEncoder) Provision(ctx caddy.Context) error {
 
 // UnmarshalCaddyfile sets up the module from Caddyfile tokens. Syntax:
 //
-//     filter {
-//         wrap <another encoder>
-//         fields {
-//             <field> <filter> {
-//                 <filter options>
-//             }
-//         }
-//     }
+//	filter {
+//	    wrap <another encoder>
+//	    fields {
+//	        <field> <filter> {
+//	            <filter options>
+//	        }
+//	    }
+//	}
 func (fe *FilterEncoder) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 	for d.Next() {
 		for d.NextBlock(0) {

modules/logging/netwriter.go

@@ -102,10 +102,9 @@ func (nw NetWriter) OpenWriter() (io.WriteCloser, error) {
 
 // UnmarshalCaddyfile sets up the handler from Caddyfile tokens. Syntax:
 //
-//     net <address> {
-//         dial_timeout <duration>
-//     }
-//
+//	net <address> {
+//	    dial_timeout <duration>
+//	}
 func (nw *NetWriter) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 	for d.Next() {
 		if !d.NextArg() {