Fix misuse of PublicKeyCallback (#32810)

Only upgrading the ssh package is not enough.

1 files changed, 62 insertions, 8 deletions

diff --git a/modules/ssh/ssh.go b/modules/ssh/ssh.go
index f8e4f569b8..6d0695ee16 100644
--- a/modules/ssh/ssh.go
+++ b/modules/ssh/ssh.go
@@ -13,10 +13,12 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"maps"
 	"net"
 	"os"
 	"os/exec"
 	"path/filepath"
+	"reflect"
 	"strconv"
 	"strings"
 	"sync"
@@ -33,9 +35,22 @@ import (
 	gossh "golang.org/x/crypto/ssh"
 )
 
-type contextKey string
-
-const giteaKeyID = contextKey("gitea-key-id")
+// 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
+
+const giteaPermissionExtensionKeyID = "gitea-perm-ext-key-id"
 
 func getExitStatusFromError(err error) int {
 	if err == nil {
@@ -61,8 +76,32 @@ 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 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) {
-	keyID := fmt.Sprintf("%d", session.Context().Value(giteaKeyID).(int64))
+	// 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.
+	sshConn := ptr[sessionPartial](session)
+	keyID := sshConn.conn.Permissions.Extensions[giteaPermissionExtensionKeyID]
 
 	command := session.RawCommand()
 
@@ -164,6 +203,23 @@ func sessionHandler(session ssh.Session) {
 }
 
 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"
+	oldCtxPerm := ctx.Permissions().Permissions
+	ctx.Permissions().Permissions = &gossh.Permissions{}
+	ctx.Permissions().Permissions.CriticalOptions = maps.Clone(oldCtxPerm.CriticalOptions)
+
+	setPermExt := func(keyID int64) {
+		ctx.Permissions().Permissions.Extensions = map[string]string{
+			giteaPermissionExtensionKeyID: fmt.Sprint(keyID),
+		}
+	}
+
 	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())
 	}
@@ -238,8 +294,7 @@ func publicKeyHandler(ctx ssh.Context, key ssh.PublicKey) bool {
 			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)
 			}
-			ctx.SetValue(giteaKeyID, pkey.ID)
-
+			setPermExt(pkey.ID)
 			return true
 		}
 
@@ -266,8 +321,7 @@ func publicKeyHandler(ctx ssh.Context, key ssh.PublicKey) bool {
 	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))
 	}
-	ctx.SetValue(giteaKeyID, pkey.ID)
-
+	setPermExt(pkey.ID)
 	return true
 }