Skip to content

Commit 8f999e5

Browse files
caarlos0belak
andauthored
fix: Avoid Permissions leaking between auth callbacks (#33)
Co-authored-by: Kaleb Elwert <[email protected]>
1 parent 4fe22b0 commit 8f999e5

File tree

2 files changed

+69
-4
lines changed

2 files changed

+69
-4
lines changed

context.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,10 @@ func newContext(srv *Server) (*sshContext, context.CancelFunc) {
120120
return ctx, cancel
121121
}
122122

123+
func resetPermissions(ctx Context) {
124+
ctx.Permissions().Permissions = &gossh.Permissions{}
125+
}
126+
123127
// this is separate from newContext because we will get ConnMetadata
124128
// at different points so it needs to be applied separately
125129
func applyConnMetadata(ctx Context, conn gossh.ConnMetadata) {

server.go

Lines changed: 65 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package ssh
22

33
import (
44
"context"
5+
"encoding/base64"
56
"errors"
67
"fmt"
78
"net"
@@ -29,6 +30,16 @@ var DefaultChannelHandlers = map[string]ChannelHandler{
2930
"session": DefaultSessionHandler,
3031
}
3132

33+
var permissionsPublicKeyExt = "gliderlabs/ssh.PublicKey"
34+
35+
func ensureNoPKInPermissions(ctx Context) error {
36+
if _, ok := ctx.Permissions().Permissions.Extensions[permissionsPublicKeyExt]; ok {
37+
return errors.New("misconfigured server: public key incorrectly set")
38+
}
39+
40+
return nil
41+
}
42+
3243
// Server defines parameters for running an SSH server. The zero value for
3344
// Server is a valid configuration. When both PasswordHandler and
3445
// PublicKeyHandler are nil, no client authentication is performed.
@@ -151,27 +162,48 @@ func (srv *Server) config(ctx Context) *gossh.ServerConfig {
151162
}
152163
if srv.PasswordHandler != nil {
153164
config.PasswordCallback = func(conn gossh.ConnMetadata, password []byte) (*gossh.Permissions, error) {
165+
resetPermissions(ctx)
154166
applyConnMetadata(ctx, conn)
155-
if ok := srv.PasswordHandler(ctx, string(password)); !ok {
167+
err := ensureNoPKInPermissions(ctx)
168+
if err != nil {
169+
return ctx.Permissions().Permissions, err
170+
}
171+
ok := srv.PasswordHandler(ctx, string(password))
172+
if !ok {
156173
return ctx.Permissions().Permissions, fmt.Errorf("permission denied")
157174
}
158175
return ctx.Permissions().Permissions, nil
159176
}
160177
}
161178
if srv.PublicKeyHandler != nil {
162179
config.PublicKeyCallback = func(conn gossh.ConnMetadata, key gossh.PublicKey) (*gossh.Permissions, error) {
180+
resetPermissions(ctx)
163181
applyConnMetadata(ctx, conn)
164-
if ok := srv.PublicKeyHandler(ctx, key); !ok {
182+
err := ensureNoPKInPermissions(ctx)
183+
if err != nil {
184+
return ctx.Permissions().Permissions, err
185+
}
186+
ok := srv.PublicKeyHandler(ctx, key)
187+
if !ok {
165188
return ctx.Permissions().Permissions, fmt.Errorf("permission denied")
166189
}
167-
ctx.SetValue(ContextKeyPublicKey, key)
190+
191+
pkStr := base64.StdEncoding.EncodeToString(key.Marshal())
192+
ctx.Permissions().Permissions.Extensions[permissionsPublicKeyExt] = pkStr
193+
168194
return ctx.Permissions().Permissions, nil
169195
}
170196
}
171197
if srv.KeyboardInteractiveHandler != nil {
172198
config.KeyboardInteractiveCallback = func(conn gossh.ConnMetadata, challenger gossh.KeyboardInteractiveChallenge) (*gossh.Permissions, error) {
199+
resetPermissions(ctx)
173200
applyConnMetadata(ctx, conn)
174-
if ok := srv.KeyboardInteractiveHandler(ctx, challenger); !ok {
201+
ok := srv.KeyboardInteractiveHandler(ctx, challenger)
202+
err := ensureNoPKInPermissions(ctx)
203+
if err != nil {
204+
return ctx.Permissions().Permissions, err
205+
}
206+
if !ok {
175207
return ctx.Permissions().Permissions, fmt.Errorf("permission denied")
176208
}
177209
return ctx.Permissions().Permissions, nil
@@ -303,6 +335,35 @@ func (srv *Server) HandleConn(newConn net.Conn) {
303335
return
304336
}
305337

338+
if sshConn.Permissions != nil {
339+
// Now that the connection was authed, if the permissionsPublicKeyExt was
340+
// attached, we need to re-parse it as a public key.
341+
if keyData, ok := sshConn.Permissions.Extensions[permissionsPublicKeyExt]; ok {
342+
decodedData, err := base64.StdEncoding.DecodeString(keyData)
343+
if err != nil {
344+
if srv.ConnectionFailedCallback != nil {
345+
srv.ConnectionFailedCallback(conn, err)
346+
}
347+
return
348+
}
349+
350+
key, err := gossh.ParsePublicKey(decodedData)
351+
if err != nil {
352+
if srv.ConnectionFailedCallback != nil {
353+
srv.ConnectionFailedCallback(conn, err)
354+
}
355+
return
356+
}
357+
358+
ctx.SetValue(ContextKeyPublicKey, key)
359+
}
360+
}
361+
362+
// Additionally, now that the connection was authed, we can take the
363+
// permissions off of the gossh.Conn and re-attach them to the Permissions
364+
// object stored in the Context.
365+
ctx.Permissions().Permissions = sshConn.Permissions
366+
306367
srv.trackConn(sshConn, true)
307368
defer srv.trackConn(sshConn, false)
308369

0 commit comments

Comments
 (0)