Skip to content

Commit

Permalink
Restore interactive PAM authentication
Browse files Browse the repository at this point in the history
#29279 caused PAM to
deadlock when performing interactive authentication. To restore
the previous semblance of functional PAM, this reverts waiting
for PAM to be complete if BPF is disabled. #29279 was specifically
added to prevent systemd, which may be invoked via a PAM module,
from moving the exec subprocess to a different cgroup. Since
cgroups are not used outside of Enhanced Session Recording this
is a stop-gap measure that can allow mose users of PAM to get an
immediate restoration of behavior while a more long term and sane
approach to performing PAM during the SSH handshake can be
considered, evaluated, and tested.

Closes #49028.
  • Loading branch information
rosstimothy committed Nov 26, 2024
1 parent 6f73d95 commit 985b1b1
Showing 1 changed file with 17 additions and 5 deletions.
22 changes: 17 additions & 5 deletions lib/srv/sess.go
Original file line number Diff line number Diff line change
Expand Up @@ -1366,12 +1366,24 @@ func (s *session) startInteractive(ctx context.Context, scx *ServerContext, p *p
Events: scx.Identity.AccessChecker.EnhancedRecordingSet(),
}

if err := s.term.WaitForChild(); err != nil {
s.logger.ErrorContext(ctx, "Child process never became ready.", "error", err)
return trace.Wrap(err)
bpfService := scx.srv.GetBPF()

// Only wait for the child to be "ready" if BPF is enabled. This is required
// because PAM might inadvertently move the child process to another cgroup
// by invoking systemd. If this happens, then the cgroup filter used by BPF
// will be looking for events in the wrong cgroup and no events will be captured.
// However, unconditionally waiting for the child to be ready results in PAM
// deadlocking because stdin/stdout/stderr which it uses to relay details from
// PAM auth modules are not properly copied until _after_ the shell request is
// replied to.
if bpfService.Enabled() {
if err := s.term.WaitForChild(); err != nil {
s.logger.ErrorContext(ctx, "Child process never became ready.", "error", err)
return trace.Wrap(err)
}
}

if cgroupID, err := scx.srv.GetBPF().OpenSession(sessionContext); err != nil {
if cgroupID, err := bpfService.OpenSession(sessionContext); err != nil {
s.logger.ErrorContext(ctx, "Failed to open enhanced recording (interactive) session.", "error", err)
return trace.Wrap(err)
} else if cgroupID > 0 {
Expand All @@ -1380,7 +1392,7 @@ func (s *session) startInteractive(ctx context.Context, scx *ServerContext, p *p
go func() {
// Close the BPF recording session once the session is closed
<-s.stopC
err = scx.srv.GetBPF().CloseSession(sessionContext)
err = bpfService.CloseSession(sessionContext)
if err != nil {
s.logger.ErrorContext(ctx, "Failed to close enhanced recording (interactive) session.", "error", err)
}
Expand Down

0 comments on commit 985b1b1

Please sign in to comment.