From d939a82225e44931d454796947a80b431b156c31 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sun, 14 Jan 2024 21:36:23 +0100 Subject: [PATCH] user: add TLS certificates Signed-off-by: Nicola Murino --- go.mod | 2 +- go.sum | 4 +- internal/acme/acme.go | 4 ++ internal/dataprovider/dataprovider.go | 33 +++++++++++++- internal/dataprovider/group.go | 1 + internal/dataprovider/user.go | 10 +++-- internal/ftpd/ftpd_test.go | 19 ++++++++ internal/ftpd/server.go | 2 +- internal/httpd/httpd_test.go | 20 +++++++++ internal/httpd/server.go | 10 +++-- internal/httpd/webadmin.go | 5 +++ internal/httpdtest/httpdtest.go | 11 +++++ internal/util/i18n.go | 1 + internal/webdavd/server.go | 2 +- internal/webdavd/webdavd_test.go | 7 +++ openapi/openapi.yaml | 4 ++ static/locales/en/translation.json | 5 ++- static/locales/it/translation.json | 5 ++- templates/webadmin/fsconfig.html | 16 +++---- templates/webadmin/user.html | 65 +++++++++++++++++++++++++++ 20 files changed, 203 insertions(+), 23 deletions(-) diff --git a/go.mod b/go.mod index 1fc0f0d5..0ed30305 100644 --- a/go.mod +++ b/go.mod @@ -53,7 +53,7 @@ require ( github.com/rs/cors v1.10.1 github.com/rs/xid v1.5.0 github.com/rs/zerolog v1.31.0 - github.com/sftpgo/sdk v0.1.6-0.20231105181545-b44c8058fc25 + github.com/sftpgo/sdk v0.1.6-0.20240114195211-3f4916cc829c github.com/shirou/gopsutil/v3 v3.23.12 github.com/spf13/afero v1.11.0 github.com/spf13/cobra v1.8.0 diff --git a/go.sum b/go.sum index f9877eb2..e3ed69b8 100644 --- a/go.sum +++ b/go.sum @@ -350,8 +350,8 @@ github.com/secsy/goftp v0.0.0-20200609142545-aa2de14babf4 h1:PT+ElG/UUFMfqy5HrxJ github.com/secsy/goftp v0.0.0-20200609142545-aa2de14babf4/go.mod h1:MnkX001NG75g3p8bhFycnyIjeQoOjGL6CEIsdE/nKSY= github.com/segmentio/asm v1.2.0 h1:9BQrFxC+YOHJlTlHGkTrFWf59nbL3XnCoFLTwDCI7ys= github.com/segmentio/asm v1.2.0/go.mod h1:BqMnlJP91P8d+4ibuonYZw9mfnzI9HfxselHZr5aAcs= -github.com/sftpgo/sdk v0.1.6-0.20231105181545-b44c8058fc25 h1:R8cTb41ZX5WSYw8q8ufTKQfOvXh7aLQWqdnteDY/96U= -github.com/sftpgo/sdk v0.1.6-0.20231105181545-b44c8058fc25/go.mod h1:6s/PFoLUd7FXG3wGlrdVhrA0SJOwri2h9kzTph/2oiU= +github.com/sftpgo/sdk v0.1.6-0.20240114195211-3f4916cc829c h1:07TYPvNbOnmKsBxjNsUr+gsILIUWflw1UYwjn1jognM= +github.com/sftpgo/sdk v0.1.6-0.20240114195211-3f4916cc829c/go.mod h1:AWoY2YYe/P1ymfTlRER/meERQjCcZZTbgVPGcPQgaqc= github.com/shirou/gopsutil/v3 v3.23.12 h1:z90NtUkp3bMtmICZKpC4+WaknU1eXtp5vtbQ11DgpE4= github.com/shirou/gopsutil/v3 v3.23.12/go.mod h1:1FrWgea594Jp7qmjHUUPlJDTPgcsb9mGnXDxavtikzM= github.com/shoenig/go-m1cpu v0.1.6 h1:nxdKQNcEB6vzgA2E2bvzKIYRuNj7XNJ4S/aRSwKzFtM= diff --git a/internal/acme/acme.go b/internal/acme/acme.go index 6c87303f..71f45717 100644 --- a/internal/acme/acme.go +++ b/internal/acme/acme.go @@ -390,6 +390,10 @@ func (c *Configuration) loadPrivateKey() (crypto.PrivateKey, error) { } keyBlock, _ := pem.Decode(keyBytes) + if keyBlock == nil { + acmeLog(logger.LevelError, "unable to parse private key from file %q: pem decoding failed", c.accountKeyPath) + return nil, errors.New("pem decoding failed") + } var privateKey crypto.PrivateKey switch keyBlock.Type { diff --git a/internal/dataprovider/dataprovider.go b/internal/dataprovider/dataprovider.go index 14a3d421..7287b555 100644 --- a/internal/dataprovider/dataprovider.go +++ b/internal/dataprovider/dataprovider.go @@ -29,6 +29,7 @@ import ( "encoding/base64" "encoding/hex" "encoding/json" + "encoding/pem" "errors" "fmt" "hash" @@ -1212,7 +1213,7 @@ func CheckCompositeCredentials(username, password, ip, loginMethod, protocol str if err != nil { return user, loginMethod, err } - if !user.IsTLSUsernameVerificationEnabled() { + if !user.IsTLSVerificationEnabled() { // for backward compatibility with 2.0.x we only check the password and change the login method here // in future updates we have to return an error user, err := CheckUserAndPass(username, password, ip, protocol) @@ -2623,6 +2624,8 @@ func copyBaseUserFilters(in sdk.BaseUserFilters) sdk.BaseUserFilters { filters.PasswordStrength = in.PasswordStrength filters.WebClient = make([]string, len(in.WebClient)) copy(filters.WebClient, in.WebClient) + filters.TLSCerts = make([]string, len(in.TLSCerts)) + copy(filters.TLSCerts, in.TLSCerts) filters.BandwidthLimits = make([]sdk.BandwidthLimit, 0, len(in.BandwidthLimits)) for _, limit := range in.BandwidthLimits { bwLimit := sdk.BandwidthLimit{ @@ -3023,6 +3026,25 @@ func validateFilterProtocols(filters *sdk.BaseUserFilters) error { return nil } +func validateTLSCerts(certs []string) error { + for idx, cert := range certs { + derBlock, _ := pem.Decode([]byte(cert)) + if derBlock == nil { + return util.NewI18nError( + util.NewValidationError(fmt.Sprintf("invalid TLS certificate %d", idx)), + util.I18nErrorInvalidTLSCert, + ) + } + if _, err := x509.ParseCertificate(derBlock.Bytes); err != nil { + return util.NewI18nError( + util.NewValidationError(fmt.Sprintf("error parsing TLS certificate %d", idx)), + util.I18nErrorInvalidTLSCert, + ) + } + } + return nil +} + func validateBaseFilters(filters *sdk.BaseUserFilters) error { checkEmptyFiltersStruct(filters) if err := validateIPFilters(filters); err != nil { @@ -3047,6 +3069,9 @@ func validateBaseFilters(filters *sdk.BaseUserFilters) error { return util.NewValidationError(fmt.Sprintf("invalid TLS username: %q", filters.TLSUsername)) } } + if err := validateTLSCerts(filters.TLSCerts); err != nil { + return err + } for _, opts := range filters.WebClient { if !util.Contains(sdk.WebClientOptions, opts) { return util.NewValidationError(fmt.Sprintf("invalid web client options %q", opts)) @@ -3312,6 +3337,12 @@ func checkUserAndTLSCertificate(user *User, protocol string, tlsCert *x509.Certi } switch protocol { case protocolFTP, protocolWebDAV: + for _, cert := range user.Filters.TLSCerts { + derBlock, _ := pem.Decode([]byte(cert)) + if derBlock != nil && bytes.Equal(derBlock.Bytes, tlsCert.Raw) { + return *user, nil + } + } if user.Filters.TLSUsername == sdk.TLSUsernameCN { if user.Username == tlsCert.Subject.CommonName { return *user, nil diff --git a/internal/dataprovider/group.go b/internal/dataprovider/group.go index 35bdbe20..cc6c28fa 100644 --- a/internal/dataprovider/group.go +++ b/internal/dataprovider/group.go @@ -179,6 +179,7 @@ func (g *Group) validateUserSettings() error { } g.UserSettings.Permissions = permissions } + g.UserSettings.Filters.TLSCerts = nil if err := validateBaseFilters(&g.UserSettings.Filters); err != nil { return err } diff --git a/internal/dataprovider/user.go b/internal/dataprovider/user.go index e12219db..d9b1663d 100644 --- a/internal/dataprovider/user.go +++ b/internal/dataprovider/user.go @@ -468,9 +468,11 @@ func (u *User) IsPasswordHashed() bool { return util.IsStringPrefixInSlice(u.Password, hashPwdPrefixes) } -// IsTLSUsernameVerificationEnabled returns true if we need to extract the username -// from the client TLS certificate -func (u *User) IsTLSUsernameVerificationEnabled() bool { +// IsTLSVerificationEnabled returns true if we need to check the TLS authentication +func (u *User) IsTLSVerificationEnabled() bool { + if len(u.Filters.TLSCerts) > 0 { + return true + } if u.Filters.TLSUsername != "" { return u.Filters.TLSUsername != sdk.TLSUsernameNone } @@ -1757,7 +1759,7 @@ func (u *User) mergePrimaryGroupFilters(filters *sdk.BaseUserFilters, replacer * if u.Filters.MaxUploadFileSize == 0 { u.Filters.MaxUploadFileSize = filters.MaxUploadFileSize } - if !u.IsTLSUsernameVerificationEnabled() { + if !u.IsTLSVerificationEnabled() { u.Filters.TLSUsername = filters.TLSUsername } if !u.Filters.Hooks.CheckPasswordDisabled { diff --git a/internal/ftpd/ftpd_test.go b/internal/ftpd/ftpd_test.go index 7e6e0d04..d1a1414b 100644 --- a/internal/ftpd/ftpd_test.go +++ b/internal/ftpd/ftpd_test.go @@ -3519,6 +3519,25 @@ func TestClientCertificateAuth(t *testing.T) { if assert.Error(t, err) { assert.Contains(t, err.Error(), "does not match username") } + // add the certs to the user + user2.Filters.TLSUsername = sdk.TLSUsernameNone + user2.Filters.TLSCerts = []string{client2Crt, client1Crt} + user2, _, err = httpdtest.UpdateUser(user2, http.StatusOK, "") + assert.NoError(t, err) + client, err = getFTPClient(user2, true, tlsConfig) + if assert.NoError(t, err) { + err = checkBasicFTP(client) + assert.NoError(t, err) + err = client.Quit() + assert.NoError(t, err) + } + user2.Filters.TLSCerts = []string{client2Crt} + user2, _, err = httpdtest.UpdateUser(user2, http.StatusOK, "") + assert.NoError(t, err) + _, err = getFTPClient(user2, true, tlsConfig) + if assert.Error(t, err) { + assert.Contains(t, err.Error(), "TLS certificate is not valid") + } // now disable certificate authentication user.Filters.DeniedLoginMethods = append(user.Filters.DeniedLoginMethods, dataprovider.LoginMethodTLSCertificate, diff --git a/internal/ftpd/server.go b/internal/ftpd/server.go index 8968cb9b..9be8711a 100644 --- a/internal/ftpd/server.go +++ b/internal/ftpd/server.go @@ -245,7 +245,7 @@ func (s *Server) VerifyConnection(cc ftpserver.ClientContext, user string, tlsCo updateLoginMetrics(&dbUser, ipAddr, dataprovider.LoginMethodTLSCertificate, err) return nil, dataprovider.ErrInvalidCredentials } - if dbUser.IsTLSUsernameVerificationEnabled() { + if dbUser.IsTLSVerificationEnabled() { dbUser, err = dataprovider.CheckUserAndTLSCert(user, ipAddr, common.ProtocolFTP, state.PeerCertificates[0]) if err != nil { return nil, err diff --git a/internal/httpd/httpd_test.go b/internal/httpd/httpd_test.go index 89900828..317093be 100644 --- a/internal/httpd/httpd_test.go +++ b/internal/httpd/httpd_test.go @@ -821,12 +821,32 @@ func TestRoleRelations(t *testing.T) { assert.NoError(t, err) } +func TestTLSCert(t *testing.T) { + u := getTestUser() + u.Filters.TLSCerts = []string{"not a cert"} + _, resp, err := httpdtest.AddUser(u, http.StatusBadRequest) + assert.NoError(t, err, string(resp)) + assert.Contains(t, string(resp), "invalid TLS certificate") + + u.Filters.TLSCerts = []string{httpsCert} + user, resp, err := httpdtest.AddUser(u, http.StatusCreated) + assert.NoError(t, err, string(resp)) + if assert.Len(t, user.Filters.TLSCerts, 1) { + assert.Equal(t, httpsCert, user.Filters.TLSCerts[0]) + } + + _, err = httpdtest.RemoveUser(user, http.StatusOK) + assert.NoError(t, err) +} + func TestBasicGroupHandling(t *testing.T) { g := getTestGroup() + g.UserSettings.Filters.TLSCerts = []string{"invalid cert"} // ignored for groups group, _, err := httpdtest.AddGroup(g, http.StatusCreated) assert.NoError(t, err) assert.Greater(t, group.CreatedAt, int64(0)) assert.Greater(t, group.UpdatedAt, int64(0)) + assert.Len(t, group.UserSettings.Filters.TLSCerts, 0) groupGet, _, err := httpdtest.GetGroupByName(group.Name, http.StatusOK) assert.NoError(t, err) assert.Equal(t, group, groupGet) diff --git a/internal/httpd/server.go b/internal/httpd/server.go index 42b30ecd..2c07fb68 100644 --- a/internal/httpd/server.go +++ b/internal/httpd/server.go @@ -48,6 +48,10 @@ import ( "github.com/drakkan/sftpgo/v2/internal/version" ) +const ( + jsonAPISuffix = "/json" +) + var ( compressor = middleware.NewCompressor(5) xForwardedProto = http.CanonicalHeaderKey("X-Forwarded-Proto") @@ -1683,7 +1687,7 @@ func (s *httpdServer) setupWebAdminRoutes() { router.With(s.checkPerm(dataprovider.PermAdminViewUsers), s.refreshCookie). Get(webUsersPath, s.handleGetWebUsers) router.With(s.checkPerm(dataprovider.PermAdminViewUsers), compressor.Handler, s.refreshCookie). - Get(webUsersPath+"/json", getAllUsers) + Get(webUsersPath+jsonAPISuffix, getAllUsers) router.With(s.checkPerm(dataprovider.PermAdminAddUsers), s.refreshCookie). Get(webUserPath, s.handleWebAddUserGet) router.With(s.checkPerm(dataprovider.PermAdminChangeUsers), s.refreshCookie). @@ -1694,7 +1698,7 @@ func (s *httpdServer) setupWebAdminRoutes() { router.With(s.checkPerm(dataprovider.PermAdminManageGroups), s.refreshCookie). Get(webGroupsPath, s.handleWebGetGroups) router.With(s.checkPerm(dataprovider.PermAdminManageGroups), compressor.Handler, s.refreshCookie). - Get(webGroupsPath+"/json", getAllGroups) + Get(webGroupsPath+jsonAPISuffix, getAllGroups) router.With(s.checkPerm(dataprovider.PermAdminManageGroups), s.refreshCookie). Get(webGroupPath, s.handleWebAddGroupGet) router.With(s.checkPerm(dataprovider.PermAdminManageGroups)).Post(webGroupPath, s.handleWebAddGroupPost) @@ -1709,7 +1713,7 @@ func (s *httpdServer) setupWebAdminRoutes() { router.With(s.checkPerm(dataprovider.PermAdminManageFolders), s.refreshCookie). Get(webFoldersPath, s.handleWebGetFolders) router.With(s.checkPerm(dataprovider.PermAdminManageFolders), compressor.Handler, s.refreshCookie). - Get(webFoldersPath+"/json", getAllFolders) + Get(webFoldersPath+jsonAPISuffix, getAllFolders) router.With(s.checkPerm(dataprovider.PermAdminManageFolders), s.refreshCookie). Get(webFolderPath, s.handleWebAddFolderGet) router.With(s.checkPerm(dataprovider.PermAdminManageFolders)).Post(webFolderPath, s.handleWebAddFolderPost) diff --git a/internal/httpd/webadmin.go b/internal/httpd/webadmin.go index aee5cb10..57cf771a 100644 --- a/internal/httpd/webadmin.go +++ b/internal/httpd/webadmin.go @@ -1961,6 +1961,10 @@ func updateRepeaterFormFields(r *http.Request) { r.Form.Add("public_keys", r.Form.Get(k)) continue } + if hasPrefixAndSuffix(k, "tls_certs[", "][tls_cert]") { + r.Form.Add("tls_certs", strings.TrimSpace(r.Form.Get(k))) + continue + } if hasPrefixAndSuffix(k, "virtual_folders[", "][vfolder_path]") { base, _ := strings.CutSuffix(k, "[vfolder_path]") r.Form.Add("vfolder_path", strings.TrimSpace(r.Form.Get(k))) @@ -2059,6 +2063,7 @@ func getUserFromPostFields(r *http.Request) (dataprovider.User, error) { if err != nil { return user, err } + filters.TLSCerts = r.Form["tls_certs"] user = dataprovider.User{ BaseUser: sdk.BaseUser{ Username: strings.TrimSpace(r.Form.Get("username")), diff --git a/internal/httpdtest/httpdtest.go b/internal/httpdtest/httpdtest.go index 9b7d2a05..953c6116 100644 --- a/internal/httpdtest/httpdtest.go +++ b/internal/httpdtest/httpdtest.go @@ -283,6 +283,7 @@ func AddGroup(group dataprovider.Group, expectedStatusCode int) (dataprovider.Gr body, _ = getResponseBody(resp) } if err == nil { + group.UserSettings.Filters.TLSCerts = nil err = checkGroup(group, newGroup) } return newGroup, body, err @@ -2412,6 +2413,16 @@ func compareUserFilterSubStructs(expected sdk.BaseUserFilters, actual sdk.BaseUs return errors.New("web client options contents mismatch") } } + + if len(expected.TLSCerts) != len(actual.TLSCerts) { + return errors.New("TLS certs mismatch") + } + for _, cert := range expected.TLSCerts { + if !util.Contains(actual.TLSCerts, cert) { + return errors.New("TLS certs content mismatch") + } + } + return compareUserFiltersEqualFields(expected, actual) } diff --git a/internal/util/i18n.go b/internal/util/i18n.go index e4480fb6..c427f264 100644 --- a/internal/util/i18n.go +++ b/internal/util/i18n.go @@ -185,6 +185,7 @@ const ( I18nErrorFsUsernameRequired = "storage.username_required" I18nAddGroupTitle = "title.add_group" I18nUpdateGroupTitle = "title.update_group" + I18nErrorInvalidTLSCert = "user.tls_cert_invalid" ) // NewI18nError returns a I18nError wrappring the provided error diff --git a/internal/webdavd/server.go b/internal/webdavd/server.go index 293125fb..8a8de264 100644 --- a/internal/webdavd/server.go +++ b/internal/webdavd/server.go @@ -293,7 +293,7 @@ func (s *webDavServer) authenticate(r *http.Request, ip string) (dataprovider.Us if cachedUser.IsExpired() { dataprovider.RemoveCachedWebDAVUser(username) } else { - if !cachedUser.User.IsTLSUsernameVerificationEnabled() { + if !cachedUser.User.IsTLSVerificationEnabled() { // for backward compatibility with 2.0.x we only check the password tlsCert = nil loginMethod = dataprovider.LoginMethodPassword diff --git a/internal/webdavd/webdavd_test.go b/internal/webdavd/webdavd_test.go index 0d64b5c7..b6b2bbdb 100644 --- a/internal/webdavd/webdavd_test.go +++ b/internal/webdavd/webdavd_test.go @@ -2800,6 +2800,13 @@ func TestClientCertificateAuth(t *testing.T) { client := getWebDavClient(user, true, tlsConfig) err = checkBasicFunc(client) assert.NoError(t, err) + user.Filters.TLSUsername = sdk.TLSUsernameNone + user.Filters.TLSCerts = []string{client1Crt} + user, _, err = httpdtest.UpdateUser(user, http.StatusOK, "") + assert.NoError(t, err) + client = getWebDavClient(user, true, tlsConfig) + err = checkBasicFunc(client) + assert.NoError(t, err) user.Filters.DeniedLoginMethods = []string{dataprovider.LoginMethodPassword, dataprovider.LoginMethodTLSCertificate} user, _, err = httpdtest.UpdateUser(user, http.StatusOK, "") diff --git a/openapi/openapi.yaml b/openapi/openapi.yaml index 915ccba5..725a69e1 100644 --- a/openapi/openapi.yaml +++ b/openapi/openapi.yaml @@ -5499,6 +5499,10 @@ components: tls_username: type: string description: 'defines the TLS certificate field to use as username. For FTP clients it must match the name provided using the "USER" command. For WebDAV, if no username is provided, the CN will be used as username. For WebDAV clients it must match the implicit or provided username. Ignored if mutual TLS is disabled. Currently the only supported value is `CommonName`' + tls_certs: + type: array + items: + type: string hooks: $ref: '#/components/schemas/HooksFilter' disable_fs_checks: diff --git a/static/locales/en/translation.json b/static/locales/en/translation.json index 17eceebb..6cbc99f1 100644 --- a/static/locales/en/translation.json +++ b/static/locales/en/translation.json @@ -467,7 +467,10 @@ "submit_export": "Generate and export users", "invalid_quota_size": "Invalid quota size", "expires_in": "Expires in", - "expires_in_help": "Account expiration as number of days from the creation. 0 means no expiration" + "expires_in_help": "Account expiration as number of days from the creation. 0 means no expiration", + "tls_certs": "TLS certificates", + "tls_cert_help": "Paste your PEM encoded TLS certificate here", + "tls_cert_invalid": "Invalid TLS certificate" }, "group": { "view_manage": "View and manage groups", diff --git a/static/locales/it/translation.json b/static/locales/it/translation.json index 4df3dd15..5fc70a25 100644 --- a/static/locales/it/translation.json +++ b/static/locales/it/translation.json @@ -467,7 +467,10 @@ "submit_export": "Genera ed esporta utenti", "invalid_quota_size": "Quota (dimensione) non valida", "expires_in": "Scadenza", - "expires_in_help": "Scadenza dell'account espressa in numero di giorni dalla creazione. 0 significa nessuna scadenza" + "expires_in_help": "Scadenza dell'account espressa in numero di giorni dalla creazione. 0 significa nessuna scadenza", + "tls_certs": "Certificati TLS", + "tls_cert_help": "Incolla qui il tuo certificato TLS codificato PEM", + "tls_cert_invalid": "Certificato TLS non valido" }, "group": { "view_manage": "Visualizza e gestisci gruppi", diff --git a/templates/webadmin/fsconfig.html b/templates/webadmin/fsconfig.html index 67f20368..fc31d305 100644 --- a/templates/webadmin/fsconfig.html +++ b/templates/webadmin/fsconfig.html @@ -746,14 +746,6 @@ explicit grant from the SFTPGo Team (support@sftpgo.com). {{- end}} {{- define "user_group_advanced"}} -
- -
- -
-
-
-
@@ -776,6 +768,14 @@ explicit grant from the SFTPGo Team (support@sftpgo.com).
+
+ +
+ +
+
+
+
diff --git a/templates/webadmin/user.html b/templates/webadmin/user.html index ac50acb8..96703dd0 100644 --- a/templates/webadmin/user.html +++ b/templates/webadmin/user.html @@ -629,6 +629,70 @@ explicit grant from the SFTPGo Team (support@sftpgo.com).
+
+
+

TLS certificates

+
+
+
+
+
+ {{- range $idx, $val := .User.Filters.TLSCerts}} +
+
+
+ +
+
+ + + + + + + + + Delete + +
+
+
+ {{- else}} +
+
+
+ +
+
+ + + + + + + + + Delete + +
+
+
+ {{- end}} +
+
+ +
+ + + Add + +
+
+
+
+ {{template "user_group_advanced" .User.Filters}}
@@ -728,6 +792,7 @@ explicit grant from the SFTPGo Team (support@sftpgo.com). initRepeater('#directory_permissions'); initRepeater('#directory_patterns'); initRepeater('#src_bandwidth_limits'); + initRepeater('#tls_certs'); initRepeaterItems(); //{{- if .Error}} //{{- if ne .LoggedUser.Filters.Preferences.VisibleUserPageSections 0}}