From b1ce6eb85bac7e492e0da0b03a39cf0f9ded3cfa Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Mon, 15 Feb 2021 19:38:53 +0100 Subject: [PATCH] web admin: allow to set an empty password for SFTPGo users --- dataprovider/dataprovider.go | 2 +- dataprovider/user.go | 5 ++ docs/howto/README.md | 1 - docs/howto/rest-api-https-auth.md | 81 ---------------------- httpd/httpd_test.go | 107 +++++++++++++++++++++++++++--- httpd/web.go | 6 +- httpdtest/httpdtest.go | 11 ++- templates/user.html | 7 +- 8 files changed, 118 insertions(+), 102 deletions(-) delete mode 100644 docs/howto/rest-api-https-auth.md diff --git a/dataprovider/dataprovider.go b/dataprovider/dataprovider.go index fe4e673c..b7cfaf50 100644 --- a/dataprovider/dataprovider.go +++ b/dataprovider/dataprovider.go @@ -1379,7 +1379,7 @@ func validateBaseParams(user *User) error { } func createUserPasswordHash(user *User) error { - if user.Password != "" && !utils.IsStringPrefixInSlice(user.Password, hashPwdPrefixes) { + if user.Password != "" && !user.IsPasswordHashed() { pwd, err := argon2id.CreateHash(user.Password, argon2Params) if err != nil { return err diff --git a/dataprovider/user.go b/dataprovider/user.go index 83eef167..6ac8be7c 100644 --- a/dataprovider/user.go +++ b/dataprovider/user.go @@ -261,6 +261,11 @@ func (u *User) HideConfidentialData() { } } +// IsPasswordHashed returns true if the password is hashed +func (u *User) IsPasswordHashed() bool { + return utils.IsStringPrefixInSlice(u.Password, hashPwdPrefixes) +} + // SetEmptySecrets sets to empty any user secret func (u *User) SetEmptySecrets() { u.FsConfig.S3Config.AccessSecret = kms.NewEmptySecret() diff --git a/docs/howto/README.md b/docs/howto/README.md index b4cf0288..5ae7dc1d 100644 --- a/docs/howto/README.md +++ b/docs/howto/README.md @@ -3,4 +3,3 @@ Here we collect step-to-step tutorials. SFTPGo users are encouraged to contribute! - [SFTPGo with PostgreSQL data provider and S3 backend](./postgresql-s3.md) -- [Expose Web Admin and REST API over HTTPS and password protected](./rest-api-https-auth.md) diff --git a/docs/howto/rest-api-https-auth.md b/docs/howto/rest-api-https-auth.md deleted file mode 100644 index 5ca99e1e..00000000 --- a/docs/howto/rest-api-https-auth.md +++ /dev/null @@ -1,81 +0,0 @@ -# Expose Web Admin and REST API over HTTPS - -This tutorial shows how to expose the SFTPGo web interface and REST API over HTTPS. - -## Preliminary Note - -Before proceeding further you need to have a SFTPGo instance already configured and running. - -We assume: - -- you are running SFTPGo as service using the dedicated `sftpgo` system user -- the SFTPGo configuration directory is `/etc/sftpgo` -- you are running SFTPGo on Ubuntu 20.04, however this instructions can be easily adapted for other Linux variants. - -## Creation of a Self-Signed Certificate - -For demostration purpose we use a self-signed certificate here. These certificates are easy to make and do not cost money. However, they do not provide all of the security properties that certificates signed by a Public Certificate Authority (CA) aim to provide, you are encouraged to use a certificate signed by a Public CA. - -When creating a new SSL certificate, one needs to specify the duration validity of the same by changing the value 365 (as appearing in the message below) to the preferred number of days. It is important to mention here that the certificate so created stands to auto-expire upon completion of one year. - -```shell -sudo mkdir /etc/sftpgo/ssl -sudo openssl req -x509 -nodes -days 365 -newkey rsa:2048 -keyout /etc/sftpgo/ssl/sftpgo.key -out /etc/sftpgo/ssl/sftpgo.crt -``` - -The above command is rather versatile, and lets you create both the self-signed SSL certificate and the server key to safeguard it, in addition to placing both of these into the `etc/sftpgo/ssl` directory. Answer to the questions to create the certificate and the key for HTTPS. - -Assign the proper permissions to the generated certificates. - -```shell -sudo chown -R sftpgo:sftpgo /etc/sftpgo/ssl -``` - -## HTTPS Setup - -Open the SFTPGo configuration. - -```shell -sudo vi /etc/sftpgo/sftpgo.json -``` - -Search for the `httpd` section and change it as follow. - -```json - "httpd": { - "bindings": [ - { - "port": 8080, - "address": "", - "enable_web_admin": true, - "enable_https": false, - "client_auth_type": 0 - } - ], - "templates_path": "/usr/share/sftpgo/templates", - "static_files_path": "/usr/share/sftpgo/static", - "backups_path": "/srv/sftpgo/backups", - "certificate_file": "/etc/sftpgo/ssl/sftpgo.crt", - "certificate_key_file": "/etc/sftpgo/ssl/sftpgo.key", - "ca_certificates": [], - "ca_revocation_lists": [] - }, -``` - -The configuration keys `certificate_file` and `certificate_key_file` point to the certificate and key we previously created. Setting an empty `address` means that the service will listen on all available network interfaces. - -Now restart the SFTPGo service to apply the changes. - -```shell -sudo systemctl restart sftpgo -``` - -You are done! Now SFTPGo web admin and REST API are exposed over HTTPS. - -You can easily replace the self-signed certificate used here with a properly signed certificate. - -The certificate could frequently change if you use something like [let's encrypt](https://letsencrypt.org/). SFTPGo allows hot-certificate reloading using the following command. - -```shell -sudo systemctl reload sftpgo -``` diff --git a/httpd/httpd_test.go b/httpd/httpd_test.go index 18f25072..79284747 100644 --- a/httpd/httpd_test.go +++ b/httpd/httpd_test.go @@ -115,6 +115,7 @@ AAAEA0E24gi8ab/XRSvJ85TGZJMe6HVmwxSG4ExPfTMwwe2n5EHjI1NnP2Yc6RrDBSJs11 6aKNVXcSsx4vFZQGUI3+AAAACW5pY29sYUBwMQECAwQ= -----END OPENSSH PRIVATE KEY-----` sftpPkeyFingerprint = "SHA256:QVQ06XHZZbYZzqfrsZcf3Yozy2WTnqQPeLOkcJCdbP0" + redactedSecret = "[**redacted**]" ) var ( @@ -899,20 +900,62 @@ func TestAddUserInvalidVirtualFolders(t *testing.T) { func TestUserPublicKey(t *testing.T) { u := getTestUser() + u.Password = "" invalidPubKey := "invalid" - validPubKey := "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQC03jj0D+djk7pxIf/0OhrxrchJTRZklofJ1NoIu4752Sq02mdXmarMVsqJ1cAjV5LBVy3D1F5U6XW4rppkXeVtd04Pxb09ehtH0pRRPaoHHlALiJt8CoMpbKYMA8b3KXPPriGxgGomvtU2T2RMURSwOZbMtpsugfjYSWenyYX+VORYhylWnSXL961LTyC21ehd6d6QnW9G7E5hYMITMY9TuQZz3bROYzXiTsgN0+g6Hn7exFQp50p45StUMfV/SftCMdCxlxuyGny2CrN/vfjO7xxOo2uv7q1qm10Q46KPWJQv+pgZ/OfL+EDjy07n5QVSKHlbx+2nT4Q0EgOSQaCTYwn3YjtABfIxWwgAFdyj6YlPulCL22qU4MYhDcA6PSBwDdf8hvxBfvsiHdM+JcSHvv8/VeJhk6CmnZxGY0fxBupov27z3yEO8nAg8k+6PaUiW1MSUfuGMF/ktB8LOstXsEPXSszuyXiOv4DaryOXUiSn7bmRqKcEFlJusO6aZP0= nicola@p1" u.PublicKeys = []string{invalidPubKey} _, _, err := httpdtest.AddUser(u, http.StatusBadRequest) assert.NoError(t, err) - u.PublicKeys = []string{validPubKey} + u.PublicKeys = []string{testPubKey} user, _, err := httpdtest.AddUser(u, http.StatusCreated) assert.NoError(t, err) - user.PublicKeys = []string{validPubKey, invalidPubKey} + + dbUser, err := dataprovider.UserExists(u.Username) + assert.NoError(t, err) + assert.Empty(t, dbUser.Password) + assert.False(t, dbUser.IsPasswordHashed()) + + user.PublicKeys = []string{testPubKey, invalidPubKey} _, _, err = httpdtest.UpdateUser(user, http.StatusBadRequest, "") assert.NoError(t, err) - user.PublicKeys = []string{validPubKey, validPubKey, validPubKey} + user.PublicKeys = []string{testPubKey, testPubKey, testPubKey} + user.Password = defaultPassword _, _, err = httpdtest.UpdateUser(user, http.StatusOK, "") assert.NoError(t, err) + + dbUser, err = dataprovider.UserExists(u.Username) + assert.NoError(t, err) + assert.NotEmpty(t, dbUser.Password) + assert.True(t, dbUser.IsPasswordHashed()) + + _, err = httpdtest.RemoveUser(user, http.StatusOK) + assert.NoError(t, err) +} + +func TestUpdateUserEmptyPassword(t *testing.T) { + u := getTestUser() + u.PublicKeys = []string{testPubKey} + user, _, err := httpdtest.AddUser(u, http.StatusCreated) + assert.NoError(t, err) + + // the password is not empty + dbUser, err := dataprovider.UserExists(u.Username) + assert.NoError(t, err) + assert.NotEmpty(t, dbUser.Password) + assert.True(t, dbUser.IsPasswordHashed()) + // now update the user and set an empty password + customUser := make(map[string]interface{}) + customUser["password"] = "" + asJSON, err := json.Marshal(customUser) + assert.NoError(t, err) + userNoPwd, _, err := httpdtest.UpdateUserWithJSON(user, http.StatusOK, "", asJSON) + assert.NoError(t, err) + assert.Equal(t, user, userNoPwd) // the password is hidden so the user must be equal + // check the password within the data provider + dbUser, err = dataprovider.UserExists(u.Username) + assert.NoError(t, err) + assert.Empty(t, dbUser.Password) + assert.False(t, dbUser.IsPasswordHashed()) + _, err = httpdtest.RemoveUser(user, http.StatusOK) assert.NoError(t, err) } @@ -4634,6 +4677,11 @@ func TestWebUserAddMock(t *testing.T) { req.Header.Set("Content-Type", contentType) rr = executeRequest(req) checkResponseCode(t, http.StatusSeeOther, rr) + + dbUser, err := dataprovider.UserExists(user.Username) + assert.NoError(t, err) + assert.NotEmpty(t, dbUser.Password) + assert.True(t, dbUser.IsPasswordHashed()) // the user already exists, was created with the above request b, contentType, _ = getMultipartFormData(form, "", "") req, _ = http.NewRequest(http.MethodPost, webUserPath, &b) @@ -4730,6 +4778,10 @@ func TestWebUserUpdateMock(t *testing.T) { setBearerForReq(req, apiToken) rr := executeRequest(req) checkResponseCode(t, http.StatusCreated, rr) + dbUser, err := dataprovider.UserExists(user.Username) + assert.NoError(t, err) + assert.NotEmpty(t, dbUser.Password) + assert.True(t, dbUser.IsPasswordHashed()) err = render.DecodeJSON(rr.Body, &user) assert.NoError(t, err) user.MaxSessions = 1 @@ -4739,6 +4791,8 @@ func TestWebUserUpdateMock(t *testing.T) { user.AdditionalInfo = "new additional info" form := make(url.Values) form.Set("username", user.Username) + form.Set("password", "") + form.Set("public_keys", testPubKey) form.Set("home_dir", user.HomeDir) form.Set("uid", "0") form.Set("gid", strconv.FormatInt(int64(user.GID), 10)) @@ -4774,6 +4828,36 @@ func TestWebUserUpdateMock(t *testing.T) { req.Header.Set("Content-Type", contentType) rr = executeRequest(req) checkResponseCode(t, http.StatusSeeOther, rr) + dbUser, err = dataprovider.UserExists(user.Username) + assert.NoError(t, err) + assert.Empty(t, dbUser.Password) + assert.False(t, dbUser.IsPasswordHashed()) + + form.Set("password", defaultPassword) + b, contentType, _ = getMultipartFormData(form, "", "") + req, _ = http.NewRequest(http.MethodPost, path.Join(webUserPath, user.Username), &b) + setJWTCookieForReq(req, webToken) + req.Header.Set("Content-Type", contentType) + rr = executeRequest(req) + checkResponseCode(t, http.StatusSeeOther, rr) + dbUser, err = dataprovider.UserExists(user.Username) + assert.NoError(t, err) + assert.NotEmpty(t, dbUser.Password) + assert.True(t, dbUser.IsPasswordHashed()) + prevPwd := dbUser.Password + + form.Set("password", redactedSecret) + b, contentType, _ = getMultipartFormData(form, "", "") + req, _ = http.NewRequest(http.MethodPost, path.Join(webUserPath, user.Username), &b) + setJWTCookieForReq(req, webToken) + req.Header.Set("Content-Type", contentType) + rr = executeRequest(req) + checkResponseCode(t, http.StatusSeeOther, rr) + dbUser, err = dataprovider.UserExists(user.Username) + assert.NoError(t, err) + assert.NotEmpty(t, dbUser.Password) + assert.True(t, dbUser.IsPasswordHashed()) + assert.Equal(t, prevPwd, dbUser.Password) req, _ = http.NewRequest(http.MethodGet, path.Join(userPath, user.Username), nil) setBearerForReq(req, apiToken) @@ -5177,6 +5261,7 @@ func TestWebUserS3Mock(t *testing.T) { form := make(url.Values) form.Set(csrfFormToken, csrfToken) form.Set("username", user.Username) + form.Set("password", redactedSecret) form.Set("home_dir", user.HomeDir) form.Set("uid", "0") form.Set("gid", strconv.FormatInt(int64(user.GID), 10)) @@ -5249,7 +5334,7 @@ func TestWebUserS3Mock(t *testing.T) { assert.Empty(t, updateUser.FsConfig.S3Config.AccessSecret.GetKey()) assert.Empty(t, updateUser.FsConfig.S3Config.AccessSecret.GetAdditionalData()) // now check that a redacted password is not saved - form.Set("s3_access_secret", "[**redacted**] ") + form.Set("s3_access_secret", redactedSecret) b, contentType, _ = getMultipartFormData(form, "", "") req, _ = http.NewRequest(http.MethodPost, path.Join(webUserPath, user.Username), &b) setJWTCookieForReq(req, webToken) @@ -5318,6 +5403,7 @@ func TestWebUserGCSMock(t *testing.T) { form := make(url.Values) form.Set(csrfFormToken, csrfToken) form.Set("username", user.Username) + form.Set("password", redactedSecret) form.Set("home_dir", user.HomeDir) form.Set("uid", "0") form.Set("gid", strconv.FormatInt(int64(user.GID), 10)) @@ -5420,6 +5506,7 @@ func TestWebUserAzureBlobMock(t *testing.T) { form := make(url.Values) form.Set(csrfFormToken, csrfToken) form.Set("username", user.Username) + form.Set("password", redactedSecret) form.Set("home_dir", user.HomeDir) form.Set("uid", "0") form.Set("gid", strconv.FormatInt(int64(user.GID), 10)) @@ -5491,7 +5578,7 @@ func TestWebUserAzureBlobMock(t *testing.T) { assert.Empty(t, updateUser.FsConfig.AzBlobConfig.AccountKey.GetKey()) assert.Empty(t, updateUser.FsConfig.AzBlobConfig.AccountKey.GetAdditionalData()) // now check that a redacted password is not saved - form.Set("az_account_key", "[**redacted**] ") + form.Set("az_account_key", redactedSecret+" ") b, contentType, _ = getMultipartFormData(form, "", "") req, _ = http.NewRequest(http.MethodPost, path.Join(webUserPath, user.Username), &b) setJWTCookieForReq(req, webToken) @@ -5535,6 +5622,7 @@ func TestWebUserCryptMock(t *testing.T) { form := make(url.Values) form.Set(csrfFormToken, csrfToken) form.Set("username", user.Username) + form.Set("password", redactedSecret) form.Set("home_dir", user.HomeDir) form.Set("uid", "0") form.Set("gid", strconv.FormatInt(int64(user.GID), 10)) @@ -5582,7 +5670,7 @@ func TestWebUserCryptMock(t *testing.T) { assert.Empty(t, updateUser.FsConfig.CryptConfig.Passphrase.GetKey()) assert.Empty(t, updateUser.FsConfig.CryptConfig.Passphrase.GetAdditionalData()) // now check that a redacted password is not saved - form.Set("crypt_passphrase", "[**redacted**] ") + form.Set("crypt_passphrase", redactedSecret+" ") b, contentType, _ = getMultipartFormData(form, "", "") req, _ = http.NewRequest(http.MethodPost, path.Join(webUserPath, user.Username), &b) setJWTCookieForReq(req, webToken) @@ -5631,6 +5719,7 @@ func TestWebUserSFTPFsMock(t *testing.T) { form := make(url.Values) form.Set(csrfFormToken, csrfToken) form.Set("username", user.Username) + form.Set("password", redactedSecret) form.Set("home_dir", user.HomeDir) form.Set("uid", "0") form.Set("gid", strconv.FormatInt(int64(user.GID), 10)) @@ -5692,8 +5781,8 @@ func TestWebUserSFTPFsMock(t *testing.T) { assert.Len(t, updateUser.FsConfig.SFTPConfig.Fingerprints, 1) assert.Contains(t, updateUser.FsConfig.SFTPConfig.Fingerprints, sftpPkeyFingerprint) // now check that a redacted credentials are not saved - form.Set("sftp_password", "[**redacted**] ") - form.Set("sftp_private_key", "[**redacted**]") + form.Set("sftp_password", redactedSecret+" ") + form.Set("sftp_private_key", redactedSecret) b, contentType, _ = getMultipartFormData(form, "", "") req, _ = http.NewRequest(http.MethodPost, path.Join(webUserPath, user.Username), &b) setJWTCookieForReq(req, webToken) diff --git a/httpd/web.go b/httpd/web.go index 994c0346..fa75dd02 100644 --- a/httpd/web.go +++ b/httpd/web.go @@ -389,6 +389,9 @@ func renderUserPage(w http.ResponseWriter, r *http.Request, user *dataprovider.U title = "User template" currentURL = webTemplateUser } + if user.Password != "" && user.IsPasswordHashed() && mode == userPageModeUpdate { + user.Password = redactedSecret + } data := userPage{ basePage: getBasePageData(title, currentURL, r), Mode: mode, @@ -1334,6 +1337,7 @@ func handleWebAddUserGet(w http.ResponseWriter, r *http.Request) { if err == nil { user.ID = 0 user.Username = "" + user.Password = "" user.SetEmptySecrets() renderUserPage(w, r, &user, userPageModeAdd, "") } else if _, ok := err.(*dataprovider.RecordNotFoundError); ok { @@ -1401,7 +1405,7 @@ func handleWebUpdateUserPost(w http.ResponseWriter, r *http.Request) { updatedUser.ID = user.ID updatedUser.Username = user.Username updatedUser.SetEmptySecretsIfNil() - if updatedUser.Password == "" { + if updatedUser.Password == redactedSecret { updatedUser.Password = user.Password } updateEncryptedSecrets(&updatedUser, user.FsConfig.S3Config.AccessSecret, user.FsConfig.AzBlobConfig.AccountKey, diff --git a/httpdtest/httpdtest.go b/httpdtest/httpdtest.go index 3d002c91..3ef524fd 100644 --- a/httpdtest/httpdtest.go +++ b/httpdtest/httpdtest.go @@ -152,15 +152,14 @@ func AddUser(user dataprovider.User, expectedStatusCode int) (dataprovider.User, return newUser, body, err } -// UpdateUser updates an existing user and checks the received HTTP Status code against expectedStatusCode. -func UpdateUser(user dataprovider.User, expectedStatusCode int, disconnect string) (dataprovider.User, []byte, error) { +// UpdateUserWithJSON update a user using the provided JSON as POST body +func UpdateUserWithJSON(user dataprovider.User, expectedStatusCode int, disconnect string, userAsJSON []byte) (dataprovider.User, []byte, error) { var newUser dataprovider.User var body []byte url, err := addDisconnectQueryParam(buildURLRelativeToBase(userPath, url.PathEscape(user.Username)), disconnect) if err != nil { return user, body, err } - userAsJSON, _ := json.Marshal(user) resp, err := sendHTTPRequest(http.MethodPut, url.String(), bytes.NewBuffer(userAsJSON), "application/json", getDefaultToken()) if err != nil { @@ -181,6 +180,12 @@ func UpdateUser(user dataprovider.User, expectedStatusCode int, disconnect strin return newUser, body, err } +// UpdateUser updates an existing user and checks the received HTTP Status code against expectedStatusCode. +func UpdateUser(user dataprovider.User, expectedStatusCode int, disconnect string) (dataprovider.User, []byte, error) { + userAsJSON, _ := json.Marshal(user) + return UpdateUserWithJSON(user, expectedStatusCode, disconnect, userAsJSON) +} + // RemoveUser removes an existing user and checks the received HTTP Status code against expectedStatusCode. func RemoveUser(user dataprovider.User, expectedStatusCode int) ([]byte, error) { var body []byte diff --git a/templates/user.html b/templates/user.html index 31309c2e..22b3aaf4 100644 --- a/templates/user.html +++ b/templates/user.html @@ -85,12 +85,7 @@
- - {{if eq .Mode 2}} - - If empty the current password will not be changed - - {{end}} +