refactor: replace gliderlabs/ssh with golang.org/x/crypto/ssh (#38837)

Migrate away from this thin ssh wrapper module while adding more test
coverage.

Removes `sessionPartial`, which hand-copied the layout of a private
`gliderlabs/ssh` struct and reinterpreted a pointer to it via
`reflect.UnsafePointer` to reach the permissions of the authenticated
connection. The layout is unchecked, so an upstream field reorder would
mismatch silently.

The builtin server only needs the session channel with `exec` and
`shell`. Serving those on `x/crypto` drops the hack and the dependency,
since `PublicKeyCallback` returns permissions per key and `x/crypto`
assigns them only after verifying the signature.

Two benign behavior changes:

1. Internal session handler errors report exit status 1 rather than 0,
so a client no longer reads a failure as success.
1. An unusable host key is fatal at startup instead of being replaced by
an ephemeral one that would trigger an error at the client.

modules/ssh/ssh.go

@@ -13,7 +13,6 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
-	"reflect"
 	"strconv"
 	"strings"
 	"sync"
@@ -25,30 +24,11 @@ import (
 	"gitea.dev/modules/log"
 	"gitea.dev/modules/process"
 	"gitea.dev/modules/setting"
+	"gitea.dev/modules/util"
 
-	"github.com/gliderlabs/ssh"
 	gossh "golang.org/x/crypto/ssh"
 )
 
-// The ssh auth overall works like this:
-// NewServerConn:
-//	serverHandshake+serverAuthenticate:
-//		PublicKeyCallback:
-//			PublicKeyHandler (our code):
-//				reset(ctx.Permissions) and set ctx.Permissions.giteaKeyID = keyID
-//		pubKey.Verify
-//		return ctx.Permissions // only reaches here, the pub key is really authenticated
-//	set conn.Permissions from serverAuthenticate
-//  sessionHandler(conn)
-//
-// Then sessionHandler should only use the "verified keyID" from the original ssh conn, but not the ctx one.
-// Otherwise, if a user provides 2 keys A (a correct one) and B (public key matches but no private key),
-// then only A succeeds to authenticate, sessionHandler will see B's keyID
-//
-// After x/crypto >= 0.31.0 (fix CVE-2024-45337), the PublicKeyCallback will be called again for the verified key,
-// it mitigates the misuse for most cases, it's still good for us to make sure we don't rely on that mitigation
-// and do not misuse the PublicKeyCallback: we should only use the verified keyID from the verified ssh conn.
-
 const giteaPermissionExtensionKeyID = "gitea-perm-ext-key-id"
 
 func getExitStatusFromError(err error) int {
@@ -75,45 +55,20 @@ func getExitStatusFromError(err error) int {
 	return waitStatus.ExitStatus()
 }
 
-// sessionPartial is the private struct from "gliderlabs/ssh/session.go"
-// We need to read the original "conn" field from "ssh.Session interface" which contains the "*session pointer"
-// https://github.com/gliderlabs/ssh/blob/d137aad99cd6f2d9495bfd98c755bec4e5dffb8c/session.go#L109-L113
-// If upstream fixes the problem and/or changes the struct, we need to follow.
-// If the struct mismatches, the builtin ssh server will fail during integration tests.
-type sessionPartial struct {
-	sync.Mutex
-	gossh.Channel
-	conn *gossh.ServerConn
-}
+func sessionHandler(session *sshSession) int {
+	// the conn permissions are the ones of the key which really authenticated, see publicKeyHandler
+	keyID := session.conn.Permissions.Extensions[giteaPermissionExtensionKeyID]
 
-func ptr[T any](intf any) *T {
-	// https://pkg.go.dev/unsafe#Pointer
-	// (1) Conversion of a *T1 to Pointer to *T2.
-	// Provided that T2 is no larger than T1 and that the two share an equivalent memory layout,
-	// this conversion allows reinterpreting data of one type as data of another type.
-	v := reflect.ValueOf(intf)
-	p := v.UnsafePointer()
-	return (*T)(p)
-}
-
-func sessionHandler(session ssh.Session) {
-	// here can't use session.Permissions() because it only uses the value from ctx, which might not be the authenticated one.
-	// so we must use the original ssh conn, which always contains the correct (verified) keyID.
-	sshSession := ptr[sessionPartial](session)
-	keyID := sshSession.conn.Permissions.Extensions[giteaPermissionExtensionKeyID]
-
-	command := session.RawCommand()
-
-	log.Trace("SSH: Payload: %v", command)
+	log.Trace("SSH: Payload: %v", session.rawCmd)
 
 	args := []string{"--config=" + setting.CustomConf, "serv", "key-" + keyID}
 	log.Trace("SSH: Arguments: %v", args)
 
-	ctx, cancel := context.WithCancel(session.Context())
+	ctx, cancel := context.WithCancel(session.ctx)
 	defer cancel()
 
 	gitProtocol := ""
-	for _, env := range session.Environ() {
+	for _, env := range session.env {
 		if strings.HasPrefix(env, "GIT_PROTOCOL=") {
 			_, gitProtocol, _ = strings.Cut(env, "=")
 			break
@@ -123,7 +78,7 @@ func sessionHandler(session ssh.Session) {
 	cmd := exec.CommandContext(ctx, setting.AppPath, args...)
 	cmd.Env = append(
 		os.Environ(),
-		"SSH_ORIGINAL_COMMAND="+command,
+		"SSH_ORIGINAL_COMMAND="+session.rawCmd,
 		"SKIP_MINWINSVC=1",
 		"GIT_PROTOCOL="+gitProtocol,
 	)
@@ -131,21 +86,21 @@ func sessionHandler(session ssh.Session) {
 	stdout, err := cmd.StdoutPipe()
 	if err != nil {
 		log.Error("SSH: StdoutPipe: %v", err)
-		return
+		return 1
 	}
 	defer stdout.Close()
 
 	stderr, err := cmd.StderrPipe()
 	if err != nil {
 		log.Error("SSH: StderrPipe: %v", err)
-		return
+		return 1
 	}
 	defer stderr.Close()
 
 	stdin, err := cmd.StdinPipe()
 	if err != nil {
 		log.Error("SSH: StdinPipe: %v", err)
-		return
+		return 1
 	}
 	defer stdin.Close()
 
@@ -155,7 +110,7 @@ func sessionHandler(session ssh.Session) {
 
 	if err = cmd.Start(); err != nil {
 		log.Error("SSH: Start: %v", err)
-		return
+		return 1
 	}
 
 	go func() {
@@ -193,52 +148,45 @@ func sessionHandler(session ssh.Session) {
 		}
 	}
 
-	if err := session.Exit(getExitStatusFromError(err)); err != nil && !errors.Is(err, io.EOF) {
-		log.Error("Session failed to exit. %s", err)
-	}
+	return getExitStatusFromError(err)
 }
 
-func publicKeyHandler(ctx ssh.Context, key ssh.PublicKey) bool {
-	// The publicKeyHandler (PublicKeyCallback) only helps to provide the candidate keys to authenticate,
-	// It does NOT really verify here, so we could only record the related information here.
-	// After authentication (Verify), the "Permissions" will be assigned to the ssh conn,
-	// then we can use it in the "session handler"
-
-	// first, reset the ctx permissions (just like https://github.com/gliderlabs/ssh/pull/243 does)
-	// it shouldn't be reused across different ssh conn (sessions), each pub key should have its own "Permissions"
-	ctx.Permissions().Permissions = &gossh.Permissions{}
-	setPermExt := func(keyID int64) {
-		ctx.Permissions().Permissions.Extensions = map[string]string{
-			giteaPermissionExtensionKeyID: strconv.FormatInt(keyID, 10),
-		}
-	}
+func keyPermissions(keyID int64) *gossh.Permissions {
+	return &gossh.Permissions{Extensions: map[string]string{
+		giteaPermissionExtensionKeyID: strconv.FormatInt(keyID, 10),
+	}}
+}
 
+// publicKeyHandler only offers the candidate keys, it does not verify them. x/crypto assigns the
+// returned Permissions to the ssh conn once it verified the signature for that key, so a user
+// offering keys A (with a private key) and B (without one) authenticates and is served as A.
+func publicKeyHandler(ctx context.Context, conn gossh.ConnMetadata, key gossh.PublicKey) (*gossh.Permissions, error) {
 	if log.IsDebug() { // <- FingerprintSHA256 is kinda expensive so only calculate it if necessary
-		log.Debug("Handle Public Key: Fingerprint: %s from %s", gossh.FingerprintSHA256(key), ctx.RemoteAddr())
+		log.Debug("Handle Public Key: Fingerprint: %s from %s", gossh.FingerprintSHA256(key), conn.RemoteAddr())
 	}
 
-	if ctx.User() != setting.SSH.BuiltinServerUser {
-		log.Warn("Invalid SSH username %s - must use %s for all git operations via ssh", ctx.User(), setting.SSH.BuiltinServerUser)
-		log.Warn("Failed authentication attempt from %s", ctx.RemoteAddr())
-		return false
+	if conn.User() != setting.SSH.BuiltinServerUser {
+		log.Warn("Invalid SSH username %s - must use %s for all git operations via ssh", conn.User(), setting.SSH.BuiltinServerUser)
+		log.Warn("Failed authentication attempt from %s", conn.RemoteAddr())
+		return nil, util.ErrPermissionDenied
 	}
 
 	// check if we have a certificate
 	if cert, ok := key.(*gossh.Certificate); ok {
 		if log.IsDebug() { // <- FingerprintSHA256 is kinda expensive so only calculate it if necessary
-			log.Debug("Handle Certificate: %s Fingerprint: %s is a certificate", ctx.RemoteAddr(), gossh.FingerprintSHA256(key))
+			log.Debug("Handle Certificate: %s Fingerprint: %s is a certificate", conn.RemoteAddr(), gossh.FingerprintSHA256(key))
 		}
 
 		if len(setting.SSH.TrustedUserCAKeys) == 0 {
 			log.Warn("Certificate Rejected: No trusted certificate authorities for this server")
-			log.Warn("Failed authentication attempt from %s", ctx.RemoteAddr())
-			return false
+			log.Warn("Failed authentication attempt from %s", conn.RemoteAddr())
+			return nil, util.ErrPermissionDenied
 		}
 
 		if cert.CertType != gossh.UserCert {
 			log.Warn("Certificate Rejected: Not a user certificate")
-			log.Warn("Failed authentication attempt from %s", ctx.RemoteAddr())
-			return false
+			log.Warn("Failed authentication attempt from %s", conn.RemoteAddr())
+			return nil, util.ErrPermissionDenied
 		}
 
 		// look for the exact principal
@@ -247,11 +195,11 @@ func publicKeyHandler(ctx ssh.Context, key ssh.PublicKey) bool {
 			pkey, err := asymkey_model.SearchPublicKeyByContentExact(ctx, principal)
 			if err != nil {
 				if asymkey_model.IsErrKeyNotExist(err) {
-					log.Debug("Principal Rejected: %s Unknown Principal: %s", ctx.RemoteAddr(), principal)
+					log.Debug("Principal Rejected: %s Unknown Principal: %s", conn.RemoteAddr(), principal)
 					continue principalLoop
 				}
 				log.Error("SearchPublicKeyByContentExact: %v", err)
-				return false
+				return nil, util.ErrPermissionDenied
 			}
 
 			c := &gossh.CertChecker{
@@ -270,7 +218,7 @@ func publicKeyHandler(ctx ssh.Context, key ssh.PublicKey) bool {
 			// check the CA of the cert
 			if !c.IsUserAuthority(cert.SignatureKey) {
 				if log.IsDebug() {
-					log.Debug("Principal Rejected: %s Untrusted Authority Signature Fingerprint %s for Principal: %s", ctx.RemoteAddr(), gossh.FingerprintSHA256(cert.SignatureKey), principal)
+					log.Debug("Principal Rejected: %s Untrusted Authority Signature Fingerprint %s for Principal: %s", conn.RemoteAddr(), gossh.FingerprintSHA256(cert.SignatureKey), principal)
 				}
 				continue principalLoop
 			}
@@ -278,44 +226,42 @@ func publicKeyHandler(ctx ssh.Context, key ssh.PublicKey) bool {
 			// validate the cert for this principal
 			if err := c.CheckCert(principal, cert); err != nil {
 				// User is presenting an invalid certificate - STOP any further processing
-				log.Error("Invalid Certificate KeyID %s with Signature Fingerprint %s presented for Principal: %s from %s", cert.KeyId, gossh.FingerprintSHA256(cert.SignatureKey), principal, ctx.RemoteAddr())
-				log.Warn("Failed authentication attempt from %s", ctx.RemoteAddr())
+				log.Error("Invalid Certificate KeyID %s with Signature Fingerprint %s presented for Principal: %s from %s", cert.KeyId, gossh.FingerprintSHA256(cert.SignatureKey), principal, conn.RemoteAddr())
+				log.Warn("Failed authentication attempt from %s", conn.RemoteAddr())
 
-				return false
+				return nil, util.ErrPermissionDenied
 			}
 
 			if log.IsDebug() { // <- FingerprintSHA256 is kinda expensive so only calculate it if necessary
-				log.Debug("Successfully authenticated: %s Certificate Fingerprint: %s Principal: %s", ctx.RemoteAddr(), gossh.FingerprintSHA256(key), principal)
+				log.Debug("Successfully authenticated: %s Certificate Fingerprint: %s Principal: %s", conn.RemoteAddr(), gossh.FingerprintSHA256(key), principal)
 			}
-			setPermExt(pkey.ID)
-			return true
+			return keyPermissions(pkey.ID), nil
 		}
 
-		log.Warn("From %s Fingerprint: %s is a certificate, but no valid principals found", ctx.RemoteAddr(), gossh.FingerprintSHA256(key))
-		log.Warn("Failed authentication attempt from %s", ctx.RemoteAddr())
-		return false
+		log.Warn("From %s Fingerprint: %s is a certificate, but no valid principals found", conn.RemoteAddr(), gossh.FingerprintSHA256(key))
+		log.Warn("Failed authentication attempt from %s", conn.RemoteAddr())
+		return nil, util.ErrPermissionDenied
 	}
 
 	if log.IsDebug() { // <- FingerprintSHA256 is kinda expensive so only calculate it if necessary
-		log.Debug("Handle Public Key: %s Fingerprint: %s is not a certificate", ctx.RemoteAddr(), gossh.FingerprintSHA256(key))
+		log.Debug("Handle Public Key: %s Fingerprint: %s is not a certificate", conn.RemoteAddr(), gossh.FingerprintSHA256(key))
 	}
 
 	pkey, err := asymkey_model.SearchPublicKeyByContent(ctx, strings.TrimSpace(string(gossh.MarshalAuthorizedKey(key))))
 	if err != nil {
 		if asymkey_model.IsErrKeyNotExist(err) {
-			log.Warn("Unknown public key: %s from %s", gossh.FingerprintSHA256(key), ctx.RemoteAddr())
-			log.Warn("Failed authentication attempt from %s", ctx.RemoteAddr())
-			return false
+			log.Warn("Unknown public key: %s from %s", gossh.FingerprintSHA256(key), conn.RemoteAddr())
+			log.Warn("Failed authentication attempt from %s", conn.RemoteAddr())
+			return nil, util.ErrPermissionDenied
 		}
 		log.Error("SearchPublicKeyByContent: %v", err)
-		return false
+		return nil, util.ErrPermissionDenied
 	}
 
 	if log.IsDebug() { // <- FingerprintSHA256 is kinda expensive so only calculate it if necessary
-		log.Debug("Successfully authenticated: %s Public Key Fingerprint: %s", ctx.RemoteAddr(), gossh.FingerprintSHA256(key))
+		log.Debug("Successfully authenticated: %s Public Key Fingerprint: %s", conn.RemoteAddr(), gossh.FingerprintSHA256(key))
 	}
-	setPermExt(pkey.ID)
-	return true
+	return keyPermissions(pkey.ID), nil
 }
 
 // sshConnectionFailed logs a failed connection
@@ -329,25 +275,6 @@ func sshConnectionFailed(conn net.Conn, err error) {
 
 // Listen starts an SSH server listening on given port.
 func Listen(host string, port int, ciphers, keyExchanges, macs []string) {
-	srv := ssh.Server{
-		Addr:             net.JoinHostPort(host, strconv.Itoa(port)),
-		PublicKeyHandler: publicKeyHandler,
-		Handler:          sessionHandler,
-		ServerConfigCallback: func(ctx ssh.Context) *gossh.ServerConfig {
-			config := &gossh.ServerConfig{}
-			config.KeyExchanges = keyExchanges
-			config.MACs = macs
-			config.Ciphers = ciphers
-			return config
-		},
-		ConnectionFailedCallback: sshConnectionFailed,
-		// We need to explicitly disable the PtyCallback so text displays
-		// properly.
-		PtyCallback: func(ctx ssh.Context, pty ssh.Pty) bool {
-			return false
-		},
-	}
-
 	hostKeyFiles := make([]string, 0, len(setting.SSH.ServerHostKeys))
 	for _, key := range setting.SSH.ServerHostKeys {
 		_, err := os.Stat(key)
@@ -372,17 +299,34 @@ func Listen(host string, port int, ciphers, keyExchanges, macs []string) {
 		}
 	}
 
+	var hostSigners []gossh.Signer
 	for _, keyFile := range hostKeyFiles {
-		log.Info("Adding SSH host key: %s", keyFile)
-		err := srv.SetOption(ssh.HostKeyFile(keyFile))
-		if err != nil {
-			log.Error("Failed to set Host Key. %s", err)
+		pemBytes, err := os.ReadFile(keyFile)
+		if err == nil {
+			var signer gossh.Signer
+			if signer, err = gossh.ParsePrivateKey(pemBytes); err == nil {
+				log.Info("Adding SSH host key: %s", keyFile)
+				hostSigners = append(hostSigners, signer)
+				continue
+			}
 		}
+		log.Error("Failed to load SSH host key %s: %v", keyFile, err)
 	}
+
+	if len(hostSigners) == 0 {
+		log.Fatal("No usable SSH host key, tried: %v", hostKeyFiles)
+	}
+
+	srv := &sshServer{
+		addr:        net.JoinHostPort(host, strconv.Itoa(port)),
+		hostSigners: hostSigners,
+		config:      gossh.Config{Ciphers: ciphers, KeyExchanges: keyExchanges, MACs: macs},
+	}
+
 	go func() {
 		_, _, finished := process.GetManager().AddTypedContext(graceful.GetManager().HammerContext(), "Service: Built-in SSH server", process.SystemProcessType, true)
 		defer finished()
-		listen(&srv)
+		listen(srv)
 	}()
 }