Skip to content

Try to detect process being forked during PAM transaction #352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions pam/pam.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,15 @@ func (h *Handle) GetItem(i Item) (unsafe.Pointer, error) {
return data, nil
}

// GetServiceName retrieves the name of the application running the PAM transaction.
func (h *Handle) GetServiceName() string {
data, err := h.GetItem(Service)
if err != nil {
return "[unknown service]"
}
return C.GoString((*C.char)(data))
}

// StartAsPamUser sets the effective privileges to that of the PAM user.
func (h *Handle) StartAsPamUser() error {
userPrivs, err := security.UserPrivileges(h.PamUser)
Expand Down
46 changes: 46 additions & 0 deletions pam_fscrypt/pam_fscrypt.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ import "C"
import (
"fmt"
"log"
"log/syslog"
"os"
"strconv"
"unsafe"

"github.com/pkg/errors"
Expand All @@ -46,6 +49,8 @@ const (
moduleName = "pam_fscrypt"
// authtokLabel tags the AUTHTOK in the PAM data.
authtokLabel = "fscrypt_authtok"
// pidLabel tags the pid in the PAM data.
pidLabel = "fscrypt_pid"
// These flags are used to toggle behavior of the PAM module.
debugFlag = "debug"

Expand Down Expand Up @@ -76,6 +81,12 @@ func Authenticate(handle *pam.Handle, _ map[string]bool) error {
}
defer handle.StopAsPamUser()

// Save the PID in the PAM data so that the Session hook can try to
// detect the unsupported situation where the process was forked.
if err := handle.SetString(pidLabel, strconv.Itoa(os.Getpid())); err != nil {
return errors.Wrap(err, "could not save pid in PAM data")
}

// If this user doesn't have a login protector, no unlocking is needed.
if _, err := loginProtector(handle); err != nil {
log.Printf("no protector, no need for AUTHTOK: %s", err)
Expand Down Expand Up @@ -128,6 +139,37 @@ func setupUserKeyringIfNeeded(handle *pam.Handle, policies []*actions.Policy) er
return handle.StartAsPamUser()
}

// The Go runtime doesn't support being forked, as it is multithreaded but
// fork() deletes all threads except one. Some programs, such as xrdp, misuse
// libpam by fork()-ing the process between pam_authenticate() and
// pam_open_session(). Try to detect such unsupported cases and bail out early
// rather than deadlocking the Go runtime, which would prevent the user from
// logging in entirely. This isn't guaranteed to work, as we are already
// running Go code here, so we may have already deadlocked. But in practice the
// deadlock doesn't occur until hashing the login passphrase is attempted.
func isUnsupportedFork(handle *pam.Handle) bool {
pidString, err := handle.GetString(pidLabel)
if err != nil {
return false
}
expectedPid, err := strconv.Atoi(pidString)
if err != nil {
log.Printf("%s parse error: %v", pidLabel, err)
return false
}
if os.Getpid() == expectedPid {
return false
}
handle.InfoMessage(fmt.Sprintf("%s couldn't automatically unlock directories, see syslog", moduleName))
if logger, err := syslog.New(syslog.LOG_WARNING, moduleName); err == nil {
fmt.Fprintf(logger,
"not unlocking directories because %s forked the process between authenticating the user and opening the session, which is incompatible with %s. See https://github.com/google/fscrypt/issues/350",
handle.GetServiceName(), moduleName)
logger.Close()
}
return true
}

// OpenSession provisions any policies protected with the login protector.
func OpenSession(handle *pam.Handle, _ map[string]bool) error {
// We will always clear the AUTHTOK data
Expand All @@ -154,6 +196,10 @@ func OpenSession(handle *pam.Handle, _ map[string]bool) error {
return nil
}

if isUnsupportedFork(handle) {
return nil
}

if err = setupUserKeyringIfNeeded(handle, policies); err != nil {
return errors.Wrapf(err, "setting up user keyring")
}
Expand Down