Quellcode durchsuchen

remove ftpserverlib fork

the correct flow is to add features to the upstream library first

Signed-off-by: Nicola Murino <[email protected]>
Nicola Murino vor 4 Wochen
Ursprung
Commit
952df50a98

+ 0 - 1
go.mod

@@ -192,7 +192,6 @@ require (
 )
 
 replace (
-	github.com/fclairamb/ftpserverlib => github.com/drakkan/ftpserverlib v0.0.0-20250204143431-e069fad14727
 	github.com/jlaffaye/ftp => github.com/drakkan/ftp v0.0.0-20240430173938-7ba8270c8e7f
 	github.com/robfig/cron/v3 => github.com/drakkan/cron/v3 v3.0.0-20230222140221-217a1e4d96c0
 )

+ 2 - 2
go.sum

@@ -130,8 +130,6 @@ github.com/drakkan/cron/v3 v3.0.0-20230222140221-217a1e4d96c0 h1:EW9gIJRmt9lzk66
 github.com/drakkan/cron/v3 v3.0.0-20230222140221-217a1e4d96c0/go.mod h1:eQICP3HwyT7UooqI/z+Ov+PtYAWygg1TEWWzGIFLtro=
 github.com/drakkan/ftp v0.0.0-20240430173938-7ba8270c8e7f h1:S9JUlrOzjK58UKoLqqb40YLyVlt0bcIFtYrvnanV3zc=
 github.com/drakkan/ftp v0.0.0-20240430173938-7ba8270c8e7f/go.mod h1:4p8lUl4vQ80L598CygL+3IFtm+3nggvvW/palOlViwE=
-github.com/drakkan/ftpserverlib v0.0.0-20250204143431-e069fad14727 h1:OwxAvQejxuEYFtuXcOxuepEjt6VPLEQ3zK+5k9p4M60=
-github.com/drakkan/ftpserverlib v0.0.0-20250204143431-e069fad14727/go.mod h1:TRdVBbJEt+KihZKGl6jgSp6H/yPc0NxMUAlMZuNHcmY=
 github.com/drakkan/webdav v0.0.0-20241026165615-b8b8f74ae71b h1:Y1tLiQ8fnxM5f3wiBjAXsHzHNwiY9BR+mXZA75nZwrs=
 github.com/drakkan/webdav v0.0.0-20241026165615-b8b8f74ae71b/go.mod h1:zOVb1QDhwwqWn2L2qZ0U3swMSO4GTSNyIwXCGO/UGWE=
 github.com/eikenb/pipeat v0.0.0-20210730190139-06b3e6902001 h1:/ZshrfQzayqRSBDodmp3rhNCHJCff+utvgBuWRbiqu4=
@@ -147,6 +145,8 @@ github.com/envoyproxy/protoc-gen-validate v1.2.1/go.mod h1:d/C80l/jxXLdfEIhX1W2T
 github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk=
 github.com/fatih/color v1.18.0 h1:S8gINlzdQ840/4pfAwic/ZE0djQEH3wM94VfqLTZcOM=
 github.com/fatih/color v1.18.0/go.mod h1:4FelSpRwEGDpQ12mAdzqdOukCy4u8WUtOY6lkT/6HfU=
+github.com/fclairamb/ftpserverlib v0.26.0 h1:86K7qms8rrguRYQ4hxVMVl5HBSymUlqu+enjdk9Ug5A=
+github.com/fclairamb/ftpserverlib v0.26.0/go.mod h1:XMm3NdvCvmBtoAVK86oERDVmoYo0GTNS5gdds4f9lpM=
 github.com/fclairamb/go-log v0.6.0 h1:1V7BJ75P2PvanLHRyGBBFjncB6d4AgEmu+BPWKbMkaU=
 github.com/fclairamb/go-log v0.6.0/go.mod h1:cyXxOw4aJwO6lrZb8GRELSw+sxO6wwkLJdsjY5xYCWA=
 github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg=

+ 0 - 12
internal/config/config.go

@@ -1201,12 +1201,6 @@ func getFTPDBindingSecurityFromEnv(idx int, binding *ftpd.Binding) bool {
 		isSet = true
 	}
 
