mirror of
https://github.com/netbirdio/netbird.git
synced 2026-10-30 08:19:09 +01:00
[client] Refuse SSH JWT auth when the client changes the user name between auth requests on one connection (#8389)
* Refuse SSH JWT auth when the client changes the user name between auth requests on one connection * Run SSH sessions and SFTP as the OS user that was authorized, taken from the connection context
This commit is contained in:
@@ -16,6 +16,7 @@ import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/gliderlabs/ssh"
|
||||
"github.com/golang-jwt/jwt/v5"
|
||||
cryptossh "golang.org/x/crypto/ssh"
|
||||
|
||||
@@ -751,3 +752,76 @@ func TestJWTMultipleAudiences(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// authRequestContext is the context passwordHandler sees: firstUser is what gliderlabs/ssh recorded
|
||||
// from the first auth request of the connection, currentUser is the name in the request being handled.
|
||||
type authRequestContext struct {
|
||||
ssh.Context
|
||||
firstUser string
|
||||
currentUser string
|
||||
}
|
||||
|
||||
func (c authRequestContext) User() string { return c.firstUser }
|
||||
|
||||
func (c authRequestContext) RemoteAddr() net.Addr {
|
||||
return &net.TCPAddr{IP: net.IPv4(100, 64, 0, 2), Port: 12345}
|
||||
}
|
||||
|
||||
func (c authRequestContext) Value(key any) any {
|
||||
if key == (preAuthConnKey{}) {
|
||||
return authRequestConn{user: c.currentUser}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
type authRequestConn struct {
|
||||
cryptossh.ConnMetadata
|
||||
user string
|
||||
}
|
||||
|
||||
func (c authRequestConn) User() string { return c.user }
|
||||
|
||||
// TestJWTRejectsUserNameChange tests that a token authorized for one OS user is not accepted
|
||||
// once the client switches to another user name on the same connection.
|
||||
func TestJWTRejectsUserNameChange(t *testing.T) {
|
||||
jwksServer, privateKey, jwksURL := setupJWKSServer(t)
|
||||
defer jwksServer.Close()
|
||||
|
||||
const (
|
||||
issuer = "https://test-issuer.example.com"
|
||||
audience = "test-audience"
|
||||
)
|
||||
|
||||
server := New(&Config{
|
||||
JWT: &JWTConfig{
|
||||
Issuer: issuer,
|
||||
Audiences: []string{audience},
|
||||
KeysLocation: jwksURL,
|
||||
},
|
||||
})
|
||||
|
||||
testUserHash, err := sshuserhash.HashUserID("test-user")
|
||||
require.NoError(t, err)
|
||||
|
||||
server.UpdateSSHAuth(&sshauth.Config{
|
||||
UserIDClaim: sshauth.DefaultUserIDClaim,
|
||||
AuthorizedUsers: []sshuserhash.UserIDHash{testUserHash},
|
||||
MachineUsers: map[string][]uint32{
|
||||
"alice": {0},
|
||||
"bob": {0},
|
||||
},
|
||||
})
|
||||
|
||||
token := generateValidJWT(t, privateKey, issuer, audience)
|
||||
|
||||
assert.True(t, server.passwordHandler(authRequestContext{firstUser: "alice", currentUser: "alice"}, token),
|
||||
"mapped OS user should be accepted")
|
||||
assert.False(t, server.passwordHandler(authRequestContext{firstUser: "root", currentUser: "root"}, token),
|
||||
"unmapped OS user should be rejected")
|
||||
assert.False(t, server.passwordHandler(authRequestContext{firstUser: "alice", currentUser: "root"}, token),
|
||||
"change to an unmapped OS user after the first request should be rejected")
|
||||
assert.False(t, server.passwordHandler(authRequestContext{firstUser: "alice", currentUser: "bob"}, token),
|
||||
"change to another mapped OS user after the first request should be rejected")
|
||||
assert.False(t, server.passwordHandler(authRequestContext{firstUser: "root", currentUser: "alice"}, token),
|
||||
"change to a mapped OS user after the first request should be rejected")
|
||||
}
|
||||
|
||||
@@ -135,6 +135,9 @@ type authKey string
|
||||
// Used to track authenticated connections for status display and port forwarding.
|
||||
type connKey string
|
||||
|
||||
// preAuthConnKey is the context key under which the connection being authenticated is stored.
|
||||
type preAuthConnKey struct{}
|
||||
|
||||
func newAuthKey(username string, remoteAddr net.Addr) authKey {
|
||||
return authKey(fmt.Sprintf("%s@%s", username, remoteAddr.String()))
|
||||
}
|
||||
@@ -431,7 +434,7 @@ func (s *Server) buildSessionInfo(state *sessionState) SessionInfo {
|
||||
|
||||
remoteAddr := session.RemoteAddr().String()
|
||||
info := SessionInfo{
|
||||
Username: session.User(),
|
||||
Username: sessionUser(session),
|
||||
RemoteAddress: remoteAddr,
|
||||
Command: cmd,
|
||||
JWTUsername: state.jwtUsername,
|
||||
@@ -686,6 +689,19 @@ func (s *Server) passwordHandler(ctx ssh.Context, password string) bool {
|
||||
remoteAddr := ctx.RemoteAddr()
|
||||
logger := s.getRequestLogger(ctx)
|
||||
|
||||
// ctx.User() is the name from the first auth request of the connection, while the session
|
||||
// runs as the name from the request that succeeds. Like OpenSSH, refuse a change of user
|
||||
// name between requests so the account that is authorized is the one the session runs as.
|
||||
conn, ok := ctx.Value(preAuthConnKey{}).(cryptossh.ConnMetadata)
|
||||
if !ok {
|
||||
logger.Warn("SSH auth denied: connection being authenticated is unknown")
|
||||
return false
|
||||
}
|
||||
if conn.User() != osUsername {
|
||||
logger.Warnf("SSH auth denied: user name changed during authentication from %q to %q", osUsername, conn.User())
|
||||
return false
|
||||
}
|
||||
|
||||
if err := s.ensureJWTValidator(); err != nil {
|
||||
logger.Errorf("JWT validator initialization failed: %v", err)
|
||||
return false
|
||||
@@ -904,6 +920,7 @@ func (s *Server) createSSHServer(addr net.Addr) (*ssh.Server, error) {
|
||||
|
||||
if s.jwtEnabled {
|
||||
server.PasswordHandler = s.passwordHandler
|
||||
server.ServerConfigCallback = trackPreAuthConn
|
||||
}
|
||||
|
||||
hostKeyPEM := ssh.HostKeyPEM(s.hostKeyPEM)
|
||||
@@ -915,6 +932,16 @@ func (s *Server) createSSHServer(addr net.Addr) (*ssh.Server, error) {
|
||||
return server, nil
|
||||
}
|
||||
|
||||
// trackPreAuthConn stores the connection being authenticated in its context, which gives
|
||||
// passwordHandler the user name of the auth request it is handling.
|
||||
func trackPreAuthConn(ctx ssh.Context) *cryptossh.ServerConfig {
|
||||
return &cryptossh.ServerConfig{
|
||||
PreAuthConnCallback: func(conn cryptossh.ServerPreAuthConn) {
|
||||
ctx.SetValue(preAuthConnKey{}, conn)
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
func (s *Server) storeRemoteForwardListener(key forwardKey, ln net.Listener) {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
|
||||
@@ -12,10 +12,17 @@ import (
|
||||
log "github.com/sirupsen/logrus"
|
||||
)
|
||||
|
||||
// sessionUser returns the OS user name the session was authenticated for. The context keeps the
|
||||
// name from the first auth request of the connection, which is the one passwordHandler checked,
|
||||
// while sess.User() is the name from the last request the client sent and may differ from it.
|
||||
func sessionUser(sess ssh.Session) string {
|
||||
return sess.Context().User()
|
||||
}
|
||||
|
||||
// associateJWTUsername extracts pending JWT username for the session and associates it with the session state.
|
||||
// Returns the JWT username (empty if none) for logging purposes.
|
||||
func (s *Server) associateJWTUsername(sess ssh.Session, sessionKey sessionKey) string {
|
||||
key := newAuthKey(sess.User(), sess.RemoteAddr())
|
||||
key := newAuthKey(sessionUser(sess), sess.RemoteAddr())
|
||||
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
@@ -61,7 +68,7 @@ func (s *Server) sessionHandler(session ssh.Session) {
|
||||
defer drainWindowChanges(winCh)
|
||||
}
|
||||
|
||||
privilegeResult, err := s.userPrivilegeCheck(session.User())
|
||||
privilegeResult, err := s.userPrivilegeCheck(sessionUser(session))
|
||||
if err != nil {
|
||||
s.handlePrivError(logger, session, err)
|
||||
return
|
||||
@@ -90,7 +97,7 @@ func (s *Server) registerSession(session ssh.Session, sessionType string) sessio
|
||||
shortID := hex.EncodeToString(hash[:4])
|
||||
|
||||
remoteAddr := session.RemoteAddr().String()
|
||||
username := session.User()
|
||||
username := sessionUser(session)
|
||||
sessionKey := sessionKey(fmt.Sprintf("%s@%s-%s", username, remoteAddr, shortID))
|
||||
|
||||
s.mu.Lock()
|
||||
|
||||
@@ -43,7 +43,7 @@ func (s *Server) sftpSubsystemHandler(sess ssh.Session) {
|
||||
}
|
||||
|
||||
result := s.CheckPrivileges(PrivilegeCheckRequest{
|
||||
RequestedUsername: sess.User(),
|
||||
RequestedUsername: sessionUser(sess),
|
||||
FeatureSupportsUserSwitch: true,
|
||||
FeatureName: FeatureSFTP,
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user