Browse Source

Fixes #855

update OpenAPI definition, add test cases, fix lint

Signed-off-by: Nicola Murino <nicola.murino@gmail.com>
Nicola Murino 3 years ago
parent
commit
7329cd804b

+ 1 - 1
Dockerfile.alpine

@@ -1,4 +1,4 @@
-FROM golang:1.18-alpine3.15 AS builder
+FROM golang:1.18-alpine3.16 AS builder
 
 ENV GOFLAGS="-mod=readonly"
 

+ 1 - 1
go.mod

@@ -157,7 +157,7 @@ require (
 	google.golang.org/genproto v0.0.0-20220527130721-00d5c0f3be58 // indirect
 	google.golang.org/grpc v1.46.2 // indirect
 	google.golang.org/protobuf v1.28.0 // indirect
-	gopkg.in/ini.v1 v1.66.4 // indirect
+	gopkg.in/ini.v1 v1.66.5 // indirect
 	gopkg.in/square/go-jose.v2 v2.6.0 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect

+ 2 - 2
go.sum

@@ -1257,8 +1257,8 @@ gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntN
 gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q=
 gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI=
 gopkg.in/inconshreveable/log15.v2 v2.0.0-20180818164646-67afb5ed74ec/go.mod h1:aPpfJ7XW+gOuirDoZ8gHhLh3kZ1B08FtV2bbmy7Jv3s=
-gopkg.in/ini.v1 v1.66.4 h1:SsAcf+mM7mRZo2nJNGt8mZCjG8ZRaNGMURJw7BsIST4=
-gopkg.in/ini.v1 v1.66.4/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
+gopkg.in/ini.v1 v1.66.5 h1:zfiCO0p88Fj4f6NR6KR5WdGMQ02U8vlDnN6HuD2xv5o=
+gopkg.in/ini.v1 v1.66.5/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
 gopkg.in/natefinch/lumberjack.v2 v2.0.0 h1:1Lc07Kr7qY4U2YPouBjpCLxpiyxIVoxqXgkXLknAOE8=
 gopkg.in/natefinch/lumberjack.v2 v2.0.0/go.mod h1:l0ndWWf7gzL7RNwBG7wST/UCcT4T24xpD6X8LsfU/+k=
 gopkg.in/square/go-jose.v2 v2.5.1/go.mod h1:M9dMgbHiYLoDGQrXy7OpJDJWiKiU//h+vD76mk0e1AI=

+ 1 - 1
httpd/api_folder.go

@@ -67,7 +67,7 @@ func updateFolder(w http.ResponseWriter, r *http.Request) {
 	currentCryptoPassphrase := folder.FsConfig.CryptConfig.Passphrase
 	currentSFTPPassword := folder.FsConfig.SFTPConfig.Password
 	currentSFTPKey := folder.FsConfig.SFTPConfig.PrivateKey
-	currentSFTPKeyPassphrase := folder.FsConfig.SFTPConfig.Passphrase
+	currentSFTPKeyPassphrase := folder.FsConfig.SFTPConfig.KeyPassphrase
 
 	folder.FsConfig.S3Config = vfs.S3FsConfig{}
 	folder.FsConfig.AzBlobConfig = vfs.AzBlobFsConfig{}

+ 1 - 1
httpd/api_group.go

@@ -72,7 +72,7 @@ func updateGroup(w http.ResponseWriter, r *http.Request) {
 	currentCryptoPassphrase := group.UserSettings.FsConfig.CryptConfig.Passphrase
 	currentSFTPPassword := group.UserSettings.FsConfig.SFTPConfig.Password
 	currentSFTPKey := group.UserSettings.FsConfig.SFTPConfig.PrivateKey
-	currentSFTPKeyPassphrase := group.UserSettings.FsConfig.SFTPConfig.Passphrase
+	currentSFTPKeyPassphrase := group.UserSettings.FsConfig.SFTPConfig.KeyPassphrase
 
 	group.UserSettings.FsConfig.S3Config = vfs.S3FsConfig{}
 	group.UserSettings.FsConfig.AzBlobConfig = vfs.AzBlobFsConfig{}

+ 3 - 3
httpd/api_user.go

@@ -134,7 +134,7 @@ func updateUser(w http.ResponseWriter, r *http.Request) {
 	currentCryptoPassphrase := user.FsConfig.CryptConfig.Passphrase
 	currentSFTPPassword := user.FsConfig.SFTPConfig.Password
 	currentSFTPKey := user.FsConfig.SFTPConfig.PrivateKey
-	currentSFTPKeyPassphrase := user.FsConfig.SFTPConfig.Passphrase
+	currentSFTPKeyPassphrase := user.FsConfig.SFTPConfig.KeyPassphrase
 
 	user.Permissions = make(map[string][]string)
 	user.FsConfig.S3Config = vfs.S3FsConfig{}
@@ -263,8 +263,8 @@ func updateEncryptedSecrets(fsConfig *vfs.Filesystem, currentS3AccessSecret, cur
 		if fsConfig.SFTPConfig.PrivateKey.IsNotPlainAndNotEmpty() {
 			fsConfig.SFTPConfig.PrivateKey = currentSFTPKey
 		}
-		if fsConfig.SFTPConfig.Passphrase.IsNotPlainAndNotEmpty() {
-			fsConfig.SFTPConfig.Passphrase = currentSFTPKeyPassphrase
+		if fsConfig.SFTPConfig.KeyPassphrase.IsNotPlainAndNotEmpty() {
+			fsConfig.SFTPConfig.KeyPassphrase = currentSFTPKeyPassphrase
 		}
 	}
 }

+ 78 - 3
httpd/httpd_test.go

@@ -202,9 +202,21 @@ AAAEA0E24gi8ab/XRSvJ85TGZJMe6HVmwxSG4ExPfTMwwe2n5EHjI1NnP2Yc6RrDBSJs11
 6aKNVXcSsx4vFZQGUI3+AAAACW5pY29sYUBwMQECAwQ=
 -----END OPENSSH PRIVATE KEY-----`
 	sftpPkeyFingerprint = "SHA256:QVQ06XHZZbYZzqfrsZcf3Yozy2WTnqQPeLOkcJCdbP0"
-	redactedSecret      = "[**redacted**]"
-	osWindows           = "windows"
-	oidcMockAddr        = "127.0.0.1:11111"
+	// password protected private key
+	testPrivateKeyPwd = `-----BEGIN OPENSSH PRIVATE KEY-----
+b3BlbnNzaC1rZXktdjEAAAAACmFlczI1Ni1jdHIAAAAGYmNyeXB0AAAAGAAAABAvfwQQcs
++PyMsCLTNFcKiQAAAAEAAAAAEAAAAzAAAAC3NzaC1lZDI1NTE5AAAAILqltfCL7IPuIQ2q
++8w23flfgskjIlKViEwMfjJR4mrbAAAAkHp5xgG8J1XW90M/fT59ZUQht8sZzzP17rEKlX
+waYKvLzDxkPK6LFIYs55W1EX1eVt/2Maq+zQ7k2SOUmhPNknsUOlPV2gytX3uIYvXF7u2F
+FTBIJuzZ+UQ14wFbraunliE9yye9DajVG1kz2cz2wVgXUbee+gp5NyFVvln+TcTxXwMsWD
+qwlk5iw/jQekxThg==
+-----END OPENSSH PRIVATE KEY-----
+`
+	testPubKeyPwd  = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAILqltfCL7IPuIQ2q+8w23flfgskjIlKViEwMfjJR4mrb"
+	privateKeyPwd  = "password"
+	redactedSecret = "[**redacted**]"
+	osWindows      = "windows"
+	oidcMockAddr   = "127.0.0.1:11111"
 )
 
 var (
@@ -13136,6 +13148,58 @@ func TestWebUploadSFTP(t *testing.T) {
 	assert.NoError(t, err)
 }
 
+func TestWebAPISFTPPasswordProtectedPrivateKey(t *testing.T) {
+	u := getTestUser()
+	u.Password = ""
+	u.PublicKeys = []string{testPubKeyPwd}
+	localUser, _, err := httpdtest.AddUser(u, http.StatusCreated)
+	assert.NoError(t, err)
+	u = getTestSFTPUser()
+	u.FsConfig.SFTPConfig.Password = kms.NewEmptySecret()
+	u.FsConfig.SFTPConfig.PrivateKey = kms.NewPlainSecret(testPrivateKeyPwd)
+	u.FsConfig.SFTPConfig.KeyPassphrase = kms.NewPlainSecret(privateKeyPwd)
+	u.HomeDir = filepath.Join(os.TempDir(), u.Username)
+	sftpUser, _, err := httpdtest.AddUser(u, http.StatusCreated)
+	assert.NoError(t, err)
+
+	webToken, err := getJWTWebClientTokenFromTestServer(sftpUser.Username, defaultPassword)
+	assert.NoError(t, err)
+	req, err := http.NewRequest(http.MethodGet, webClientFilesPath, nil)
+	assert.NoError(t, err)
+	setJWTCookieForReq(req, webToken)
+	rr := executeRequest(req)
+	checkResponseCode(t, http.StatusOK, rr)
+	// update the user, the key must be preserved
+	assert.Equal(t, sdkkms.SecretStatusSecretBox, sftpUser.FsConfig.SFTPConfig.KeyPassphrase.GetStatus())
+	_, _, err = httpdtest.UpdateUser(sftpUser, http.StatusOK, "")
+	assert.NoError(t, err)
+	req, err = http.NewRequest(http.MethodGet, webClientFilesPath, nil)
+	assert.NoError(t, err)
+	setJWTCookieForReq(req, webToken)
+	rr = executeRequest(req)
+	checkResponseCode(t, http.StatusOK, rr)
+	// using a wrong passphrase or no passphrase should fail
+	sftpUser.FsConfig.SFTPConfig.KeyPassphrase = kms.NewPlainSecret("wrong")
+	_, _, err = httpdtest.UpdateUser(sftpUser, http.StatusOK, "")
+	assert.NoError(t, err)
+	_, err = getJWTWebClientTokenFromTestServer(sftpUser.Username, defaultPassword)
+	assert.Error(t, err)
+	sftpUser.FsConfig.SFTPConfig.KeyPassphrase = kms.NewEmptySecret()
+	_, _, err = httpdtest.UpdateUser(sftpUser, http.StatusOK, "")
+	assert.NoError(t, err)
+	_, err = getJWTWebClientTokenFromTestServer(sftpUser.Username, defaultPassword)
+	assert.Error(t, err)
+
+	_, err = httpdtest.RemoveUser(sftpUser, http.StatusOK)
+	assert.NoError(t, err)
+	_, err = httpdtest.RemoveUser(localUser, http.StatusOK)
+	assert.NoError(t, err)
+	err = os.RemoveAll(localUser.GetHomeDir())
+	assert.NoError(t, err)
+	err = os.RemoveAll(sftpUser.GetHomeDir())
+	assert.NoError(t, err)
+}
+
 func TestWebUploadMultipartFormReadError(t *testing.T) {
 	user, _, err := httpdtest.AddUser(getTestUser(), http.StatusCreated)
 	assert.NoError(t, err)
@@ -17198,6 +17262,7 @@ func TestWebUserSFTPFsMock(t *testing.T) {
 	user.FsConfig.SFTPConfig.Username = "sftpuser"
 	user.FsConfig.SFTPConfig.Password = kms.NewPlainSecret("pwd")
 	user.FsConfig.SFTPConfig.PrivateKey = kms.NewPlainSecret(sftpPrivateKey)
+	user.FsConfig.SFTPConfig.KeyPassphrase = kms.NewPlainSecret(testPrivateKeyPwd)
 	user.FsConfig.SFTPConfig.Fingerprints = []string{sftpPkeyFingerprint}
 	user.FsConfig.SFTPConfig.Prefix = "/home/sftpuser"
 	user.FsConfig.SFTPConfig.DisableCouncurrentReads = true
@@ -17243,6 +17308,7 @@ func TestWebUserSFTPFsMock(t *testing.T) {
 	form.Set("sftp_username", user.FsConfig.SFTPConfig.Username)
 	form.Set("sftp_password", user.FsConfig.SFTPConfig.Password.GetPayload())
 	form.Set("sftp_private_key", user.FsConfig.SFTPConfig.PrivateKey.GetPayload())
+	form.Set("sftp_key_passphrase", user.FsConfig.SFTPConfig.KeyPassphrase.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")
@@ -17270,6 +17336,10 @@ func TestWebUserSFTPFsMock(t *testing.T) {
 	assert.NotEmpty(t, updateUser.FsConfig.SFTPConfig.PrivateKey.GetPayload())
 	assert.Empty(t, updateUser.FsConfig.SFTPConfig.PrivateKey.GetKey())
 	assert.Empty(t, updateUser.FsConfig.SFTPConfig.PrivateKey.GetAdditionalData())
+	assert.Equal(t, sdkkms.SecretStatusSecretBox, updateUser.FsConfig.SFTPConfig.KeyPassphrase.GetStatus())
+	assert.NotEmpty(t, updateUser.FsConfig.SFTPConfig.KeyPassphrase.GetPayload())
+	assert.Empty(t, updateUser.FsConfig.SFTPConfig.KeyPassphrase.GetKey())
+	assert.Empty(t, updateUser.FsConfig.SFTPConfig.KeyPassphrase.GetAdditionalData())
 	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)
@@ -17280,6 +17350,7 @@ func TestWebUserSFTPFsMock(t *testing.T) {
 	// now check that a redacted credentials are not saved
 	form.Set("sftp_password", redactedSecret+" ")
 	form.Set("sftp_private_key", redactedSecret)
+	form.Set("sftp_key_passphrase", redactedSecret)
 	b, contentType, _ = getMultipartFormData(form, "", "")
 	req, _ = http.NewRequest(http.MethodPost, path.Join(webUserPath, user.Username), &b)
 	setJWTCookieForReq(req, webToken)
@@ -17301,6 +17372,10 @@ func TestWebUserSFTPFsMock(t *testing.T) {
 	assert.Equal(t, updateUser.FsConfig.SFTPConfig.PrivateKey.GetPayload(), lastUpdatedUser.FsConfig.SFTPConfig.PrivateKey.GetPayload())
 	assert.Empty(t, lastUpdatedUser.FsConfig.SFTPConfig.PrivateKey.GetKey())
 	assert.Empty(t, lastUpdatedUser.FsConfig.SFTPConfig.PrivateKey.GetAdditionalData())
+	assert.Equal(t, sdkkms.SecretStatusSecretBox, lastUpdatedUser.FsConfig.SFTPConfig.KeyPassphrase.GetStatus())
+	assert.Equal(t, updateUser.FsConfig.SFTPConfig.KeyPassphrase.GetPayload(), lastUpdatedUser.FsConfig.SFTPConfig.KeyPassphrase.GetPayload())
+	assert.Empty(t, lastUpdatedUser.FsConfig.SFTPConfig.KeyPassphrase.GetKey())
+	assert.Empty(t, lastUpdatedUser.FsConfig.SFTPConfig.KeyPassphrase.GetAdditionalData())
 	req, _ = http.NewRequest(http.MethodDelete, path.Join(userPath, user.Username), nil)
 	setBearerForReq(req, apiToken)
 	rr = executeRequest(req)

+ 5 - 4
httpd/webadmin.go

@@ -1219,7 +1219,7 @@ func getSFTPConfig(r *http.Request) (vfs.SFTPFsConfig, error) {
 	config.Username = r.Form.Get("sftp_username")
 	config.Password = getSecretFromFormField(r, "sftp_password")
 	config.PrivateKey = getSecretFromFormField(r, "sftp_private_key")
-	config.Passphrase = getSecretFromFormField(r, "sftp_passphrase")
+	config.KeyPassphrase = getSecretFromFormField(r, "sftp_key_passphrase")
 	fingerprintsFormValue := r.Form.Get("sftp_fingerprints")
 	config.Fingerprints = getSliceFromDelimitedValues(fingerprintsFormValue, "\n")
 	config.Prefix = r.Form.Get("sftp_prefix")
@@ -2203,7 +2203,7 @@ func (s *httpdServer) handleWebUpdateUserPost(w http.ResponseWriter, r *http.Req
 	}
 	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.Passphrase)
+		user.FsConfig.SFTPConfig.Password, user.FsConfig.SFTPConfig.PrivateKey, user.FsConfig.SFTPConfig.KeyPassphrase)
 
 	updatedUser = getUserFromTemplate(updatedUser, userTemplateFields{
 		Username:   updatedUser.Username,
@@ -2337,7 +2337,7 @@ func (s *httpdServer) handleWebUpdateFolderPost(w http.ResponseWriter, r *http.R
 	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.Passphrase)
+		folder.FsConfig.SFTPConfig.Password, folder.FsConfig.SFTPConfig.PrivateKey, folder.FsConfig.SFTPConfig.KeyPassphrase)
 
 	updatedFolder = getFolderFromTemplate(updatedFolder, updatedFolder.Name)
 
@@ -2501,7 +2501,8 @@ func (s *httpdServer) handleWebUpdateGroupPost(w http.ResponseWriter, r *http.Re
 	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.Passphrase)
+		group.UserSettings.FsConfig.SFTPConfig.Password, group.UserSettings.FsConfig.SFTPConfig.PrivateKey,
+		group.UserSettings.FsConfig.SFTPConfig.KeyPassphrase)
 
 	err = dataprovider.UpdateGroup(&updatedGroup, group.Users, claims.Username, ipAddr)
 	if err != nil {

+ 3 - 0
httpdtest/httpdtest.go

@@ -1521,6 +1521,9 @@ func compareSFTPFsConfig(expected *vfs.Filesystem, actual *vfs.Filesystem) error
 	if err := checkEncryptedSecret(expected.SFTPConfig.PrivateKey, actual.SFTPConfig.PrivateKey); err != nil {
 		return fmt.Errorf("SFTPFs private key mismatch: %v", err)
 	}
+	if err := checkEncryptedSecret(expected.SFTPConfig.KeyPassphrase, actual.SFTPConfig.KeyPassphrase); err != nil {
+		return fmt.Errorf("SFTPFs private key passphrase mismatch: %v", err)
+	}
 	if expected.SFTPConfig.Prefix != actual.SFTPConfig.Prefix {
 		if expected.SFTPConfig.Prefix != "" && actual.SFTPConfig.Prefix != "/" {
 			return errors.New("SFTPFs prefix mismatch")

+ 1 - 1
kms/kms.go

@@ -86,7 +86,7 @@ func NewEmptySecret() *Secret {
 
 // NewPlainSecret stores the give payload in a plain text secret
 func NewPlainSecret(payload string) *Secret {
-	return NewSecret(sdkkms.SecretStatusPlain, payload, "", "")
+	return NewSecret(sdkkms.SecretStatusPlain, strings.TrimSpace(payload), "", "")
 }
 
 // Initialize configures the KMS support

+ 2 - 0
openapi/openapi.yaml

@@ -5035,6 +5035,8 @@ components:
           $ref: '#/components/schemas/Secret'
         private_key:
           $ref: '#/components/schemas/Secret'
+        key_passphrase:
+          $ref: '#/components/schemas/Secret'
         fingerprints:
           type: array
           items:

+ 49 - 0
sftpd/sftpd_test.go

@@ -102,6 +102,18 @@ NbbCNsVroqKlChT5wyPNGS+phi2bPARBno7WSDvshTZ7dAVEP2c9MJW0XwoSevwKlhgSdt
 RLFFQ/5nclJSdzPBOmQouC0OBcMFSrYtMeknJ4VvueVvve5HcHFaEsaMc7ABAGaLYaBQOm
 iixITGvaNZh/tjAAAACW5pY29sYUBwMQE=
 -----END OPENSSH PRIVATE KEY-----`
+	// password protected private key
+	testPrivateKeyPwd = `-----BEGIN OPENSSH PRIVATE KEY-----
+b3BlbnNzaC1rZXktdjEAAAAACmFlczI1Ni1jdHIAAAAGYmNyeXB0AAAAGAAAABAvfwQQcs
++PyMsCLTNFcKiQAAAAEAAAAAEAAAAzAAAAC3NzaC1lZDI1NTE5AAAAILqltfCL7IPuIQ2q
++8w23flfgskjIlKViEwMfjJR4mrbAAAAkHp5xgG8J1XW90M/fT59ZUQht8sZzzP17rEKlX
+waYKvLzDxkPK6LFIYs55W1EX1eVt/2Maq+zQ7k2SOUmhPNknsUOlPV2gytX3uIYvXF7u2F
+FTBIJuzZ+UQ14wFbraunliE9yye9DajVG1kz2cz2wVgXUbee+gp5NyFVvln+TcTxXwMsWD
+qwlk5iw/jQekxThg==
+-----END OPENSSH PRIVATE KEY-----
+`
+	testPubKeyPwd = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAILqltfCL7IPuIQ2q+8w23flfgskjIlKViEwMfjJR4mrb"
+	privateKeyPwd = "password"
 	// test CA user key.
 	// % ssh-keygen -f ca_user_key
 	testCAUserKey = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDF5fcwZHiyixmnE6IlOZJpZhWXoh62gN+yadAA0GJ509SAEaZVLPDP8S5RsE8mUikR3wxynVshxHeqMhrkS+RlNbhSlOXDdNg94yTrq/xF8Z/PgKRInvef74k5i7bAIytza7jERzFJ/ujTEy3537T5k5EYQJ15ZQGuvzynSdv+6o99SjI4jFplyQOZ2QcYbEAmhHm5GgQlIiEFG/RlDtLksOulKZxOY3qPzP0AyQxtZJXn/5vG40aW9LTbwxCJqWlgrkFXMqAAVCbuU5YspwhiXmKt1PsldiXw23oloa4caCKN1jzbFiGuZNXEU2Ebx7JIvjQCPaUYwLjEbkRDxDqN/vmwZqBuKYiuG9Eafx+nFSQkr7QYb5b+mT+/1IFHnmeRGn38731kBqtH7tpzC/t+soRX9p2HtJM+9MYhblO2OqTSPGTlxihWUkyiRBekpAhaiHld16TsG+A3bOJHrojGcX+5g6oGarKGLAMcykL1X+rZqT993Mo6d2Z7q43MOXE= root@p1"
@@ -632,6 +644,43 @@ func TestBasicSFTPFsHandling(t *testing.T) {
 	assert.NoError(t, err)
 }
 
+func TestSFTPFsPasswordProtectedPrivateKey(t *testing.T) {
+	usePubKey := false
+	u := getTestUser(true)
+	u.PublicKeys = []string{testPubKeyPwd}
+	baseUser, _, err := httpdtest.AddUser(u, http.StatusCreated)
+	assert.NoError(t, err)
+	u = getTestSFTPUser(usePubKey)
+	u.FsConfig.SFTPConfig.PrivateKey = kms.NewPlainSecret(testPrivateKeyPwd)
+	u.FsConfig.SFTPConfig.KeyPassphrase = kms.NewPlainSecret(privateKeyPwd)
+	user, _, err := httpdtest.AddUser(u, http.StatusCreated)
+	assert.NoError(t, err)
+	conn, client, err := getSftpClient(user, usePubKey)
+	if assert.NoError(t, err) {
+		defer conn.Close()
+		defer client.Close()
+		err = checkBasicSFTP(client)
+		assert.NoError(t, err)
+	}
+	// update the user, the key must be preserved
+	_, _, err = httpdtest.UpdateUser(user, http.StatusOK, "")
+	assert.NoError(t, err)
+	conn, client, err = getSftpClient(user, usePubKey)
+	if assert.NoError(t, err) {
+		defer conn.Close()
+		defer client.Close()
+		err = checkBasicSFTP(client)
+		assert.NoError(t, err)
+	}
+
+	_, err = httpdtest.RemoveUser(user, http.StatusOK)
+	assert.NoError(t, err)
+	_, err = httpdtest.RemoveUser(baseUser, http.StatusOK)
+	assert.NoError(t, err)
+	err = os.RemoveAll(baseUser.GetHomeDir())
+	assert.NoError(t, err)
+}
+
 func TestSFTPFsEscapeHomeDir(t *testing.T) {
 	usePubKey := true
 	baseUser, _, err := httpdtest.AddUser(getTestUser(usePubKey), http.StatusCreated)

+ 7 - 7
templates/webadmin/fsconfig.html

@@ -419,18 +419,19 @@
         <div class="form-group row fsconfig fsconfig-sftpfs">
             <label for="idSFTPPrivateKey" class="col-sm-2 col-form-label">Private key</label>
             <div class="col-sm-10">
-                <textarea type="password" class="form-control" id="idSFTPPrivateKey" name="sftp_private_key"
+                <textarea class="form-control" id="idSFTPPrivateKey" name="sftp_private_key"
                     rows="3">{{if .SFTPConfig.PrivateKey.IsEncrypted}}{{.RedactedSecret}}{{else}}{{.SFTPConfig.PrivateKey.GetPayload}}{{end}}</textarea>
             </div>
         </div>
 
         <div class="form-group row fsconfig fsconfig-sftpfs">
-            <label for="idSFTPPassphrase" class="col-sm-2 col-form-label">Passphrase</label>
+            <label for="idSFTPPassphrase" class="col-sm-2 col-form-label">Key Passphrase</label>
             <div class="col-sm-10">
-                <input type="text" class="form-control" id="idSFTPPassphrase" name="sftp_passphrase" placeholder=""
-                value="{{if .SFTPConfig.Passphrase.IsEncrypted}}{{.RedactedSecret}}{{else}}{{.SFTPConfig.Passphrase.GetPayload}}{{end}}" maxlength="255" aria-describedby="SFTPPassphraseHelpBlock">
+                <input type="password" class="form-control" id="idSFTPPassphrase" name="sftp_key_passphrase" placeholder=""
+                    value="{{if .SFTPConfig.KeyPassphrase.IsEncrypted}}{{.RedactedSecret}}{{else}}{{.SFTPConfig.KeyPassphrase.GetPayload}}{{end}}"
+                    aria-describedby="SFTPPassphraseHelpBlock">
                 <small id="SFTPPassphraseHelpBlock" class="form-text text-muted">
-                    A passphrase is a word or phrase that protects private key files,its used in case of encrypted private keys.
+                    Passphrase used to protect your private key, if any
                 </small>
             </div>
         </div>
@@ -441,8 +442,7 @@
                 <textarea class="form-control" id="idSFTPFingerprints" name="sftp_fingerprints" rows="3"
                     aria-describedby="SFTPFingerprintsHelpBlock">{{range .SFTPConfig.Fingerprints}}{{.}}&#10;{{end}}</textarea>
                 <small id="SFTPFingerprintsHelpBlock" class="form-text text-muted">
-                    SHA256 fingerprints to validate when connecting to the external SFTP server, one per line. If
-                    empty any host key will be accepted: this is a security risk!
+                    SHA256 fingerprints to validate when connecting to the external SFTP server, one per line. If empty any host key will be accepted: this is a security risk!
                 </small>
             </div>
         </div>

+ 8 - 16
vfs/filesystem.go

@@ -26,7 +26,7 @@ func (f *Filesystem) SetEmptySecrets() {
 	f.CryptConfig.Passphrase = kms.NewEmptySecret()
 	f.SFTPConfig.Password = kms.NewEmptySecret()
 	f.SFTPConfig.PrivateKey = kms.NewEmptySecret()
-	f.SFTPConfig.Passphrase = kms.NewEmptySecret()
+	f.SFTPConfig.KeyPassphrase = kms.NewEmptySecret()
 }
 
 // SetEmptySecretsIfNil sets the secrets to empty if nil
@@ -52,8 +52,8 @@ func (f *Filesystem) SetEmptySecretsIfNil() {
 	if f.SFTPConfig.PrivateKey == nil {
 		f.SFTPConfig.PrivateKey = kms.NewEmptySecret()
 	}
-	if f.SFTPConfig.Passphrase == nil {
-		f.SFTPConfig.Passphrase = kms.NewEmptySecret()
+	if f.SFTPConfig.KeyPassphrase == nil {
+		f.SFTPConfig.KeyPassphrase = kms.NewEmptySecret()
 	}
 }
 
@@ -76,15 +76,7 @@ func (f *Filesystem) SetNilSecretsIfEmpty() {
 	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
-	}
-	if f.SFTPConfig.Passphrase != nil && f.SFTPConfig.Passphrase.IsEmpty() {
-		f.SFTPConfig.Passphrase = nil
-	}
+	f.SFTPConfig.setNilSecretsIfEmpty()
 }
 
 // IsEqual returns true if the fs is equal to other
@@ -198,7 +190,7 @@ func (f *Filesystem) HasRedactedSecret() bool {
 		if f.SFTPConfig.PrivateKey.IsRedacted() {
 			return true
 		}
-		if f.SFTPConfig.Passphrase.IsRedacted() {
+		if f.SFTPConfig.KeyPassphrase.IsRedacted() {
 			return true
 		}
 	}
@@ -284,9 +276,9 @@ func (f *Filesystem) GetACopy() Filesystem {
 				DisableCouncurrentReads: f.SFTPConfig.DisableCouncurrentReads,
 				BufferSize:              f.SFTPConfig.BufferSize,
 			},
-			Password:   f.SFTPConfig.Password.Clone(),
-			PrivateKey: f.SFTPConfig.PrivateKey.Clone(),
-			Passphrase: f.SFTPConfig.Passphrase.Clone(),
+			Password:      f.SFTPConfig.Password.Clone(),
+			PrivateKey:    f.SFTPConfig.PrivateKey.Clone(),
+			KeyPassphrase: f.SFTPConfig.KeyPassphrase.Clone(),
 		},
 	}
 	if len(f.SFTPConfig.Fingerprints) > 0 {

+ 1 - 1
vfs/folder.go

@@ -168,7 +168,7 @@ func (v *BaseVirtualFolder) HasRedactedSecret() bool {
 		if v.FsConfig.SFTPConfig.PrivateKey.IsRedacted() {
 			return true
 		}
-		if v.FsConfig.SFTPConfig.Passphrase.IsRedacted() {
+		if v.FsConfig.SFTPConfig.KeyPassphrase.IsRedacted() {
 			return true
 		}
 	}

+ 37 - 22
vfs/sftpfs.go

@@ -40,8 +40,8 @@ type SFTPFsConfig struct {
 	sdk.BaseSFTPFsConfig
 	Password               *kms.Secret `json:"password,omitempty"`
 	PrivateKey             *kms.Secret `json:"private_key,omitempty"`
+	KeyPassphrase          *kms.Secret `json:"key_passphrase,omitempty"`
 	forbiddenSelfUsernames []string    `json:"-"`
-	Passphrase             *kms.Secret `json:"passphrase,omitempty"`
 }
 
 // HideConfidentialData hides confidential data
@@ -52,8 +52,20 @@ func (c *SFTPFsConfig) HideConfidentialData() {
 	if c.PrivateKey != nil {
 		c.PrivateKey.Hide()
 	}
-	if c.Passphrase != nil {
-		c.Passphrase.Hide()
+	if c.KeyPassphrase != nil {
+		c.KeyPassphrase.Hide()
+	}
+}
+
+func (c *SFTPFsConfig) setNilSecretsIfEmpty() {
+	if c.Password != nil && c.Password.IsEmpty() {
+		c.Password = nil
+	}
+	if c.PrivateKey != nil && c.PrivateKey.IsEmpty() {
+		c.PrivateKey = nil
+	}
+	if c.KeyPassphrase != nil && c.KeyPassphrase.IsEmpty() {
+		c.KeyPassphrase = nil
 	}
 }
 
@@ -86,7 +98,7 @@ func (c *SFTPFsConfig) isEqual(other *SFTPFsConfig) bool {
 	if !c.Password.IsEqual(other.Password) {
 		return false
 	}
-	if !c.Passphrase.IsEqual(other.Passphrase) {
+	if !c.KeyPassphrase.IsEqual(other.KeyPassphrase) {
 		return false
 	}
 	return c.PrivateKey.IsEqual(other.PrivateKey)
@@ -99,8 +111,8 @@ func (c *SFTPFsConfig) setEmptyCredentialsIfNil() {
 	if c.PrivateKey == nil {
 		c.PrivateKey = kms.NewEmptySecret()
 	}
-	if c.Passphrase == nil {
-		c.Passphrase = kms.NewEmptySecret()
+	if c.KeyPassphrase == nil {
+		c.KeyPassphrase = kms.NewEmptySecret()
 	}
 }
 
@@ -147,6 +159,12 @@ func (c *SFTPFsConfig) validateCredentials() error {
 	if !c.PrivateKey.IsEmpty() && !c.PrivateKey.IsValidInput() {
 		return errors.New("invalid private key")
 	}
+	if c.KeyPassphrase.IsEncrypted() && !c.KeyPassphrase.IsValid() {
+		return errors.New("invalid encrypted private key passphrase")
+	}
+	if !c.KeyPassphrase.IsEmpty() && !c.KeyPassphrase.IsValidInput() {
+		return errors.New("invalid private key passphrase")
+	}
 	return nil
 }
 
@@ -167,10 +185,10 @@ func (c *SFTPFsConfig) ValidateAndEncryptCredentials(additionalData string) erro
 			return util.NewValidationError(fmt.Sprintf("could not encrypt SFTP fs private key: %v", err))
 		}
 	}
-	if c.Passphrase.IsPlain() {
-		c.Passphrase.SetAdditionalData(additionalData)
-		if err := c.Passphrase.Encrypt(); err != nil {
-			return err
+	if c.KeyPassphrase.IsPlain() {
+		c.KeyPassphrase.SetAdditionalData(additionalData)
+		if err := c.KeyPassphrase.Encrypt(); err != nil {
+			return util.NewValidationError(fmt.Sprintf("could not encrypt SFTP fs private key passphrase: %v", err))
 		}
 	}
 	return nil
@@ -211,8 +229,8 @@ func NewSFTPFs(connectionID, mountPath, localTempDir string, forbiddenSelfUserna
 			return nil, err
 		}
 	}
-	if !config.Passphrase.IsEmpty() {
-		if err := config.Passphrase.TryDecrypt(); err != nil {
+	if !config.KeyPassphrase.IsEmpty() {
+		if err := config.KeyPassphrase.TryDecrypt(); err != nil {
 			return nil, err
 		}
 	}
@@ -800,18 +818,15 @@ func (fs *SFTPFs) createConnection() error {
 	}
 	if fs.config.PrivateKey.GetPayload() != "" {
 		var signer ssh.Signer
-		if fs.config.Passphrase.GetPayload() != "" {
-			signer, err = ssh.ParsePrivateKeyWithPassphrase([]byte(fs.config.PrivateKey.GetPayload()), []byte(fs.config.Passphrase.GetPayload()))
-			if err != nil {
-				fs.err <- err
-				return err
-			}
+		if fs.config.KeyPassphrase.GetPayload() != "" {
+			signer, err = ssh.ParsePrivateKeyWithPassphrase([]byte(fs.config.PrivateKey.GetPayload()),
+				[]byte(fs.config.KeyPassphrase.GetPayload()))
 		} else {
 			signer, err = ssh.ParsePrivateKey([]byte(fs.config.PrivateKey.GetPayload()))
-			if err != nil {
-				fs.err <- err
-				return err
-			}
+		}
+		if err != nil {
+			fs.err <- err
+			return fmt.Errorf("sftpfs: unable to parse the private key: %w", err)
 		}
 		clientConfig.Auth = append(clientConfig.Auth, ssh.PublicKeys(signer))
 	}