From dc845fa2f4d5167941e4ff77d2e7d37b76176528 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sat, 14 Nov 2020 19:19:41 +0100 Subject: [PATCH] webdav: fix permission errors if the client try to read multiple times --- go.mod | 2 +- go.sum | 4 ++-- sftpd/server.go | 20 ++++++++++---------- webdavd/file.go | 3 +-- webdavd/webdavd_test.go | 11 ++++++----- 5 files changed, 20 insertions(+), 20 deletions(-) diff --git a/go.mod b/go.mod index 249b8f1a..8d273b04 100644 --- a/go.mod +++ b/go.mod @@ -40,7 +40,7 @@ require ( github.com/spf13/jwalterweatherman v1.1.0 // indirect github.com/spf13/viper v1.7.1 github.com/stretchr/testify v1.6.1 - github.com/studio-b12/gowebdav v0.0.0-20200303150724-9380631c29a1 + github.com/studio-b12/gowebdav v0.0.0-20200929080739-bdacfab94796 go.etcd.io/bbolt v1.3.5 go.uber.org/automaxprocs v1.3.0 golang.org/x/crypto v0.0.0-20201112155050-0c6587e931a9 diff --git a/go.sum b/go.sum index a57e0447..647ceed2 100644 --- a/go.sum +++ b/go.sum @@ -479,8 +479,8 @@ github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81P github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/studio-b12/gowebdav v0.0.0-20200303150724-9380631c29a1 h1:TPyHV/OgChqNcnYqCoCvIFjR9TU60gFXXBKnhOBzVEI= -github.com/studio-b12/gowebdav v0.0.0-20200303150724-9380631c29a1/go.mod h1:gCcfDlA1Y7GqOaeEKw5l9dOGx1VLdc/HuQSlQAaZ30s= +github.com/studio-b12/gowebdav v0.0.0-20200929080739-bdacfab94796 h1:vkok9HUaplVRvHgwuyEGxBhdGtOUdMr7/KprS+6k8TQ= +github.com/studio-b12/gowebdav v0.0.0-20200929080739-bdacfab94796/go.mod h1:gCcfDlA1Y7GqOaeEKw5l9dOGx1VLdc/HuQSlQAaZ30s= github.com/subosito/gotenv v1.2.0 h1:Slr1R9HxAlEKefgq5jn9U+DnETlIUa6HfgEzj0g5d7s= github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw= github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= diff --git a/sftpd/server.go b/sftpd/server.go index b30adf6b..30c49103 100644 --- a/sftpd/server.go +++ b/sftpd/server.go @@ -128,7 +128,7 @@ func (e *authenticationError) Error() string { } // Initialize the SFTP server and add a persistent listener to handle inbound SFTP connections. -func (c Configuration) Initialize(configDir string) error { +func (c *Configuration) Initialize(configDir string) error { serverConfig := &ssh.ServerConfig{ NoClientAuth: false, MaxAuthTries: c.MaxAuthTries, @@ -205,7 +205,7 @@ func (c Configuration) Initialize(configDir string) error { } } -func (c Configuration) configureSecurityOptions(serverConfig *ssh.ServerConfig) { +func (c *Configuration) configureSecurityOptions(serverConfig *ssh.ServerConfig) { if len(c.KexAlgorithms) > 0 { serverConfig.KeyExchanges = c.KexAlgorithms } @@ -217,7 +217,7 @@ func (c Configuration) configureSecurityOptions(serverConfig *ssh.ServerConfig) } } -func (c Configuration) configureLoginBanner(serverConfig *ssh.ServerConfig, configDir string) { +func (c *Configuration) configureLoginBanner(serverConfig *ssh.ServerConfig, configDir string) { if len(c.LoginBannerFile) > 0 { bannerFilePath := c.LoginBannerFile if !filepath.IsAbs(bannerFilePath) { @@ -236,7 +236,7 @@ func (c Configuration) configureLoginBanner(serverConfig *ssh.ServerConfig, conf } } -func (c Configuration) configureKeyboardInteractiveAuth(serverConfig *ssh.ServerConfig) { +func (c *Configuration) configureKeyboardInteractiveAuth(serverConfig *ssh.ServerConfig) { if len(c.KeyboardInteractiveHook) == 0 { return } @@ -266,7 +266,7 @@ func (c Configuration) configureKeyboardInteractiveAuth(serverConfig *ssh.Server } // AcceptInboundConnection handles an inbound connection to the server instance and determines if the request should be served or not. -func (c Configuration) AcceptInboundConnection(conn net.Conn, config *ssh.ServerConfig) { +func (c *Configuration) AcceptInboundConnection(conn net.Conn, config *ssh.ServerConfig) { defer func() { if r := recover(); r != nil { logger.Error(logSender, "", "panic in AcceptInboundConnection: %#v stack strace: %v", r, string(debug.Stack())) @@ -379,7 +379,7 @@ func (c Configuration) AcceptInboundConnection(conn net.Conn, config *ssh.Server } } -func (c Configuration) handleSftpConnection(channel ssh.Channel, connection *Connection) { +func (c *Configuration) handleSftpConnection(channel ssh.Channel, connection *Connection) { defer func() { if r := recover(); r != nil { logger.Error(logSender, "", "panic in handleSftpConnection: %#v stack strace: %v", r, string(debug.Stack())) @@ -405,7 +405,7 @@ func (c Configuration) handleSftpConnection(channel ssh.Channel, connection *Con } } -func (c Configuration) createHandler(connection *Connection) sftp.Handlers { +func (c *Configuration) createHandler(connection *Connection) sftp.Handlers { return sftp.Handlers{ FileGet: connection, FilePut: connection, @@ -633,7 +633,7 @@ func (c *Configuration) initializeCertChecker(configDir string) error { return nil } -func (c Configuration) validatePublicKeyCredentials(conn ssh.ConnMetadata, pubKey ssh.PublicKey) (*ssh.Permissions, error) { +func (c *Configuration) validatePublicKeyCredentials(conn ssh.ConnMetadata, pubKey ssh.PublicKey) (*ssh.Permissions, error) { var err error var user dataprovider.User var keyID string @@ -682,7 +682,7 @@ func (c Configuration) validatePublicKeyCredentials(conn ssh.ConnMetadata, pubKe return sshPerm, err } -func (c Configuration) validatePasswordCredentials(conn ssh.ConnMetadata, pass []byte) (*ssh.Permissions, error) { +func (c *Configuration) validatePasswordCredentials(conn ssh.ConnMetadata, pass []byte) (*ssh.Permissions, error) { var err error var user dataprovider.User var sshPerm *ssh.Permissions @@ -699,7 +699,7 @@ func (c Configuration) validatePasswordCredentials(conn ssh.ConnMetadata, pass [ return sshPerm, err } -func (c Configuration) validateKeyboardInteractiveCredentials(conn ssh.ConnMetadata, client ssh.KeyboardInteractiveChallenge) (*ssh.Permissions, error) { +func (c *Configuration) validateKeyboardInteractiveCredentials(conn ssh.ConnMetadata, client ssh.KeyboardInteractiveChallenge) (*ssh.Permissions, error) { var err error var user dataprovider.User var sshPerm *ssh.Permissions diff --git a/webdavd/file.go b/webdavd/file.go index e83a44bc..14695aea 100644 --- a/webdavd/file.go +++ b/webdavd/file.go @@ -136,8 +136,6 @@ func (f *webDavFile) Read(p []byte) (n int, err error) { return 0, errTransferAborted } if atomic.LoadInt32(&f.readTryed) == 0 { - atomic.StoreInt32(&f.readTryed, 1) - if !f.Connection.User.HasPerm(dataprovider.PermDownload, path.Dir(f.GetVirtualPath())) { return 0, f.Connection.GetPermissionDeniedError() } @@ -146,6 +144,7 @@ func (f *webDavFile) Read(p []byte) (n int, err error) { f.Connection.Log(logger.LevelWarn, "reading file %#v is not allowed", f.GetVirtualPath()) return 0, f.Connection.GetPermissionDeniedError() } + atomic.StoreInt32(&f.readTryed, 1) } f.Connection.UpdateLastActivity() diff --git a/webdavd/webdavd_test.go b/webdavd/webdavd_test.go index 43986a30..2e0f7887 100644 --- a/webdavd/webdavd_test.go +++ b/webdavd/webdavd_test.go @@ -528,18 +528,19 @@ func TestDownloadErrors(t *testing.T) { u.Permissions[path.Join("/", subDir1)] = []string{dataprovider.PermListItems} u.Permissions[path.Join("/", subDir2)] = []string{dataprovider.PermListItems, dataprovider.PermUpload, dataprovider.PermDelete, dataprovider.PermDownload} + // use an unknown mime to trigger content type detection u.Filters.FileExtensions = []dataprovider.ExtensionsFilter{ { Path: "/sub2", AllowedExtensions: []string{}, - DeniedExtensions: []string{".zip"}, + DeniedExtensions: []string{".zipp"}, }, } user, _, err := httpd.AddUser(u, http.StatusOK) assert.NoError(t, err) client := getWebDavClient(user) - testFilePath1 := filepath.Join(user.HomeDir, subDir1, "file.zip") - testFilePath2 := filepath.Join(user.HomeDir, subDir2, "file.zip") + testFilePath1 := filepath.Join(user.HomeDir, subDir1, "file.zipp") + testFilePath2 := filepath.Join(user.HomeDir, subDir2, "file.zipp") err = os.MkdirAll(filepath.Dir(testFilePath1), os.ModePerm) assert.NoError(t, err) err = os.MkdirAll(filepath.Dir(testFilePath2), os.ModePerm) @@ -549,9 +550,9 @@ func TestDownloadErrors(t *testing.T) { err = ioutil.WriteFile(testFilePath2, []byte("file2"), os.ModePerm) assert.NoError(t, err) localDownloadPath := filepath.Join(homeBasePath, testDLFileName) - err = downloadFile(path.Join("/", subDir1, "file.zip"), localDownloadPath, 5, client) + err = downloadFile(path.Join("/", subDir1, "file.zipp"), localDownloadPath, 5, client) assert.Error(t, err) - err = downloadFile(path.Join("/", subDir2, "file.zip"), localDownloadPath, 5, client) + err = downloadFile(path.Join("/", subDir2, "file.zipp"), localDownloadPath, 5, client) assert.Error(t, err) err = downloadFile(path.Join("missing.zip"), localDownloadPath, 5, client) assert.Error(t, err)