Browse Source

sftpfs: add an option to disable concurrent reads

Nicola Murino 4 years ago
parent
commit
055506e518

+ 63 - 58
cmd/portable.go

@@ -21,58 +21,59 @@ import (
 )
 
 var (
-	directoryToServe             string
-	portableSFTPDPort            int
-	portableAdvertiseService     bool
-	portableAdvertiseCredentials bool
-	portableUsername             string
-	portablePassword             string
-	portableLogFile              string
-	portableLogVerbose           bool
-	portablePublicKeys           []string
-	portablePermissions          []string
-	portableSSHCommands          []string
-	portableAllowedPatterns      []string
-	portableDeniedPatterns       []string
-	portableFsProvider           int
-	portableS3Bucket             string
-	portableS3Region             string
-	portableS3AccessKey          string
-	portableS3AccessSecret       string
-	portableS3Endpoint           string
-	portableS3StorageClass       string
-	portableS3KeyPrefix          string
-	portableS3ULPartSize         int
-	portableS3ULConcurrency      int
-	portableGCSBucket            string
-	portableGCSCredentialsFile   string
-	portableGCSAutoCredentials   int
-	portableGCSStorageClass      string
-	portableGCSKeyPrefix         string
-	portableFTPDPort             int
-	portableFTPSCert             string
-	portableFTPSKey              string
-	portableWebDAVPort           int
-	portableWebDAVCert           string
-	portableWebDAVKey            string
-	portableAzContainer          string
-	portableAzAccountName        string
-	portableAzAccountKey         string
-	portableAzEndpoint           string
-	portableAzAccessTier         string
-	portableAzSASURL             string
-	portableAzKeyPrefix          string
-	portableAzULPartSize         int
-	portableAzULConcurrency      int
-	portableAzUseEmulator        bool
-	portableCryptPassphrase      string
-	portableSFTPEndpoint         string
-	portableSFTPUsername         string
-	portableSFTPPassword         string
-	portableSFTPPrivateKeyPath   string
-	portableSFTPFingerprints     []string
-	portableSFTPPrefix           string
-	portableCmd                  = &cobra.Command{
+	directoryToServe                   string
+	portableSFTPDPort                  int
+	portableAdvertiseService           bool
+	portableAdvertiseCredentials       bool
+	portableUsername                   string
+	portablePassword                   string
+	portableLogFile                    string
+	portableLogVerbose                 bool
+	portablePublicKeys                 []string
+	portablePermissions                []string
+	portableSSHCommands                []string
+	portableAllowedPatterns            []string
+	portableDeniedPatterns             []string
+	portableFsProvider                 int
+	portableS3Bucket                   string
+	portableS3Region                   string
+	portableS3AccessKey                string
+	portableS3AccessSecret             string
+	portableS3Endpoint                 string
+	portableS3StorageClass             string
+	portableS3KeyPrefix                string
+	portableS3ULPartSize               int
+	portableS3ULConcurrency            int
+	portableGCSBucket                  string
+	portableGCSCredentialsFile         string
+	portableGCSAutoCredentials         int
+	portableGCSStorageClass            string
+	portableGCSKeyPrefix               string
+	portableFTPDPort                   int
+	portableFTPSCert                   string
+	portableFTPSKey                    string
+	portableWebDAVPort                 int
+	portableWebDAVCert                 string
+	portableWebDAVKey                  string
+	portableAzContainer                string
+	portableAzAccountName              string
+	portableAzAccountKey               string
+	portableAzEndpoint                 string
+	portableAzAccessTier               string
+	portableAzSASURL                   string
+	portableAzKeyPrefix                string
+	portableAzULPartSize               int
+	portableAzULConcurrency            int
+	portableAzUseEmulator              bool
+	portableCryptPassphrase            string
+	portableSFTPEndpoint               string
+	portableSFTPUsername               string
+	portableSFTPPassword               string
+	portableSFTPPrivateKeyPath         string
+	portableSFTPFingerprints           []string
+	portableSFTPPrefix                 string
+	portableSFTPDisableConcurrentReads bool
+	portableCmd                        = &cobra.Command{
 		Use:   "portable",
 		Short: "Serve a single directory",
 		Long: `To serve the current working directory with auto generated credentials simply
@@ -184,12 +185,13 @@ Please take a look at the usage below to customize the serving parameters`,
 							Passphrase: kms.NewPlainSecret(portableCryptPassphrase),
 						},
 						SFTPConfig: vfs.SFTPFsConfig{
-							Endpoint:     portableSFTPEndpoint,
-							Username:     portableSFTPUsername,
-							Password:     kms.NewPlainSecret(portableSFTPPassword),
-							PrivateKey:   kms.NewPlainSecret(portableSFTPPrivateKey),
-							Fingerprints: portableSFTPFingerprints,
-							Prefix:       portableSFTPPrefix,
+							Endpoint:                portableSFTPEndpoint,
+							Username:                portableSFTPUsername,
+							Password:                kms.NewPlainSecret(portableSFTPPassword),
+							PrivateKey:              kms.NewPlainSecret(portableSFTPPrivateKey),
+							Fingerprints:            portableSFTPFingerprints,
+							Prefix:                  portableSFTPPrefix,
+							DisableCouncurrentReads: portableSFTPDisableConcurrentReads,
 						},
 					},
 					Filters: dataprovider.UserFilters{
@@ -317,6 +319,9 @@ key for SFTP provider`)
 	portableCmd.Flags().StringVar(&portableSFTPPrefix, "sftp-prefix", "", `SFTP prefix allows restrict all
 operations to a given path within the
 remote SFTP server`)
+	portableCmd.Flags().BoolVar(&portableSFTPDisableConcurrentReads, "sftp-disable-concurrent-reads", false, `Concurrent reads are safe to use and
+disabling them will degrade performance.
+Disable for read once servers`)
 	rootCmd.AddCommand(portableCmd)
 }
 

+ 6 - 5
dataprovider/user.go

@@ -934,11 +934,12 @@ func (u *User) getACopy() User {
 			Passphrase: u.FsConfig.CryptConfig.Passphrase.Clone(),
 		},
 		SFTPConfig: vfs.SFTPFsConfig{
-			Endpoint:   u.FsConfig.SFTPConfig.Endpoint,
-			Username:   u.FsConfig.SFTPConfig.Username,
-			Password:   u.FsConfig.SFTPConfig.Password.Clone(),
-			PrivateKey: u.FsConfig.SFTPConfig.PrivateKey.Clone(),
-			Prefix:     u.FsConfig.SFTPConfig.Prefix,
+			Endpoint:                u.FsConfig.SFTPConfig.Endpoint,
+			Username:                u.FsConfig.SFTPConfig.Username,
+			Password:                u.FsConfig.SFTPConfig.Password.Clone(),
+			PrivateKey:              u.FsConfig.SFTPConfig.PrivateKey.Clone(),
+			Prefix:                  u.FsConfig.SFTPConfig.Prefix,
+			DisableCouncurrentReads: u.FsConfig.SFTPConfig.DisableCouncurrentReads,
 		},
 	}
 	if len(u.FsConfig.SFTPConfig.Fingerprints) > 0 {

+ 3 - 0
docs/portable-mode.md

@@ -91,6 +91,9 @@ Flags:
                                         parallel (default 2)
       --s3-upload-part-size int         The buffer size for multipart uploads
                                         (MB) (default 5)
+      --sftp-disable-concurrent-reads   Concurrent reads are safe to use and
+                                        disabling them will degrade performance.
+                                        Disable for read once servers
       --sftp-endpoint string            SFTP endpoint as host:port for SFTP
                                         provider
       --sftp-fingerprints strings       SFTP fingerprints to verify remote host

+ 1 - 1
go.mod

@@ -40,7 +40,7 @@ require (
 	github.com/otiai10/copy v1.5.0
 	github.com/pelletier/go-toml v1.8.1 // indirect
 	github.com/pires/go-proxyproto v0.5.0
-	github.com/pkg/sftp v1.12.1-0.20210222152308-b8102da57e75
+	github.com/pkg/sftp v1.12.1-0.20210306114423-5b7da38a9cdb
 	github.com/prometheus/client_golang v1.9.0
 	github.com/prometheus/common v0.18.0 // indirect
 	github.com/prometheus/procfs v0.6.0 // indirect

+ 2 - 2
go.sum

@@ -573,8 +573,8 @@ github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
 github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pkg/profile v1.2.1/go.mod h1:hJw3o1OdXxsrSjjVksARp5W95eeEaEfptyVZyv6JUPA=
 github.com/pkg/sftp v1.10.1/go.mod h1:lYOWFsE0bwd1+KfKJaKeuokY15vzFx25BLbzYYoAxZI=
-github.com/pkg/sftp v1.12.1-0.20210222152308-b8102da57e75 h1:M1mYcYovX5NYD+nZSSqligrmYg3lsiMBS0DycBpDjXY=
-github.com/pkg/sftp v1.12.1-0.20210222152308-b8102da57e75/go.mod h1:41g+FIPlQUTDCveupEmEA65IoiQFrtgCeDopC4ajGIM=
+github.com/pkg/sftp v1.12.1-0.20210306114423-5b7da38a9cdb h1:kMNGy8CB8a49gKT+Sb7+ReG15NzJBKviZ8Zds9f0aHM=
+github.com/pkg/sftp v1.12.1-0.20210306114423-5b7da38a9cdb/go.mod h1:41g+FIPlQUTDCveupEmEA65IoiQFrtgCeDopC4ajGIM=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/posener/complete v1.1.1/go.mod h1:em0nMJCgc9GFtwrmVmEMR/ZL6WyhyjMBndrE9hABlRI=

+ 7 - 0
httpd/httpd_test.go

@@ -1523,9 +1523,11 @@ func TestUserSFTPFs(t *testing.T) {
 	assert.Contains(t, string(resp), "invalid endpoint")
 
 	user.FsConfig.SFTPConfig.Endpoint = "127.0.0.1:2022"
+	user.FsConfig.SFTPConfig.DisableCouncurrentReads = true
 	user, _, err = httpdtest.UpdateUser(user, http.StatusOK, "")
 	assert.NoError(t, err)
 	assert.Equal(t, "/", user.FsConfig.SFTPConfig.Prefix)
+	assert.True(t, user.FsConfig.SFTPConfig.DisableCouncurrentReads)
 	initialPwdPayload := user.FsConfig.SFTPConfig.Password.GetPayload()
 	initialPkeyPayload := user.FsConfig.SFTPConfig.PrivateKey.GetPayload()
 	assert.Equal(t, kms.SecretStatusSecretBox, user.FsConfig.SFTPConfig.Password.GetStatus())
@@ -1542,6 +1544,7 @@ func TestUserSFTPFs(t *testing.T) {
 	user.FsConfig.SFTPConfig.PrivateKey.SetStatus(kms.SecretStatusSecretBox)
 	user.FsConfig.SFTPConfig.PrivateKey.SetAdditionalData("adata")
 	user.FsConfig.SFTPConfig.PrivateKey.SetKey("fake key")
+	user.FsConfig.SFTPConfig.DisableCouncurrentReads = false
 	user, bb, err := httpdtest.UpdateUser(user, http.StatusOK, "")
 	assert.NoError(t, err, string(bb))
 	assert.Equal(t, kms.SecretStatusSecretBox, user.FsConfig.SFTPConfig.Password.GetStatus())
@@ -1552,6 +1555,7 @@ func TestUserSFTPFs(t *testing.T) {
 	assert.Equal(t, initialPkeyPayload, user.FsConfig.SFTPConfig.PrivateKey.GetPayload())
 	assert.Empty(t, user.FsConfig.SFTPConfig.PrivateKey.GetAdditionalData())
 	assert.Empty(t, user.FsConfig.SFTPConfig.PrivateKey.GetKey())
+	assert.False(t, user.FsConfig.SFTPConfig.DisableCouncurrentReads)
 
 	_, err = httpdtest.RemoveUser(user, http.StatusOK)
 	assert.NoError(t, err)
@@ -5823,6 +5827,7 @@ func TestWebUserSFTPFsMock(t *testing.T) {
 	user.FsConfig.SFTPConfig.PrivateKey = kms.NewPlainSecret(sftpPrivateKey)
 	user.FsConfig.SFTPConfig.Fingerprints = []string{sftpPkeyFingerprint}
 	user.FsConfig.SFTPConfig.Prefix = "/home/sftpuser"
+	user.FsConfig.SFTPConfig.DisableCouncurrentReads = true
 	form := make(url.Values)
 	form.Set(csrfFormToken, csrfToken)
 	form.Set("username", user.Username)
@@ -5859,6 +5864,7 @@ func TestWebUserSFTPFsMock(t *testing.T) {
 	form.Set("sftp_private_key", user.FsConfig.SFTPConfig.PrivateKey.GetPayload())
 	form.Set("sftp_fingerprints", user.FsConfig.SFTPConfig.Fingerprints[0])
 	form.Set("sftp_prefix", user.FsConfig.SFTPConfig.Prefix)
+	form.Set("sftp_disable_concurrent_reads", "true")
 	b, contentType, _ = getMultipartFormData(form, "", "")
 	req, _ = http.NewRequest(http.MethodPost, path.Join(webUserPath, user.Username), &b)
 	setJWTCookieForReq(req, webToken)
@@ -5885,6 +5891,7 @@ func TestWebUserSFTPFsMock(t *testing.T) {
 	assert.Equal(t, updateUser.FsConfig.SFTPConfig.Prefix, user.FsConfig.SFTPConfig.Prefix)
 	assert.Equal(t, updateUser.FsConfig.SFTPConfig.Username, user.FsConfig.SFTPConfig.Username)
 	assert.Equal(t, updateUser.FsConfig.SFTPConfig.Endpoint, user.FsConfig.SFTPConfig.Endpoint)
+	assert.True(t, updateUser.FsConfig.SFTPConfig.DisableCouncurrentReads)
 	assert.Len(t, updateUser.FsConfig.SFTPConfig.Fingerprints, 1)
 	assert.Contains(t, updateUser.FsConfig.SFTPConfig.Fingerprints, sftpPkeyFingerprint)
 	// now check that a redacted credentials are not saved

+ 3 - 0
httpd/schema/openapi.yaml

@@ -1524,6 +1524,9 @@ components:
         prefix:
           type: string
           description: Specifying a prefix you can restrict all operations to a given path within the remote SFTP server.
+        disable_concurrent_reads:
+          type: boolean
+          description: Concurrent reads are safe to use and disabling them will degrade performance. Some servers automatically delete files once they are downloaded. Using concurrent reads is problematic with such servers.
     FilesystemConfig:
       type: object
       properties:

+ 1 - 0
httpd/web.go

@@ -730,6 +730,7 @@ func getSFTPConfig(r *http.Request) vfs.SFTPFsConfig {
 	fingerprintsFormValue := r.Form.Get("sftp_fingerprints")
 	config.Fingerprints = getSliceFromDelimitedValues(fingerprintsFormValue, "\n")
 	config.Prefix = r.Form.Get("sftp_prefix")
+	config.DisableCouncurrentReads = len(r.Form.Get("sftp_disable_concurrent_reads")) > 0
 	return config
 }
 

+ 3 - 0
httpdtest/httpdtest.go

@@ -1050,6 +1050,9 @@ func compareSFTPFsConfig(expected *dataprovider.User, actual *dataprovider.User)
 	if expected.FsConfig.SFTPConfig.Username != actual.FsConfig.SFTPConfig.Username {
 		return errors.New("SFTPFs username mismatch")
 	}
+	if expected.FsConfig.SFTPConfig.DisableCouncurrentReads != actual.FsConfig.SFTPConfig.DisableCouncurrentReads {
+		return errors.New("SFTPFs disable_concurrent_reads mismatch")
+	}
 	if err := checkEncryptedSecret(expected.FsConfig.SFTPConfig.Password, actual.FsConfig.SFTPConfig.Password); err != nil {
 		return fmt.Errorf("SFTPFs password mismatch: %v", err)
 	}

+ 5 - 2
service/service_portable.go

@@ -25,8 +25,8 @@ import (
 )
 
 // StartPortableMode starts the service in portable mode
-func (s *Service) StartPortableMode(sftpdPort, ftpPort, webdavPort int, enabledSSHCommands []string, advertiseService, advertiseCredentials bool,
-	ftpsCert, ftpsKey, webDavCert, webDavKey string) error {
+func (s *Service) StartPortableMode(sftpdPort, ftpPort, webdavPort int, enabledSSHCommands []string, advertiseService,
+	advertiseCredentials bool, ftpsCert, ftpsKey, webDavCert, webDavKey string) error {
 	if s.PortableMode != 1 {
 		return fmt.Errorf("service is not configured for portable mode")
 	}
@@ -49,6 +49,9 @@ func (s *Service) StartPortableMode(sftpdPort, ftpPort, webdavPort int, enabledS
 	httpdConf := config.GetHTTPDConfig()
 	httpdConf.Bindings = nil
 	config.SetHTTPDConfig(httpdConf)
+	telemetryConf := config.GetTelemetryConfig()
+	telemetryConf.BindPort = 0
+	config.SetTelemetryConfig(telemetryConf)
 	sftpdConf := config.GetSFTPDConfig()
 	sftpdConf.MaxAuthTries = 12
 	sftpdConf.Bindings = []sftpd.Binding{

+ 22 - 0
sftpd/sftpd_test.go

@@ -394,6 +394,7 @@ func TestBasicSFTPFsHandling(t *testing.T) {
 	assert.NoError(t, err)
 	u := getTestSFTPUser(usePubKey)
 	u.QuotaSize = 6553600
+	u.FsConfig.SFTPConfig.DisableCouncurrentReads = true
 	user, _, err := httpdtest.AddUser(u, http.StatusCreated)
 	assert.NoError(t, err)
 	client, err := getSftpClient(user, usePubKey)
@@ -726,6 +727,27 @@ func TestProxyProtocol(t *testing.T) {
 	assert.NoError(t, err)
 }
 
+func TestRealPath(t *testing.T) {
+	usePubKey := true
+	u := getTestUser(usePubKey)
+	user, _, err := httpdtest.AddUser(u, http.StatusCreated)
+	assert.NoError(t, err)
+	client, err := getSftpClient(user, usePubKey)
+	if assert.NoError(t, err) {
+		defer client.Close()
+		p, err := client.RealPath("../..")
+		assert.NoError(t, err)
+		assert.Equal(t, "/", p)
+		p, err = client.RealPath("../test")
+		assert.NoError(t, err)
+		assert.Equal(t, "/test", p)
+	}
+	_, err = httpdtest.RemoveUser(user, http.StatusOK)
+	assert.NoError(t, err)
+	err = os.RemoveAll(user.GetHomeDir())
+	assert.NoError(t, err)
+}
+
 func TestUploadResume(t *testing.T) {
 	usePubKey := false
 	u := getTestUser(usePubKey)

+ 7 - 0
templates/user.html

@@ -616,6 +616,13 @@
                 </div>
             </div>
 
+            <div class="form-group sftp">
+                <div class="form-check">
+                    <input type="checkbox" class="form-check-input" id="idDisableConcurrentReads" name="sftp_disable_concurrent_reads" {{if .User.FsConfig.SFTPConfig.DisableCouncurrentReads}}checked{{end}}>
+                    <label for="idDisableConcurrentReads" class="form-check-label">Disable concurrent reads</label>
+                </div>
+            </div>
+
             <div class="form-group row sftp">
                 <label for="idSFTPPassword" class="col-sm-2 col-form-label">Password</label>
                 <div class="col-sm-10">

+ 9 - 0
vfs/sftpfs.go

@@ -38,6 +38,10 @@ type SFTPFsConfig struct {
 	Fingerprints []string    `json:"fingerprints,omitempty"`
 	// Prefix is the path prefix to strip from SFTP resource paths.
 	Prefix string `json:"prefix,omitempty"`
+	// Concurrent reads are safe to use and disabling them will degrade performance.
+	// Some servers automatically delete files once they are downloaded.
+	// Using concurrent reads is problematic with such servers.
+	DisableCouncurrentReads bool `json:"disable_concurrent_reads,omitempty"`
 }
 
 func (c *SFTPFsConfig) setEmptyCredentialsIfNil() {
@@ -584,6 +588,11 @@ func (fs *SFTPFs) createConnection() error {
 		fs.err <- err
 		return err
 	}
+	if fs.config.DisableCouncurrentReads {
+		fsLog(fs, logger.LevelDebug, "disabling concurrent reads")
+		opt := sftp.UseConcurrentReads(false)
+		opt(fs.sftpClient) //nolint:errcheck
+	}
 	go fs.wait()
 	return nil
 }