263893 2022-05-10 10:43:38 +0000 pam_exec.so in auth stack with expose_authtok option makes su segfault 2023-06-29 07:32:27 +0000 1 1 1 Unclassified Base System bin 13.1-RELEASE Any Any Closed FIXED https://reviews.freebsd.org/D35169 crash --- Affects Some People --- 1 zirias khng lwhsu nyan titus oldest_to_newest 1210625 0 zirias 2022-05-10 10:43:38 +0000 su(1) segfaults when there's pam_exec.so in the "auth" stack with the option expose_authtok. To reproduce, use the following "auth" config in /etc/pam.d/system: auth sufficient pam_exec.so expose_authtok /usr/bin/false auth required pam_unix.so use_first_pass nullok When removing the 'use_first_pass' option from 'pam_unix.so', su asks for a password a second time (as expected), but still segfaults. When removing the 'expose_authtok' option from 'pam_exec.so', the segfault is gone. A lot of (probably irrelevant) context is here: https://forums.freebsd.org/threads/su-segfaults-when-adding-some-custom-pam_exec-to-the-auth-stack.85112/ 1210658 1 titus 2022-05-10 13:53:22 +0000 from pam_exec(8) expose_authtok Write the authentication token to the program's standard input stream, followed by a NUL character. Ignored for pam_sm_setcred(). problem is that it is not ignored when code _pam_exec() is trying to retrieve the auth token when it is called from pam_sm_setcred pam_get_item will set item to null; PAM_AUTHTOK item is set to null when pam_authenticate finishes then a strlen is performed on null and it segfaults if (options->use_first_pass || strcmp(func, "pam_sm_setcred") == 0) { /* don't prompt, only expose existing token */ rc = pam_get_item(pamh, PAM_AUTHTOK, &item); authtok = item; } ...... authtok_size = strlen(authtok) + 1; // <= bombs here 1210689 2 nyan 2022-05-10 18:31:11 +0000 This patch should fix it. https://reviews.freebsd.org/D35169 1212751 3 koobs 2022-05-25 00:53:52 +0000 The branch main has been updated by khng: URL: https://cgit.FreeBSD.org/src/commit/?id=b75e0eed345d2ab047a6b1b00a9a7c3bf92e992c commit b75e0eed345d2ab047a6b1b00a9a7c3bf92e992c Author: Yan Ka Chiu <nyan@myuji.xyz> AuthorDate: 2022-05-22 16:33:02 +0000 Commit: Ka Ho Ng <khng@FreeBSD.org> CommitDate: 2022-05-22 16:36:48 +0000 pam_exec: fix segfault when authtok is null According to pam_exec(8), the `expose_authtok` option should be ignored when the service function is `pam_sm_setcred`. Currently `pam_exec` only prevent prompt for anth token when `expose_authtok` is set on `pam_sm_setcred`. This subsequently led to segfault when there isn't an existing auth token available. Bug reported on this: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=263893 After reading https://reviews.freebsd.org/rS349556 I am not sure if the default behaviour supposed to be simply not prompt for authentication token, or is it to ignore the option entirely as stated in the man page. This patch is therefore only adding an additional NULL check on the item `pam_get_item` provide, and exit with `PAM_SYSTEM_ERR` when such item is NULL. MFC after: 1 week Reviewed by: des, khng Differential Revision: https://reviews.freebsd.org/D35169 1212752 4 koobs 2022-05-25 00:54:10 +0000 ^Triage: Assign to committer resolving 1213370 5 zirias 2022-05-28 08:55:24 +0000 Thanks for the commit! What would be necessary to create an EN to get this fix on releng/* after MFC? Background: xscreensaver dropped all support for external PAM helpers in 6.03 and kscreenlocker from KDE plasma also removed their own helper in 5.25. I have a workaround based on pam_exec ready (port is prepared and tested), but it won't work without this fix. So, consequently, some port updates would be blocked... 1215865 6 zirias 2022-06-15 07:11:09 +0000 I'll give some more context explaining why it's important to get this fix in the -RELEASE versions. Background is that pam_unix.so requires the caller to have root privileges for authentication. LinuxPAM's pam_unix.so contains a workaround specifically for authenticating as yourself, by calling its own suid-root helper. Screen lockers start taking that for granted (so far xscreensaver and kscreenlocker from KDE plasma). Adding the same kind of workaround to FreeBSD's pam_unix.so was rejected by des@ and others, with des@ specifically recommending to use pam_exec.so instead. The clean and correct solution might be some authentication service that could be used by pam_unix.so instead of accessing the passwd database directly, but this is a complex thing to do, it's certainly off the table for a timely solution. So, calling a helper with pam_exec.so is the unintrusive way to have a workaround similar to LinuxPAM and make these screenlockers "just work" (given a correctly configured PAM service policy is installed for them). A pam_exec.so that might crash when used for authentication blocks that solution. The only other option remaining would be to patch in support for external suid-root helpers in the affected screen lockers. That's not sustainable in the longer run, IMHO we shouldn't even start doing that... 1216261 7 nyan 2022-06-18 09:39:03 +0000 This patch can be MFC to 13 but does not need for stable/12 as the regression is introduced in 13 along with the introduction of use_first_pass option in pam_exec 1216347 8 zirias 2022-06-19 13:22:52 +0000 (In reply to nyan from comment #7) Thanks, tested and confirmed 12.3-RELEASE is not affected. Still the fix is needed for 13.0-RELEASE and 13.1-RELEASE to move forward with port updates. 1217377 9 zirias 2022-06-27 07:48:32 +0000 Thanks for MFC to stable/13! Should probably be flagged here to correctly reflect progress, but it seems I don't have the required privileges to do so… Now, how would the process continue for ENs to get it on the releases? Anything I can do to help here? 1221435 10 lwhsu 2022-08-02 13:33:36 +0000 Secteam responsed that this is in the queue for the next release. 1222179 11 zirias 2022-08-08 12:53:19 +0000 (In reply to Li-Wen Hsu from comment #10) That's great, thanks! Then I guess this PR can be closed (and mfc-stable13 set to + as it already happened)?