[GH-ISSUE #44] TCP proxy connection remains open (CLOSED_WAIT) #22

Closed
opened 2026-05-05 10:57:19 -06:00 by gitea-mirror · 6 comments
Owner

Originally created by @goriccardo on GitHub (Nov 14, 2017).
Original GitHub issue: https://github.com/mmatczuk/go-http-tunnel/issues/44

In a client TCP Proxy, when a local connection is closed the function call transfer(local, r, ...) does not return, effectively preventing the local connection to be properly closed.

To reproduce:

  1. Create a TCP tunnel
  2. Connect to the tunnel
  3. Close the tunnel connection
  4. invoke:
lsof -i -a -p `pidof tunnel`

At least one connection marked CLOSED_WAIT is present.

A quick and dirty solution would be to close the response reader after the local connection dies:

From 736802f154da59e23b4f1d89baafa49a1f579815 Mon Sep 17 00:00:00 2001
From: Riccardo Gori <goriccardo@gmail.com>
Date: Tue, 14 Nov 2017 16:07:31 +0100
Subject: [PATCH] Properly close proxy connection

---
 tcpproxy.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tcpproxy.go b/tcpproxy.go
index f7dc199..d72bb76 100644
--- a/tcpproxy.go
+++ b/tcpproxy.go
@@ -104,6 +104,7 @@ func (p *TCPProxy) Proxy(w io.Writer, r io.ReadCloser, msg *proto.ControlMessage
 			"dst", msg.ForwardedBy,
 			"src", target,
 		))
+		r.Close()
 		close(done)
 	}()
 
-- 
2.9.5

Originally created by @goriccardo on GitHub (Nov 14, 2017). Original GitHub issue: https://github.com/mmatczuk/go-http-tunnel/issues/44 In a client TCP Proxy, when a `local` connection is closed the function call `transfer(local, r, ...)` does not return, effectively preventing the `local` connection to be properly closed. To reproduce: 1. Create a TCP tunnel 2. Connect to the tunnel 3. Close the tunnel connection 4. invoke: ```shell lsof -i -a -p `pidof tunnel` ``` At least one connection marked CLOSED_WAIT is present. A quick and dirty solution would be to close the response reader after the local connection dies: ```patch From 736802f154da59e23b4f1d89baafa49a1f579815 Mon Sep 17 00:00:00 2001 From: Riccardo Gori <goriccardo@gmail.com> Date: Tue, 14 Nov 2017 16:07:31 +0100 Subject: [PATCH] Properly close proxy connection --- tcpproxy.go | 1 + 1 file changed, 1 insertion(+) diff --git a/tcpproxy.go b/tcpproxy.go index f7dc199..d72bb76 100644 --- a/tcpproxy.go +++ b/tcpproxy.go @@ -104,6 +104,7 @@ func (p *TCPProxy) Proxy(w io.Writer, r io.ReadCloser, msg *proto.ControlMessage "dst", msg.ForwardedBy, "src", target, )) + r.Close() close(done) }() -- 2.9.5 ```
Author
Owner

@mmatczuk commented on GitHub (Nov 14, 2017):

I think it may be related to this https://github.com/mmatczuk/go-http-tunnel/blob/master/utils.go#L37

<!-- gh-comment-id:344292710 --> @mmatczuk commented on GitHub (Nov 14, 2017): I think it may be related to this https://github.com/mmatczuk/go-http-tunnel/blob/master/utils.go#L37
Author
Owner

@goriccardo commented on GitHub (Nov 14, 2017):

The problem was more widespread than I initially thought as also the server is not closing the connections.

The fix I found is to make sure that when one transfer returns also its dual get closed. Here is my patch:

From ba111f2716296225011e031e2903e68b4a20ce06 Mon Sep 17 00:00:00 2001
From: Riccardo Gori <goriccardo@gmail.com>
Date: Tue, 14 Nov 2017 16:07:31 +0100
Subject: [PATCH] Properly close proxy connection

---
 server.go   | 12 +++++++-----
 tcpproxy.go |  4 ++++
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/server.go b/server.go
index 7bf2ca2..f15d4f9 100644
--- a/server.go
+++ b/server.go
@@ -520,6 +520,11 @@ func (s *Server) proxyConn(identifier id.ID, conn net.Conn, msg *proto.ControlMe
 		return err
 	}
 
