From a7b159aebb154b234265943eea1bc71a52994826 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Thu, 31 Mar 2022 21:49:06 +0200 Subject: [PATCH] ssh user certs: add a revoked list Signed-off-by: Nicola Murino --- config/config.go | 2 + docs/full-configuration.md | 1 + service/service_windows.go | 5 +++ service/signals_unix.go | 5 +++ sftpd/internal_test.go | 20 +++++++++ sftpd/server.go | 87 ++++++++++++++++++++++++++++++++++++-- sftpd/sftpd_test.go | 57 +++++++++++++++++++++++++ sftpgo.json | 1 + 8 files changed, 174 insertions(+), 4 deletions(-) diff --git a/config/config.go b/config/config.go index f06ce49c..4913eca1 100644 --- a/config/config.go +++ b/config/config.go @@ -199,6 +199,7 @@ func Init() { Ciphers: []string{}, MACs: []string{}, TrustedUserCAKeys: []string{}, + RevokedUserCertsFile: "", LoginBannerFile: "", EnabledSSHCommands: []string{}, KeyboardInteractiveAuthentication: false, @@ -1542,6 +1543,7 @@ func setViperDefaults() { viper.SetDefault("sftpd.ciphers", globalConf.SFTPD.Ciphers) viper.SetDefault("sftpd.macs", globalConf.SFTPD.MACs) viper.SetDefault("sftpd.trusted_user_ca_keys", globalConf.SFTPD.TrustedUserCAKeys) + viper.SetDefault("sftpd.revoked_user_certs_file", globalConf.SFTPD.RevokedUserCertsFile) viper.SetDefault("sftpd.login_banner_file", globalConf.SFTPD.LoginBannerFile) viper.SetDefault("sftpd.enabled_ssh_commands", sftpd.GetDefaultSSHCommands()) viper.SetDefault("sftpd.keyboard_interactive_authentication", globalConf.SFTPD.KeyboardInteractiveAuthentication) diff --git a/docs/full-configuration.md b/docs/full-configuration.md index 59459e95..5d9b7159 100644 --- a/docs/full-configuration.md +++ b/docs/full-configuration.md @@ -112,6 +112,7 @@ The configuration file contains the following sections: - `ciphers`, list of strings. Allowed ciphers in preference order. Leave empty to use default values. The supported values are: `aes128-gcm@openssh.com`, `aes256-gcm@openssh.com`, `chacha20-poly1305@openssh.com`, `aes128-ctr`, `aes192-ctr`, `aes256-ctr`, `aes128-cbc`, `aes192-cbc`, `aes256-cbc`, `3des-cbc`, `arcfour256`, `arcfour128`, `arcfour`. Default values: `aes128-gcm@openssh.com`, `aes256-gcm@openssh.com`, `chacha20-poly1305@openssh.com`, `aes128-ctr`, `aes192-ctr`, `aes256-ctr`. Please note that the ciphers disabled by default are insecure, you should expect that an active attacker can recover plaintext if you enable them. - `macs`, list of strings. Available MAC (message authentication code) algorithms in preference order. Leave empty to use default values. The supported values are: `hmac-sha2-256-etm@openssh.com`, `hmac-sha2-256`, `hmac-sha2-512-etm@openssh.com`, `hmac-sha2-512`, `hmac-sha1`, `hmac-sha1-96`. Default values: `hmac-sha2-256-etm@openssh.com`, `hmac-sha2-256`. - `trusted_user_ca_keys`, list of public keys paths of certificate authorities that are trusted to sign user certificates for authentication. The paths can be absolute or relative to the configuration directory. + - `revoked_user_certs_file`, path to a file containing the revoked user certificates. The path can be absolute or relative to the configuration directory. It must contain a JSON list with the public key fingerprints of the revoked certificates. Example content: `["SHA256:bsBRHC/xgiqBJdSuvSTNpJNLTISP/G356jNMCRYC5Es","SHA256:119+8cL/HH+NLMawRsJx6CzPF1I3xC+jpM60bQHXGE8"]`. The revocation list can be reloaded on demand sending a `SIGHUP` signal on Unix based systems and a `paramchange` request to the running service on Windows. Default: "". - `login_banner_file`, path to the login banner file. The contents of the specified file, if any, are sent to the remote user before authentication is allowed. It can be a path relative to the config dir or an absolute one. Leave empty to disable login banner. - `enabled_ssh_commands`, list of enabled SSH commands. `*` enables all supported commands. More information can be found [here](./ssh-commands.md). - `keyboard_interactive_authentication`, boolean. This setting specifies whether keyboard interactive authentication is allowed. If no keyboard interactive hook or auth plugin is defined the default is to prompt for the user password and then the one time authentication code, if defined. Default: `false`. diff --git a/service/service_windows.go b/service/service_windows.go index 8e717fee..16e2fe6f 100644 --- a/service/service_windows.go +++ b/service/service_windows.go @@ -17,6 +17,7 @@ import ( "github.com/drakkan/sftpgo/v2/httpd" "github.com/drakkan/sftpgo/v2/logger" "github.com/drakkan/sftpgo/v2/plugin" + "github.com/drakkan/sftpgo/v2/sftpd" "github.com/drakkan/sftpgo/v2/telemetry" "github.com/drakkan/sftpgo/v2/webdavd" ) @@ -141,6 +142,10 @@ loop: if err != nil { logger.Warn(logSender, "", "error reloading common configs: %v", err) } + err = sftpd.Reload() + if err != nil { + logger.Warn(logSender, "", "error reloading sftpd revoked certificates: %v", err) + } case rotateLogCmd: logger.Debug(logSender, "", "Received log file rotation request") err := logger.RotateLogFile() diff --git a/service/signals_unix.go b/service/signals_unix.go index d8a4b4c8..22a24480 100644 --- a/service/signals_unix.go +++ b/service/signals_unix.go @@ -14,6 +14,7 @@ import ( "github.com/drakkan/sftpgo/v2/httpd" "github.com/drakkan/sftpgo/v2/logger" "github.com/drakkan/sftpgo/v2/plugin" + "github.com/drakkan/sftpgo/v2/sftpd" "github.com/drakkan/sftpgo/v2/telemetry" "github.com/drakkan/sftpgo/v2/webdavd" ) @@ -61,6 +62,10 @@ func handleSIGHUP() { if err != nil { logger.Warn(logSender, "", "error reloading common configs: %v", err) } + err = sftpd.Reload() + if err != nil { + logger.Warn(logSender, "", "error reloading sftpd revoked certificates: %v", err) + } } func handleSIGUSR1() { diff --git a/sftpd/internal_test.go b/sftpd/internal_test.go index 8fe11426..bcf30148 100644 --- a/sftpd/internal_test.go +++ b/sftpd/internal_test.go @@ -2126,3 +2126,23 @@ func TestFolderPrefix(t *testing.T) { c.checkFolderPrefix() assert.Empty(t, c.FolderPrefix) } + +func TestLoadRevokedUserCertsFile(t *testing.T) { + r := revokedCertificates{ + certs: map[string]bool{}, + } + err := r.load() + assert.NoError(t, err) + r.filePath = filepath.Join(os.TempDir(), "sub", "testrevoked") + err = os.MkdirAll(filepath.Dir(r.filePath), os.ModePerm) + assert.NoError(t, err) + err = os.WriteFile(r.filePath, []byte(`no json`), 0644) + assert.NoError(t, err) + err = r.load() + assert.Error(t, err) + r.filePath = filepath.Dir(r.filePath) + err = r.load() + assert.Error(t, err) + err = os.RemoveAll(r.filePath) + assert.NoError(t, err) +} diff --git a/sftpd/server.go b/sftpd/server.go index 3417af4b..09232796 100644 --- a/sftpd/server.go +++ b/sftpd/server.go @@ -13,6 +13,7 @@ import ( "path/filepath" "runtime/debug" "strings" + "sync" "time" "github.com/pkg/sftp" @@ -55,6 +56,10 @@ var ( "hmac-sha2-512-etm@openssh.com", "hmac-sha2-512", "hmac-sha1", "hmac-sha1-96", } + + revokedCertManager = revokedCertificates{ + certs: map[string]bool{}, + } ) // Binding defines the configuration for a network listener @@ -109,6 +114,11 @@ type Configuration struct { // that are trusted to sign user certificates for authentication. // The paths can be absolute or relative to the configuration directory TrustedUserCAKeys []string `json:"trusted_user_ca_keys" mapstructure:"trusted_user_ca_keys"` + // Path to a file containing the revoked user certificates. + // This file must contain a JSON list with the public key fingerprints of the revoked certificates. + // Example content: + // ["SHA256:bsBRHC/xgiqBJdSuvSTNpJNLTISP/G356jNMCRYC5Es","SHA256:119+8cL/HH+NLMawRsJx6CzPF1I3xC+jpM60bQHXGE8"] + RevokedUserCertsFile string `json:"revoked_user_certs_file" mapstructure:"revoked_user_certs_file"` // LoginBannerFile the contents of the specified file, if any, are sent to // the remote user before authentication is allowed. LoginBannerFile string `json:"login_banner_file" mapstructure:"login_banner_file"` @@ -858,7 +868,16 @@ func (c *Configuration) initializeCertChecker(configDir string) error { return false }, } - return nil + if c.RevokedUserCertsFile != "" { + if !util.IsFileInputValid(c.RevokedUserCertsFile) { + return fmt.Errorf("invalid revoked user certificate: %#v", c.RevokedUserCertsFile) + } + if !filepath.IsAbs(c.RevokedUserCertsFile) { + c.RevokedUserCertsFile = filepath.Join(configDir, c.RevokedUserCertsFile) + } + } + revokedCertManager.filePath = c.RevokedUserCertsFile + return revokedCertManager.load() } func (c *Configuration) validatePublicKeyCredentials(conn ssh.ConnMetadata, pubKey ssh.PublicKey) (*ssh.Permissions, error) { @@ -872,7 +891,9 @@ func (c *Configuration) validatePublicKeyCredentials(conn ssh.ConnMetadata, pubK method := dataprovider.SSHLoginMethodPublicKey ipAddr := util.GetIPFromRemoteAddress(conn.RemoteAddr().String()) cert, ok := pubKey.(*ssh.Certificate) + var certFingerprint string if ok { + certFingerprint = ssh.FingerprintSHA256(cert.Key) if cert.CertType != ssh.UserCert { err = fmt.Errorf("ssh: cert has type %d", cert.CertType) user.Username = conn.User() @@ -886,8 +907,13 @@ func (c *Configuration) validatePublicKeyCredentials(conn ssh.ConnMetadata, pubK return nil, err } if len(cert.ValidPrincipals) == 0 { - err = fmt.Errorf("ssh: certificate %s has no valid principals, user: \"%s\"", - ssh.FingerprintSHA256(pubKey), conn.User()) + err = fmt.Errorf("ssh: certificate %s has no valid principals, user: \"%s\"", certFingerprint, conn.User()) + user.Username = conn.User() + updateLoginMetrics(&user, ipAddr, method, err) + return nil, err + } + if revokedCertManager.isRevoked(certFingerprint) { + err = fmt.Errorf("ssh: certificate %s is revoked", certFingerprint) user.Username = conn.User() updateLoginMetrics(&user, ipAddr, method, err) return nil, err @@ -901,7 +927,7 @@ func (c *Configuration) validatePublicKeyCredentials(conn ssh.ConnMetadata, pubK } if user, keyID, err = dataprovider.CheckUserAndPubKey(conn.User(), pubKey.Marshal(), ipAddr, common.ProtocolSSH, ok); err == nil { if ok { - keyID = fmt.Sprintf("%s: ID: %s, serial: %v, CA %s %s", ssh.FingerprintSHA256(pubKey), + keyID = fmt.Sprintf("%s: ID: %s, serial: %v, CA %s %s", certFingerprint, cert.KeyId, cert.Serial, cert.Type(), ssh.FingerprintSHA256(cert.SignatureKey)) } if user.IsPartialAuth(method) { @@ -980,3 +1006,56 @@ func updateLoginMetrics(user *dataprovider.User, ip, method string, err error) { metric.AddLoginResult(method, err) dataprovider.ExecutePostLoginHook(user, method, ip, common.ProtocolSSH, err) } + +type revokedCertificates struct { + filePath string + mu sync.RWMutex + certs map[string]bool +} + +func (r *revokedCertificates) load() error { + if r.filePath == "" { + return nil + } + logger.Debug(logSender, "", "loading revoked user certificate file %#v", r.filePath) + info, err := os.Stat(r.filePath) + if err != nil { + return fmt.Errorf("unable to load revoked user certificate file %#v: %w", r.filePath, err) + } + maxSize := int64(1048576 * 5) // 5MB + if info.Size() > maxSize { + return fmt.Errorf("unable to load revoked user certificate file %#v size too big: %v/%v bytes", + r.filePath, info.Size(), maxSize) + } + content, err := os.ReadFile(r.filePath) + if err != nil { + return fmt.Errorf("unable to read revoked user certificate file %#v: %w", r.filePath, err) + } + var certs []string + err = json.Unmarshal(content, &certs) + if err != nil { + return fmt.Errorf("unable to parse revoked user certificate file %#v: %w", r.filePath, err) + } + + r.mu.Lock() + defer r.mu.Unlock() + + r.certs = map[string]bool{} + for _, fp := range certs { + r.certs[fp] = true + } + logger.Debug(logSender, "", "revoked user certificate file %#v loaded, entries: %v", r.filePath, len(r.certs)) + return nil +} + +func (r *revokedCertificates) isRevoked(fp string) bool { + r.mu.RLock() + defer r.mu.RUnlock() + + return r.certs[fp] +} + +// Reload reloads the list of revoked user certificates +func Reload() error { + return revokedCertManager.load() +} diff --git a/sftpd/sftpd_test.go b/sftpd/sftpd_test.go index 0e875cb3..62faf54b 100644 --- a/sftpd/sftpd_test.go +++ b/sftpd/sftpd_test.go @@ -135,6 +135,7 @@ var ( pubKeyPath string privateKeyPath string trustedCAUserKey string + revokeUserCerts string gitWrapPath string extAuthPath string keyIntAuthPath string @@ -236,6 +237,7 @@ func TestMain(m *testing.M) { createInitialFiles(scriptArgs) sftpdConf.TrustedUserCAKeys = append(sftpdConf.TrustedUserCAKeys, trustedCAUserKey) + sftpdConf.RevokedUserCertsFile = revokeUserCerts go func() { logger.Debug(logSender, "", "initializing SFTP server with config %+v", sftpdConf) @@ -320,6 +322,7 @@ func TestMain(m *testing.M) { os.Remove(pubKeyPath) os.Remove(privateKeyPath) os.Remove(trustedCAUserKey) + os.Remove(revokeUserCerts) os.Remove(gitWrapPath) os.Remove(extAuthPath) os.Remove(preLoginPath) @@ -393,6 +396,22 @@ func TestInitialization(t *testing.T) { if assert.Error(t, err) { assert.Contains(t, err.Error(), "unsupported key-exchange algorithm") } + sftpdConf.RevokedUserCertsFile = "." + err = sftpdConf.Initialize(configDir) + assert.Error(t, err) + sftpdConf.RevokedUserCertsFile = "a missing file" + err = sftpdConf.Initialize(configDir) + assert.ErrorIs(t, err, os.ErrNotExist) + + err = createTestFile(revokeUserCerts, 10*1024*1024) + sftpdConf.RevokedUserCertsFile = revokeUserCerts + err = sftpdConf.Initialize(configDir) + assert.Error(t, err) + + err = os.WriteFile(revokeUserCerts, []byte(`[]`), 0644) + assert.NoError(t, err) + err = sftpdConf.Initialize(configDir) + assert.Error(t, err) } func TestBasicSFTPHandling(t *testing.T) { @@ -1802,6 +1821,34 @@ func TestLoginUserCert(t *testing.T) { defer client.Close() assert.NoError(t, checkBasicSFTP(client)) } + // revoke the certificate + certs := []string{"SHA256:OkxVB1ImSJ2XeI8nA2Wg+6zJVlxdevD1FYBSEJjFEN4"} + data, err := json.Marshal(certs) + assert.NoError(t, err) + err = os.WriteFile(revokeUserCerts, data, 0644) + assert.NoError(t, err) + err = sftpd.Reload() + assert.NoError(t, err) + conn, client, err = getCustomAuthSftpClient(user, []ssh.AuthMethod{ssh.PublicKeys(signer)}, "") + if !assert.Error(t, err) { + client.Close() + conn.Close() + } + // if we remove the revoked certificate login should work again + certs = []string{"SHA256:bsBRHC/xgiqBJdSuvSTNpJNLTISP/G356jNMCRYC5Es, SHA256:1kxVB1ImSJ2XeI8nA2Wg+6zJVlxdevD1FYBSEJjFEN4"} + data, err = json.Marshal(certs) + assert.NoError(t, err) + err = os.WriteFile(revokeUserCerts, data, 0644) + assert.NoError(t, err) + err = sftpd.Reload() + assert.NoError(t, err) + conn, client, err = getCustomAuthSftpClient(user, []ssh.AuthMethod{ssh.PublicKeys(signer)}, "") + if assert.NoError(t, err) { + defer conn.Close() + defer client.Close() + assert.NoError(t, checkBasicSFTP(client)) + } + // try login using a cert signed from an untrusted CA signer, err = getSignerForUserCert([]byte(testCertUntrustedCA)) assert.NoError(t, err) @@ -1869,6 +1916,11 @@ func TestLoginUserCert(t *testing.T) { conn.Close() } + err = os.WriteFile(revokeUserCerts, []byte(`[]`), 0644) + assert.NoError(t, err) + err = sftpd.Reload() + assert.NoError(t, err) + _, err = httpdtest.RemoveUser(user, http.StatusOK) assert.NoError(t, err) err = os.RemoveAll(user.GetHomeDir()) @@ -10865,6 +10917,7 @@ func createInitialFiles(scriptArgs string) { checkPwdPath = filepath.Join(homeBasePath, "checkpwd.sh") preDownloadPath = filepath.Join(homeBasePath, "predownload.sh") preUploadPath = filepath.Join(homeBasePath, "preupload.sh") + revokeUserCerts = filepath.Join(homeBasePath, "revoked_certs.json") err := os.WriteFile(pubKeyPath, []byte(testPubKey+"\n"), 0600) if err != nil { logger.WarnToConsole("unable to save public key to file: %v", err) @@ -10882,4 +10935,8 @@ func createInitialFiles(scriptArgs string) { if err != nil { logger.WarnToConsole("unable to save trusted CA user key: %v", err) } + err = os.WriteFile(revokeUserCerts, []byte(`[]`), 0644) + if err != nil { + logger.WarnToConsole("unable to save revoked user certs: %v", err) + } } diff --git a/sftpgo.json b/sftpgo.json index 80f17504..fafa81f0 100644 --- a/sftpgo.json +++ b/sftpgo.json @@ -67,6 +67,7 @@ "ciphers": [], "macs": [], "trusted_user_ca_keys": [], + "revoked_user_certs_file": "", "login_banner_file": "", "enabled_ssh_commands": [ "md5sum",