diff --git a/internal/config/config.go b/internal/config/config.go index e34aabb6..23419d88 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -278,6 +278,8 @@ func Init() { PublicKeyAlgorithms: []string{}, TrustedUserCAKeys: []string{}, RevokedUserCertsFile: "", + OPKSSHPath: "", + OPKSSHChecksum: "", LoginBannerFile: "", EnabledSSHCommands: []string{}, KeyboardInteractiveAuthentication: true, @@ -2095,6 +2097,8 @@ func setViperDefaults() { viper.SetDefault("sftpd.public_key_algorithms", globalConf.SFTPD.PublicKeyAlgorithms) viper.SetDefault("sftpd.trusted_user_ca_keys", globalConf.SFTPD.TrustedUserCAKeys) viper.SetDefault("sftpd.revoked_user_certs_file", globalConf.SFTPD.RevokedUserCertsFile) + viper.SetDefault("sftpd.opkssh_path", globalConf.SFTPD.OPKSSHPath) + viper.SetDefault("sftpd.opkssh_checksum", globalConf.SFTPD.OPKSSHChecksum) 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/internal/sftpd/internal_test.go b/internal/sftpd/internal_test.go index 7f749d8e..1a928162 100644 --- a/internal/sftpd/internal_test.go +++ b/internal/sftpd/internal_test.go @@ -16,6 +16,7 @@ package sftpd import ( "bytes" + "context" "errors" "fmt" "io" @@ -1818,6 +1819,7 @@ func TestCanReadSymlink(t *testing.T) { } func TestAuthenticationErrors(t *testing.T) { + sftpAuthError := newAuthenticationError(nil, "", "") loginMethod := dataprovider.SSHLoginMethodPassword username := "test user" err := newAuthenticationError(fmt.Errorf("cannot validate credentials: %w", util.NewRecordNotFoundError("not found")), @@ -1842,3 +1844,42 @@ func TestAuthenticationErrors(t *testing.T) { assert.ErrorIs(t, err, sftpAuthError) assert.NotErrorIs(t, err, util.ErrNotFound) } + +type mockCommandExecutor struct { + Output []byte + Err error +} + +func (f mockCommandExecutor) CombinedOutput(ctx context.Context, name string, args ...string) ([]byte, error) { + return f.Output, f.Err +} + +func TestVerifyWithOPKSSH(t *testing.T) { + sshCert := []byte(`ssh-rsa-cert-v01@openssh.com AAAAHHNzaC1yc2EtY2VydC12MDFAb3BlbnNzaC5jb20AAAAg4+hKHVPKv183MU/Q7XD/mzDBFSc2YY3eraltxLMGJo0AAAADAQABAAABAQCe6jMoy1xCQgiZkZJ7gi6NLj4uRqz2OaUGK/OJYZTfBqK+SlS9iymAluHu9K+cc4+0qxx0gn7dRTJWINSgzvca6ayYe995EKgD1hE5krh9BH0bRrXB+hGqyslcZOgLNO+v8jYojClQbRtET2tS+xb4k33GCuL5wgla2790ZgOQgs7huQUjG0S8c1W+EYt6fI4cWE/DeEBnv9sqryS8rOb0PbM6WUd7XBadwySFWYQUX0ei56GNt12Z4gADEGlFQV/OnV0PvnTcAMGUl0rfToPgJ4jgogWKoTVWuZ9wyA/x+2LRLRvgm2a969ig937/AH0i0Wq+FzqfK7EXQ99Yf5K/AAAAAAAAAAAAAAACAAAAFGhvc3QuZXhhbXBsZS5jb20ta2V5AAAAFAAAABBob3N0LmV4YW1wbGUuY29tAAAAAGXEzYAAAAAAd8sP4wAAAAAAAAAAAAAAAAAAARcAAAAHc3NoLXJzYQAAAAMBAAEAAAEBAL4PXUPSERufZWCW/hhEnylk3IeMgaa+2HcNY5Cur77a8fYy6OYZAPF+vhJUT0akwGUpTeXAZumAgHECDrJlw1J+jo9ZVT0AKDo0wU77IzNzYxob7+dpB02NJ7DLAXmPauQ07Zc5pWJFVKtmuh7YH9pjYtNXSMOXye7k06PBGzX+ztIt7nPWvD9fR2mZeTSoljeBCGZHwdlnV2ESQlQbBoEI93RPxqxJh/UCDatQPhpDbyverr2ZvB9Y45rqsx6ZVmu5RXl3MfBU1U21W/4ia2di3PybyD4rSmVoam0efcqxo6cBKSHe26OFoTuS9zgdH0iCWL37vqOFmJ7eH91M3nMAAAEUAAAADHJzYS1zaGEyLTI1NgAAAQA/ByIegNZYJRRl413S/8LxGvTZnbxsPwaluoJ/54niGZV9P28THz7d9jXfSHPjalhH93jNPfTYXvI4opnDC37ua1Nu8KKfk40IWXnnDdZLWraUxEidIzhmfVtz8kGdGoFQ8H0EzubL7zKNOTlfSfOoDlmQVOuxT/+eh2mEp4ri0/+8J1mLfLBr8tREX0/iaNjK+RKdcyTMicKursAYMCDdu8vlaphxea+ocyHM9izSX/l33t44V13ueTqIOh2Zbl2UE2k+jk+0dc1CmV0SEoiWiIyt8TRM4yQry1vPlQLsrf28sYM/QMwnhCVhyZO3vs5F25aQWrB9d51VEzBW9/fd host.example.com`) + key, _, _, _, err := ssh.ParseAuthorizedKey(sshCert) //nolint:dogsled + require.NoError(t, err) + cert, ok := key.(*ssh.Certificate) + require.True(t, ok) + c := Configuration{} + c.executor = mockCommandExecutor{ + Err: errors.New("test error"), + } + err = c.verifyWithOPKSSH("user", cert) + assert.Error(t, err) + + c.executor = mockCommandExecutor{} + err = c.verifyWithOPKSSH("", cert) + assert.Error(t, err) + + c.executor = mockCommandExecutor{ + Output: ssh.MarshalAuthorizedKey(cert), + } + err = c.verifyWithOPKSSH("", cert) + assert.Error(t, err) + + c.executor = mockCommandExecutor{ + Output: ssh.MarshalAuthorizedKey(cert.SignatureKey), + } + err = c.verifyWithOPKSSH("", cert) + assert.NoError(t, err) +} diff --git a/internal/sftpd/server.go b/internal/sftpd/server.go index 54d50159..3bee040d 100644 --- a/internal/sftpd/server.go +++ b/internal/sftpd/server.go @@ -16,6 +16,8 @@ package sftpd import ( "bytes" + "context" + "crypto/sha256" "encoding/hex" "encoding/json" "errors" @@ -25,6 +27,7 @@ import ( "maps" "net" "os" + "os/exec" "path/filepath" "runtime/debug" "slices" @@ -82,10 +85,20 @@ var ( revokedCertManager = revokedCertificates{ certs: map[string]bool{}, } - - sftpAuthError = newAuthenticationError(nil, "", "") ) +type commandExecutor interface { + CombinedOutput(ctx context.Context, name string, args ...string) ([]byte, error) +} + +type defaultExecutor struct{} + +func (d defaultExecutor) CombinedOutput(ctx context.Context, name string, args ...string) ([]byte, error) { + cmd := exec.CommandContext(ctx, name, args...) + cmd.Env = []string{} + return cmd.CombinedOutput() +} + // Binding defines the configuration for a network listener type Binding struct { // The address to listen on. A blank value means listen on all available network interfaces. @@ -150,6 +163,10 @@ type Configuration struct { // Example content: // ["SHA256:bsBRHC/xgiqBJdSuvSTNpJNLTISP/G356jNMCRYC5Es","SHA256:119+8cL/HH+NLMawRsJx6CzPF1I3xC+jpM60bQHXGE8"] RevokedUserCertsFile string `json:"revoked_user_certs_file" mapstructure:"revoked_user_certs_file"` + // Absolute path to the opkssh binary used for OpenPubkey SSH integration + OPKSSHPath string `json:"opkssh_path" mapstructure:"opkssh_path"` + // Expected SHA256 checksum of the opkssh binary. It is verified at application startup + OPKSSHChecksum string `json:"opkssh_checksum" mapstructure:"opkssh_checksum"` // 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"` @@ -185,6 +202,7 @@ type Configuration struct { PasswordAuthentication bool `json:"password_authentication" mapstructure:"password_authentication"` certChecker *ssh.CertChecker parsedUserCAKeys []ssh.PublicKey + executor commandExecutor } type authenticationError struct { @@ -342,6 +360,7 @@ func (c *Configuration) loadFromProvider() error { // Initialize the SFTP server and add a persistent listener to handle inbound SFTP connections. func (c *Configuration) Initialize(configDir string) error { + c.executor = defaultExecutor{} if err := c.loadFromProvider(); err != nil { return fmt.Errorf("unable to load configs from provider: %w", err) } @@ -365,6 +384,9 @@ func (c *Configuration) Initialize(configDir string) error { if err := c.initializeCertChecker(configDir); err != nil { return err } + if err := c.initializeOPKSSH(); err != nil { + return err + } c.configureKeyboardInteractiveAuth(serverConfig) c.configureLoginBanner(serverConfig, configDir) c.checkSSHCommands() @@ -1069,6 +1091,49 @@ func (c *Configuration) loadHostCertificates(configDir string) ([]hostCertificat return certs, nil } +func (c *Configuration) initializeOPKSSH() error { + if c.OPKSSHPath != "" { + if len(c.parsedUserCAKeys) > 0 { + return errors.New("opkssh and certificate authorities are mutually exclusive") + } + if !util.IsFileInputValid(c.OPKSSHPath) || !filepath.IsAbs(c.OPKSSHPath) { + return fmt.Errorf("opkssh path %q is not valid, it must be an absolute path", c.OPKSSHPath) + } + if c.OPKSSHChecksum == "" { + if _, err := os.Stat(c.OPKSSHPath); err != nil { + return fmt.Errorf("error validating opkssh path %q: %w", c.OPKSSHPath, err) + } + } else { + if err := util.VerifyFileChecksum(c.OPKSSHPath, sha256.New(), c.OPKSSHChecksum, 100*1024*1024); err != nil { + return fmt.Errorf("error validating opkssh checksum: %w", err) + } + } + } + + return nil +} + +func (c *Configuration) verifyWithOPKSSH(username string, cert *ssh.Certificate) error { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + args := []string{"verify", username, util.BytesToString(ssh.MarshalAuthorizedKey(cert)), cert.Type()} + out, err := c.executor.CombinedOutput(ctx, c.OPKSSHPath, args...) + if err != nil { + logger.Debug(logSender, "", "unable to execute opk verifier: %s", string(out)) + return fmt.Errorf("unable to execute opk verifier: %w", err) + } + pubKey, _, _, _, err := ssh.ParseAuthorizedKey(out) //nolint:dogsled + if err != nil { + logger.Debug(logSender, "", "unable to validate the opk verifier output: %s", string(out)) + return fmt.Errorf("unable to validate the opk verifier output: %w", err) + } + if !bytes.Equal(pubKey.Marshal(), cert.SignatureKey.Marshal()) { + return errors.New("unable to validate opk result") + } + return nil +} + func (c *Configuration) initializeCertChecker(configDir string) error { for _, keyPath := range c.TrustedUserCAKeys { keyPath = strings.TrimSpace(keyPath) @@ -1144,34 +1209,43 @@ func (c *Configuration) validatePublicKeyCredentials(conn ssh.ConnMetadata, pubK 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() - updateLoginMetrics(&user, ipAddr, method, err) - return nil, err - } - if !c.certChecker.IsUserAuthority(cert.SignatureKey) { - err := errors.New("ssh: certificate signed by unrecognized authority") - user.Username = conn.User() - updateLoginMetrics(&user, ipAddr, method, err) - return nil, err - } - if len(cert.ValidPrincipals) == 0 { - 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 - } - if err := c.certChecker.CheckCert(conn.User(), cert); err != nil { - user.Username = conn.User() - updateLoginMetrics(&user, ipAddr, method, err) - return nil, err + if c.OPKSSHPath != "" { + if err := c.verifyWithOPKSSH(conn.User(), cert); err != nil { + err := fmt.Errorf("ssh: verification with OPK failed: %v", err) + user.Username = conn.User() + updateLoginMetrics(&user, ipAddr, method, err) + return nil, err + } + } else { + if cert.CertType != ssh.UserCert { + err := fmt.Errorf("ssh: cert has type %d", cert.CertType) + user.Username = conn.User() + updateLoginMetrics(&user, ipAddr, method, err) + return nil, err + } + if !c.certChecker.IsUserAuthority(cert.SignatureKey) { + err := errors.New("ssh: certificate signed by unrecognized authority") + user.Username = conn.User() + updateLoginMetrics(&user, ipAddr, method, err) + return nil, err + } + if len(cert.ValidPrincipals) == 0 { + 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 + } + if err := c.certChecker.CheckCert(conn.User(), cert); err != nil { + user.Username = conn.User() + updateLoginMetrics(&user, ipAddr, method, err) + return nil, err + } } certPerm = &cert.Permissions } diff --git a/internal/sftpd/sftpd_test.go b/internal/sftpd/sftpd_test.go index 135571a9..ba47edab 100644 --- a/internal/sftpd/sftpd_test.go +++ b/internal/sftpd/sftpd_test.go @@ -488,6 +488,17 @@ func TestInitialization(t *testing.T) { assert.NoError(t, err) sftpdConf.HostKeys = nil sftpdConf.HostCertificates = nil + sftpdConf.OPKSSHPath = "relative path" + err = sftpdConf.Initialize(configDir) + assert.Error(t, err) + sftpdConf.OPKSSHPath = filepath.Join(os.TempDir(), "missing path") + err = sftpdConf.Initialize(configDir) + assert.Error(t, err) + sftpdConf.OPKSSHChecksum = "invalid checksum" + err = sftpdConf.Initialize(configDir) + assert.Error(t, err) + sftpdConf.OPKSSHPath = "" + sftpdConf.OPKSSHChecksum = "" sftpdConf.RevokedUserCertsFile = "." err = sftpdConf.Initialize(configDir) assert.Error(t, err) diff --git a/internal/util/util.go b/internal/util/util.go index d3eb7dc1..11715062 100644 --- a/internal/util/util.go +++ b/internal/util/util.go @@ -23,6 +23,7 @@ import ( "crypto/rand" "crypto/rsa" "crypto/sha256" + "crypto/subtle" "crypto/tls" "crypto/x509" "encoding/hex" @@ -30,6 +31,7 @@ import ( "encoding/pem" "errors" "fmt" + "hash" "io" "io/fs" "math" @@ -930,3 +932,40 @@ func SlicesEqual(s1, s2 []string) bool { return true } + +// VerifyFileChecksum computes the hash of the given file using the provided +// hash algorithm and compares it against the expected checksum (in hex format). +// It returns an error if the checksum does not match or if the operation fails. +func VerifyFileChecksum(filePath string, h hash.Hash, expectedHex string, maxSize int64) error { + expected, err := hex.DecodeString(expectedHex) + if err != nil { + return fmt.Errorf("invalid checksum %q: %w", expectedHex, err) + } + + f, err := os.Open(filePath) + if err != nil { + return err + } + defer f.Close() + + if maxSize > 0 { + fi, err := f.Stat() + if err != nil { + return err + } + if fi.Size() > maxSize { + return fmt.Errorf("file too large: %s", ByteCountIEC(fi.Size())) + } + } + + if _, err := io.Copy(h, f); err != nil { + return err + } + + actual := h.Sum(nil) + if subtle.ConstantTimeCompare(actual, expected) != 1 { + return errors.New("checksum mismatch") + } + + return nil +} diff --git a/sftpgo.json b/sftpgo.json index 98b425dc..e128c22b 100644 --- a/sftpgo.json +++ b/sftpgo.json @@ -100,6 +100,8 @@ "public_key_algorithms": [], "trusted_user_ca_keys": [], "revoked_user_certs_file": "", + "opkssh_path": "", + "opkssh_checksum": "", "login_banner_file": "", "enabled_ssh_commands": [ "md5sum",