fix(ssh): fetch ssh key by fingerprint (#39423)

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

cmd/keys.go

@@ -74,7 +74,7 @@ func runKeys(ctx context.Context, c *cli.Command) error {
 
 	setup(ctx, c.Bool("debug"))
 
-	authorizedString, extra := private.AuthorizedPublicKeyByContent(ctx, content)
+	authorizedString, extra := private.AuthorizedPublicKeyForSSH(ctx, content)
 	// do not use handleCliResponseExtra or cli.NewExitError, if it exists immediately, it breaks some tests like Test_CmdKeys
 	if extra.Error != nil {
 		return extra.Error

cmd/serv.go

@@ -343,8 +343,8 @@ func runServ(ctx context.Context, c *cli.Command) error {
 
 	// Update user key activity.
 	if results.PublicKeyID > 0 {
-		if err = private.UpdatePublicKeyInRepo(ctx, results.PublicKeyID, results.RepoID); err != nil {
-			return fail(ctx, "Failed to update public key", "UpdatePublicKeyInRepo: %v", err)
+		if err = private.UpdatePublicKeyLastUsed(ctx, results.PublicKeyID, results.RepoID); err != nil {
+			return fail(ctx, "Failed to update public key", "UpdatePublicKeyLastUsed: %v", err)
 		}
 	}
 

models/asymkey/ssh_key.go

@@ -175,12 +175,16 @@ func GetPublicKeyByID(ctx context.Context, keyID int64) (*PublicKey, error) {
 	return key, nil
 }
 
-// SearchPublicKeyByContent searches content as prefix (leak e-mail part)
-// and returns public key found.
-func SearchPublicKeyByContent(ctx context.Context, content string) (*PublicKey, error) {
+func SearchPublicKeyForSSH(ctx context.Context, sshPubKey string) (*PublicKey, error) {
+	// this function is designed to only accept SSH public keys,
+	// because there might be different methods to calculate the fingerprint in the future (at the moment: "SHA256:...")
+	fingerprint, err := CalcFingerprint(sshPubKey)
+	if err != nil {
+		return nil, err
+	}
 	key := new(PublicKey)
 	has, err := db.GetEngine(ctx).
-		Where("content like ?", content+"%").
+		Where("fingerprint = ?", fingerprint).
 		Get(key)
 	if err != nil {
 		return nil, err
@@ -190,12 +194,12 @@ func SearchPublicKeyByContent(ctx context.Context, content string) (*PublicKey,
 	return key, nil
 }
 
-// SearchPublicKeyByContentExact searches content
-// and returns public key found.
-func SearchPublicKeyByContentExact(ctx context.Context, content string) (*PublicKey, error) {
+func SearchPrincipalKey(ctx context.Context, principalKey string) (*PublicKey, error) {
+	// FIXME: this function is wrong, and there is no index on the content column
+	// In the future, the existing principal keys should be migrated to use the fingerprint ("principal:{name}") instead of the content
 	key := new(PublicKey)
 	has, err := db.GetEngine(ctx).
-		Where("content = ?", content).
+		Where("content = ?", principalKey).
 		Get(key)
 	if err != nil {
 		return nil, err
@@ -321,10 +325,10 @@ func deleteKeysMarkedForDeletion(ctx context.Context, keys []string) (bool, erro
 	return db.WithTx2(ctx, func(ctx context.Context) (bool, error) {
 		// Delete keys marked for deletion
 		var sshKeysNeedUpdate bool
-		for _, KeyToDelete := range keys {
-			key, err := SearchPublicKeyByContent(ctx, KeyToDelete)
+		for _, sshKeyToDelete := range keys {
+			key, err := SearchPublicKeyForSSH(ctx, sshKeyToDelete)
 			if err != nil {
-				log.Error("SearchPublicKeyByContent: %v", err)
+				log.Error("SearchPublicKeyForSSH: %v", err)
 				continue
 			}
 			if _, err = db.DeleteByID[PublicKey](ctx, key.ID); err != nil {

modules/log/logger_global.go

@@ -34,10 +34,6 @@ func Debug(format string, v ...any) {
 	Log(1, DEBUG, format, v...)
 }
 
-func IsDebug() bool {
-	return GetLevel() <= DEBUG
-}
-
 func Info(format string, v ...any) {
 	Log(1, INFO, format, v...)
 }

modules/private/key.go

@@ -10,19 +10,14 @@ import (
 	"gitea.dev/modules/setting"
 )
 
-// UpdatePublicKeyInRepo update public key and if necessary deploy key updates
-func UpdatePublicKeyInRepo(ctx context.Context, keyID, repoID int64) error {
-	// Ask for running deliver hook and test pull request tasks.
+func UpdatePublicKeyLastUsed(ctx context.Context, keyID, repoID int64) error {
 	reqURL := setting.LocalURL + fmt.Sprintf("api/internal/ssh/%d/update/%d", keyID, repoID)
 	req := newInternalRequestAPI(ctx, reqURL, "POST")
 	_, extra := requestJSONResp(req, &ResponseText{})
 	return extra.Error
 }
 
-// AuthorizedPublicKeyByContent searches content as prefix (leak e-mail part)
-// and returns public key found.
-func AuthorizedPublicKeyByContent(ctx context.Context, content string) (*ResponseText, ResponseExtra) {
-	// Ask for running deliver hook and test pull request tasks.
+func AuthorizedPublicKeyForSSH(ctx context.Context, content string) (*ResponseText, ResponseExtra) {
 	reqURL := setting.LocalURL + "api/internal/ssh/authorized_keys"
 	req := newInternalRequestAPI(ctx, reqURL, "POST")
 	req.Param("content", content)

modules/ssh/ssh.go

@@ -159,9 +159,9 @@ func keyPermissions(keyID int64) *gossh.Permissions {
 // 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), conn.RemoteAddr())
-	}
+	sshPubKey := string(gossh.MarshalAuthorizedKey(key))
+	fingerprint := gossh.FingerprintSHA256(key)
+	log.Debug("Handle Public Key: Fingerprint: %s from %s", fingerprint, conn.RemoteAddr())
 
 	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)
@@ -171,9 +171,7 @@ func publicKeyHandler(ctx context.Context, conn gossh.ConnMetadata, key gossh.Pu
 
 	// 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", conn.RemoteAddr(), gossh.FingerprintSHA256(key))
-		}
+		log.Debug("Handle Certificate: %s Fingerprint: %s is a certificate", conn.RemoteAddr(), fingerprint)
 
 		if len(setting.SSH.TrustedUserCAKeys) == 0 {
 			log.Warn("Certificate Rejected: No trusted certificate authorities for this server")
@@ -187,78 +185,68 @@ func publicKeyHandler(ctx context.Context, conn gossh.ConnMetadata, key gossh.Pu
 			return nil, util.ErrPermissionDenied
 		}
 
-		// look for the exact principal
-	principalLoop:
+		certChecker := &gossh.CertChecker{
+			IsUserAuthority: func(auth gossh.PublicKey) bool {
+				marshaled := auth.Marshal()
+				for _, k := range setting.SSH.TrustedUserCAKeysParsed {
+					if bytes.Equal(marshaled, k.Marshal()) {
+						return true
+					}
+				}
+				return false
+			},
+		}
+
+		// check the CA of the cert
+		if !certChecker.IsUserAuthority(cert.SignatureKey) {
+			log.Warn("Principal Rejected: %s Untrusted Authority Signature Fingerprint %s", conn.RemoteAddr(), gossh.FingerprintSHA256(cert.SignatureKey))
+			log.Warn("Failed authentication attempt from %s", conn.RemoteAddr())
+			return nil, util.ErrPermissionDenied
+		}
+
+	principalLoop: // look for the exact principal
 		for _, principal := range cert.ValidPrincipals {
-			pkey, err := asymkey_model.SearchPublicKeyByContentExact(ctx, principal)
+			pkey, err := asymkey_model.SearchPrincipalKey(ctx, principal)
 			if err != nil {
 				if asymkey_model.IsErrKeyNotExist(err) {
 					log.Debug("Principal Rejected: %s Unknown Principal: %s", conn.RemoteAddr(), principal)
 					continue principalLoop
 				}
-				log.Error("SearchPublicKeyByContentExact: %v", err)
+				log.Error("SearchPrincipalKey: %v", err)
 				return nil, util.ErrPermissionDenied
 			}
 
-			c := &gossh.CertChecker{
-				IsUserAuthority: func(auth gossh.PublicKey) bool {
-					marshaled := auth.Marshal()
-					for _, k := range setting.SSH.TrustedUserCAKeysParsed {
-						if bytes.Equal(marshaled, k.Marshal()) {
-							return true
-						}
-					}
-
-					return false
-				},
-			}
-
-			// 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", conn.RemoteAddr(), gossh.FingerprintSHA256(cert.SignatureKey), principal)
-				}
-				continue principalLoop
-			}
-
 			// validate the cert for this principal
-			if err := c.CheckCert(principal, cert); err != nil {
+			if err := certChecker.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, conn.RemoteAddr())
+				log.Warn("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 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", conn.RemoteAddr(), gossh.FingerprintSHA256(key), principal)
-			}
+			log.Debug("Successfully authenticated: %s Certificate Fingerprint: %s Principal: %s", conn.RemoteAddr(), fingerprint, principal)
 			return keyPermissions(pkey.ID), nil
 		}
 
-		log.Warn("From %s Fingerprint: %s is a certificate, but no valid principals found", conn.RemoteAddr(), gossh.FingerprintSHA256(key))
+		log.Warn("From %s Fingerprint: %s is a certificate, but no valid principals found", conn.RemoteAddr(), fingerprint)
 		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", conn.RemoteAddr(), gossh.FingerprintSHA256(key))
-	}
+	log.Debug("Handle Public Key: %s Fingerprint: %s is not a certificate", conn.RemoteAddr(), fingerprint)
 
-	pkey, err := asymkey_model.SearchPublicKeyByContent(ctx, strings.TrimSpace(string(gossh.MarshalAuthorizedKey(key))))
+	pkey, err := asymkey_model.SearchPublicKeyForSSH(ctx, sshPubKey)
 	if err != nil {
 		if asymkey_model.IsErrKeyNotExist(err) {
-			log.Warn("Unknown public key: %s from %s", gossh.FingerprintSHA256(key), conn.RemoteAddr())
+			log.Warn("Unknown public key: %s from %s", fingerprint, conn.RemoteAddr())
 			log.Warn("Failed authentication attempt from %s", conn.RemoteAddr())
 			return nil, util.ErrPermissionDenied
 		}
-		log.Error("SearchPublicKeyByContent: %v", err)
+		log.Error("SearchPublicKeyForSSH: %v", err)
 		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", conn.RemoteAddr(), gossh.FingerprintSHA256(key))
-	}
+	log.Debug("Successfully authenticated: %s Public Key Fingerprint: %s", conn.RemoteAddr(), fingerprint)
 	return keyPermissions(pkey.ID), nil
 }
 

routers/private/internal.go

@@ -78,7 +78,7 @@ func Routes() *web.Router {
 	r.AfterRouting(common.AuditOrigin(audit_model.OriginSystem))
 
 	r.Get("/dummy", misc.DummyOK)
-	r.Post("/ssh/authorized_keys", AuthorizedPublicKeyByContent)
+	r.Post("/ssh/authorized_keys", AuthorizedPublicKeyForSSH)
 	r.Post("/ssh/{id}/update/{repoid}", UpdatePublicKeyInRepo)
 	r.Post("/ssh/log", bind(private.SSHLogOption{}), SSHLog)
 	r.Post("/hook/pre-receive/{owner}/{repo}", RepoAssignment, bind(private.HookOptions{}), HookPreReceive)

routers/private/key.go

@@ -37,12 +37,9 @@ func UpdatePublicKeyInRepo(ctx *context.PrivateContext) {
 	ctx.PlainText(http.StatusOK, "success")
 }
 
-// AuthorizedPublicKeyByContent searches content as prefix (without comment part)
-// and returns public key found.
-func AuthorizedPublicKeyByContent(ctx *context.PrivateContext) {
+func AuthorizedPublicKeyForSSH(ctx *context.PrivateContext) {
 	content := ctx.FormString("content")
-
-	publicKey, err := asymkey_model.SearchPublicKeyByContent(ctx, content)
+	publicKey, err := asymkey_model.SearchPublicKeyForSSH(ctx, content)
 	if err != nil {
 		ctx.PrivateInternalErrorf("%v", err)
 		return

services/auth/httpsign.go

@@ -167,13 +167,13 @@ func VerifyCert(r *http.Request) (*asymkey_model.PublicKey, error) {
 	// Now for each of the certificate valid principals
 	for _, principal := range cert.ValidPrincipals {
 		// Look in the db for the public key
-		publicKey, err := asymkey_model.SearchPublicKeyByContentExact(r.Context(), principal)
+		publicKey, err := asymkey_model.SearchPrincipalKey(r.Context(), principal)
 		if asymkey_model.IsErrKeyNotExist(err) {
 			// No public key matches this principal - try the next principal
 			continue
 		} else if err != nil {
 			// this error will be a db error therefore we can't solve this and we should abort
-			log.Error("SearchPublicKeyByContentExact: %v", err)
+			log.Error("SearchPrincipalKey: %v", err)
 			return nil, err
 		}