From f7cb37ca5a85f566023e5589440f6e4b11287c8b Mon Sep 17 00:00:00 2001 From: Gusted Date: Wed, 11 Dec 2024 23:14:27 +0100 Subject: [PATCH] fix: ensure correct ssh public key is used for authentication - The root cause is described in https://github.com/golang/crypto/commit/b4f1988a35dee11ec3e05d6bf3e90b695fbd8909 - Move to a fork of `github.com/gliderlabs/ssh` that exposes the permissions that was chosen by `x/crypto/ssh` after succesfully authenticating, this is the recommended mitigation by the Golang security team. The fork exposes this, since `gliderlabs/ssh` instead relies on context values to do so, which is vulnerable to the same attack, although partially mitigated by the fix in `x/crypto/ssh` it would not be good practice and defense deep to rely on it. - Existing tests covers that the functionality is preserved. - No tests are added to ensure it fixes the described security, the exploit relies on non-standard SSH behavior it would be too hard to craft SSH packets to exploit this. (cherry picked from commit 3e1b03838e455d88089b4d6683443e1adfb4752e) Conflicts: go.mod go.sum trivial context conflict --- go.mod | 2 ++ go.sum | 4 ++-- modules/ssh/ssh.go | 17 +++++++++-------- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/go.mod b/go.mod index 7bac9c10ce..4714b645b2 100644 --- a/go.mod +++ b/go.mod @@ -309,3 +309,5 @@ exclude github.com/goccy/go-json v0.4.11 exclude github.com/satori/go.uuid v1.2.0 replace github.com/mholt/archiver/v3 => code.forgejo.org/forgejo/archiver/v3 v3.5.1 + +replace github.com/gliderlabs/ssh => code.forgejo.org/forgejo/ssh v0.0.0-20241211213324-5fc306ca0616 diff --git a/go.sum b/go.sum index e50c59f267..7fcdc12284 100644 --- a/go.sum +++ b/go.sum @@ -39,6 +39,8 @@ code.forgejo.org/forgejo/archiver/v3 v3.5.1 h1:UmmbA7D5550uf71SQjarmrn6yKwOGxtEj code.forgejo.org/forgejo/archiver/v3 v3.5.1/go.mod h1:e3dqJ7H78uzsRSEACH1joayhuSyhnonssnDhppzS1L4= code.forgejo.org/forgejo/reply v1.0.1 h1:usZi5yx7/g0D+xtGPJEM6mCvoDNdWvmtJu5J9/B/KBI= code.forgejo.org/forgejo/reply v1.0.1/go.mod h1:RyZUfzQLc+fuLIGjTSQWDAJWPiL4WtKXB/FifT5fM7U= +code.forgejo.org/forgejo/ssh v0.0.0-20241211213324-5fc306ca0616 h1:kEZL84+02jY9RxXM4zHBWZ3Fml0B09cmP1LGkDsCfIA= +code.forgejo.org/forgejo/ssh v0.0.0-20241211213324-5fc306ca0616/go.mod h1:zpHEXBstFnQYtGnB8k8kQLol82umzn/2/snG7alWVD8= code.gitea.io/actions-proto-go v0.4.0 h1:OsPBPhodXuQnsspG1sQ4eRE1PeoZyofd7+i73zCwnsU= code.gitea.io/actions-proto-go v0.4.0/go.mod h1:mn7Wkqz6JbnTOHQpot3yDeHx+O5C9EGhMEE+htvHBas= code.gitea.io/gitea-vet v0.2.3 h1:gdFmm6WOTM65rE8FUBTRzeQZYzXePKSSB1+r574hWwI= @@ -278,8 +280,6 @@ github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nos github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM= github.com/fxamacker/cbor/v2 v2.5.0 h1:oHsG0V/Q6E/wqTS2O1Cozzsy69nqCiguo5Q1a1ADivE= github.com/fxamacker/cbor/v2 v2.5.0/go.mod h1:TA1xS00nchWmaBnEIxPSE5oHLuJBAVvqrtAnWBwBCVo= -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/go-ap/activitypub v0.0.0-20231114162308-e219254dc5c9 h1:j2TrkUG/NATGi/EQS+MvEoF79CxiRUmT16ErFroNcKI= github.com/go-ap/activitypub v0.0.0-20231114162308-e219254dc5c9/go.mod h1:cJ9Ye0ZNSMN7RzZDBRY3E+8M3Bpf/R1JX22Ir9yX6WI= github.com/go-ap/errors v0.0.0-20231003111023-183eef4b31b7 h1:I2nuhyVI/48VXoRCCZR2hYBgnSXa+EuDJf/VyX06TC0= diff --git a/modules/ssh/ssh.go b/modules/ssh/ssh.go index f8e4f569b8..6ee10f718b 100644 --- a/modules/ssh/ssh.go +++ b/modules/ssh/ssh.go @@ -11,7 +11,6 @@ import ( "crypto/x509" "encoding/pem" "errors" - "fmt" "io" "net" "os" @@ -33,10 +32,6 @@ import ( gossh "golang.org/x/crypto/ssh" ) -type contextKey string - -const giteaKeyID = contextKey("gitea-key-id") - func getExitStatusFromError(err error) int { if err == nil { return 0 @@ -62,7 +57,7 @@ func getExitStatusFromError(err error) int { } func sessionHandler(session ssh.Session) { - keyID := fmt.Sprintf("%d", session.Context().Value(giteaKeyID).(int64)) + keyID := session.ConnPermissions().Extensions["forgejo-key-id"] command := session.RawCommand() @@ -238,7 +233,10 @@ 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) + if ctx.Permissions().Extensions == nil { + ctx.Permissions().Extensions = map[string]string{} + } + ctx.Permissions().Extensions["forgejo-key-id"] = strconv.FormatInt(pkey.ID, 10) return true } @@ -266,7 +264,10 @@ 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) + if ctx.Permissions().Extensions == nil { + ctx.Permissions().Extensions = map[string]string{} + } + ctx.Permissions().Extensions["forgejo-key-id"] = strconv.FormatInt(pkey.ID, 10) return true }