Skip to content

pam_fingwit: fail closed when no decision can be reached - #63

Open
nikaiw wants to merge 5 commits into
xapp-project:masterfrom
nikaiw:hardening/fail-closed-and-tests
Open

nikaiw wants to merge 5 commits into
xapp-project:masterfrom
nikaiw:hardening/fail-closed-and-tests

Conversation

@nikaiw

@nikaiw nikaiw commented Sep 20, 2026 •

Copy link
Copy Markdown

Follow-up to #62. Four separate fixes, one commit each. The diff below still
contains #62, so please read the commits.

  1. waitpid() was called once and status was never set. A signal during the
    wait returns EINTR. The module then read junk from the stack, decided
    authentication from it, and returned while the helper was still running.
    The child also used exit() instead of _exit().

  2. Any exit code other than PAM_AUTHINFO_UNAVAIL became PAM_IGNORE. So a
    traceback (1) or a missing interpreter (127) let fprintd run. This is why
    Sudo produces a ModuleNotFoundError #51 shows a fingerprint prompt after the error.

  3. pam_fingwit.py read debug after the settings lookup, so the error
    handler failed with UnboundLocalError. That handler and the empty
    username case both returned PAM_IGNORE. They now skip fprintd. Errors go
    to LOG_WARNING.

  4. tests/pam_stack_check.sh tests the module with a temporary PAM service
    that copies fingwit.pam_config. It needs root, removes its own file at
    the end, and does not touch common-auth.

Each commit's tests fail on the commit before it. 29 + 5 pass as a normal user,
4 more as root.

@nikaiw
nikaiw force-pushed the hardening/fail-closed-and-tests branch from 13cebae to c8896eb Compare September 20, 2026 14:33
execvp("python3", ...) resolves through PATH, and the forked child keeps
euid 0 because sudo, pkexec and the polkit agent are setuid root. Any local
user could have their own python3 run as root:

    printf '#!/bin/bash -p\nid > /tmp/pwned\n' > /tmp/evil/python3
    PATH=/tmp/evil:$PATH sudo -k anything

PYTHONPATH, GIO_EXTRA_MODULES and XDG_DATA_DIRS reached the same place by
other routes.

Use execve() with an absolute path and an environment allowlist. Building
it before the fork also keeps setenv() and malloc() out of the child.

This also fixes the "No module named 'gi'" traceback under sudo: a pyenv or
conda python3 was being picked up instead of the system one.

tests/ gains a harness that loads the module and reads back its decision,
so these two cases stay covered.

Closes: xapp-project#54
Closes: xapp-project#51
status was read uninitialised whenever a signal interrupted waitpid(), so
the module decided authentication from stack garbage and returned while the
helper was still running. PAM callers install handlers: SIGCHLD, SIGALRM
and SIGWINCH all reach us.

The child also left through exit() when execve() failed, flushing stdio
buffers and running atexit() handlers it inherited from the caller.
Every exit code other than PAM_AUTHINFO_UNAVAIL mapped to PAM_IGNORE, so a
traceback (1), a missing interpreter (127) or a crash all let fprintd run.
That is the fingerprint prompt after the ModuleNotFoundError in xapp-project#51.

This module exists to keep fingerprint auth away from initial login on an
encrypted home. Crashing the helper should not get you there.
debug was parsed after the settings lookup, the first thing that can fail,
so the error handler itself died with UnboundLocalError. Parse it first.

The handler and the missing-username path both returned PAM_IGNORE, which
allows fprintd. Return PAM_AUTHINFO_UNAVAIL instead: the user loses the
fingerprint prompt, the password still works.

Log at LOG_WARNING rather than only under debug, so these reach auth.log
without reconfiguring PAM first.
The unit driver reports the module's decision but not how the stack reacts
to it. pam_stack_check.sh installs a throwaway service using the same
[authinfo_unavail=1 default=ignore] jump as fingwit.pam_config, with
pam_deny.so where pam_fprintd.so sits.

Needs root for /etc/pam.d. It installs only its own service file and
removes it on exit; common-auth is untouched.
@nikaiw
nikaiw force-pushed the hardening/fail-closed-and-tests branch from c8896eb to 6816a61 Compare September 20, 2026 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant