add additional data to MFA secrets and fix pointers management

This commit is contained in:
Nicola Murino 2021-09-05 14:10:12 +02:00
parent b1d54f69d9
commit 59140a6d51
No known key found for this signature in database
GPG key ID: 2F1FB59433D5A8CB
7 changed files with 152 additions and 22 deletions

View file

@ -53,7 +53,7 @@ type TOTPConfig struct {
Secret *kms.Secret `json:"secret,omitempty"`
}
func (c *TOTPConfig) validate() error {
func (c *TOTPConfig) validate(username string) error {
if !c.Enabled {
c.ConfigName = ""
c.Secret = kms.NewEmptySecret()
@ -69,6 +69,7 @@ func (c *TOTPConfig) validate() error {
return util.NewValidationError("totp: secret is mandatory")
}
if c.Secret.IsPlain() {
c.Secret.SetAdditionalData(username)
if err := c.Secret.Encrypt(); err != nil {
return util.NewValidationError(fmt.Sprintf("totp: unable to encrypt secret: %v", err))
}
@ -161,6 +162,7 @@ func (a *Admin) validateRecoveryCodes() error {
return util.NewValidationError("mfa: recovery code cannot be empty")
}
if code.Secret.IsPlain() {
code.Secret.SetAdditionalData(a.Username)
if err := code.Secret.Encrypt(); err != nil {
return util.NewValidationError(fmt.Sprintf("mfa: unable to encrypt recovery code: %v", err))
}
@ -196,7 +198,7 @@ func (a *Admin) validate() error {
if a.hasRedactedSecret() {
return util.NewValidationError("cannot save an admin with a redacted secret")
}
if err := a.Filters.TOTPConfig.validate(); err != nil {
if err := a.Filters.TOTPConfig.validate(a.Username); err != nil {
return err
}
if err := a.validateRecoveryCodes(); err != nil {

View file

@ -1339,7 +1339,7 @@ func validateUserVirtualFolders(user *User) error {
return nil
}
func validateUserTOTPConfig(c *sdk.TOTPConfig) error {
func validateUserTOTPConfig(c *sdk.TOTPConfig, username string) error {
if !c.Enabled {
c.ConfigName = ""
c.Secret = kms.NewEmptySecret()
@ -1356,6 +1356,7 @@ func validateUserTOTPConfig(c *sdk.TOTPConfig) error {
return util.NewValidationError("totp: secret is mandatory")
}
if c.Secret.IsPlain() {
c.Secret.SetAdditionalData(username)
if err := c.Secret.Encrypt(); err != nil {
return util.NewValidationError(fmt.Sprintf("totp: unable to encrypt secret: %v", err))
}
@ -1379,6 +1380,7 @@ func validateUserRecoveryCodes(user *User) error {
return util.NewValidationError("mfa: recovery code cannot be empty")
}
if code.Secret.IsPlain() {
code.Secret.SetAdditionalData(user.Username)
if err := code.Secret.Encrypt(); err != nil {
return util.NewValidationError(fmt.Sprintf("mfa: unable to encrypt recovery code: %v", err))
}
@ -1675,7 +1677,7 @@ func ValidateUser(user *User) error {
if user.hasRedactedSecret() {
return util.NewValidationError("cannot save a user with a redacted secret")
}
if err := validateUserTOTPConfig(&user.Filters.TOTPConfig); err != nil {
if err := validateUserTOTPConfig(&user.Filters.TOTPConfig, user.Username); err != nil {
return err
}
if err := validateUserRecoveryCodes(user); err != nil {

View file

@ -306,20 +306,16 @@ The configuration can be read from JSON, TOML, YAML, HCL, envfile and Java prope
## Binding to privileged ports
On Linux, if you want to use Internet domain privileged ports (port numbers less than 1024) instead of running the SFTPGo service as root user you can set the `cap_net_bind_service` capability on the `sftpgo` binary. To set the capability you need to be root:
On Linux, if you want to use Internet domain privileged ports (port numbers less than 1024) instead of running the SFTPGo service as root user you can set the `cap_net_bind_service` capability on the `sftpgo` binary. To set the capability you can use the following command:
```shell
root@myhost # setcap cap_net_bind_service=+ep /usr/bin/sftpgo
```
Check that the capability is added:
```shell
root@myhost # getcap /usr/bin/sftpgo
$ sudo setcap cap_net_bind_service=+ep /usr/bin/sftpgo
# Check that the capability is added:
$ getcap /usr/bin/sftpgo
/usr/bin/sftpgo cap_net_bind_service=ep
```
Now you can use privileged ports such as 21, 22, 443 etc.. without running the SFTPGo service as root user.
Now you can use privileged ports such as 21, 22, 443 etc.. without running the SFTPGo service as root user. You have to set the `cap_net_bind_service` capability each time you update the `sftpgo` binary.
## Environment variables

View file

@ -95,6 +95,8 @@ func updateAdmin(w http.ResponseWriter, r *http.Request) {
adminID := admin.ID
totpConfig := admin.Filters.TOTPConfig
recoveryCodes := admin.Filters.RecoveryCodes
admin.Filters.TOTPConfig = dataprovider.TOTPConfig{}
admin.Filters.RecoveryCodes = nil
err = render.DecodeJSON(r.Body, &admin)
if err != nil {
sendAPIResponse(w, r, err, "", http.StatusBadRequest)

View file

@ -207,11 +207,12 @@ func saveUserTOTPConfig(username string, r *http.Request, recoveryCodes []sdk.Re
return err
}
currentTOTPSecret := user.Filters.TOTPConfig.Secret
user.Filters.TOTPConfig.Secret = nil
err = render.DecodeJSON(r.Body, &user.Filters.TOTPConfig)
if err != nil {
return util.NewValidationError(fmt.Sprintf("unable to decode JSON body: %v", err))
}
if user.Filters.TOTPConfig.Secret != nil && !user.Filters.TOTPConfig.Secret.IsPlain() {
if user.Filters.TOTPConfig.Secret == nil || !user.Filters.TOTPConfig.Secret.IsPlain() {
user.Filters.TOTPConfig.Secret = currentTOTPSecret
}
if user.CountUnusedRecoveryCodes() < 5 && user.Filters.TOTPConfig.Enabled {
@ -226,6 +227,7 @@ func saveAdminTOTPConfig(username string, r *http.Request, recoveryCodes []sdk.R
return err
}
currentTOTPSecret := admin.Filters.TOTPConfig.Secret
admin.Filters.TOTPConfig.Secret = nil
err = render.DecodeJSON(r.Body, &admin.Filters.TOTPConfig)
if err != nil {
return util.NewValidationError(fmt.Sprintf("unable to decode JSON body: %v", err))
@ -233,7 +235,7 @@ func saveAdminTOTPConfig(username string, r *http.Request, recoveryCodes []sdk.R
if admin.CountUnusedRecoveryCodes() < 5 && admin.Filters.TOTPConfig.Enabled {
admin.Filters.RecoveryCodes = recoveryCodes
}
if admin.Filters.TOTPConfig.Secret != nil && !admin.Filters.TOTPConfig.Secret.IsPlain() {
if admin.Filters.TOTPConfig.Secret == nil || !admin.Filters.TOTPConfig.Secret.IsPlain() {
admin.Filters.TOTPConfig.Secret = currentTOTPSecret
}
return dataprovider.UpdateAdmin(&admin)

View file

@ -160,6 +160,8 @@ func updateUser(w http.ResponseWriter, r *http.Request) {
user.FsConfig.GCSConfig = vfs.GCSFsConfig{}
user.FsConfig.CryptConfig = vfs.CryptFsConfig{}
user.FsConfig.SFTPConfig = vfs.SFTPFsConfig{}
user.Filters.TOTPConfig = sdk.TOTPConfig{}
user.Filters.RecoveryCodes = nil
user.VirtualFolders = nil
err = render.DecodeJSON(r.Body, &user)
if err != nil {

View file

@ -661,6 +661,83 @@ func TestHTTPUserAuthentication(t *testing.T) {
assert.NoError(t, err)
}
func TestPermMFADisabled(t *testing.T) {
u := getTestUser()
u.Filters.WebClient = []string{sdk.WebClientMFADisabled}
user, _, err := httpdtest.AddUser(u, http.StatusCreated)
assert.NoError(t, err)
configName, _, secret, _, err := mfa.GenerateTOTPSecret(mfa.GetAvailableTOTPConfigNames()[0], user.Username)
assert.NoError(t, err)
token, err := getJWTAPIUserTokenFromTestServer(defaultUsername, defaultPassword)
assert.NoError(t, err)
userTOTPConfig := sdk.TOTPConfig{
Enabled: true,
ConfigName: configName,
Secret: kms.NewPlainSecret(secret),
Protocols: []string{common.ProtocolSSH},
}
asJSON, err := json.Marshal(userTOTPConfig)
assert.NoError(t, err)
req, err := http.NewRequest(http.MethodPost, userTOTPSavePath, bytes.NewBuffer(asJSON))
assert.NoError(t, err)
setBearerForReq(req, token)
rr := executeRequest(req)
checkResponseCode(t, http.StatusForbidden, rr) // MFA is disabled for this user
user.Filters.WebClient = []string{sdk.WebClientWriteDisabled}
user, _, err = httpdtest.UpdateUser(user, http.StatusOK, "")
assert.NoError(t, err)
token, err = getJWTAPIUserTokenFromTestServer(defaultUsername, defaultPassword)
assert.NoError(t, err)
req, err = http.NewRequest(http.MethodPost, userTOTPSavePath, bytes.NewBuffer(asJSON))
assert.NoError(t, err)
setBearerForReq(req, token)
rr = executeRequest(req)
checkResponseCode(t, http.StatusOK, rr)
// now we cannot disable MFA for this user
user.Filters.WebClient = []string{sdk.WebClientMFADisabled}
_, resp, err := httpdtest.UpdateUser(user, http.StatusBadRequest, "")
assert.NoError(t, err)
assert.Contains(t, string(resp), "multi-factor authentication cannot be disabled for a user with an active configuration")
saveReq := make(map[string]bool)
saveReq["enabled"] = false
asJSON, err = json.Marshal(saveReq)
assert.NoError(t, err)
req, err = http.NewRequest(http.MethodPost, userTOTPSavePath, bytes.NewBuffer(asJSON))
assert.NoError(t, err)
setBearerForReq(req, token)
rr = executeRequest(req)
checkResponseCode(t, http.StatusOK, rr)
user.Filters.RecoveryCodes = []sdk.RecoveryCode{
{
Secret: kms.NewPlainSecret(shortuuid.New()),
},
}
user, resp, err = httpdtest.UpdateUser(user, http.StatusOK, "")
assert.NoError(t, err, string(resp))
assert.Contains(t, user.Filters.WebClient, sdk.WebClientMFADisabled)
assert.Len(t, user.Filters.RecoveryCodes, 12)
req, err = http.NewRequest(http.MethodGet, user2FARecoveryCodesPath, nil)
assert.NoError(t, err)
setBearerForReq(req, token)
rr = executeRequest(req)
checkResponseCode(t, http.StatusOK, rr)
var recCodes []recoveryCode
err = json.Unmarshal(rr.Body.Bytes(), &recCodes)
assert.NoError(t, err)
assert.Len(t, recCodes, 12)
_, err = httpdtest.RemoveUser(user, http.StatusOK)
assert.NoError(t, err)
err = os.RemoveAll(user.GetHomeDir())
assert.NoError(t, err)
}
func TestLoginUserAPITOTP(t *testing.T) {
user, _, err := httpdtest.AddUser(getTestUser(), http.StatusCreated)
assert.NoError(t, err)
@ -4812,10 +4889,18 @@ func TestAdminTOTP(t *testing.T) {
assert.Equal(t, kms.SecretStatusSecretBox, admin.Filters.TOTPConfig.Secret.GetStatus())
admin.Filters.TOTPConfig = dataprovider.TOTPConfig{
Enabled: false,
ConfigName: shortuuid.New(),
Secret: kms.NewEmptySecret(),
}
admin, _, err = httpdtest.UpdateAdmin(admin, http.StatusOK)
assert.NoError(t, err)
admin.Filters.RecoveryCodes = []sdk.RecoveryCode{
{
Secret: kms.NewEmptySecret(),
},
}
admin, resp, err := httpdtest.UpdateAdmin(admin, http.StatusOK)
assert.NoError(t, err, string(resp))
assert.True(t, admin.Filters.TOTPConfig.Enabled)
assert.Len(t, admin.Filters.RecoveryCodes, 12)
// if we use token we should get no recovery codes
req, err = http.NewRequest(http.MethodGet, admin2FARecoveryCodesPath, nil)
assert.NoError(t, err)
@ -5378,14 +5463,15 @@ func TestMFAErrors(t *testing.T) {
setBearerForReq(req, userToken)
rr = executeRequest(req)
checkResponseCode(t, http.StatusBadRequest, rr)
assert.Contains(t, rr.Body.String(), "cannot save a user with a redacted secret")
// previous secret will be preserved and we have no secret saved
assert.Contains(t, rr.Body.String(), "totp: secret is mandatory")
req, err = http.NewRequest(http.MethodPost, adminTOTPSavePath, bytes.NewBuffer(asJSON))
assert.NoError(t, err)
setBearerForReq(req, adminToken)
rr = executeRequest(req)
checkResponseCode(t, http.StatusBadRequest, rr)
assert.Contains(t, rr.Body.String(), "cannot save an admin with a redacted secret")
assert.Contains(t, rr.Body.String(), "totp: secret is mandatory")
_, err = httpdtest.RemoveUser(user, http.StatusOK)
assert.NoError(t, err)
@ -5612,16 +5698,18 @@ func TestWebUserTOTP(t *testing.T) {
assert.NoError(t, err)
totpCfg := user.Filters.TOTPConfig
assert.True(t, totpCfg.Enabled)
secretPayload := totpCfg.Secret.GetPayload()
assert.Equal(t, totpGenResp.ConfigName, totpCfg.ConfigName)
assert.Empty(t, totpCfg.Secret.GetKey())
assert.Empty(t, totpCfg.Secret.GetAdditionalData())
assert.NotEmpty(t, totpCfg.Secret.GetPayload())
assert.NotEmpty(t, secretPayload)
assert.Equal(t, kms.SecretStatusSecretBox, totpCfg.Secret.GetStatus())
assert.Len(t, totpCfg.Protocols, 1)
assert.Contains(t, totpCfg.Protocols, common.ProtocolSSH)
// update protocols only
userTOTPConfig = sdk.TOTPConfig{
Protocols: []string{common.ProtocolSSH, common.ProtocolFTP},
Secret: kms.NewEmptySecret(),
}
asJSON, err = json.Marshal(userTOTPConfig)
assert.NoError(t, err)
@ -5634,6 +5722,7 @@ func TestWebUserTOTP(t *testing.T) {
// update the user, TOTP should not be affected
user.Filters.TOTPConfig = sdk.TOTPConfig{
Enabled: false,
Secret: kms.NewEmptySecret(),
}
_, _, err = httpdtest.UpdateUser(user, http.StatusOK, "")
assert.NoError(t, err)
@ -5644,7 +5733,7 @@ func TestWebUserTOTP(t *testing.T) {
assert.Equal(t, totpCfg.ConfigName, user.Filters.TOTPConfig.ConfigName)
assert.Empty(t, user.Filters.TOTPConfig.Secret.GetKey())
assert.Empty(t, user.Filters.TOTPConfig.Secret.GetAdditionalData())
assert.Equal(t, totpCfg.Secret.GetPayload(), user.Filters.TOTPConfig.Secret.GetPayload())
assert.Equal(t, secretPayload, user.Filters.TOTPConfig.Secret.GetPayload())
assert.Equal(t, kms.SecretStatusSecretBox, user.Filters.TOTPConfig.Secret.GetStatus())
assert.Len(t, user.Filters.TOTPConfig.Protocols, 2)
assert.Contains(t, user.Filters.TOTPConfig.Protocols, common.ProtocolSSH)
@ -10033,6 +10122,41 @@ func TestWebAdminBasicMock(t *testing.T) {
admin, _, err = httpdtest.GetAdminByUsername(altAdminUsername, http.StatusOK)
assert.NoError(t, err)
assert.True(t, admin.Filters.TOTPConfig.Enabled)
secretPayload := admin.Filters.TOTPConfig.Secret.GetPayload()
assert.NotEmpty(t, secretPayload)
adminTOTPConfig = dataprovider.TOTPConfig{
Enabled: true,
ConfigName: configName,
Secret: kms.NewEmptySecret(),
}
asJSON, err = json.Marshal(adminTOTPConfig)
assert.NoError(t, err)
req, err = http.NewRequest(http.MethodPost, webAdminTOTPSavePath, bytes.NewBuffer(asJSON))
assert.NoError(t, err)
setJWTCookieForReq(req, altToken)
setCSRFHeaderForReq(req, csrfToken)
rr = executeRequest(req)
checkResponseCode(t, http.StatusOK, rr)
admin, _, err = httpdtest.GetAdminByUsername(altAdminUsername, http.StatusOK)
assert.NoError(t, err)
assert.True(t, admin.Filters.TOTPConfig.Enabled)
assert.Equal(t, secretPayload, admin.Filters.TOTPConfig.Secret.GetPayload())
adminTOTPConfig = dataprovider.TOTPConfig{
Enabled: true,
ConfigName: configName,
Secret: nil,
}
asJSON, err = json.Marshal(adminTOTPConfig)
assert.NoError(t, err)
req, err = http.NewRequest(http.MethodPost, webAdminTOTPSavePath, bytes.NewBuffer(asJSON))
assert.NoError(t, err)
setJWTCookieForReq(req, altToken)
setCSRFHeaderForReq(req, csrfToken)
rr = executeRequest(req)
checkResponseCode(t, http.StatusOK, rr)
req, _ = http.NewRequest(http.MethodGet, webAdminsPath+"?qlimit=a", nil)
setJWTCookieForReq(req, token)