handle allowed bpf events in SSH access permit consistently (#66919)

* handle allowed bpf events in SSH access permit consistently

* log error if closing the session fails

* fix tests failing

* add regression test

lib/srv/sess.go

@@ -1665,37 +1665,42 @@ func (s *session) startExec(ctx context.Context, channel ssh.Channel, scx *Serve
 		return trace.BadParameter("cannot start exec with BPF enabled without an ssh access permit (this is a bug)")
 	}
 
-	auditSessID, err := execRequest.ReadAuditSessionID()
-	if err != nil {
-		s.logger.ErrorContext(ctx, "Failed to get child PID", "error", err)
-		return trace.Wrap(err)
-	}
+	var sessionContext *bpf.SessionContext
+	if bpfEnabled && len(eventsMap) != 0 {
+		auditSessID, err := execRequest.ReadAuditSessionID()
+		if err != nil {
+			s.logger.ErrorContext(ctx, "Failed to get child PID", "error", err)
+			return trace.Wrap(err)
+		}
 
-	// Open a BPF recording session.
-	sessionContext := &bpf.SessionContext{
-		Context:               scx.srv.Context(),
-		AuditSessionID:        auditSessID,
-		Emitter:               scx.BPFEmitter(),
-		Namespace:             scx.srv.GetNamespace(),
-		SessionID:             string(s.id),
-		ServerID:              scx.srv.ID(),
-		ServerHostname:        scx.srv.GetInfo().GetHostname(),
-		Login:                 scx.Identity.Login,
-		User:                  scx.Identity.TeleportUser,
-		UserOriginClusterName: scx.Identity.OriginClusterName,
-		Events:                eventsMap,
-	}
+		// Open a BPF recording session.
+		sessionContext = &bpf.SessionContext{
+			Context:               scx.srv.Context(),
+			AuditSessionID:        auditSessID,
+			Emitter:               scx.BPFEmitter(),
+			Namespace:             scx.srv.GetNamespace(),
+			SessionID:             string(s.id),
+			ServerID:              scx.srv.ID(),
+			ServerHostname:        scx.srv.GetInfo().GetHostname(),
+			Login:                 scx.Identity.Login,
+			User:                  scx.Identity.TeleportUser,
+			UserOriginClusterName: scx.Identity.OriginClusterName,
+			UserRoles:             scx.Identity.MappedRoles,
+			UserTraits:            scx.Identity.Traits,
+			Events:                eventsMap,
+		}
 
-	if err := scx.srv.GetBPF().OpenSession(sessionContext); err != nil {
-		s.logger.ErrorContext(
-			ctx, "Failed to open enhanced recording (exec) session.",
-			"command", execRequest.GetCommand(),
-			"error", err,
-		)
-		return trace.Wrap(err)
-	}
+		if err := scx.srv.GetBPF().OpenSession(sessionContext); err != nil {
+			s.logger.ErrorContext(
+				ctx, "Failed to open enhanced recording (exec) session.",
+				"command", execRequest.GetCommand(),
+				"error", err,
+			)
+			return trace.Wrap(err)
+		}
 
-	s.setHasEnhancedRecording(bpfEnabled && len(eventsMap) > 0)
+		s.setHasEnhancedRecording(true)
+	}
 
 	s.logger.DebugContext(ctx, "Waiting for continue signal.")
 
@@ -1709,17 +1714,27 @@ func (s *session) startExec(ctx context.Context, channel ssh.Channel, scx *Serve
 		result := execRequest.Wait()
 		scx.SendExecResult(ctx, result)
 
-		// Wait a little bit to let all events filter through before closing the
-		// BPF session so everything can be recorded.
+		// Wait a little bit to let all events filter through before
+		// closing the BPF session so everything can be recorded.
+		//
+		// TODO: This sleep is also necessary to prevent the SSH server
+		// from closing an SSH session without sending the exit status
+		// to the client. The SSH server code in handleSessionRequests
+		// should be refactored to consistently wait for the exit result
+		// before cleaning up.
 		time.Sleep(2 * time.Second)
 
-		err = scx.srv.GetBPF().CloseSession(sessionContext)
-		if err != nil {
-			s.logger.ErrorContext(ctx, "Failed to close enhanced recording (exec) session.", "error", err)
+		if sessionContext != nil {
+			err = scx.srv.GetBPF().CloseSession(sessionContext)
+			if err != nil {
+				s.logger.ErrorContext(ctx, "Failed to close enhanced recording (exec) session.", "error", err)
+			}
 		}
 
 		s.emitSessionEndEvent()
-		s.Close()
+		if err := s.Close(); err != nil {
+			s.logger.WarnContext(ctx, "Failed to close session.", "error", err)
+		}
 
 		s.io.Close()
 		close(s.doneCh)

lib/srv/sess_test.go

@@ -45,6 +45,7 @@ import (
 	apissh "github.com/gravitational/teleport/api/ssh"
 	"github.com/gravitational/teleport/api/types"
 	apievents "github.com/gravitational/teleport/api/types/events"
+	"github.com/gravitational/teleport/lib/bpf"
 	"github.com/gravitational/teleport/lib/defaults"
 	"github.com/gravitational/teleport/lib/events"
 	"github.com/gravitational/teleport/lib/events/eventstest"
@@ -1086,6 +1087,44 @@ func TestSessionRecordingModes(t *testing.T) {
 	}
 }
 
+// TestBPFEnabledAndNoPermitEvents tests that when ESR is enabled and no
+// permit events are configured for the user, opening a non-interactive
+// session does not attempt to open a ESR session.
+func TestBPFEnabledAndNoPermitEvents(t *testing.T) {
+	t.Parallel()
+
+	srv := newMockServer(t)
+	bpfSrv := &countingBPF{enabled: true}
+	srv.bpf = bpfSrv
+
+	reg, err := NewSessionRegistry(SessionRegistryConfig{
+		Srv:                   srv,
+		SessionTrackerService: srv.auth,
+	})
+	require.NoError(t, err)
+	t.Cleanup(reg.Close)
+
+	scx := newTestServerContext(t, reg.Srv, nil, &decisionpb.SSHAccessPermit{})
+	execRequest := &mockExec{command: "true"}
+	scx.execRequest = execRequest
+
+	clientChan, serverChan := newMockSSHChannel(t)
+	clientChan.Drain()
+	require.NoError(t, reg.OpenExecSession(t.Context(), serverChan, scx))
+
+	sess := scx.getSession()
+	require.NotNil(t, sess)
+	select {
+	case <-sess.doneCh:
+	case <-time.After(3 * time.Second):
+		require.Fail(t, "exec session did not complete")
+	}
+
+	require.Zero(t, bpfSrv.openSessionCalls.Load())
+	require.Zero(t, bpfSrv.closeSessionCalls.Load())
+	require.False(t, sess.hasEnhancedRecording)
+}
+
 func TestStopSessionWithoutClientDisconnect(t *testing.T) {
 	t.Parallel()
 
@@ -1226,6 +1265,64 @@ func (s sessionEvaluator) IsModerated() bool {
 	return s.moderated
 }
 
+type mockExec struct {
+	command string
+}
+
+func (s *mockExec) GetCommand() string {
+	return s.command
+}
+
+func (s *mockExec) SetCommand(command string) {
+	s.command = command
+}
+
+func (s *mockExec) Start(ctx context.Context, channel ssh.Channel) error {
+	return nil
+}
+
+func (s *mockExec) Wait() ExecResult {
+	return ExecResult{Command: s.command}
+}
+
+func (s *mockExec) ReadAuditSessionID() (uint32, error) {
+	return 0, nil
+}
+
+func (s *mockExec) Continue() {}
+
+func (s *mockExec) PID() int {
+	return 0
+}
+
+type countingBPF struct {
+	enabled           bool
+	openSessionCalls  atomic.Int32
+	closeSessionCalls atomic.Int32
+}
+
+func (c *countingBPF) OpenSession(ctx *bpf.SessionContext) error {
+	c.openSessionCalls.Add(1)
+	return nil
+}
+
+func (c *countingBPF) CloseSession(ctx *bpf.SessionContext) error {
+	c.closeSessionCalls.Add(1)
+	return nil
+}
+
+func (c *countingBPF) Close(restarting bool) error {
+	return nil
+}
+
+func (c *countingBPF) Enabled() bool {
+	return c.enabled
+}
+
+func (c *countingBPF) LostEvents() bpf.EventCount {
+	return bpf.EventCount{}
+}
+
 func TestTrackingSession(t *testing.T) {
 	t.Parallel()
 	ctx := t.Context()