Skip to content

Commit d1c1e3c

Browse files
authored
Fine tune ssh related comments and code (#32846)
Add more comments to explain the ssh problem, and rename `sshConn` to `sshSession`
1 parent 1cfb718 commit d1c1e3c

File tree

1 file changed

+6
-6
lines changed

1 file changed

+6
-6
lines changed

modules/ssh/ssh.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"errors"
1414
"fmt"
1515
"io"
16-
"maps"
1716
"net"
1817
"os"
1918
"os/exec"
@@ -49,6 +48,10 @@ import (
4948
// Then sessionHandler should only use the "verified keyID" from the original ssh conn, but not the ctx one.
5049
// Otherwise, if a user provides 2 keys A (a correct one) and B (public key matches but no private key),
5150
// then only A succeeds to authenticate, sessionHandler will see B's keyID
51+
//
52+
// After x/crypto >= 0.31.0 (fix CVE-2024-45337), the PublicKeyCallback will be called again for the verified key,
53+
// it mitigates the misuse for most cases, it's still good for us to make sure we don't rely on that mitigation
54+
// and do not misuse the PublicKeyCallback: we should only use the verified keyID from the verified ssh conn.
5255

5356
const giteaPermissionExtensionKeyID = "gitea-perm-ext-key-id"
5457

@@ -100,8 +103,8 @@ func ptr[T any](intf any) *T {
100103
func sessionHandler(session ssh.Session) {
101104
// here can't use session.Permissions() because it only uses the value from ctx, which might not be the authenticated one.
102105
// so we must use the original ssh conn, which always contains the correct (verified) keyID.
103-
sshConn := ptr[sessionPartial](session)
104-
keyID := sshConn.conn.Permissions.Extensions[giteaPermissionExtensionKeyID]
106+
sshSession := ptr[sessionPartial](session)
107+
keyID := sshSession.conn.Permissions.Extensions[giteaPermissionExtensionKeyID]
105108

106109
command := session.RawCommand()
107110

@@ -210,10 +213,7 @@ func publicKeyHandler(ctx ssh.Context, key ssh.PublicKey) bool {
210213

211214
// first, reset the ctx permissions (just like https://github.com/gliderlabs/ssh/pull/243 does)
212215
// it shouldn't be reused across different ssh conn (sessions), each pub key should have its own "Permissions"
213-
oldCtxPerm := ctx.Permissions().Permissions
214216
ctx.Permissions().Permissions = &gossh.Permissions{}
215-
ctx.Permissions().Permissions.CriticalOptions = maps.Clone(oldCtxPerm.CriticalOptions)
216-
217217
setPermExt := func(keyID int64) {
218218
ctx.Permissions().Permissions.Extensions = map[string]string{
219219
giteaPermissionExtensionKeyID: fmt.Sprint(keyID),

0 commit comments

Comments
 (0)