From 61dc4e1eb260427d9a39ccaeeb75fe85be7dcccc Mon Sep 17 00:00:00 2001 From: Christophe Fergeau Date: Tue, 20 Aug 2024 18:04:55 +0200 Subject: [PATCH] deps: Revert inetaf/tcpproxy commit 2862066 This causes a regression in gvproxy when it's used by podman: https://github.com/containers/podman/issues/23616 Thanks to Maciej Szlosarczyk for investigating and finding the faulty commit! Reverting inetaf/tcpproxy commit 2862066 is a bit convoluted, as we need to first undo the module name change (inet.af/tcpproxy -> github.com/inetaf/tcpproxy) done in commit 600910ca and then a go module `replace` directive to redirect the no-longer existing inet.af/tcpproxy to the commit we want in github.com/inetaf/tcpproxy/ This way, the module name in gvisor-tap-vsock go.mod and in github.com/inetaf/tcpproxy go.mod are the same (inet.af/tcpproxy), and we can use older commits in this repository. It's unclear what's causing the regression, as the commit log/PR description/associated issue don't provide useful details: https://github.com/inetaf/tcpproxy/commit/2862066fc2a9405880f212f71230425bdfe9950e The best I could find is: https://github.com/tailscale/tailscale/pull/10070 > The close in the handler sometimes occurs before the buffered data is forwarded. The proxy could be improved to perform a half-close dance, such that it will only mutually close once both halves are closed or both halves error. and https://github.com/inetaf/tcpproxy/issues/21 which seems to be the same issue as https://github.com/inetaf/tcpproxy/pull/38 which is the issue fixed by the commit triggering the regression. What could be happening is that before inetaf/tcpproxy commit 2862066, as soon as one side of the connection was closed, the other half was also closed, while after commit 2862066, the tcpproxy code waits for both halves of the connection to be closed. So maybe we are missing a connection close somewhere in gvproxy's code :-/ Signed-off-by: Christophe Fergeau --- go.mod | 4 +- go.sum | 4 +- pkg/services/forwarder/ports.go | 2 +- pkg/services/forwarder/tcp.go | 2 +- pkg/virtualnetwork/mux.go | 2 +- .../inetaf => inet.af}/tcpproxy/.travis.yml | 0 .../tcpproxy/CONTRIBUTING.md | 0 .../inetaf => inet.af}/tcpproxy/LICENSE | 0 .../inetaf => inet.af}/tcpproxy/README.md | 2 +- .../inetaf => inet.af}/tcpproxy/http.go | 0 .../inetaf => inet.af}/tcpproxy/listener.go | 0 .../inetaf => inet.af}/tcpproxy/sni.go | 0 .../inetaf => inet.af}/tcpproxy/tcpproxy.go | 40 ++++--------------- vendor/modules.txt | 7 ++-- 14 files changed, 21 insertions(+), 42 deletions(-) rename vendor/{github.com/inetaf => inet.af}/tcpproxy/.travis.yml (100%) rename vendor/{github.com/inetaf => inet.af}/tcpproxy/CONTRIBUTING.md (100%) rename vendor/{github.com/inetaf => inet.af}/tcpproxy/LICENSE (100%) rename vendor/{github.com/inetaf => inet.af}/tcpproxy/README.md (59%) rename vendor/{github.com/inetaf => inet.af}/tcpproxy/http.go (100%) rename vendor/{github.com/inetaf => inet.af}/tcpproxy/listener.go (100%) rename vendor/{github.com/inetaf => inet.af}/tcpproxy/sni.go (100%) rename vendor/{github.com/inetaf => inet.af}/tcpproxy/tcpproxy.go (95%) diff --git a/go.mod b/go.mod index a7b49af06..15a7c1b15 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,6 @@ require ( github.com/coreos/stream-metadata-go v0.4.4 github.com/dustin/go-humanize v1.0.1 github.com/google/gopacket v1.1.19 - github.com/inetaf/tcpproxy v0.0.0-20240214030015-3ce58045626c github.com/insomniacslk/dhcp v0.0.0-20240710054256-ddd8a41251c9 github.com/linuxkit/virtsock v0.0.0-20220523201153-1a23e78aa7a2 github.com/mdlayher/vsock v1.2.1 @@ -27,6 +26,7 @@ require ( golang.org/x/sync v0.8.0 golang.org/x/sys v0.24.0 gvisor.dev/gvisor v0.0.0-20231023213702-2691a8f9b1cf + inet.af/tcpproxy v0.0.0-20220326234310-be3ee21c9fa0 ) require ( @@ -49,3 +49,5 @@ require ( gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) + +replace inet.af/tcpproxy => github.com/inetaf/tcpproxy v0.0.0-20221017015627-91f861402626 diff --git a/go.sum b/go.sum index 9c0689a11..3440f2720 100644 --- a/go.sum +++ b/go.sum @@ -40,8 +40,8 @@ github.com/google/gopacket v1.1.19/go.mod h1:iJ8V8n6KS+z2U1A8pUwu8bW5SyEMkXJB8Yo github.com/google/pprof v0.0.0-20240424215950-a892ee059fd6 h1:k7nVchz72niMH6YLQNvHSdIE7iqsQxK1P41mySCvssg= github.com/google/pprof v0.0.0-20240424215950-a892ee059fd6/go.mod h1:kf6iHlnVGwgKolg33glAes7Yg/8iWP8ukqeldJSO7jw= github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU= -github.com/inetaf/tcpproxy v0.0.0-20240214030015-3ce58045626c h1:gYfYE403/nlrGNYj6BEOs9ucLCAGB9gstlSk92DttTg= -github.com/inetaf/tcpproxy v0.0.0-20240214030015-3ce58045626c/go.mod h1:Di7LXRyUcnvAcLicFhtM9/MlZl/TNgRSDHORM2c6CMI= +github.com/inetaf/tcpproxy v0.0.0-20221017015627-91f861402626 h1:oeu2cpk2bBlSgMQiSQIBJ8+FZsTqMG9fwdPez/weEbk= +github.com/inetaf/tcpproxy v0.0.0-20221017015627-91f861402626/go.mod h1:Tojt5kmHpDIR2jMojxzZK2w2ZR7OILODmUo2gaSwjrk= github.com/insomniacslk/dhcp v0.0.0-20240710054256-ddd8a41251c9 h1:LZJWucZz7ztCqY6Jsu7N9g124iJ2kt/O62j3+UchZFg= github.com/insomniacslk/dhcp v0.0.0-20240710054256-ddd8a41251c9/go.mod h1:KclMyHxX06VrVr0DJmeFSUb1ankt7xTfoOA35pCkoic= github.com/josharian/native v1.1.0 h1:uuaP0hAbW7Y4l0ZRQ6C9zfb7Mg1mbFKry/xzDAfmtLA= diff --git a/pkg/services/forwarder/ports.go b/pkg/services/forwarder/ports.go index 7d6c06c2d..887092cbd 100644 --- a/pkg/services/forwarder/ports.go +++ b/pkg/services/forwarder/ports.go @@ -17,12 +17,12 @@ import ( "github.com/containers/gvisor-tap-vsock/pkg/sshclient" "github.com/containers/gvisor-tap-vsock/pkg/types" - "github.com/inetaf/tcpproxy" log "github.com/sirupsen/logrus" "gvisor.dev/gvisor/pkg/tcpip" "gvisor.dev/gvisor/pkg/tcpip/adapters/gonet" "gvisor.dev/gvisor/pkg/tcpip/network/ipv4" "gvisor.dev/gvisor/pkg/tcpip/stack" + "inet.af/tcpproxy" ) type PortsForwarder struct { diff --git a/pkg/services/forwarder/tcp.go b/pkg/services/forwarder/tcp.go index 6493c94c6..9026d662d 100644 --- a/pkg/services/forwarder/tcp.go +++ b/pkg/services/forwarder/tcp.go @@ -6,13 +6,13 @@ import ( "net" "sync" - "github.com/inetaf/tcpproxy" log "github.com/sirupsen/logrus" "gvisor.dev/gvisor/pkg/tcpip" "gvisor.dev/gvisor/pkg/tcpip/adapters/gonet" "gvisor.dev/gvisor/pkg/tcpip/stack" "gvisor.dev/gvisor/pkg/tcpip/transport/tcp" "gvisor.dev/gvisor/pkg/waiter" + "inet.af/tcpproxy" ) const linkLocalSubnet = "169.254.0.0/16" diff --git a/pkg/virtualnetwork/mux.go b/pkg/virtualnetwork/mux.go index cc61c5d75..94e89e030 100644 --- a/pkg/virtualnetwork/mux.go +++ b/pkg/virtualnetwork/mux.go @@ -8,11 +8,11 @@ import ( "strconv" "github.com/containers/gvisor-tap-vsock/pkg/types" - "github.com/inetaf/tcpproxy" log "github.com/sirupsen/logrus" "gvisor.dev/gvisor/pkg/tcpip" "gvisor.dev/gvisor/pkg/tcpip/adapters/gonet" "gvisor.dev/gvisor/pkg/tcpip/network/ipv4" + "inet.af/tcpproxy" ) func (n *VirtualNetwork) Mux() *http.ServeMux { diff --git a/vendor/github.com/inetaf/tcpproxy/.travis.yml b/vendor/inet.af/tcpproxy/.travis.yml similarity index 100% rename from vendor/github.com/inetaf/tcpproxy/.travis.yml rename to vendor/inet.af/tcpproxy/.travis.yml diff --git a/vendor/github.com/inetaf/tcpproxy/CONTRIBUTING.md b/vendor/inet.af/tcpproxy/CONTRIBUTING.md similarity index 100% rename from vendor/github.com/inetaf/tcpproxy/CONTRIBUTING.md rename to vendor/inet.af/tcpproxy/CONTRIBUTING.md diff --git a/vendor/github.com/inetaf/tcpproxy/LICENSE b/vendor/inet.af/tcpproxy/LICENSE similarity index 100% rename from vendor/github.com/inetaf/tcpproxy/LICENSE rename to vendor/inet.af/tcpproxy/LICENSE diff --git a/vendor/github.com/inetaf/tcpproxy/README.md b/vendor/inet.af/tcpproxy/README.md similarity index 59% rename from vendor/github.com/inetaf/tcpproxy/README.md rename to vendor/inet.af/tcpproxy/README.md index 8181ceb91..f526c213a 100644 --- a/vendor/github.com/inetaf/tcpproxy/README.md +++ b/vendor/inet.af/tcpproxy/README.md @@ -1,5 +1,5 @@ # tcpproxy -For library usage, see https://pkg.go.dev/github.com/inetaf/tcpproxy/ +For library usage, see https://godoc.org/inet.af/tcpproxy/ For CLI usage, see https://github.com/inetaf/tcpproxy/blob/master/cmd/tlsrouter/README.md diff --git a/vendor/github.com/inetaf/tcpproxy/http.go b/vendor/inet.af/tcpproxy/http.go similarity index 100% rename from vendor/github.com/inetaf/tcpproxy/http.go rename to vendor/inet.af/tcpproxy/http.go diff --git a/vendor/github.com/inetaf/tcpproxy/listener.go b/vendor/inet.af/tcpproxy/listener.go similarity index 100% rename from vendor/github.com/inetaf/tcpproxy/listener.go rename to vendor/inet.af/tcpproxy/listener.go diff --git a/vendor/github.com/inetaf/tcpproxy/sni.go b/vendor/inet.af/tcpproxy/sni.go similarity index 100% rename from vendor/github.com/inetaf/tcpproxy/sni.go rename to vendor/inet.af/tcpproxy/sni.go diff --git a/vendor/github.com/inetaf/tcpproxy/tcpproxy.go b/vendor/inet.af/tcpproxy/tcpproxy.go similarity index 95% rename from vendor/github.com/inetaf/tcpproxy/tcpproxy.go rename to vendor/inet.af/tcpproxy/tcpproxy.go index 1f03e3201..5d178c633 100644 --- a/vendor/github.com/inetaf/tcpproxy/tcpproxy.go +++ b/vendor/inet.af/tcpproxy/tcpproxy.go @@ -345,30 +345,8 @@ func UnderlyingConn(c net.Conn) net.Conn { return c } -func tcpConn(c net.Conn) (t *net.TCPConn, ok bool) { - if c, ok := UnderlyingConn(c).(*net.TCPConn); ok { - return c, ok - } - if c, ok := c.(*net.TCPConn); ok { - return c, ok - } - return nil, false -} - func goCloseConn(c net.Conn) { go c.Close() } -func closeRead(c net.Conn) { - if c, ok := tcpConn(c); ok { - c.CloseRead() - } -} - -func closeWrite(c net.Conn) { - if c, ok := tcpConn(c); ok { - c.CloseWrite() - } -} - // HandleConn implements the Target interface. func (dp *DialProxy) HandleConn(src net.Conn) { ctx := context.Background() @@ -393,19 +371,20 @@ func (dp *DialProxy) HandleConn(src net.Conn) { defer goCloseConn(src) if ka := dp.keepAlivePeriod(); ka > 0 { - for _, c := range []net.Conn{src, dst} { - if c, ok := tcpConn(c); ok { - c.SetKeepAlive(true) - c.SetKeepAlivePeriod(ka) - } + if c, ok := UnderlyingConn(src).(*net.TCPConn); ok { + c.SetKeepAlive(true) + c.SetKeepAlivePeriod(ka) + } + if c, ok := dst.(*net.TCPConn); ok { + c.SetKeepAlive(true) + c.SetKeepAlivePeriod(ka) } } - errc := make(chan error, 2) + errc := make(chan error, 1) go proxyCopy(errc, src, dst) go proxyCopy(errc, dst, src) <-errc - <-errc } func (dp *DialProxy) sendProxyHeader(w io.Writer, src net.Conn) error { @@ -441,9 +420,6 @@ func (dp *DialProxy) sendProxyHeader(w io.Writer, src net.Conn) error { // It's a named function instead of a func literal so users get // named goroutines in debug goroutine stack dumps. func proxyCopy(errc chan<- error, dst, src net.Conn) { - defer closeRead(src) - defer closeWrite(dst) - // Before we unwrap src and/or dst, copy any buffered data. if wc, ok := src.(*Conn); ok && len(wc.Peeked) > 0 { if _, err := dst.Write(wc.Peeked); err != nil { diff --git a/vendor/modules.txt b/vendor/modules.txt index 7db5c4720..aca28bb65 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -41,9 +41,6 @@ github.com/google/go-cmp/cmp/internal/value ## explicit; go 1.12 github.com/google/gopacket github.com/google/gopacket/layers -# github.com/inetaf/tcpproxy v0.0.0-20240214030015-3ce58045626c -## explicit; go 1.16 -github.com/inetaf/tcpproxy # github.com/insomniacslk/dhcp v0.0.0-20240710054256-ddd8a41251c9 ## explicit; go 1.20 github.com/insomniacslk/dhcp/dhcpv4 @@ -272,3 +269,7 @@ gvisor.dev/gvisor/pkg/tcpip/transport/tcp gvisor.dev/gvisor/pkg/tcpip/transport/tcpconntrack gvisor.dev/gvisor/pkg/tcpip/transport/udp gvisor.dev/gvisor/pkg/waiter +# inet.af/tcpproxy v0.0.0-20220326234310-be3ee21c9fa0 => github.com/inetaf/tcpproxy v0.0.0-20221017015627-91f861402626 +## explicit; go 1.16 +inet.af/tcpproxy +# inet.af/tcpproxy => github.com/inetaf/tcpproxy v0.0.0-20221017015627-91f861402626