diff --git a/pkg/util/vhost/http.go b/pkg/util/vhost/http.go index 33b11223..9394e913 100644 --- a/pkg/util/vhost/http.go +++ b/pkg/util/vhost/http.go @@ -206,12 +206,28 @@ func checkRouteAuthByRequest(req *http.Request, rc *RouteConfig) bool { } func (rp *HTTPReverseProxy) connectHandler(rw http.ResponseWriter, req *http.Request) { + if req.ContentLength > 0 || len(req.TransferEncoding) > 0 { + http.Error(rw, http.StatusText(http.StatusBadRequest), http.StatusBadRequest) + return + } + hj, ok := rw.(http.Hijacker) if !ok { rw.WriteHeader(http.StatusInternalServerError) return } + // Forward only CONNECT headers; the original Body must not be used after Hijack. + outreq := req.Clone(req.Context()) + outreq.Body = nil + outreq.GetBody = nil + outreq.ContentLength = 0 + outreq.TransferEncoding = nil + outreq.Trailer = nil + outreq.Header.Del("Content-Length") + outreq.Header.Del("Transfer-Encoding") + outreq.Header.Del("Trailer") + client, buffered, err := hj.Hijack() if err != nil { rw.WriteHeader(http.StatusInternalServerError) @@ -224,7 +240,11 @@ func (rp *HTTPReverseProxy) connectHandler(rw http.ResponseWriter, req *http.Req client.Close() return } - _ = req.Write(remote) + if err := outreq.Write(remote); err != nil { + _ = remote.Close() + _ = client.Close() + return + } go libio.Join(remote, libio.WrapReadWriteCloser(buffered.Reader, client, client.Close)) } diff --git a/pkg/util/vhost/http_test.go b/pkg/util/vhost/http_test.go index e993da48..e661ca08 100644 --- a/pkg/util/vhost/http_test.go +++ b/pkg/util/vhost/http_test.go @@ -7,6 +7,8 @@ import ( "net" "net/http" "net/http/httptest" + "strings" + "sync/atomic" "testing" "time" @@ -17,8 +19,17 @@ import ( ) func TestHTTPConnectTunnel(t *testing.T) { - for _, pipelined := range []bool{false, true} { - t.Run(fmt.Sprintf("pipelined=%v", pipelined), func(t *testing.T) { + for _, tc := range []struct { + name string + framing string + pipelined bool + }{ + {name: "plain"}, + {name: "plain-early", pipelined: true}, + {name: "cl0", framing: "Content-Length: 0\r\n"}, + {name: "cl0-early", framing: "Content-Length: 0\r\n", pipelined: true}, + } { + t.Run(tc.name, func(t *testing.T) { remote, backend := net.Pipe() defer remote.Close() defer backend.Close() @@ -26,7 +37,9 @@ func TestHTTPConnectTunnel(t *testing.T) { rp := NewHTTPReverseProxy(HTTPReverseProxyOptions{}, NewRouters()) require.NoError(t, rp.Register(RouteConfig{ - Domain: "target.example.com", + Domain: "target.example.com", + Username: "alice", + Password: "secret", CreateConnFn: func(string) (net.Conn, error) { return remote, nil }, @@ -48,8 +61,10 @@ func TestHTTPConnectTunnel(t *testing.T) { require.NoError(t, client.SetDeadline(time.Now().Add(5*time.Second))) require.NoError(t, listener.PutConn(incoming)) - request := "CONNECT target.example.com:443 HTTP/1.1\r\nHost: target.example.com:443\r\n\r\n" - if pipelined { + proxyAuth := httppkg.BasicAuth("alice", "secret") + request := "CONNECT target.example.com:443 HTTP/1.1\r\nHost: target.example.com:443\r\n" + + "X-Connect-Test: tunnel\r\nProxy-Authorization: " + proxyAuth + "\r\n" + tc.framing + "\r\n" + if tc.pipelined { // One pipe write puts the tunnel data in net/http's request buffer. request += "early data" } @@ -61,7 +76,12 @@ func TestHTTPConnectTunnel(t *testing.T) { defer req.Body.Close() require.Equal(t, http.MethodConnect, req.Method) require.Equal(t, "target.example.com:443", req.Host) - if pipelined { + require.Equal(t, "target.example.com:443", req.URL.Host) + require.Equal(t, "tunnel", req.Header.Get("X-Connect-Test")) + require.Equal(t, proxyAuth, req.Header.Get("Proxy-Authorization")) + require.Zero(t, req.ContentLength) + require.Empty(t, req.TransferEncoding) + if tc.pipelined { data := make([]byte, len("early data")) _, err = io.ReadFull(backendReader, data) require.NoError(t, err) @@ -94,6 +114,155 @@ func TestHTTPConnectTunnel(t *testing.T) { } } +func TestHTTPConnectRejectsContent(t *testing.T) { + for _, tc := range []struct { + name string + framing string + body string + }{ + {name: "content-length", framing: "Content-Length: 1\r\n", body: "X"}, + {name: "chunked", framing: "Transfer-Encoding: chunked\r\n", body: "1\r\nX\r\n0\r\n\r\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + backendCalls := 0 + rp := NewHTTPReverseProxy(HTTPReverseProxyOptions{}, NewRouters()) + require.NoError(t, rp.Register(RouteConfig{ + Domain: "target.example.com", + CreateConnFn: func(string) (net.Conn, error) { + backendCalls++ + return nil, io.ErrClosedPipe + }, + })) + req, err := http.ReadRequest(bufio.NewReader(strings.NewReader( + "CONNECT target.example.com:443 HTTP/1.1\r\nHost: target.example.com:443\r\n" + tc.framing + "\r\n" + tc.body))) + require.NoError(t, err) + defer req.Body.Close() + rw := &connectTestResponseWriter{ResponseRecorder: httptest.NewRecorder()} + + rp.ServeHTTP(rw, req) + + require.Equal(t, http.StatusBadRequest, rw.Code) + require.Zero(t, rw.hijackCalls, "reject content before Hijack") + require.Zero(t, backendCalls, "reject content before creating a backend") + }) + } +} + +func TestHTTPConnectHeaderWriteFailure(t *testing.T) { + for _, originalBody := range []bool{false, true} { + t.Run(fmt.Sprintf("originalBody=%v", originalBody), func(t *testing.T) { + client, incoming := net.Pipe() + defer client.Close() + defer incoming.Close() + remote, backend := net.Pipe() + defer remote.Close() + defer backend.Close() + clientConn := &connectTestConn{Conn: incoming} + remoteConn := &connectTestConn{Conn: remote, failWrites: true} + rp := NewHTTPReverseProxy(HTTPReverseProxyOptions{}, NewRouters()) + require.NoError(t, rp.Register(RouteConfig{ + Domain: "target.example.com", + CreateConnFn: func(string) (net.Conn, error) { + return remoteConn, nil + }, + })) + req := rp.injectRequestInfoToCtx(httptest.NewRequest(http.MethodConnect, "target.example.com:443", nil)) + require.Equal(t, "target.example.com:443", req.Host) + body := &connectTestBody{} + if originalBody { + req.Body = body + } + requestBody := req.Body + getBodyCalls := 0 + req.GetBody = func() (io.ReadCloser, error) { + getBodyCalls++ + return body, nil + } + req.Header.Set("Content-Length", "0") + req.Header.Set("Trailer", "X-Connect-Trailer") + req.Trailer = http.Header{"X-Connect-Trailer": {"value"}} + originalHeader, originalTrailer := req.Header.Clone(), req.Trailer.Clone() + rw := &connectTestResponseWriter{ + ResponseRecorder: httptest.NewRecorder(), + conn: clientConn, + buffered: bufio.NewReadWriter(bufio.NewReader(clientConn), bufio.NewWriter(clientConn)), + } + + rp.connectHandler(rw, req) + + require.Equal(t, 1, rw.hijackCalls) + require.Zero(t, body.reads, "do not read the original Body after Hijack") + require.Zero(t, body.closes, "do not close the original Body after Hijack") + require.Positive(t, remoteConn.writes.Load(), "exercise CONNECT header write failure") + require.True(t, remoteConn.closed.Load(), "close backend on CONNECT header write failure") + require.True(t, clientConn.closed.Load(), "close client on CONNECT header write failure") + require.Zero(t, remoteConn.reads.Load(), "do not start Join after header write failure") + require.Zero(t, clientConn.reads.Load(), "do not start Join after header write failure") + require.Zero(t, getBodyCalls) + require.Equal(t, requestBody, req.Body) + require.NotNil(t, req.GetBody) + require.Equal(t, originalHeader, req.Header) + require.Equal(t, originalTrailer, req.Trailer) + }) + } +} + +type connectTestResponseWriter struct { + *httptest.ResponseRecorder + conn net.Conn + buffered *bufio.ReadWriter + hijackCalls int +} + +func (rw *connectTestResponseWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) { + rw.hijackCalls++ + if rw.conn == nil { + return nil, nil, io.ErrClosedPipe + } + return rw.conn, rw.buffered, nil +} + +type connectTestConn struct { + net.Conn + failWrites bool + closed atomic.Bool + reads atomic.Int32 + writes atomic.Int32 +} + +func (c *connectTestConn) Read(p []byte) (int, error) { + c.reads.Add(1) + return c.Conn.Read(p) +} + +func (c *connectTestConn) Write(p []byte) (int, error) { + c.writes.Add(1) + if c.failWrites { + return 0, io.ErrClosedPipe + } + return c.Conn.Write(p) +} + +func (c *connectTestConn) Close() error { + c.closed.Store(true) + return c.Conn.Close() +} + +type connectTestBody struct { + reads int + closes int +} + +func (b *connectTestBody) Read([]byte) (int, error) { + b.reads++ + return 0, io.ErrUnexpectedEOF +} + +func (b *connectTestBody) Close() error { + b.closes++ + return nil +} + func TestHTTPServerProtocols(t *testing.T) { rp := NewHTTPReverseProxy(HTTPReverseProxyOptions{}, NewRouters()) protocols := new(http.Protocols)