ssh user certs: add a revoked list

Signed-off-by: Nicola Murino <nicola.murino@gmail.com>

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)

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`.

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()

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() {

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)
+}

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()
+}

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)
+	}
 }

sftpgo.json

@@ -67,6 +67,7 @@
     "ciphers": [],
     "macs": [],
     "trusted_user_ca_keys": [],
+    "revoked_user_certs_file": "",
     "login_banner_file": "",
     "enabled_ssh_commands": [
       "md5sum",