Active transfer local IP resolver

README.md

@@ -128,6 +128,7 @@ type Settings struct {
 	PassiveTransferPortRange PasvPortGetter   // (Optional) Port Range for data connections. Random if not specified
 	PassiveTransferPortMultiplexing bool      // Allow different client IPs to share passive listener ports
 	ActiveTransferPortNon20  bool             // Do not impose the port 20 for active data transfer (#88, RFC 1579)
+	ActiveTransferLocalIPResolver ActiveTransferLocalIPResolver // (Optional) Local IP to dial active data connections from
 	IdleTimeout              int              // Maximum inactivity time before disconnecting (#58)
 	ConnectionTimeout        int              // Maximum time to establish passive or active transfer connections
 	DisableMLSD              bool             // Disable MLSD support

driver.go

@@ -315,6 +315,11 @@ func (r PortMappingRange) NumberAttempts() int {
 // to use in the response to the PASV command, or an error if a public IP cannot be determined.
 type PublicIPResolver func(ClientContext) (string, error)
 
+// ActiveTransferLocalIPResolver takes the ClientContext of a control connection and returns the
+// local IP address the server dials from when it opens an active mode (PORT or EPRT) data
+// connection to that client. A nil result leaves the choice to the operating system.
+type ActiveTransferLocalIPResolver func(ClientContext) net.IP
+
 // TLSRequirement is the enumerable that represents the supported TLS mode
 type TLSRequirement int8
 
@@ -374,4 +379,10 @@ type Settings struct {
 	ActiveConnectionsCheck DataConnectionRequirement
 	// PasvConnectionsCheck defines the security requirements for passive connections
 	PasvConnectionsCheck DataConnectionRequirement
+	// ActiveTransferLocalIPResolver (Optional) selects the local IP address active mode (PORT, EPRT)
+	// data connections are dialed from. Without it the operating system picks the source address
+	// from its routing table, which on a multi homed host is not necessarily the address the client
+	// connected to, and clients and firewalls generally expect the data connection to come from
+	// that address. Only the address is taken from it, the port still follows ActiveTransferPortNon20.
+	ActiveTransferLocalIPResolver ActiveTransferLocalIPResolver
 }

transfer_active.go

@@ -56,10 +56,17 @@ func (c *clientHandler) handlePORT(param string) error {
 		}
 	}
 
+	var laddr net.IP
+
+	if resolver := c.server.settings.ActiveTransferLocalIPResolver; resolver != nil {
+		laddr = resolver(c)
+	}
+
 	c.transferMu.Lock()
 
 	c.transfer = &activeTransferHandler{
 		raddr:     raddr,
+		laddr:     laddr,
 		settings:  c.server.settings,
 		tlsConfig: tlsConfig,
 	}
@@ -78,6 +85,7 @@ var _ transferHandler = (*activeTransferHandler)(nil)
 // Active connection
 type activeTransferHandler struct {
 	raddr     *net.TCPAddr // Remote address of the client
+	laddr     net.IP       // Local address to dial from, nil lets the operating system choose
 	conn      net.Conn     // Connection used to connect to him
 	settings  *Settings    // Settings
 	tlsConfig *tls.Config  // not nil if the active connection requires TLS
@@ -97,8 +105,11 @@ func (a *activeTransferHandler) Open() (net.Conn, error) {
 	dialer := &net.Dialer{Timeout: timeout}
 
 	if !a.settings.ActiveTransferPortNon20 {
-		dialer.LocalAddr, _ = net.ResolveTCPAddr("tcp", ":20")
+		// Several transfers may dial from port 20 at once, so the socket must be reusable
+		dialer.LocalAddr = &net.TCPAddr{IP: a.laddr, Port: 20}
 		dialer.Control = Control
+	} else if a.laddr != nil {
+		dialer.LocalAddr = &net.TCPAddr{IP: a.laddr}
 	}
 
 	conn, err := dialer.Dial("tcp", a.raddr.String())

transfer_active_test.go

@@ -2,6 +2,7 @@
 package ftpserver
 
 import (
+	"io"
 	"net"
 	"regexp"
 	"testing"
@@ -59,3 +60,82 @@ func TestActiveTransferFromPort20(t *testing.T) {
 	_, err = client.ReadDir("/")
 	require.NoError(t, err)
 }
+
+func TestActiveTransferLocalIPResolver(t *testing.T) {
+	// The address the server is asked to dial from has to exist on this host. Every 127/8 address
+	// does on Linux, not everywhere, so the test steps aside where it cannot be bound.
+	const sourceIP = "127.0.0.2"
+
+	listenConfig := &net.ListenConfig{}
+
+	probe, err := listenConfig.Listen(t.Context(), "tcp", net.JoinHostPort(sourceIP, "0"))
+	if err != nil {
+		t.Skipf("Binding on %s is not supported here: %v", sourceIP, err)
+	}
+
+	require.NoError(t, probe.Close())
+
+	resolvedFor := make(chan string, 1)
+	server := NewTestServerWithTestDriver(t, &TestServerDriver{
+		Settings: &Settings{
+			ActiveTransferPortNon20: true,
+			ActiveTransferLocalIPResolver: func(cc ClientContext) net.IP {
+				resolvedFor <- cc.LocalAddr().String()
+
+				return net.ParseIP(sourceIP)
+			},
+		},
+	})
+
+	client, err := goftp.DialConfig(goftp.Config{User: authUser, Password: authPass}, server.Addr())
+	require.NoError(t, err, "Couldn't connect")
+
+	defer func() { panicOnError(client.Close()) }()
+
+	raw, err := client.OpenRawConn()
+	require.NoError(t, err, "Couldn't open raw connection")
+
+	defer func() { require.NoError(t, raw.Close()) }()
+
+	// We play the client side of the data connection ourselves, to see which address dials in
+	dataListener, err := listenConfig.Listen(t.Context(), "tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+
+	defer func() { require.NoError(t, dataListener.Close()) }()
+
+	dataAddr := requireTCPAddr(t, dataListener.Addr())
+
+	returnCode, response, err := raw.SendCommand("PORT 127,0,0,1,%d,%d", dataAddr.Port/256, dataAddr.Port%256)
+	require.NoError(t, err)
+	require.Equal(t, StatusOK, returnCode, response)
+
+	returnCode, response, err = raw.SendCommand("LIST")
+	require.NoError(t, err)
+	require.Equal(t, StatusFileStatusOK, returnCode, response)
+
+	dataConn, err := dataListener.Accept()
+	require.NoError(t, err)
+
+	peer := requireTCPAddr(t, dataConn.RemoteAddr())
+	require.Equal(t, sourceIP, peer.IP.String(), "the data connection must come from the resolved address")
+
+	_, err = io.Copy(io.Discard, dataConn)
+	require.NoError(t, err)
+	require.NoError(t, dataConn.Close())
+
+	returnCode, response, err = raw.ReadResponse()
+	require.NoError(t, err)
+	require.Equal(t, StatusClosingDataConn, returnCode, response)
+
+	// The resolver saw the control connection, whose local address is the server's listening address
+	require.Equal(t, server.Addr(), <-resolvedFor)
+}
+
+func requireTCPAddr(t *testing.T, addr net.Addr) *net.TCPAddr {
+	t.Helper()
+
+	tcpAddr, ok := addr.(*net.TCPAddr)
+	require.True(t, ok, "expected a TCP address, got %T", addr)
+
+	return tcpAddr
+}