Skip to content

Commit

Permalink
Restore interactive PAM authentication (#49487)
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 27, 2024
1 parent 8223c66 commit 3604b33
Showing 1 changed file with 24 additions and 6 deletions.
30 changes: 24 additions & 6 deletions lib/srv/sess.go
Original file line number Diff line number Diff line change
Expand Up @@ -1345,12 +1345,31 @@ func (s *session) startInteractive(ctx context.Context, scx *ServerContext, p *p
Events: scx.Identity.AccessChecker.EnhancedRecordingSet(),
}

if err := s.term.WaitForChild(); err != nil {
s.log.WithError(err).Error("Child process never became ready")
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.log.WithError(err).Error("Child process never became ready")
return trace.Wrap(err)
}
} else {
// Clean up the read half of the pipe, and set it to nil so that when the
// ServerContext is closed it doesn't attempt to a second close.
if err := s.scx.readyr.Close(); err != nil {
s.log.WithError(err).Error("Failed closing child ready pipe")
}
s.scx.readyr = nil
}

if cgroupID, err := scx.srv.GetBPF().OpenSession(sessionContext); err != nil {
if cgroupID, err := bpfService.OpenSession(sessionContext); err != nil {
s.log.WithError(err).Error("Failed to open enhanced recording (interactive) session")
return trace.Wrap(err)
} else if cgroupID > 0 {
Expand All @@ -1359,8 +1378,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)
if err != nil {
if err := bpfService.CloseSession(sessionContext); err != nil {
s.log.WithError(err).Error("Failed to close enhanced recording (interactive) session")
}
}()
Expand Down

0 comments on commit 3604b33

Please sign in to comment.