From 2bcf05ca4531a5066e46d77caea30d9314e3c8fc Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Fri, 26 Apr 2024 16:17:24 +0200 Subject: [PATCH] refactor for secrets management in API and private key handling in SFTPFs Signed-off-by: Nicola Murino --- internal/httpd/api_folder.go | 5 +---- internal/httpd/api_group.go | 15 +------------- internal/httpd/api_user.go | 40 +++++++++++++++--------------------- internal/httpd/webadmin.go | 17 +++------------ internal/vfs/sftpfs.go | 40 +++++++++++++++++++++++------------- 5 files changed, 47 insertions(+), 70 deletions(-) diff --git a/internal/httpd/api_folder.go b/internal/httpd/api_folder.go index a6ec8489..debc8483 100644 --- a/internal/httpd/api_folder.go +++ b/internal/httpd/api_folder.go @@ -89,10 +89,7 @@ func updateFolder(w http.ResponseWriter, r *http.Request) { updatedFolder.ID = folder.ID updatedFolder.Name = folder.Name updatedFolder.FsConfig.SetEmptySecretsIfNil() - updateEncryptedSecrets(&updatedFolder.FsConfig, folder.FsConfig.S3Config.AccessSecret, folder.FsConfig.AzBlobConfig.AccountKey, - folder.FsConfig.AzBlobConfig.SASURL, folder.FsConfig.GCSConfig.Credentials, folder.FsConfig.CryptConfig.Passphrase, - folder.FsConfig.SFTPConfig.Password, folder.FsConfig.SFTPConfig.PrivateKey, folder.FsConfig.SFTPConfig.KeyPassphrase, - folder.FsConfig.HTTPConfig.Password, folder.FsConfig.HTTPConfig.APIKey) + updateEncryptedSecrets(&updatedFolder.FsConfig, &folder.FsConfig) err = dataprovider.UpdateFolder(&updatedFolder, folder.Users, folder.Groups, claims.Username, util.GetIPFromRemoteAddress(r.RemoteAddr), claims.Role) diff --git a/internal/httpd/api_group.go b/internal/httpd/api_group.go index 416f6bdc..125319d5 100644 --- a/internal/httpd/api_group.go +++ b/internal/httpd/api_group.go @@ -79,17 +79,6 @@ func updateGroup(w http.ResponseWriter, r *http.Request) { return } - currentS3AccessSecret := group.UserSettings.FsConfig.S3Config.AccessSecret - currentAzAccountKey := group.UserSettings.FsConfig.AzBlobConfig.AccountKey - currentAzSASUrl := group.UserSettings.FsConfig.AzBlobConfig.SASURL - currentGCSCredentials := group.UserSettings.FsConfig.GCSConfig.Credentials - currentCryptoPassphrase := group.UserSettings.FsConfig.CryptConfig.Passphrase - currentSFTPPassword := group.UserSettings.FsConfig.SFTPConfig.Password - currentSFTPKey := group.UserSettings.FsConfig.SFTPConfig.PrivateKey - currentSFTPKeyPassphrase := group.UserSettings.FsConfig.SFTPConfig.KeyPassphrase - currentHTTPPassword := group.UserSettings.FsConfig.HTTPConfig.Password - currentHTTPAPIKey := group.UserSettings.FsConfig.HTTPConfig.APIKey - var updatedGroup dataprovider.Group err = render.DecodeJSON(r.Body, &updatedGroup) if err != nil { @@ -99,9 +88,7 @@ func updateGroup(w http.ResponseWriter, r *http.Request) { updatedGroup.ID = group.ID updatedGroup.Name = group.Name updatedGroup.UserSettings.FsConfig.SetEmptySecretsIfNil() - updateEncryptedSecrets(&updatedGroup.UserSettings.FsConfig, currentS3AccessSecret, currentAzAccountKey, currentAzSASUrl, - currentGCSCredentials, currentCryptoPassphrase, currentSFTPPassword, currentSFTPKey, currentSFTPKeyPassphrase, - currentHTTPPassword, currentHTTPAPIKey) + updateEncryptedSecrets(&updatedGroup.UserSettings.FsConfig, &group.UserSettings.FsConfig) err = dataprovider.UpdateGroup(&updatedGroup, group.Users, claims.Username, util.GetIPFromRemoteAddress(r.RemoteAddr), claims.Role) if err != nil { diff --git a/internal/httpd/api_user.go b/internal/httpd/api_user.go index 551aed11..7b55082a 100644 --- a/internal/httpd/api_user.go +++ b/internal/httpd/api_user.go @@ -27,7 +27,6 @@ import ( "github.com/drakkan/sftpgo/v2/internal/common" "github.com/drakkan/sftpgo/v2/internal/dataprovider" - "github.com/drakkan/sftpgo/v2/internal/kms" "github.com/drakkan/sftpgo/v2/internal/logger" "github.com/drakkan/sftpgo/v2/internal/smtp" "github.com/drakkan/sftpgo/v2/internal/util" @@ -186,10 +185,7 @@ func updateUser(w http.ResponseWriter, r *http.Request) { updatedUser.Filters.TOTPConfig = user.Filters.TOTPConfig updatedUser.LastPasswordChange = user.LastPasswordChange updatedUser.SetEmptySecretsIfNil() - updateEncryptedSecrets(&updatedUser.FsConfig, user.FsConfig.S3Config.AccessSecret, user.FsConfig.AzBlobConfig.AccountKey, - user.FsConfig.AzBlobConfig.SASURL, user.FsConfig.GCSConfig.Credentials, user.FsConfig.CryptConfig.Passphrase, - user.FsConfig.SFTPConfig.Password, user.FsConfig.SFTPConfig.PrivateKey, user.FsConfig.SFTPConfig.KeyPassphrase, - user.FsConfig.HTTPConfig.Password, user.FsConfig.HTTPConfig.APIKey) + updateEncryptedSecrets(&updatedUser.FsConfig, &user.FsConfig) if claims.Role != "" { updatedUser.Role = claims.Role } @@ -275,58 +271,54 @@ func disconnectUser(username, admin, role string) { } } -func updateEncryptedSecrets(fsConfig *vfs.Filesystem, currentS3AccessSecret, currentAzAccountKey, currentAzSASUrl, - currentGCSCredentials, currentCryptoPassphrase, currentSFTPPassword, currentSFTPKey, currentSFTPKeyPassphrase, - currentHTTPPassword, currentHTTPAPIKey *kms.Secret) { +func updateEncryptedSecrets(fsConfig *vfs.Filesystem, currentFsConfig *vfs.Filesystem) { // we use the new access secret if plain or empty, otherwise the old value switch fsConfig.Provider { case sdk.S3FilesystemProvider: if fsConfig.S3Config.AccessSecret.IsNotPlainAndNotEmpty() { - fsConfig.S3Config.AccessSecret = currentS3AccessSecret + fsConfig.S3Config.AccessSecret = currentFsConfig.S3Config.AccessSecret } case sdk.AzureBlobFilesystemProvider: if fsConfig.AzBlobConfig.AccountKey.IsNotPlainAndNotEmpty() { - fsConfig.AzBlobConfig.AccountKey = currentAzAccountKey + fsConfig.AzBlobConfig.AccountKey = currentFsConfig.AzBlobConfig.AccountKey } if fsConfig.AzBlobConfig.SASURL.IsNotPlainAndNotEmpty() { - fsConfig.AzBlobConfig.SASURL = currentAzSASUrl + fsConfig.AzBlobConfig.SASURL = currentFsConfig.AzBlobConfig.SASURL } case sdk.GCSFilesystemProvider: // for GCS credentials will be cleared if we enable automatic credentials // so keep the old credentials here if no new credentials are provided if !fsConfig.GCSConfig.Credentials.IsPlain() { - fsConfig.GCSConfig.Credentials = currentGCSCredentials + fsConfig.GCSConfig.Credentials = currentFsConfig.GCSConfig.Credentials } case sdk.CryptedFilesystemProvider: if fsConfig.CryptConfig.Passphrase.IsNotPlainAndNotEmpty() { - fsConfig.CryptConfig.Passphrase = currentCryptoPassphrase + fsConfig.CryptConfig.Passphrase = currentFsConfig.CryptConfig.Passphrase } case sdk.SFTPFilesystemProvider: - updateSFTPFsEncryptedSecrets(fsConfig, currentSFTPPassword, currentSFTPKey, currentSFTPKeyPassphrase) + updateSFTPFsEncryptedSecrets(fsConfig, currentFsConfig) case sdk.HTTPFilesystemProvider: - updateHTTPFsEncryptedSecrets(fsConfig, currentHTTPPassword, currentHTTPAPIKey) + updateHTTPFsEncryptedSecrets(fsConfig, currentFsConfig) } } -func updateSFTPFsEncryptedSecrets(fsConfig *vfs.Filesystem, currentSFTPPassword, currentSFTPKey, - currentSFTPKeyPassphrase *kms.Secret, -) { +func updateSFTPFsEncryptedSecrets(fsConfig *vfs.Filesystem, currentFsConfig *vfs.Filesystem) { if fsConfig.SFTPConfig.Password.IsNotPlainAndNotEmpty() { - fsConfig.SFTPConfig.Password = currentSFTPPassword + fsConfig.SFTPConfig.Password = currentFsConfig.SFTPConfig.Password } if fsConfig.SFTPConfig.PrivateKey.IsNotPlainAndNotEmpty() { - fsConfig.SFTPConfig.PrivateKey = currentSFTPKey + fsConfig.SFTPConfig.PrivateKey = currentFsConfig.SFTPConfig.PrivateKey } if fsConfig.SFTPConfig.KeyPassphrase.IsNotPlainAndNotEmpty() { - fsConfig.SFTPConfig.KeyPassphrase = currentSFTPKeyPassphrase + fsConfig.SFTPConfig.KeyPassphrase = currentFsConfig.SFTPConfig.KeyPassphrase } } -func updateHTTPFsEncryptedSecrets(fsConfig *vfs.Filesystem, currentHTTPPassword, currentHTTPAPIKey *kms.Secret) { +func updateHTTPFsEncryptedSecrets(fsConfig *vfs.Filesystem, currentFsConfig *vfs.Filesystem) { if fsConfig.HTTPConfig.Password.IsNotPlainAndNotEmpty() { - fsConfig.HTTPConfig.Password = currentHTTPPassword + fsConfig.HTTPConfig.Password = currentFsConfig.HTTPConfig.Password } if fsConfig.HTTPConfig.APIKey.IsNotPlainAndNotEmpty() { - fsConfig.HTTPConfig.APIKey = currentHTTPAPIKey + fsConfig.HTTPConfig.APIKey = currentFsConfig.HTTPConfig.APIKey } } diff --git a/internal/httpd/webadmin.go b/internal/httpd/webadmin.go index 84abbbf8..76a0e6f8 100644 --- a/internal/httpd/webadmin.go +++ b/internal/httpd/webadmin.go @@ -3411,10 +3411,7 @@ func (s *httpdServer) handleWebUpdateUserPost(w http.ResponseWriter, r *http.Req if updatedUser.Password == redactedSecret { updatedUser.Password = user.Password } - updateEncryptedSecrets(&updatedUser.FsConfig, user.FsConfig.S3Config.AccessSecret, user.FsConfig.AzBlobConfig.AccountKey, - user.FsConfig.AzBlobConfig.SASURL, user.FsConfig.GCSConfig.Credentials, user.FsConfig.CryptConfig.Passphrase, - user.FsConfig.SFTPConfig.Password, user.FsConfig.SFTPConfig.PrivateKey, user.FsConfig.SFTPConfig.KeyPassphrase, - user.FsConfig.HTTPConfig.Password, user.FsConfig.HTTPConfig.APIKey) + updateEncryptedSecrets(&updatedUser.FsConfig, &user.FsConfig) updatedUser = getUserFromTemplate(updatedUser, userTemplateFields{ Username: updatedUser.Username, @@ -3556,10 +3553,7 @@ func (s *httpdServer) handleWebUpdateFolderPost(w http.ResponseWriter, r *http.R updatedFolder.Name = folder.Name updatedFolder.FsConfig = fsConfig updatedFolder.FsConfig.SetEmptySecretsIfNil() - updateEncryptedSecrets(&updatedFolder.FsConfig, folder.FsConfig.S3Config.AccessSecret, folder.FsConfig.AzBlobConfig.AccountKey, - folder.FsConfig.AzBlobConfig.SASURL, folder.FsConfig.GCSConfig.Credentials, folder.FsConfig.CryptConfig.Passphrase, - folder.FsConfig.SFTPConfig.Password, folder.FsConfig.SFTPConfig.PrivateKey, folder.FsConfig.SFTPConfig.KeyPassphrase, - folder.FsConfig.HTTPConfig.Password, folder.FsConfig.HTTPConfig.APIKey) + updateEncryptedSecrets(&updatedFolder.FsConfig, &folder.FsConfig) updatedFolder = getFolderFromTemplate(updatedFolder, updatedFolder.Name) @@ -3720,12 +3714,7 @@ func (s *httpdServer) handleWebUpdateGroupPost(w http.ResponseWriter, r *http.Re updatedGroup.Name = group.Name updatedGroup.SetEmptySecretsIfNil() - updateEncryptedSecrets(&updatedGroup.UserSettings.FsConfig, group.UserSettings.FsConfig.S3Config.AccessSecret, - group.UserSettings.FsConfig.AzBlobConfig.AccountKey, group.UserSettings.FsConfig.AzBlobConfig.SASURL, - group.UserSettings.FsConfig.GCSConfig.Credentials, group.UserSettings.FsConfig.CryptConfig.Passphrase, - group.UserSettings.FsConfig.SFTPConfig.Password, group.UserSettings.FsConfig.SFTPConfig.PrivateKey, - group.UserSettings.FsConfig.SFTPConfig.KeyPassphrase, group.UserSettings.FsConfig.HTTPConfig.Password, - group.UserSettings.FsConfig.HTTPConfig.APIKey) + updateEncryptedSecrets(&updatedGroup.UserSettings.FsConfig, &group.UserSettings.FsConfig) err = dataprovider.UpdateGroup(&updatedGroup, group.Users, claims.Username, ipAddr, claims.Role) if err != nil { diff --git a/internal/vfs/sftpfs.go b/internal/vfs/sftpfs.go index b4085708..cc3e502f 100644 --- a/internal/vfs/sftpfs.go +++ b/internal/vfs/sftpfs.go @@ -67,6 +67,27 @@ type SFTPFsConfig struct { PrivateKey *kms.Secret `json:"private_key,omitempty"` KeyPassphrase *kms.Secret `json:"key_passphrase,omitempty"` forbiddenSelfUsernames []string `json:"-"` + signer ssh.Signer +} + +func (c *SFTPFsConfig) populateSigner() error { + if c.PrivateKey.GetPayload() != "" { + signer, err := c.getSigner() + if err != nil { + return fmt.Errorf("sftpfs: unable to parse the private key: %w", err) + } + c.signer = signer + return nil + } + return nil +} + +func (c *SFTPFsConfig) getSigner() (ssh.Signer, error) { + if c.KeyPassphrase.GetPayload() != "" { + return ssh.ParsePrivateKeyWithPassphrase([]byte(c.PrivateKey.GetPayload()), + []byte(c.KeyPassphrase.GetPayload())) + } + return ssh.ParsePrivateKey([]byte(c.PrivateKey.GetPayload())) } // HideConfidentialData hides confidential data @@ -331,6 +352,9 @@ func NewSFTPFs(connectionID, mountPath, localTempDir string, forbiddenSelfUserna return nil, err } } + if err := config.populateSigner(); err != nil { + return nil, err + } config.forbiddenSelfUsernames = forbiddenSelfUsernames sftpFs := &SFTPFs{ connectionID: connectionID, @@ -931,14 +955,6 @@ func (c *sftpConnection) OpenConnection() error { return c.openConnNoLock() } -func (c *sftpConnection) getSigner() (ssh.Signer, error) { - if c.config.KeyPassphrase.GetPayload() != "" { - return ssh.ParsePrivateKeyWithPassphrase([]byte(c.config.PrivateKey.GetPayload()), - []byte(c.config.KeyPassphrase.GetPayload())) - } - return ssh.ParsePrivateKey([]byte(c.config.PrivateKey.GetPayload())) -} - func (c *sftpConnection) openConnNoLock() error { if c.isConnected { logger.Debug(c.logSender, "", "reusing connection") @@ -976,12 +992,8 @@ func (c *sftpConnection) openConnNoLock() error { Timeout: 10 * time.Second, ClientVersion: fmt.Sprintf("SSH-2.0-SFTPGo_%v", version.Get().Version), } - if c.config.PrivateKey.GetPayload() != "" { - signer, err := c.getSigner() - if err != nil { - return fmt.Errorf("sftpfs: unable to parse the private key: %w", err) - } - clientConfig.Auth = append(clientConfig.Auth, ssh.PublicKeys(signer)) + if c.config.signer != nil { + clientConfig.Auth = append(clientConfig.Auth, ssh.PublicKeys(c.config.signer)) } if c.config.Password.GetPayload() != "" { clientConfig.Auth = append(clientConfig.Auth, ssh.Password(c.config.Password.GetPayload()))