-	tlsSessionReuse, ok := lookupIntFromEnv(fmt.Sprintf("SFTPGO_FTPD__BINDINGS__%v__TLS_SESSION_REUSE", idx), 32)
-	if ok {
-		binding.TLSSessionReuse = int(tlsSessionReuse)
-		isSet = true
-	}
-
 	tlsVer, ok := lookupIntFromEnv(fmt.Sprintf("SFTPGO_FTPD__BINDINGS__%v__MIN_TLS_VERSION", idx), 32)
 	if ok {
 		binding.MinTLSVersion = int(tlsVer)
@@ -1237,12 +1231,6 @@ func getFTPDBindingSecurityFromEnv(idx int, binding *ftpd.Binding) bool {
 		isSet = true
 	}
 
-	ignoreASCIITransferType, ok := lookupIntFromEnv(fmt.Sprintf("SFTPGO_FTPD__BINDINGS__%d__IGNORE_ASCII_TRANSFER_TYPE", idx), 32)
-	if ok {
-		binding.IgnoreASCIITransferType = int(ignoreASCIITransferType)
-		isSet = true
-	}
-
 	return isSet
 }
 

+ 0 - 6
internal/config/config_test.go

@@ -942,7 +942,6 @@ func TestFTPDBindingsFromEnv(t *testing.T) {
 	os.Setenv("SFTPGO_FTPD__BINDINGS__0__PORT", "2200")
 	os.Setenv("SFTPGO_FTPD__BINDINGS__0__APPLY_PROXY_CONFIG", "f")
 	os.Setenv("SFTPGO_FTPD__BINDINGS__0__TLS_MODE", "2")
-	os.Setenv("SFTPGO_FTPD__BINDINGS__0__TLS_SESSION_REUSE", "1")
 	os.Setenv("SFTPGO_FTPD__BINDINGS__0__FORCE_PASSIVE_IP", "127.0.1.2")
 	os.Setenv("SFTPGO_FTPD__BINDINGS__0__PASSIVE_IP_OVERRIDES__0__IP", "172.16.1.1")
 	os.Setenv("SFTPGO_FTPD__BINDINGS__0__PASSIVE_HOST", "127.0.1.3")
@@ -967,7 +966,6 @@ func TestFTPDBindingsFromEnv(t *testing.T) {
 		os.Unsetenv("SFTPGO_FTPD__BINDINGS__0__PORT")
 		os.Unsetenv("SFTPGO_FTPD__BINDINGS__0__APPLY_PROXY_CONFIG")
 		os.Unsetenv("SFTPGO_FTPD__BINDINGS__0__TLS_MODE")
-		os.Unsetenv("SFTPGO_FTPD__BINDINGS__0__TLS_SESSION_REUSE")
 		os.Unsetenv("SFTPGO_FTPD__BINDINGS__0__FORCE_PASSIVE_IP")
 		os.Unsetenv("SFTPGO_FTPD__BINDINGS__0__PASSIVE_IP_OVERRIDES__0__IP")
 		os.Unsetenv("SFTPGO_FTPD__BINDINGS__0__PASSIVE_HOST")
@@ -996,7 +994,6 @@ func TestFTPDBindingsFromEnv(t *testing.T) {
 	require.Equal(t, "127.0.0.1", bindings[0].Address)
 	require.False(t, bindings[0].ApplyProxyConfig)
 	require.Equal(t, 2, bindings[0].TLSMode)
-	require.Equal(t, 1, bindings[0].TLSSessionReuse)
 	require.Equal(t, 12, bindings[0].MinTLSVersion)
 	require.Equal(t, "127.0.1.2", bindings[0].ForcePassiveIP)
 	require.Len(t, bindings[0].PassiveIPOverrides, 0)
@@ -1008,12 +1005,10 @@ func TestFTPDBindingsFromEnv(t *testing.T) {
 	require.False(t, bindings[0].Debug)
 	require.Equal(t, 1, bindings[0].PassiveConnectionsSecurity)
 	require.Equal(t, 0, bindings[0].ActiveConnectionsSecurity)
-	require.Equal(t, 0, bindings[0].IgnoreASCIITransferType)
 	require.Equal(t, 2203, bindings[1].Port)
 	require.Equal(t, "127.0.1.1", bindings[1].Address)
 	require.True(t, bindings[1].ApplyProxyConfig) // default value
 	require.Equal(t, 1, bindings[1].TLSMode)
-	require.Equal(t, 0, bindings[1].TLSSessionReuse)
 	require.Equal(t, 13, bindings[1].MinTLSVersion)
 	require.Equal(t, "127.0.1.1", bindings[1].ForcePassiveIP)
 	require.Empty(t, bindings[1].PassiveHost)
@@ -1026,7 +1021,6 @@ func TestFTPDBindingsFromEnv(t *testing.T) {
 	require.Nil(t, bindings[1].TLSCipherSuites)
 	require.Equal(t, 0, bindings[1].PassiveConnectionsSecurity)
 	require.Equal(t, 1, bindings[1].ActiveConnectionsSecurity)
-	require.Equal(t, 1, bindings[1].IgnoreASCIITransferType)
 	require.True(t, bindings[1].Debug)
 	require.Equal(t, "cert.crt", bindings[1].CertificateFile)
 	require.Equal(t, "cert.key", bindings[1].CertificateKeyFile)

+ 0 - 11
internal/ftpd/ftpd.go

@@ -66,8 +66,6 @@ type Binding struct {
 	// Set to 1 to require TLS for both data and control connection.
 	// Set to 2 to enable implicit TLS
 	TLSMode int `json:"tls_mode" mapstructure:"tls_mode"`
-	// 0 disabled, 1 required
-	TLSSessionReuse int `json:"tls_session_reuse" mapstructure:"tls_session_reuse"`
 	// Certificate and matching private key for this specific binding, if empty the global
 	// ones will be used, if any
 	CertificateFile    string `json:"certificate_file" mapstructure:"certificate_file"`
@@ -109,11 +107,6 @@ type Binding struct {
 	// Please note that disabling the security checks you will make the FTP service vulnerable to bounce attacks
 	// on active data connections, so change the default value only if you are on a trusted/internal network
 	ActiveConnectionsSecurity int `json:"active_connections_security" mapstructure:"active_connections_security"`
-	// Set to 1 to silently ignore any client requests to perform ASCII translations via the TYPE command.
-	// That is, FTP clients can request ASCII translations, and SFTPGo will respond as the client expects,
-	// but will not actually perform the translation for either uploads or downloads. This behavior can be
-	// useful in circumstances involving older/mainframe clients and EBCDIC files.
-	IgnoreASCIITransferType int `json:"ignore_ascii_transfer_type" mapstructure:"ignore_ascii_transfer_type"`
 	// Debug enables the FTP debug mode. In debug mode, every FTP command will be logged
 	Debug   bool `json:"debug" mapstructure:"debug"`
 	ciphers []uint16
@@ -141,10 +134,6 @@ func (b *Binding) isTLSModeValid() bool {
 	return b.TLSMode >= 0 && b.TLSMode <= 2
 }
 
-func (b *Binding) isTLSSessionReuseValid() bool {
-	return b.TLSSessionReuse >= 0 && b.TLSSessionReuse <= 1
-}
-
 func (b *Binding) checkSecuritySettings() error {
 	if b.PassiveConnectionsSecurity < 0 || b.PassiveConnectionsSecurity > 1 {
 		return fmt.Errorf("invalid passive_connections_security: %v", b.PassiveConnectionsSecurity)

+ 4 - 70
internal/ftpd/ftpd_test.go

@@ -443,7 +443,6 @@ func TestMain(m *testing.M) { //nolint:gocyclo
 			CertificateFile:    certPath,
 			CertificateKeyFile: keyPath,
 			TLSMode:            1,
-			TLSSessionReuse:    1,
 			ClientAuthType:     2,
 		},
 	}
@@ -530,16 +529,6 @@ func TestInitializationFailure(t *testing.T) {
 	require.Error(t, err)
 	require.Contains(t, err.Error(), "the provided passive IP \"127001\" is not valid")
 	ftpdConf.Bindings[1].ForcePassiveIP = ""
-	ftpdConf.Bindings[1].TLSMode = 2
-	ftpdConf.Bindings[1].TLSSessionReuse = 1
-	err = ftpdConf.Initialize(configDir)
-	require.Error(t, err, "TLS session resumption should not be supported with implicit FTPS")
-	ftpdConf.Bindings[1].TLSMode = 0
-	ftpdConf.Bindings[1].TLSSessionReuse = 100
-	err = ftpdConf.Initialize(configDir)
-	require.Error(t, err)
-	assert.Contains(t, err.Error(), "unsupported TLS reuse mode")
-	ftpdConf.Bindings[1].TLSSessionReuse = 0
 	err = ftpdConf.Initialize(configDir)
 	require.Error(t, err)
 
@@ -3165,12 +3154,12 @@ func TestMODEType(t *testing.T) {
 	if assert.NoError(t, err) {
 		code, response, err := client.SendCommand("MODE s")
 		assert.NoError(t, err)
-		assert.Equal(t, ftp.StatusCommandOK, code)
-		assert.Equal(t, "Transfer mode set to 'S'", response)
+		assert.Equal(t, ftp.StatusNotImplementedParameter, code)
+		assert.Equal(t, "Unsupported mode", response)
 		code, response, err = client.SendCommand("MODE S")
 		assert.NoError(t, err)
 		assert.Equal(t, ftp.StatusCommandOK, code)
-		assert.Equal(t, "Transfer mode set to 'S'", response)
+		assert.Equal(t, "Using stream mode", response)
 
 		code, _, err = client.SendCommand("MODE Z")
 		assert.NoError(t, err)
@@ -3178,7 +3167,7 @@ func TestMODEType(t *testing.T) {
 
 		code, _, err = client.SendCommand("MODE SS")
 		assert.NoError(t, err)
-		assert.Equal(t, ftp.StatusBadArguments, code)
+		assert.Equal(t, ftp.StatusNotImplementedParameter, code)
 
 		err = client.Quit()
 		assert.NoError(t, err)
@@ -3538,61 +3527,6 @@ func TestCombine(t *testing.T) {
 	assert.NoError(t, err)
 }
 
-func TestTLSSessionReuse(t *testing.T) {
-	u := getTestUser()
-	user, _, err := httpdtest.AddUser(u, http.StatusCreated)
-	assert.NoError(t, err)
-
-	client, err := getFTPClientWithSessionReuse(user, nil)
-	if assert.NoError(t, err) {
-		err = checkBasicFTP(client)
-		assert.NoError(t, err)
-
-		testFilePath := filepath.Join(homeBasePath, testFileName)
-		testFileSize := int64(65535)
-		err = createTestFile(testFilePath, testFileSize)
-		assert.NoError(t, err)
-
-		err = ftpUploadFile(testFilePath, testFileName, testFileSize, client, 0)
-		assert.NoError(t, err)
-
-		localDownloadPath := filepath.Join(homeBasePath, testDLFileName)
-		err = ftpDownloadFile(testFileName, localDownloadPath, testFileSize, client, 0)
-		assert.NoError(t, err)
-
-		entries, err := client.List("/")
-		assert.NoError(t, err)
-		assert.Len(t, entries, 1)
-
-		err = client.Quit()
-		assert.NoError(t, err)
-		err = os.Remove(testFilePath)
-		assert.NoError(t, err)
-		err = os.Remove(localDownloadPath)
-		assert.NoError(t, err)
-	}
-
-	// this TLS config does not support session resumption
-	tlsConfig := &tls.Config{
-		ServerName:         "localhost",
-		InsecureSkipVerify: true, // use this for tests only
-		MinVersion:         tls.VersionTLS12,
-	}
-	client, err = getFTPClientWithSessionReuse(user, tlsConfig)
-	if assert.NoError(t, err) {
-		err = checkBasicFTP(client)
-		assert.Error(t, err)
-
-		err = client.Quit()
-		assert.NoError(t, err)
-	}
-
-	_, err = httpdtest.RemoveUser(user, http.StatusOK)
-	assert.NoError(t, err)
-	err = os.RemoveAll(user.GetHomeDir())
-	assert.NoError(t, err)
-}
-
 func TestClientCertificateAuthRevokedCert(t *testing.T) {
 	u := getTestUser()
 	u.Username = tlsClient2Username

+ 30 - 4
internal/ftpd/handler.go

@@ -291,7 +291,7 @@ func (c *Connection) Symlink(oldname, newname string) error {
 }
 
 // ReadDir implements ClientDriverExtensionFilelist
-func (c *Connection) ReadDir(name string) (ftpserver.DirLister, error) {
+func (c *Connection) ReadDir(name string) ([]os.FileInfo, error) {
 	c.UpdateLastActivity()
 
 	if c.doWildcardListDir {
@@ -306,16 +306,21 @@ func (c *Connection) ReadDir(name string) (ftpserver.DirLister, error) {
 		if err != nil {
 			return nil, err
 		}
-		return &patternDirLister{
+		patternLister := &patternDirLister{
 			DirLister:      lister,
 			pattern:        baseName,
 			lastCommand:    c.clientContext.GetLastCommand(),
 			dirName:        name,
 			connectionPath: c.clientContext.Path(),
-		}, nil
+		}
+		return consumeDirLister(patternLister)
 	}
 
-	return c.ListDir(name)
+	lister, err := c.ListDir(name)
+	if err != nil {
+		return nil, err
+	}
+	return consumeDirLister(lister)
 }
 
 // GetHandle implements ClientDriverExtentionFileTransfer
@@ -583,3 +588,24 @@ func (l *patternDirLister) Next(limit int) ([]os.FileInfo, error) {
 		}
 	}
 }
+
+func consumeDirLister(lister vfs.DirLister) ([]os.FileInfo, error) {
+	defer lister.Close()
+
+	var results []os.FileInfo
+
+	for {
+		files, err := lister.Next(vfs.ListerBatchSize)
+		finished := errors.Is(err, io.EOF)
+		results = append(results, files...)
+		if err != nil && !finished {
+			return results, err
+		}
+		if finished {
+			lister.Close()
+			break
+		}
+	}
+
+	return results, nil
+}

+ 2 - 10
internal/ftpd/server.go

@@ -110,11 +110,7 @@ func (s *Server) GetSettings() (*ftpserver.Settings, error) {
 		return nil, fmt.Errorf("unsupported TLS mode: %d", s.binding.TLSMode)
 	}
 
-	if !s.binding.isTLSSessionReuseValid() {
-		return nil, fmt.Errorf("unsupported TLS reuse mode %d", s.binding.TLSSessionReuse)
-	}
-
-	if (s.binding.TLSMode > 0 || s.binding.TLSSessionReuse > 0) && certMgr == nil {
+	if s.binding.TLSMode > 0 && certMgr == nil {
 		return nil, errors.New("to enable TLS you need to provide a certificate")
 	}
 
@@ -128,13 +124,11 @@ func (s *Server) GetSettings() (*ftpserver.Settings, error) {
 		ConnectionTimeout:        20,
 		Banner:                   s.statusBanner,
 		TLSRequired:              ftpserver.TLSRequirement(s.binding.TLSMode),
-		TLSSessionReuse:          ftpserver.TLSSessionReuse(s.binding.TLSSessionReuse),
 		DisableSite:              !s.config.EnableSite,
 		DisableActiveMode:        s.config.DisableActiveMode,
 		EnableHASH:               s.config.HASHSupport > 0,
 		EnableCOMB:               s.config.CombineSupport > 0,
 		DefaultTransferType:      ftpserver.TransferTypeBinary,
-		IgnoreASCIITranferType:   s.binding.IgnoreASCIITransferType == 1,
 		ActiveConnectionsCheck:   ftpserver.DataConnectionRequirement(s.binding.ActiveConnectionsSecurity),
 		PasvConnectionsCheck:     ftpserver.DataConnectionRequirement(s.binding.PassiveConnectionsSecurity),
 	}, nil
@@ -292,9 +286,7 @@ func (s *Server) buildTLSConfig() {
 			s.binding.GetAddress(), s.binding.ciphers, certID)
 		if s.binding.isMutualTLSEnabled() {
 			s.tlsConfig.ClientCAs = certMgr.GetRootCAs()
-			if s.binding.TLSSessionReuse != int(ftpserver.TLSSessionReuseRequired) {
-				s.tlsConfig.VerifyConnection = s.verifyTLSConnection
-			}
+			s.tlsConfig.VerifyConnection = s.verifyTLSConnection
 			switch s.binding.ClientAuthType {
 			case 1:
 				s.tlsConfig.ClientAuth = tls.RequireAndVerifyClientCert