don't expose error messages from pre-actions and post connect hooks

always return a generic error instead to avoid leaking internal info

Signed-off-by: Nicola Murino <nicola.murino@gmail.com>
This commit is contained in:
Nicola Murino 2023-02-28 18:01:09 +01:00
parent dba088daed
commit fad6af11e5
No known key found for this signature in database
GPG key ID: 935D2952DEC4EECF
9 changed files with 32 additions and 27 deletions

4
go.mod
View file

@ -35,7 +35,7 @@ require (
github.com/hashicorp/go-hclog v1.4.0
github.com/hashicorp/go-plugin v1.4.8
github.com/hashicorp/go-retryablehttp v0.7.2
github.com/jackc/pgx/v5 v5.3.0
github.com/jackc/pgx/v5 v5.3.1
github.com/jlaffaye/ftp v0.0.0-20201112195030-9aae4d151126
github.com/klauspost/compress v1.16.0
github.com/lestrrat-go/jwx/v2 v2.0.8
@ -157,7 +157,7 @@ require (
golang.org/x/tools v0.6.0 // indirect
golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
google.golang.org/appengine v1.6.7 // indirect
google.golang.org/genproto v0.0.0-20230223222841-637eb2293923 // indirect
google.golang.org/genproto v0.0.0-20230227214838-9b19f0bdc514 // indirect
google.golang.org/grpc v1.53.0 // indirect
google.golang.org/protobuf v1.28.1 // indirect
gopkg.in/ini.v1 v1.67.0 // indirect

12
go.sum
View file

@ -215,8 +215,8 @@ cloud.google.com/go/iot v1.3.0/go.mod h1:r7RGh2B61+B8oz0AGE+J72AhA0G7tdXItODWsaA
cloud.google.com/go/iot v1.4.0/go.mod h1:dIDxPOn0UvNDUMD8Ger7FIaTuvMkj+aGk94RPP0iV+g=
cloud.google.com/go/kms v1.5.0/go.mod h1:QJS2YY0eJGBg3mnDfuaCyLauWwBJiHRboYxJ++1xJNg=
cloud.google.com/go/kms v1.6.0/go.mod h1:Jjy850yySiasBUDi6KFUwUv2n1+o7QZFyuUJg6OgjA0=
cloud.google.com/go/kms v1.7.0 h1:8FCf8C7qfOuSr6YzOQ4RGjJvswSRFeOpur3nHOlJbio=
cloud.google.com/go/kms v1.7.0/go.mod h1:k2UdVoNIHLJi/Rnng6dN0vlq7lS3jHSDiZasft+gmYE=
cloud.google.com/go/kms v1.8.0 h1:VrJLOsMRzW7IqTTYn+OYupqF3iKSE060Nrn+PECrYjg=
cloud.google.com/go/language v1.4.0/go.mod h1:F9dRpNFQmJbkaop6g0JhSBXCNlO90e1KWx5iDdxbWic=
cloud.google.com/go/language v1.6.0/go.mod h1:6dJ8t3B+lUYfStgls25GusK04NLh3eDLQnWM3mdEbhI=
cloud.google.com/go/language v1.7.0/go.mod h1:DJ6dYN/W+SQOjF8e1hLQXMF21AkH2w9wiPzPCJa2MIE=
@ -224,8 +224,8 @@ cloud.google.com/go/language v1.8.0/go.mod h1:qYPVHf7SPoNNiCL2Dr0FfEFNil1qi3pQEy
cloud.google.com/go/lifesciences v0.5.0/go.mod h1:3oIKy8ycWGPUyZDR/8RNnTOYevhaMLqh5vLUXs9zvT8=
cloud.google.com/go/lifesciences v0.6.0/go.mod h1:ddj6tSX/7BOnhxCSd3ZcETvtNr8NZ6t/iPhY2Tyfu08=
cloud.google.com/go/longrunning v0.1.1/go.mod h1:UUFxuDWkv22EuY93jjmDMFT5GPQKeFVJBIF6QlTqdsE=
cloud.google.com/go/longrunning v0.3.0 h1:NjljC+FYPV3uh5/OwWT6pVU+doBqMg2x/rZlE+CamDs=
cloud.google.com/go/longrunning v0.3.0/go.mod h1:qth9Y41RRSUE69rDcOn6DdK3HfQfsUI0YSmW3iIlLJc=
cloud.google.com/go/longrunning v0.4.1 h1:v+yFJOfKC3yZdY6ZUI933pIYdhyhV8S3NpWrXWmg7jM=
cloud.google.com/go/managedidentities v1.3.0/go.mod h1:UzlW3cBOiPrzucO5qWkNkh0w33KFtBJU281hacNvsdE=
cloud.google.com/go/managedidentities v1.4.0/go.mod h1:NWSBYbEMgqmbZsLIyKvxrYbtqOsxY1ZrGM+9RgDqInM=
cloud.google.com/go/mediatranslation v0.5.0/go.mod h1:jGPUhGTybqsPQn91pNXw0xVHfuJ3leR1wj37oU3y1f4=
@ -1352,8 +1352,8 @@ github.com/jackc/pgx/v4 v4.12.1-0.20210724153913-640aa07df17c/go.mod h1:1QD0+tgS
github.com/jackc/pgx/v4 v4.16.0/go.mod h1:N0A9sFdWzkw/Jy1lwoiB64F2+ugFZi987zRxcPez/wI=
github.com/jackc/pgx/v4 v4.16.1/go.mod h1:SIhx0D5hoADaiXZVyv+3gSm3LCIIINTVO0PficsvWGQ=
github.com/jackc/pgx/v4 v4.17.2/go.mod h1:lcxIZN44yMIrWI78a5CpucdD14hX0SBDbNRvjDBItsw=
github.com/jackc/pgx/v5 v5.3.0 h1:/NQi8KHMpKWHInxXesC8yD4DhkXPrVhmnwYkjp9AmBA=
github.com/jackc/pgx/v5 v5.3.0/go.mod h1:t3JDKnCBlYIc0ewLF0Q7B8MXmoIaBOZj/ic7iHozM/8=
github.com/jackc/pgx/v5 v5.3.1 h1:Fcr8QJ1ZeLi5zsPZqQeUZhNhxfkkKBOgJuYkJHoBOtU=
github.com/jackc/pgx/v5 v5.3.1/go.mod h1:t3JDKnCBlYIc0ewLF0Q7B8MXmoIaBOZj/ic7iHozM/8=
github.com/jackc/puddle v0.0.0-20190413234325-e4ced69a3a2b/go.mod h1:m4B5Dj62Y0fbyuIc15OsIqK0+JU8nkqQjsgx7dvjSWk=
github.com/jackc/puddle v0.0.0-20190608224051-11cab39313c9/go.mod h1:m4B5Dj62Y0fbyuIc15OsIqK0+JU8nkqQjsgx7dvjSWk=
github.com/jackc/puddle v1.1.3/go.mod h1:m4B5Dj62Y0fbyuIc15OsIqK0+JU8nkqQjsgx7dvjSWk=
@ -2719,8 +2719,8 @@ google.golang.org/genproto v0.0.0-20221109142239-94d6d90a7d66/go.mod h1:rZS5c/ZV
google.golang.org/genproto v0.0.0-20221118155620-16455021b5e6/go.mod h1:rZS5c/ZVYMaOGBfO68GWtjOw/eLaZM1X6iVtgjZ+EWg=
google.golang.org/genproto v0.0.0-20221201164419-0e50fba7f41c/go.mod h1:rZS5c/ZVYMaOGBfO68GWtjOw/eLaZM1X6iVtgjZ+EWg=
google.golang.org/genproto v0.0.0-20221201204527-e3fa12d562f3/go.mod h1:rZS5c/ZVYMaOGBfO68GWtjOw/eLaZM1X6iVtgjZ+EWg=
google.golang.org/genproto v0.0.0-20230223222841-637eb2293923 h1:znp6mq/drrY+6khTAlJUDNFFcDGV2ENLYKpMq8SyCds=
google.golang.org/genproto v0.0.0-20230223222841-637eb2293923/go.mod h1:3Dl5ZL0q0isWJt+FVcfpQyirqemEuLAK/iFvg1UP1Hw=
google.golang.org/genproto v0.0.0-20230227214838-9b19f0bdc514 h1:rtNKfB++wz5mtDY2t5C8TXlU5y52ojSu7tZo0z7u8eQ=
google.golang.org/genproto v0.0.0-20230227214838-9b19f0bdc514/go.mod h1:TvhZT5f700eVlTNwND1xoEZQeWTB2RY/65kplwl/bFA=
google.golang.org/grpc v0.0.0-20160317175043-d3ddb4469d5a/go.mod h1:yo6s7OP7yaDglbqo1J04qKzAhqBH6lvTonzMVmEdcZw=
google.golang.org/grpc v1.17.0/go.mod h1:6QZJwpn2B+Zp71q/5VxRsJ6NXXVCE5NRUHRo+f3cWCs=
google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c=

View file

@ -737,7 +737,7 @@ func (c *Configuration) ExecutePostConnectHook(ipAddr, protocol string) error {
if err != nil {
logger.Warn(protocol, "", "Login from ip %q denied, invalid post connect hook %q: %v",
ipAddr, c.PostConnectHook, err)
return err
return getPermissionDeniedError(protocol)
}
q := url.Query()
q.Add("ip", ipAddr)
@ -747,19 +747,19 @@ func (c *Configuration) ExecutePostConnectHook(ipAddr, protocol string) error {
resp, err := httpclient.RetryableGet(url.String())
if err != nil {
logger.Warn(protocol, "", "Login from ip %q denied, error executing post connect hook: %v", ipAddr, err)
return err
return getPermissionDeniedError(protocol)
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
logger.Warn(protocol, "", "Login from ip %q denied, post connect hook response code: %v", ipAddr, resp.StatusCode)
return errUnexpectedHTTResponse
return getPermissionDeniedError(protocol)
}
return nil
}
if !filepath.IsAbs(c.PostConnectHook) {
err := fmt.Errorf("invalid post connect hook %q", c.PostConnectHook)
logger.Warn(protocol, "", "Login from ip %q denied: %v", ipAddr, err)
return err
return getPermissionDeniedError(protocol)
}
timeout, env, args := command.GetConfig(c.PostConnectHook, command.HookPostConnect)
ctx, cancel := context.WithTimeout(context.Background(), timeout)
@ -772,8 +772,9 @@ func (c *Configuration) ExecutePostConnectHook(ipAddr, protocol string) error {
err := cmd.Run()
if err != nil {
logger.Warn(protocol, "", "Login from ip %q denied, connect hook error: %v", ipAddr, err)
return getPermissionDeniedError(protocol)
}
return err
return nil
}
// SSHConnection defines an ssh connection.

View file

@ -1566,14 +1566,7 @@ func (c *BaseConnection) GetErrorForDeniedFile(policy int) error {
// GetPermissionDeniedError returns an appropriate permission denied error for the connection protocol
func (c *BaseConnection) GetPermissionDeniedError() error {
switch c.protocol {
case ProtocolSFTP:
return sftp.ErrSSHFxPermissionDenied
case ProtocolWebDAV, ProtocolFTP, ProtocolHTTP, ProtocolOIDC, ProtocolHTTPShare, ProtocolDataRetention:
return os.ErrPermission
default:
return ErrPermissionDenied
}
return getPermissionDeniedError(c.protocol)
}
// GetNotExistError returns an appropriate not exist error for the connection protocol
@ -1723,6 +1716,17 @@ func (c *BaseConnection) GetFsAndResolvedPath(virtualPath string) (vfs.Fs, strin
return fs, fsPath, nil
}
func getPermissionDeniedError(protocol string) error {
switch protocol {
case ProtocolSFTP:
return sftp.ErrSSHFxPermissionDenied
case ProtocolWebDAV, ProtocolFTP, ProtocolHTTP, ProtocolOIDC, ProtocolHTTPShare, ProtocolDataRetention:
return os.ErrPermission
default:
return ErrPermissionDenied
}
}
func keepConnectionAlive(c *BaseConnection, done chan bool, interval time.Duration) {
ticker := time.NewTicker(interval)
defer func() {

View file

@ -406,7 +406,7 @@ func (c *Connection) handleFTPUploadToNewFile(fs vfs.Fs, flags int, resolvedPath
}
if _, err := common.ExecutePreAction(c.BaseConnection, common.OperationPreUpload, resolvedPath, requestPath, 0, 0); err != nil {
c.Log(logger.LevelDebug, "upload for file %q denied by pre action: %v", requestPath, err)
return nil, fmt.Errorf("%w, denied by pre-upload action", ftpserver.ErrFileNameNotAllowed)
return nil, ftpserver.ErrFileNameNotAllowed
}
file, w, cancelFn, err := fs.Create(filePath, flags)
if err != nil {
@ -451,7 +451,7 @@ func (c *Connection) handleFTPUploadToExistingFile(fs vfs.Fs, flags int, resolve
}
if _, err := common.ExecutePreAction(c.BaseConnection, common.OperationPreUpload, resolvedPath, requestPath, fileSize, flags); err != nil {
c.Log(logger.LevelDebug, "upload for file %q denied by pre action: %v", requestPath, err)
return nil, fmt.Errorf("%w, denied by pre-upload action", ftpserver.ErrFileNameNotAllowed)
return nil, ftpserver.ErrFileNameNotAllowed
}
if common.Config.IsAtomicUploadEnabled() && fs.IsAtomicUploadSupported() {

View file

@ -173,7 +173,7 @@ func (s *Server) ClientConnected(cc ftpserver.ClientContext) (string, error) {
return fmt.Sprintf("Access denied: %v", err.Error()), err
}
if err := common.Config.ExecutePostConnectHook(ipAddr, common.ProtocolFTP); err != nil {
return "Access denied by post connect hook", err
return "Access denied", err
}
connID := fmt.Sprintf("%v_%v", s.ID, cc.ID())
user := dataprovider.User{}

View file

@ -431,7 +431,7 @@ func (t *oidcToken) getUser(r *http.Request) error {
}
if err := common.Config.ExecutePostConnectHook(ipAddr, common.ProtocolOIDC); err != nil {
updateLoginMetrics(&user, dataprovider.LoginMethodIDP, ipAddr, err)
return fmt.Errorf("access denied by post connect hook: %w", err)
return fmt.Errorf("access denied: %w", err)
}
if err := user.CheckLoginConditions(); err != nil {
updateLoginMetrics(&user, dataprovider.LoginMethodIDP, ipAddr, err)

View file

@ -908,7 +908,7 @@ func TestOIDCToken(t *testing.T) {
err = token.getUser(req)
if assert.Error(t, err) {
assert.Contains(t, err.Error(), "access denied by post connect hook")
assert.Contains(t, err.Error(), "access denied")
}
common.Config.PostConnectHook = ""

View file

@ -246,7 +246,7 @@ func (s *httpdServer) handleWebClientLoginPost(w http.ResponseWriter, r *http.Re
}
if err := common.Config.ExecutePostConnectHook(ipAddr, protocol); err != nil {
s.renderClientLoginPage(w, fmt.Sprintf("access denied by post connect hook: %v", err), ipAddr)
s.renderClientLoginPage(w, fmt.Sprintf("access denied: %v", err), ipAddr)
return
}