+	resp, err := s.httpClient.Do(req)
+	if err != nil {
+		return fmt.Errorf("io error: %s", err)
+	}
+
 	done := make(chan struct{})
 	go func() {
 		transfer(pw, conn, log.NewContext(s.logger).With(
@@ -527,19 +532,16 @@ func (s *Server) proxyConn(identifier id.ID, conn net.Conn, msg *proto.ControlMe
 			"dst", identifier,
 			"src", conn.RemoteAddr(),
 		))
+		resp.Body.Close()
 		close(done)
 	}()
 
-	resp, err := s.httpClient.Do(req)
-	if err != nil {
-		return fmt.Errorf("io error: %s", err)
-	}
-
 	transfer(conn, resp.Body, log.NewContext(s.logger).With(
 		"dir", "client to user",
 		"dst", conn.RemoteAddr(),
 		"src", identifier,
 	))
+	conn.Close()
 
 	<-done
 
diff --git a/tcpproxy.go b/tcpproxy.go
index f7dc199..9897851 100644
--- a/tcpproxy.go
+++ b/tcpproxy.go
@@ -104,6 +104,8 @@ func (p *TCPProxy) Proxy(w io.Writer, r io.ReadCloser, msg *proto.ControlMessage
 			"dst", msg.ForwardedBy,
 			"src", target,
 		))
+		// Make sure the reader is closed after transfer returns
+		r.Close()
 		close(done)
 	}()
 
@@ -111,6 +113,8 @@ func (p *TCPProxy) Proxy(w io.Writer, r io.ReadCloser, msg *proto.ControlMessage
 		"dst", target,
 		"src", msg.ForwardedBy,
 	))
+	// Make sure local is closed after transfer returns
+	defer local.Close()
 
 	<-done
 }
-- 
2.9.5

<!-- gh-comment-id:344420135 --> @goriccardo commented on GitHub (Nov 14, 2017): The problem was more widespread than I initially thought as also the server is not closing the connections. The fix I found is to make sure that when one transfer returns also its dual get closed. Here is my patch: ```patch From ba111f2716296225011e031e2903e68b4a20ce06 Mon Sep 17 00:00:00 2001 From: Riccardo Gori <goriccardo@gmail.com> Date: Tue, 14 Nov 2017 16:07:31 +0100 Subject: [PATCH] Properly close proxy connection --- server.go | 12 +++++++----- tcpproxy.go | 4 ++++ 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/server.go b/server.go index 7bf2ca2..f15d4f9 100644 --- a/server.go +++ b/server.go @@ -520,6 +520,11 @@ func (s *Server) proxyConn(identifier id.ID, conn net.Conn, msg *proto.ControlMe return err } + resp, err := s.httpClient.Do(req) + if err != nil { + return fmt.Errorf("io error: %s", err) + } + done := make(chan struct{}) go func() { transfer(pw, conn, log.NewContext(s.logger).With( @@ -527,19 +532,16 @@ func (s *Server) proxyConn(identifier id.ID, conn net.Conn, msg *proto.ControlMe "dst", identifier, "src", conn.RemoteAddr(), )) + resp.Body.Close() close(done) }() - resp, err := s.httpClient.Do(req) - if err != nil { - return fmt.Errorf("io error: %s", err) - } - transfer(conn, resp.Body, log.NewContext(s.logger).With( "dir", "client to user", "dst", conn.RemoteAddr(), "src", identifier, )) + conn.Close() <-done diff --git a/tcpproxy.go b/tcpproxy.go index f7dc199..9897851 100644 --- a/tcpproxy.go +++ b/tcpproxy.go @@ -104,6 +104,8 @@ func (p *TCPProxy) Proxy(w io.Writer, r io.ReadCloser, msg *proto.ControlMessage "dst", msg.ForwardedBy, "src", target, )) + // Make sure the reader is closed after transfer returns + r.Close() close(done) }() @@ -111,6 +113,8 @@ func (p *TCPProxy) Proxy(w io.Writer, r io.ReadCloser, msg *proto.ControlMessage "dst", target, "src", msg.ForwardedBy, )) + // Make sure local is closed after transfer returns + defer local.Close() <-done } -- 2.9.5 ```
Author
Owner

@mmatczuk commented on GitHub (Nov 14, 2017):

@goriccardo PTAL at https://github.com/mmatczuk/go-http-tunnel/pull/45

<!-- gh-comment-id:344431430 --> @mmatczuk commented on GitHub (Nov 14, 2017): @goriccardo PTAL at https://github.com/mmatczuk/go-http-tunnel/pull/45
Author
Owner

@mmatczuk commented on GitHub (Nov 14, 2017):

BTW the second patch would block forever as while sending the request pr is empty.

<!-- gh-comment-id:344433695 --> @mmatczuk commented on GitHub (Nov 14, 2017): BTW the second patch would block forever as while sending the request pr is empty.
Author
Owner

@goriccardo commented on GitHub (Nov 15, 2017):

#45 works for me, thanks.

<!-- gh-comment-id:344515296 --> @goriccardo commented on GitHub (Nov 15, 2017): #45 works for me, thanks.
Author
Owner

@mmatczuk commented on GitHub (Nov 15, 2017):

Thanks!

<!-- gh-comment-id:344516212 --> @mmatczuk commented on GitHub (Nov 15, 2017): Thanks!
Sign in to join this conversation.
No labels
pull-request
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/go-http-tunnel#22
No description provided.