Skip to content

Commit

Permalink
[v17] Skip session data recording for networking requests (#49033)
Browse files Browse the repository at this point in the history
* Skip session data recording for networking requests.

* Add regression test.

* Add individual event validation to test.
  • Loading branch information
Joerger authored Nov 15, 2024
1 parent eb7afee commit 2c52c6c
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 0 deletions.
6 changes: 6 additions & 0 deletions lib/srv/ctx.go
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,12 @@ func (c *ServerContext) takeClosers() []io.Closer {
// When the ServerContext (connection) is closed, emit "session.data" event
// containing how much data was transmitted and received over the net.Conn.
func (c *ServerContext) reportStats(conn utils.Stater) {
// We may not want to record session data for this connection context, e.g. if this is
// for a networking subprocess tied to a shell process.
if c.SessionRecordingConfig.GetMode() == types.RecordOff {
return
}

// Never emit session data events for the proxy or from a Teleport node if
// sessions are being recorded at the proxy (this would result in double
// events).
Expand Down
3 changes: 3 additions & 0 deletions lib/srv/regular/sshserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -1180,6 +1180,7 @@ func (s *Server) startNetworkingProcess(scx *srv.ServerContext) (*networking.Pro
if err != nil {
return nil, trace.Wrap(err)
}
nsctx.SessionRecordingConfig.SetMode(types.RecordOff)
nsctx.ExecType = teleport.NetworkingSubCommand
scx.Parent().AddCloser(nsctx)

Expand Down Expand Up @@ -1460,6 +1461,7 @@ func (s *Server) handleDirectTCPIPRequest(ctx context.Context, ccx *sshutils.Con
}
scx.IsTestStub = s.isTestStub
scx.AddCloser(channel)
scx.SessionRecordingConfig.SetMode(types.RecordOff)
scx.ExecType = teleport.ChanDirectTCPIP
scx.SrcAddr = sshutils.JoinHostPort(req.Orig, req.OrigPort)
scx.DstAddr = sshutils.JoinHostPort(req.Host, req.Port)
Expand Down Expand Up @@ -2173,6 +2175,7 @@ func (s *Server) createForwardingContext(ctx context.Context, ccx *sshutils.Conn
scx.ExecType = teleport.TCPIPForwardRequest
scx.SrcAddr = listenAddr
scx.DstAddr = ccx.NetConn.RemoteAddr().String()
scx.SessionRecordingConfig.SetMode(types.RecordOff)
scx.SetAllowFileCopying(s.allowFileCopying)

if err := s.canPortForward(scx); err != nil {
Expand Down
121 changes: 121 additions & 0 deletions lib/srv/regular/sshserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,127 @@ loop:
require.Equal(t, "echo 1", execEvent.CommandMetadata.Command)
}

// TestSessionAuditLog tests that the expected audit events are emitted for a
// session with various subsystems involved.
//
// Note: This is a regression test for a bug which resulted in extra, empty session.data
// events for networking requests.
// See https://github.com/gravitational/teleport/issues/48728.
func TestSessionAuditLog(t *testing.T) {
ctx := context.Background()
t.Parallel()
f := newFixtureWithoutDiskBasedLogging(t)

// Set up a mock emitter so we can capture audit events.
emitter := eventstest.NewChannelEmitter(32)
f.ssh.srv.StreamEmitter = events.StreamerAndEmitter{
Streamer: events.NewDiscardStreamer(),
Emitter: emitter,
}

// Enable x11 forwarding
f.ssh.srv.x11 = &x11.ServerConfig{
Enabled: true,
DisplayOffset: x11.DefaultDisplayOffset,
MaxDisplay: x11.DefaultMaxDisplays,
}

// Allow x11, agent, and port forwarding for the user.
roleName := services.RoleNameForUser(f.user)
role, err := f.testSrv.Auth().GetRole(ctx, roleName)
require.NoError(t, err)
roleOptions := role.GetOptions()
roleOptions.PermitX11Forwarding = types.NewBool(true)
roleOptions.ForwardAgent = types.NewBool(true)
roleOptions.PortForwarding = types.NewBoolOption(true)
role.SetOptions(roleOptions)
_, err = f.testSrv.Auth().UpsertRole(ctx, role)
require.NoError(t, err)

nextEvent := func() apievents.AuditEvent {
select {
case event := <-emitter.C():
return event
case <-time.After(time.Second):
require.Fail(t, "timed out waiting for event")
}
return nil
}

// Start a new session
se, err := f.ssh.clt.NewSession(ctx)
require.NoError(t, err)

// start interactive SSH session (new shell):
err = se.Shell(ctx)
require.NoError(t, err)

e := nextEvent()
startEvent, ok := e.(*apievents.SessionStart)
require.True(t, ok, "expected SessionStart event but got event of type %T", e)
require.NotEmpty(t, startEvent.SessionID, "expected non empty sessionID")
sessionID := startEvent.SessionID

// Request agent forwarding, no individual event emitted.
err = agent.RequestAgentForwarding(se.Session)
require.NoError(t, err)

// Request x11 forwarding, event should be emitted immediately.
clientXAuthEntry, err := x11.NewFakeXAuthEntry(x11.Display{})
require.NoError(t, err)
err = x11.RequestForwarding(se.Session, clientXAuthEntry)
require.NoError(t, err)

x11Event := nextEvent()
require.IsType(t, &apievents.X11Forward{}, x11Event, "expected X11Forward event but got event of tgsype %T", x11Event)

// Request a remote port forwarding listener. The event is logged at the end of the session.
listener, err := f.ssh.clt.Listen("tcp", "127.0.0.1:0")
require.NoError(t, err)

// Start up a test server that uses the port forwarded listener.
ts := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
fmt.Fprintln(w, "hello, world")
}))
t.Cleanup(ts.Close)
ts.Listener = listener
ts.Start()

// Request forward to remote port. Each dial should result in a new event. Note that we don't
// know what port the server will forward the connection on, so we don't have an easy way to
// validate the event's addr field.
conn, err := f.ssh.clt.DialContext(context.Background(), "tcp", listener.Addr().String())
require.NoError(t, err)
conn.Close()

directPortForwardEvent := nextEvent()
require.IsType(t, &apievents.PortForward{}, directPortForwardEvent, "expected PortForward event but got event of type %T", directPortForwardEvent)

// End the session. Session leave, data, and end events should be emitted, along with the remote
// port forwarding event.
se.Close()

e = nextEvent()
leaveEvent, ok := e.(*apievents.SessionLeave)
require.True(t, ok, "expected SessionLeave event but got event of type %T", e)
require.Equal(t, sessionID, leaveEvent.SessionID)

e = nextEvent()
dataEvent, ok := e.(*apievents.SessionData)
require.True(t, ok, "expected SessionData event but got event of type %T", e)
require.Equal(t, sessionID, dataEvent.SessionID)

e = nextEvent()
remotePortForwardEvent, ok := e.(*apievents.PortForward)
require.True(t, ok, "expected PortForward event but got event of type %T", e)
require.Equal(t, listener.Addr().String(), remotePortForwardEvent.Addr)

e = nextEvent()
endEvent, ok := e.(*apievents.SessionEnd)
require.True(t, ok, "expected SessionEnd event but got event of type %T", e)
require.Equal(t, sessionID, endEvent.SessionID)
}

func newProxyClient(t *testing.T, testSvr *auth.TestServer) (*authclient.Client, string) {
// create proxy client used in some tests
proxyID := uuid.New().String()
Expand Down

0 comments on commit 2c52c6c

Please sign in to comment.