gravitational/teleport
1
0
Fork 0
mirror of https://github.com/gravitational/teleport.git synced 2026-10-11 22:49:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

make session ID env vars agree (#53652)

Browse Source
This commit is contained in:
Andrew LeFevre
2025-05-29 17:36:05 +00:00
committed by GitHub
parent fd4fd2a4be
commit bb9bcca12c
10 changed files with 52 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files
integration/integration_test.go
+2 -6
View File
@@ -2246,17 +2246,14 @@ func testEnvironmentVariables(t *testing.T, suite *integrationTestSuite) {
})
require.NoError(t, err)
tc.SessionID = uuid.NewString()
// The SessionID and Web address should be set in the session env vars.
cmd := []string{"printenv", sshutils.SessionEnvVar, ";", "printenv", teleport.SSHSessionWebProxyAddr}
// The Web address should be set in the session env vars.
cmd := []string{"printenv", teleport.SSHSessionWebProxyAddr}
out := &bytes.Buffer{}
tc.Stdout = out
tc.Stdin = nil
err = tc.SSH(ctx, cmd)
require.NoError(t, err)
output := out.String()
require.Contains(t, output, tc.SessionID)
require.Contains(t, output, tc.WebProxyAddr)
term := NewTerminal(250)
@@ -2265,7 +2262,6 @@ func testEnvironmentVariables(t *testing.T, suite *integrationTestSuite) {
err = tc.SSH(ctx, nil)
require.NoError(t, err)
output = term.AllOutput()
require.Contains(t, output, tc.SessionID)
require.Contains(t, output, tc.WebProxyAddr)
}
lib/client/client_test.go
+2 -4
View File
@@ -28,7 +28,6 @@ import (
"testing"
"time"
"github.com/google/go-cmp/cmp"
"github.com/gravitational/trace"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -70,9 +69,8 @@ func TestNewSession(t *testing.T) {
ses, err = newSession(ctx, nc, nil, env, nil, nil, nil, true)
require.NoError(t, err)
require.NotNil(t, ses)
require.Empty(t, cmp.Diff(ses.env, env))
// the session ID must be taken from tne environ map, if passed:
require.Equal(t, "session-id", string(ses.id))
// the session ID must be unset from tne environ map, if we are not joining a session:
require.Empty(t, ses.id)
}
// TestProxyConnection verifies that client or server-side disconnect
lib/client/session.go
+2 -13
View File
@@ -150,20 +150,9 @@ func newSession(ctx context.Context,
}
}
// new session!
} else {
// TODO(capnspacehook): DELETE IN 17.0.0
// clients shouldn't set TELEPORT_SESSION when they aren't joining
// a session, and won't need to once all supported Proxy/Node
// versions set the session ID for new sessions
sid, ok := ns.env[sshutils.SessionEnvVar]
if !ok {
sid = string(session.NewID())
}
ns.id = session.ID(sid)
}
ns.env[sshutils.SessionEnvVar] = string(ns.id)
ns.env[sshutils.SessionEnvVar] = string(ns.id)
}
// Close the Terminal when finished.
ns.closeWait.Add(1)
lib/srv/ctx.go
+8 -19
View File
@@ -298,10 +298,6 @@ type ServerContext struct {
// term holds PTY if it was requested by the session.
term Terminal
// sessionID holds the session ID that will be used when a new
// session is created.
sessionID rsession.ID
// session holds the active session (if there's an active one).
session *session
@@ -576,12 +572,7 @@ func (c *ServerContext) ID() int {
// SessionID returns the ID of the session in the context.
func (c *ServerContext) SessionID() rsession.ID {
c.mu.RLock()
defer c.mu.RUnlock()
if c.session == nil {
return c.sessionID
}
return c.session.id
return c.ConnectionContext.GetSessionID()
}
// GetServer returns the underlying server which this context was created in.
@@ -598,32 +589,29 @@ func (c *ServerContext) CreateOrJoinSession(ctx context.Context, reg *SessionReg
// its ID will be added to the environment
ssid, found := c.getEnvLocked(sshutils.SessionEnvVar)
if !found {
c.sessionID = rsession.NewID()
c.Logger.DebugContext(ctx, "Will create new session for SSH connection")
return nil
}
// make sure whatever session is requested is a valid session
id, err := rsession.ParseID(ssid)
if err != nil {
return trace.BadParameter("invalid session ID")
return trace.BadParameter("invalid session ID %s", ssid)
}
// update ctx with the session if it exists
if sess, found := reg.findSession(*id); found {
c.sessionID = *id
c.session = sess
c.ConnectionContext.SetSessionID(*id)
c.Logger.DebugContext(ctx, "Joining active SSH session", "session_id", c.session.id)
} else {
// TODO(capnspacehook): DELETE IN 17.0.0 - by then all supported
// TODO(capnspacehook): DELETE IN 19.0.0 - by then all supported
// clients should only set TELEPORT_SESSION when they want to
// join a session. Always return an error instead of using a
// new ID.
//
// to prevent the user from controlling the session ID, generate
// a new one
c.sessionID = rsession.NewID()
c.Logger.DebugContext(ctx, "Creating new SSH session")
// The session ID the client sent was not found, ignore it; the
// connection's ID will be used as the session ID later.
c.Logger.DebugContext(ctx, "Sent session ID not found, using connection ID")
}
return nil
@@ -1146,6 +1134,7 @@ func buildEnvironment(ctx *ServerContext) []string {
}
if session.id != "" {
env.AddTrusted(teleport.SSHSessionID, string(session.id))
env.AddTrusted(sshutils.SessionEnvVar, string(session.id))
}
}
lib/srv/ctx_test.go
+16 -8
View File
@@ -24,6 +24,7 @@ import (
"github.com/gogo/protobuf/proto"
"github.com/google/go-cmp/cmp"
"github.com/gravitational/trace"
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/testing/protocmp"
@@ -281,13 +282,12 @@ func TestCreateOrJoinSession(t *testing.T) {
tests := []struct {
name string
sessionID string
expectedErr bool
wantSameSessionID bool
}{
{
name: "no session ID",
wantSameSessionID: false,
name: "no session ID",
},
// TODO(capnspacehook): Check that an error is returned in v17
{
name: "new session ID",
sessionID: string(rsession.NewID()),
@@ -326,12 +326,20 @@ func TestCreateOrJoinSession(t *testing.T) {
}
err = scx.CreateOrJoinSession(ctx, registry)
require.NoError(t, err)
require.False(t, scx.sessionID.IsZero())
if tt.wantSameSessionID {
require.Equal(t, parsedSessionID.String(), scx.sessionID.String())
if tt.expectedErr {
require.True(t, trace.IsNotFound(err))
} else {
require.NotEqual(t, parsedSessionID.String(), scx.sessionID.String())
require.NoError(t, err)
}
sessID := scx.GetSessionID()
require.False(t, sessID.IsZero())
if tt.wantSameSessionID {
require.Equal(t, parsedSessionID.String(), sessID.String())
require.Equal(t, *parsedSessionID, scx.GetSessionID())
} else {
require.NotEqual(t, parsedSessionID.String(), sessID.String())
require.NotEqual(t, *parsedSessionID, scx.GetSessionID())
}
})
}
lib/srv/exec_linux_test.go
+1
View File
@@ -84,6 +84,7 @@ func TestOSCommandPrep(t *testing.T) {
"TERM=xterm",
fmt.Sprintf("SSH_TTY=%v", scx.session.term.TTYName()),
"SSH_SESSION_ID=xxx",
"TELEPORT_SESSION=xxx",
"SSH_TELEPORT_HOST_UUID=testID",
"SSH_TELEPORT_CLUSTER_NAME=localhost",
"SSH_TELEPORT_USER=teleportUser",
lib/srv/exec_test.go
+1 -1
View File
@@ -120,7 +120,7 @@ func TestEmitExecAuditEvent(t *testing.T) {
require.Equal(t, "abc", execEvent.ForwardedBy)
require.Equal(t, expectedHostname, execEvent.ServerHostname)
require.Equal(t, "testNamespace", execEvent.ServerNamespace)
require.Equal(t, "xxx", execEvent.SessionID)
require.NotEqual(t, "xxx", execEvent.SessionID)
require.Equal(t, "10.0.0.5:4817", execEvent.RemoteAddr)
require.Equal(t, "127.0.0.1:3022", execEvent.LocalAddr)
require.NotEmpty(t, events.EventID)
lib/srv/mock.go
-2
View File
@@ -45,7 +45,6 @@ import (
"github.com/gravitational/teleport/lib/fixtures"
"github.com/gravitational/teleport/lib/service/servicecfg"
"github.com/gravitational/teleport/lib/services"
rsession "github.com/gravitational/teleport/lib/session"
"github.com/gravitational/teleport/lib/sshca"
"github.com/gravitational/teleport/lib/sshutils"
"github.com/gravitational/teleport/lib/utils"
@@ -79,7 +78,6 @@ func newTestServerContext(t *testing.T, srv Server, sessionJoiningRoleSet servic
IsTestStub: true,
ClusterName: clusterName,
srv: srv,
sessionID: rsession.NewID(),
Identity: IdentityContext{
UnmappedIdentity: ident,
Login: usr.Username,
lib/sshutils/ctx.go
+20
View File
@@ -30,12 +30,16 @@ import (
"golang.org/x/crypto/ssh"
"golang.org/x/crypto/ssh/agent"
rsession "github.com/gravitational/teleport/lib/session"
"github.com/gravitational/teleport/lib/sshutils/networking"
"github.com/gravitational/teleport/lib/teleagent"
)
// ConnectionContext manages connection-level state.
type ConnectionContext struct {
// sessionID is the Teleport session ID that all child ServerContexts will inherit.
sessionID rsession.ID
// NetConn is the base connection object.
NetConn net.Conn
@@ -95,6 +99,7 @@ func SetConnectionContextClock(clock clockwork.Clock) ConnectionContextOption {
func NewConnectionContext(ctx context.Context, nconn net.Conn, sconn *ssh.ServerConn, opts ...ConnectionContextOption) (context.Context, *ConnectionContext) {
ctx, cancel := context.WithCancel(ctx)
ccx := &ConnectionContext{
sessionID: rsession.NewID(),
NetConn: nconn,
ServerConn: sconn,
env: make(map[string]string),
@@ -136,6 +141,21 @@ func (a *agentChannel) Close() error {
a.ch.Close())
}
// GetSessionID returns the Teleport session ID that all child ServerContexts will inherit.
func (c *ConnectionContext) GetSessionID() rsession.ID {
c.mu.RLock()
defer c.mu.RUnlock()
return c.sessionID
}
// SetSessionID sets the Teleport session ID that all child ServerContexts will inherit.
func (c *ConnectionContext) SetSessionID(sessionID rsession.ID) {
c.mu.Lock()
c.sessionID = sessionID
c.mu.Unlock()
}
// StartAgentChannel sets up a new agent forwarding channel against this connection. The channel
// is automatically closed when either ConnectionContext, or the supplied context.Context
// gets canceled.
lib/web/apiserver.go
-1
View File
@@ -3917,7 +3917,6 @@ func (h *Handler) generateSession(ctx context.Context, req *TerminalRequest, clu
ServerHostname: host,
ServerHostPort: port,
Moderated: accessEvaluator.IsModerated(),
ID: session.NewID(),
Created: time.Now().UTC(),
LastActive: time.Now().UTC(),
Namespace: apidefaults.Namespace,