diff --git a/client_handler.go b/client_handler.go index 1c0c3d99..98d93650 100644 --- a/client_handler.go +++ b/client_handler.go @@ -3,6 +3,7 @@ package ftpserver import ( "bufio" "compress/flate" + "crypto/tls" "errors" "fmt" "io" @@ -62,6 +63,8 @@ var ( errNoTransferConnection = errors.New("unable to open transfer: no transfer connection") errTLSRequired = errors.New("unable to open transfer: TLS is required") errInvalidTLSRequirement = errors.New("invalid TLS requirement") + errProtMismatch = errors.New("unable to open transfer: the data connection does not " + + "match the requested protection level, issue PASV or PORT again") ) func getHashMapping() map[string]HASHAlgo { @@ -736,6 +739,19 @@ func (c *clientHandler) TransferOpen(info string) (io.ReadWriter, error) { return nil, err } + // PASV/PORT prepare the data connection with the protection in force at that time, so a PROT + // received afterwards can leave it different from the one the client asked for. + if _, isTLS := conn.(*tls.Conn); isTLS != c.HasTLSForTransfers() { + c.logger.Warn( + "Data connection protection does not match the PROT command", + "encrypted", isTLS, + "requested", c.HasTLSForTransfers()) + + c.writeMessage(StatusCannotOpenDataConnection, errProtMismatch.Error()) + + return nil, errProtMismatch + } + var transferStream io.ReadWriter = conn if c.transferMode == TransferModeDeflate { diff --git a/handle_dirs_test.go b/handle_dirs_test.go index eb0d4d5f..5c223ba8 100644 --- a/handle_dirs_test.go +++ b/handle_dirs_test.go @@ -458,6 +458,81 @@ func TestPerClientTLSTransfer(t *testing.T) { require.Equal(t, "unable to open transfer: TLS is required", response) } +func TestTLSTransferPROTAfterDataCommand(t *testing.T) { + for _, active := range []bool{false, true} { + t.Run(fmt.Sprintf("active=%t", active), func(t *testing.T) { + req := require.New(t) + server := NewTestServerWithTestDriver(t, &TestServerDriver{ + TLS: true, + Settings: &Settings{ + TLSRequired: MandatoryEncryption, + ActiveTransferPortNon20: true, + }, + }) + + conf := goftp.Config{ + User: authUser, + Password: authPass, + TLSConfig: &tls.Config{ + InsecureSkipVerify: true, //nolint:gosec + }, + TLSMode: goftp.TLSExplicit, + } + + client, err := goftp.DialConfig(conf, server.Addr()) + req.NoError(err, "Couldn't connect") + + defer func() { panicOnError(client.Close()) }() + + raw, err := client.OpenRawConn() + req.NoError(err, "Couldn't open raw connection") + + defer func() { require.NoError(t, raw.Close()) }() + + returnCode, response, err := raw.SendCommand("PROT C") + req.NoError(err) + req.Equal(StatusOK, returnCode, response) + + // the data connection is prepared while PROT C is in force + if active { + var dataListener net.Listener + + dataListener, err = (&net.ListenConfig{}).Listen(t.Context(), "tcp", "127.0.0.1:0") + req.NoError(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) + req.NoError(err) + req.Equal(StatusOK, returnCode, response) + } else { + returnCode, response, err = raw.SendCommand("PASV") + req.NoError(err) + req.Equal(StatusEnteringPASV, returnCode, response) + + var dataConn net.Conn + + dialer := &net.Dialer{Timeout: 5 * time.Second} + dataConn, err = dialer.DialContext(t.Context(), "tcp", + fmt.Sprintf("127.0.0.1:%d", getPortFromPASVResponse(t, response))) + req.NoError(err) + + defer func() { require.NoError(t, dataConn.Close()) }() + } + + returnCode, response, err = raw.SendCommand("PROT P") + req.NoError(err) + req.Equal(StatusOK, returnCode, response) + + returnCode, response, err = raw.SendCommand("MLSD /") + req.NoError(err) + req.Equal(StatusCannotOpenDataConnection, returnCode, response) + req.Equal(errProtMismatch.Error(), response) + }) + } +} + func TestDirListingBeforeLogin(t *testing.T) { s := NewTestServer(t, false) dialer := &net.Dialer{Timeout: 5 * time.Second}