Mirrors
/
gitea
mirror of https://github.com/go-gitea/gitea.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fine tune ssh related comments and code (#32846) 

Add more comments to explain the ssh problem, 
and rename `sshConn` to `sshSession`
This commit is contained in:
wxiaoguang 2024-12-15 14:07:50 +08:00 committed by GitHub
parent 1cfb718976
commit d1c1e3cbcc
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 6 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
modules/ssh/ssh.go
View File

@ -13,7 +13,6 @@ import (
"errors" "errors"
"fmt" "fmt"
"io" "io"
"maps"
"net" "net"
"os" "os"
"os/exec" "os/exec"
@ -49,6 +48,10 @@ import (
// Then sessionHandler should only use the "verified keyID" from the original ssh conn, but not the ctx one. // 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), // 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 // 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" const giteaPermissionExtensionKeyID = "gitea-perm-ext-key-id"
@ -100,8 +103,8 @@ func ptr[T any](intf any) *T {
func sessionHandler(session ssh.Session) { 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. // 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. // so we must use the original ssh conn, which always contains the correct (verified) keyID.
sshConn := ptr[sessionPartial](session) sshSession := ptr[sessionPartial](session)
keyID := sshConn.conn.Permissions.Extensions[giteaPermissionExtensionKeyID] keyID := sshSession.conn.Permissions.Extensions[giteaPermissionExtensionKeyID]
command := session.RawCommand() command := session.RawCommand()
@ -210,10 +213,7 @@ func publicKeyHandler(ctx ssh.Context, key ssh.PublicKey) bool {
// first, reset the ctx permissions (just like https://github.com/gliderlabs/ssh/pull/243 does) // 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" // 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 = &gossh.Permissions{}
ctx.Permissions().Permissions.CriticalOptions = maps.Clone(oldCtxPerm.CriticalOptions)
setPermExt := func(keyID int64) { setPermExt := func(keyID int64) {
ctx.Permissions().Permissions.Extensions = map[string]string{ ctx.Permissions().Permissions.Extensions = map[string]string{
giteaPermissionExtensionKeyID: fmt.Sprint(keyID), giteaPermissionExtensionKeyID: fmt.Sprint(keyID),