mirror of
				https://github.com/go-gitea/gitea.git
				synced 2025-10-31 19:45:25 +01:00 
			
		
		
		
	Fix misuse of PublicKeyCallback (#32810)
Only upgrading the ssh package is not enough.
This commit is contained in:
		
							parent
							
								
									30008fcfcf
								
							
						
					
					
						commit
						2910f384d5
					
				
							
								
								
									
										2
									
								
								go.mod
									
									
									
									
									
								
							
							
						
						
									
										2
									
								
								go.mod
									
									
									
									
									
								
							| @ -48,7 +48,7 @@ require ( | ||||
| 	github.com/ethantkoenig/rupture v1.0.1 | ||||
| 	github.com/felixge/fgprof v0.9.5 | ||||
| 	github.com/fsnotify/fsnotify v1.7.0 | ||||
| 	github.com/gliderlabs/ssh v0.3.7 | ||||
| 	github.com/gliderlabs/ssh v0.3.8 | ||||
| 	github.com/go-ap/activitypub v0.0.0-20240910141749-b4b8c8aa484c | ||||
| 	github.com/go-ap/jsonld v0.0.0-20221030091449-f2a191312c73 | ||||
| 	github.com/go-chi/chi/v5 v5.1.0 | ||||
|  | ||||
							
								
								
									
										4
									
								
								go.sum
									
									
									
									
									
								
							
							
						
						
									
										4
									
								
								go.sum
									
									
									
									
									
								
							| @ -293,8 +293,8 @@ github.com/fxamacker/cbor/v2 v2.7.0 h1:iM5WgngdRBanHcxugY4JySA0nk1wZorNOpTgCMedv | ||||
| github.com/fxamacker/cbor/v2 v2.7.0/go.mod h1:pxXPTn3joSm21Gbwsv0w9OSA2y1HFR9qXEeXQVeNoDQ= | ||||
| github.com/git-lfs/pktline v0.0.0-20230103162542-ca444d533ef1 h1:mtDjlmloH7ytdblogrMz1/8Hqua1y8B4ID+bh3rvod0= | ||||
| github.com/git-lfs/pktline v0.0.0-20230103162542-ca444d533ef1/go.mod h1:fenKRzpXDjNpsIBhuhUzvjCKlDjKam0boRAenTE0Q6A= | ||||
| github.com/gliderlabs/ssh v0.3.7 h1:iV3Bqi942d9huXnzEF2Mt+CY9gLu8DNM4Obd+8bODRE= | ||||
| github.com/gliderlabs/ssh v0.3.7/go.mod h1:zpHEXBstFnQYtGnB8k8kQLol82umzn/2/snG7alWVD8= | ||||
| github.com/gliderlabs/ssh v0.3.8 h1:a4YXD1V7xMF9g5nTkdfnja3Sxy1PVDCj1Zg4Wb8vY6c= | ||||
| github.com/gliderlabs/ssh v0.3.8/go.mod h1:xYoytBv1sV0aL3CavoDuJIQNURXkkfPA/wxQ1pL1fAU= | ||||
| github.com/glycerine/go-unsnap-stream v0.0.0-20181221182339-f9677308dec2/go.mod h1:/20jfyN9Y5QPEAprSgKAUr+glWDY39ZiUEAYOEv5dsE= | ||||
| github.com/glycerine/goconvey v0.0.0-20190410193231-58a59202ab31/go.mod h1:Ogl1Tioa0aV7gstGFO7KhffUsb9M4ydbEbbxpcEDc24= | ||||
| github.com/go-ap/activitypub v0.0.0-20240910141749-b4b8c8aa484c h1:82lzmsy5Nr6JA6HcLRVxGfbdSoWfW45C6jnY3zFS7Ks= | ||||
|  | ||||
| @ -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 | ||||
| // 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 giteaKeyID = contextKey("gitea-key-id") | ||||
| 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 | ||||
| } | ||||
| 
 | ||||
|  | ||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user