diff --git a/dataprovider/bolt.go b/dataprovider/bolt.go index f73f861a..e88d0e01 100644 --- a/dataprovider/bolt.go +++ b/dataprovider/bolt.go @@ -576,7 +576,7 @@ func (p *BoltProvider) getUsers(limit int, offset int, order string) ([]User, er } user, err := joinUserAndFolders(v, folderBucket) if err == nil { - user.HideConfidentialData() + user.PrepareForRendering() users = append(users, user) } if len(users) >= limit { @@ -591,7 +591,7 @@ func (p *BoltProvider) getUsers(limit int, offset int, order string) ([]User, er } user, err := joinUserAndFolders(v, folderBucket) if err == nil { - user.HideConfidentialData() + user.PrepareForRendering() users = append(users, user) } if len(users) >= limit { @@ -649,7 +649,7 @@ func (p *BoltProvider) getFolders(limit, offset int, order string) ([]vfs.BaseVi if err != nil { return err } - folder.HideConfidentialData() + folder.PrepareForRendering() folders = append(folders, folder) if len(folders) >= limit { break @@ -666,7 +666,7 @@ func (p *BoltProvider) getFolders(limit, offset int, order string) ([]vfs.BaseVi if err != nil { return err } - folder.HideConfidentialData() + folder.PrepareForRendering() folders = append(folders, folder) if len(folders) >= limit { break diff --git a/dataprovider/dataprovider.go b/dataprovider/dataprovider.go index a8c8b6f9..e9294d89 100644 --- a/dataprovider/dataprovider.go +++ b/dataprovider/dataprovider.go @@ -2224,7 +2224,7 @@ func ExecutePostLoginHook(user *User, loginMethod, ip, protocol string, err erro status = "1" } - user.HideConfidentialData() + user.PrepareForRendering() userAsJSON, err := json.Marshal(user) if err != nil { providerLog(logger.LevelWarn, "error serializing user in post login hook: %v", err) @@ -2422,7 +2422,7 @@ func executeAction(operation string, user *User) { } user = &u } - user.HideConfidentialData() + user.PrepareForRendering() userAsJSON, err := json.Marshal(user) if err != nil { providerLog(logger.LevelWarn, "unable to serialize user as JSON for operation %#v: %v", operation, err) diff --git a/dataprovider/memory.go b/dataprovider/memory.go index c587f4a2..e7f88781 100644 --- a/dataprovider/memory.go +++ b/dataprovider/memory.go @@ -330,7 +330,7 @@ func (p *MemoryProvider) getUsers(limit int, offset int, order string) ([]User, } u := p.dbHandle.users[username] user := u.getACopy() - user.HideConfidentialData() + user.PrepareForRendering() users = append(users, user) if len(users) >= limit { break @@ -345,7 +345,7 @@ func (p *MemoryProvider) getUsers(limit int, offset int, order string) ([]User, username := p.dbHandle.usernames[i] u := p.dbHandle.users[username] user := u.getACopy() - user.HideConfidentialData() + user.PrepareForRendering() users = append(users, user) if len(users) >= limit { break @@ -635,7 +635,7 @@ func (p *MemoryProvider) getFolders(limit, offset int, order string) ([]vfs.Base } f := p.dbHandle.vfolders[name] folder := f.GetACopy() - folder.HideConfidentialData() + folder.PrepareForRendering() folders = append(folders, folder) if len(folders) >= limit { break @@ -650,7 +650,7 @@ func (p *MemoryProvider) getFolders(limit, offset int, order string) ([]vfs.Base name := p.dbHandle.vfoldersNames[i] f := p.dbHandle.vfolders[name] folder := f.GetACopy() - folder.HideConfidentialData() + folder.PrepareForRendering() folders = append(folders, folder) if len(folders) >= limit { break diff --git a/dataprovider/sqlcommon.go b/dataprovider/sqlcommon.go index 468614b7..79b29e0a 100644 --- a/dataprovider/sqlcommon.go +++ b/dataprovider/sqlcommon.go @@ -484,7 +484,7 @@ func sqlCommonGetUsers(limit int, offset int, order string, dbHandle sqlQuerier) if err != nil { return users, err } - u.HideConfidentialData() + u.PrepareForRendering() users = append(users, u) } } @@ -832,7 +832,7 @@ func sqlCommonGetFolders(limit, offset int, order string, dbHandle sqlQuerier) ( folder.FsConfig = fs } } - folder.HideConfidentialData() + folder.PrepareForRendering() folders = append(folders, folder) } diff --git a/dataprovider/user.go b/dataprovider/user.go index 31771629..090200bd 100644 --- a/dataprovider/user.go +++ b/dataprovider/user.go @@ -278,8 +278,8 @@ func (u *User) CheckFsRoot(connectionID string) error { return nil } -// HideConfidentialData hides user confidential data -func (u *User) HideConfidentialData() { +// hideConfidentialData hides user confidential data +func (u *User) hideConfidentialData() { u.Password = "" switch u.FsConfig.Provider { case vfs.S3FilesystemProvider: @@ -294,9 +294,17 @@ func (u *User) HideConfidentialData() { u.FsConfig.SFTPConfig.Password.Hide() u.FsConfig.SFTPConfig.PrivateKey.Hide() } +} + +// PrepareForRendering prepares a user for rendering. +// It hides confidential data and set to nil the empty secrets +// so they are not serialized +func (u *User) PrepareForRendering() { + u.hideConfidentialData() + u.FsConfig.SetNilSecretsIfEmpty() for idx := range u.VirtualFolders { folder := &u.VirtualFolders[idx] - folder.HideConfidentialData() + folder.PrepareForRendering() } } diff --git a/httpd/api_folder.go b/httpd/api_folder.go index 749f312b..8d6e9e15 100644 --- a/httpd/api_folder.go +++ b/httpd/api_folder.go @@ -88,7 +88,7 @@ func renderFolder(w http.ResponseWriter, r *http.Request, name string, status in sendAPIResponse(w, r, err, "", getRespStatus(err)) return } - folder.HideConfidentialData() + folder.PrepareForRendering() if status != http.StatusOK { ctx := context.WithValue(r.Context(), render.StatusCtxKey, status) render.JSON(w, r.WithContext(ctx), folder) diff --git a/httpd/api_user.go b/httpd/api_user.go index 99e5edfe..495079dd 100644 --- a/httpd/api_user.go +++ b/httpd/api_user.go @@ -40,7 +40,7 @@ func renderUser(w http.ResponseWriter, r *http.Request, username string, status sendAPIResponse(w, r, err, "", getRespStatus(err)) return } - user.HideConfidentialData() + user.PrepareForRendering() if status != http.StatusOK { ctx := context.WithValue(r.Context(), render.StatusCtxKey, status) render.JSON(w, r.WithContext(ctx), user) diff --git a/httpd/httpd_test.go b/httpd/httpd_test.go index 3c41e91a..d037d5aa 100644 --- a/httpd/httpd_test.go +++ b/httpd/httpd_test.go @@ -1400,7 +1400,7 @@ func TestUserS3Config(t *testing.T) { user.FsConfig.S3Config.UploadConcurrency = 4 user, body, err = httpdtest.UpdateUser(user, http.StatusOK, "") assert.NoError(t, err, string(body)) - assert.True(t, user.FsConfig.S3Config.AccessSecret.IsEmpty()) + assert.Nil(t, user.FsConfig.S3Config.AccessSecret) _, err = httpdtest.RemoveUser(user, http.StatusOK) assert.NoError(t, err) user.Password = defaultPassword @@ -1408,7 +1408,7 @@ func TestUserS3Config(t *testing.T) { // shared credential test for add instead of update user, _, err = httpdtest.AddUser(user, http.StatusCreated) assert.NoError(t, err) - assert.True(t, user.FsConfig.S3Config.AccessSecret.IsEmpty()) + assert.Nil(t, user.FsConfig.S3Config.AccessSecret) _, err = httpdtest.RemoveUser(user, http.StatusOK) assert.NoError(t, err) } @@ -1551,7 +1551,7 @@ func TestUserAzureBlobConfig(t *testing.T) { user.FsConfig.AzBlobConfig.UploadConcurrency = 4 user, _, err = httpdtest.UpdateUser(user, http.StatusOK, "") assert.NoError(t, err) - assert.True(t, user.FsConfig.AzBlobConfig.AccountKey.IsEmpty()) + assert.Nil(t, user.FsConfig.AzBlobConfig.AccountKey) _, err = httpdtest.RemoveUser(user, http.StatusOK) assert.NoError(t, err) user.Password = defaultPassword @@ -1559,7 +1559,7 @@ func TestUserAzureBlobConfig(t *testing.T) { // sas test for add instead of update user, _, err = httpdtest.AddUser(user, http.StatusCreated) assert.NoError(t, err) - assert.True(t, user.FsConfig.AzBlobConfig.AccountKey.IsEmpty()) + assert.Nil(t, user.FsConfig.AzBlobConfig.AccountKey) _, err = httpdtest.RemoveUser(user, http.StatusOK) assert.NoError(t, err) } @@ -1681,7 +1681,7 @@ func TestUserSFTPFs(t *testing.T) { user, _, err = httpdtest.AddUser(user, http.StatusCreated) assert.NoError(t, err) initialPkeyPayload = user.FsConfig.SFTPConfig.PrivateKey.GetPayload() - assert.Empty(t, user.FsConfig.SFTPConfig.Password.GetStatus()) + assert.Nil(t, user.FsConfig.SFTPConfig.Password) assert.Equal(t, kms.SecretStatusSecretBox, user.FsConfig.SFTPConfig.PrivateKey.GetStatus()) assert.NotEmpty(t, initialPkeyPayload) assert.Empty(t, user.FsConfig.SFTPConfig.PrivateKey.GetAdditionalData()) @@ -5811,7 +5811,7 @@ func TestWebUserS3Mock(t *testing.T) { var userGet dataprovider.User err = render.DecodeJSON(rr.Body, &userGet) assert.NoError(t, err) - assert.True(t, userGet.FsConfig.S3Config.AccessSecret.IsEmpty()) + assert.Nil(t, userGet.FsConfig.S3Config.AccessSecret) req, _ = http.NewRequest(http.MethodDelete, path.Join(userPath, user.Username), nil) setBearerForReq(req, apiToken) diff --git a/kms/kms.go b/kms/kms.go index 5055790d..399aee5a 100644 --- a/kms/kms.go +++ b/kms/kms.go @@ -6,6 +6,7 @@ import ( "errors" "os" "strings" + "sync" "time" "github.com/drakkan/sftpgo/utils" @@ -144,11 +145,15 @@ func (c *Configuration) getSecretProvider(base baseSecret) SecretProvider { // Secret defines the struct used to store confidential data type Secret struct { + sync.RWMutex provider SecretProvider } // MarshalJSON return the JSON encoding of the Secret object func (s *Secret) MarshalJSON() ([]byte, error) { + s.RLock() + defer s.RUnlock() + return json.Marshal(&baseSecret{ Status: s.provider.GetStatus(), Payload: s.provider.GetPayload(), @@ -161,6 +166,9 @@ func (s *Secret) MarshalJSON() ([]byte, error) { // UnmarshalJSON parses the JSON-encoded data and stores the result // in the Secret object func (s *Secret) UnmarshalJSON(data []byte) error { + s.Lock() + defer s.Unlock() + baseSecret := baseSecret{} err := json.Unmarshal(data, &baseSecret) if err != nil { @@ -191,6 +199,9 @@ func (s *Secret) UnmarshalJSON(data []byte) error { // Clone returns a copy of the secret object func (s *Secret) Clone() *Secret { + s.RLock() + defer s.RUnlock() + baseSecret := baseSecret{ Status: s.provider.GetStatus(), Payload: s.provider.GetPayload(), @@ -227,11 +238,17 @@ func (s *Secret) Clone() *Secret { // This isn't a pointer receiver because we don't want to pass // a pointer to html template func (s *Secret) IsEncrypted() bool { + s.RLock() + defer s.RUnlock() + return s.provider.IsEncrypted() } // IsPlain returns true if the secret is in plain text func (s *Secret) IsPlain() bool { + s.RLock() + defer s.RUnlock() + return s.provider.GetStatus() == SecretStatusPlain } @@ -239,56 +256,89 @@ func (s *Secret) IsPlain() bool { // This is an utility method, we update the secret for an existing user // if it is empty or plain func (s *Secret) IsNotPlainAndNotEmpty() bool { + s.RLock() + defer s.RUnlock() + return !s.IsPlain() && !s.IsEmpty() } // IsRedacted returns true if the secret is redacted func (s *Secret) IsRedacted() bool { + s.RLock() + defer s.RUnlock() + return s.provider.GetStatus() == SecretStatusRedacted } // GetPayload returns the secret payload func (s *Secret) GetPayload() string { + s.RLock() + defer s.RUnlock() + return s.provider.GetPayload() } // GetAdditionalData returns the secret additional data func (s *Secret) GetAdditionalData() string { + s.RLock() + defer s.RUnlock() + return s.provider.GetAdditionalData() } // GetStatus returns the secret status func (s *Secret) GetStatus() SecretStatus { + s.RLock() + defer s.RUnlock() + return s.provider.GetStatus() } // GetKey returns the secret key func (s *Secret) GetKey() string { + s.RLock() + defer s.RUnlock() + return s.provider.GetKey() } // GetMode returns the secret mode func (s *Secret) GetMode() int { + s.RLock() + defer s.RUnlock() + return s.provider.GetMode() } // SetAdditionalData sets the given additional data func (s *Secret) SetAdditionalData(value string) { + s.Lock() + defer s.Unlock() + s.provider.SetAdditionalData(value) } // SetStatus sets the status for this secret func (s *Secret) SetStatus(value SecretStatus) { + s.Lock() + defer s.Unlock() + s.provider.SetStatus(value) } // SetKey sets the key for this secret func (s *Secret) SetKey(value string) { + s.Lock() + defer s.Unlock() + s.provider.SetKey(value) } // IsEmpty returns true if all fields are empty func (s *Secret) IsEmpty() bool { + s.RLock() + defer s.RUnlock() + if s.provider.GetStatus() != "" { return false } @@ -306,6 +356,9 @@ func (s *Secret) IsEmpty() bool { // IsValid returns true if the secret is not empty and valid func (s *Secret) IsValid() bool { + s.RLock() + defer s.RUnlock() + if !s.IsValidInput() { return false } @@ -325,6 +378,9 @@ func (s *Secret) IsValid() bool { // IsValidInput returns true if the secret is a valid user input func (s *Secret) IsValidInput() bool { + s.RLock() + defer s.RUnlock() + if !utils.IsStringInSlice(s.provider.GetStatus(), validSecretStatuses) { return false } @@ -336,16 +392,25 @@ func (s *Secret) IsValidInput() bool { // Hide hides info to decrypt data func (s *Secret) Hide() { + s.Lock() + defer s.Unlock() + s.provider.SetKey("") s.provider.SetAdditionalData("") } // Encrypt encrypts a plain text Secret object func (s *Secret) Encrypt() error { + s.Lock() + defer s.Unlock() + return s.provider.Encrypt() } // Decrypt decrypts a Secret object func (s *Secret) Decrypt() error { + s.Lock() + defer s.Unlock() + return s.provider.Decrypt() } diff --git a/vfs/filesystem.go b/vfs/filesystem.go index edceb129..9bbdc670 100644 --- a/vfs/filesystem.go +++ b/vfs/filesystem.go @@ -48,6 +48,30 @@ func (f *Filesystem) SetEmptySecretsIfNil() { } } +// SetNilSecretsIfEmpty set the secrets to nil if empty. +// This is useful before rendering as JSON so the empty fields +// will not be serialized. +func (f *Filesystem) SetNilSecretsIfEmpty() { + if f.S3Config.AccessSecret != nil && f.S3Config.AccessSecret.IsEmpty() { + f.S3Config.AccessSecret = nil + } + if f.GCSConfig.Credentials != nil && f.GCSConfig.Credentials.IsEmpty() { + f.GCSConfig.Credentials = nil + } + if f.AzBlobConfig.AccountKey != nil && f.AzBlobConfig.AccountKey.IsEmpty() { + f.AzBlobConfig.AccountKey = nil + } + if f.CryptConfig.Passphrase != nil && f.CryptConfig.Passphrase.IsEmpty() { + f.CryptConfig.Passphrase = nil + } + if f.SFTPConfig.Password != nil && f.SFTPConfig.Password.IsEmpty() { + f.SFTPConfig.Password = nil + } + if f.SFTPConfig.PrivateKey != nil && f.SFTPConfig.PrivateKey.IsEmpty() { + f.SFTPConfig.PrivateKey = nil + } +} + // GetACopy returns a copy func (f *Filesystem) GetACopy() Filesystem { f.SetEmptySecretsIfNil() diff --git a/vfs/folder.go b/vfs/folder.go index 4ddacb4a..c09e7640 100644 --- a/vfs/folder.go +++ b/vfs/folder.go @@ -79,8 +79,8 @@ func (v *BaseVirtualFolder) IsLocalOrLocalCrypted() bool { return v.FsConfig.Provider == LocalFilesystemProvider || v.FsConfig.Provider == CryptedFilesystemProvider } -// HideConfidentialData hides folder confidential data -func (v *BaseVirtualFolder) HideConfidentialData() { +// hideConfidentialData hides folder confidential data +func (v *BaseVirtualFolder) hideConfidentialData() { switch v.FsConfig.Provider { case S3FilesystemProvider: v.FsConfig.S3Config.AccessSecret.Hide() @@ -96,6 +96,14 @@ func (v *BaseVirtualFolder) HideConfidentialData() { } } +// PrepareForRendering prepares a folder for rendering. +// It hides confidential data and set to nil the empty secrets +// so they are not serialized +func (v *BaseVirtualFolder) PrepareForRendering() { + v.hideConfidentialData() + v.FsConfig.SetEmptySecretsIfNil() +} + // HasRedactedSecret returns true if the folder has a redacted secret func (v *BaseVirtualFolder) HasRedactedSecret() bool { switch v.FsConfig.Provider { diff --git a/webdavd/internal_test.go b/webdavd/internal_test.go index ae9afe1d..434b467d 100644 --- a/webdavd/internal_test.go +++ b/webdavd/internal_test.go @@ -1331,7 +1331,6 @@ func TestUserCacheIsolation(t *testing.T) { if assert.True(t, ok) { cachedUser := result.(*dataprovider.CachedUser).User assert.Equal(t, vfs.LocalFilesystemProvider, cachedUser.FsConfig.Provider) - // FIXME: should we really allow to modify the cached users concurrently????? assert.False(t, cachedUser.FsConfig.S3Config.AccessSecret.IsEncrypted()) }