diff --git a/README.md b/README.md index 0088820f..ae4a96c1 100644 --- a/README.md +++ b/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 diff --git a/driver.go b/driver.go index bf93ca24..51fad40a 100644 --- a/driver.go +++ b/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 } diff --git a/transfer_active.go b/transfer_active.go index dd28516a..1beb3b6b 100644 --- a/transfer_active.go +++ b/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()) diff --git a/transfer_active_test.go b/transfer_active_test.go index 7e130cce..cb338a34 100644 --- a/transfer_active_test.go +++ b/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 +}