Bug 214377
| Summary: | Explore for a way for pam_cap.so to preserve the Ambient vector | ||
|---|---|---|---|
| Product: | Tools | Reporter: | Andrew G. Morgan (morgan) |
| Component: | libcap | Assignee: | Andrew G. Morgan (morgan) |
| Status: | RESOLVED CODE_FIX | ||
| Severity: | enhancement | ||
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | n/a | Subsystem: | |
| Regression: | No | Bisected commit-id: | |
|
Description
Andrew G. Morgan
2021-09-12 23:28:44 UTC
So I've been exploring adding "session" support to pam_cap.so in addition to the pre-existing "auth" (+setcred) support. It appears to work in my limited testing, so I'm going to record the fact here and investigate more testing. HEAD contains this commit: https://git.kernel.org/pub/scm/libs/libcap/libcap.git/commit/?id=fc6253b9de68dafae1927b2bcbfcef9e9ec6e05a The basic structure is you can now configure PAM with a config like this: #%PAM-1.0 auth required pam_cap.so use_session keepcaps auth required pam_unix.so account required pam_unix.so password required pam_unix.so session required pam_unix.so session optional pam_cap.so Here the "auth" part prepares the application with "keepcaps", and the "use_session" instructs the module to defer applying any IAB tuple for the user until *session* open time (which is after the setuid() call by the application). Björn Fischer says: --- As I like this approach very much to keep everything within the PAM layer, this does not work without further modifications: login.c calls pam_open_session() with root privileges way before calling setuid() via change_uid(). Also, it does some effort (like forking) to call pam_close_session() with root privileges as well. --- So, I'll look a little harder (I have one idea): add a cleanup() function from the auth module that executes the IAB setting. OK, another attempt: https://git.kernel.org/pub/scm/libs/libcap/libcap.git/commit/?id=2c3b8949f4374db5285865ad8ce1bdf49d6f24c6 The suggested config for this module for an app, that wants to support the Ambient vector, is thus now: ``` #%PAM-1.0 auth required pam_cap.so keepcaps defer auth required pam_unix.so account required pam_unix.so password required pam_unix.so session required pam_unix.so ``` If this doesn't work, I think I'm out of ideas. I tried it on a more mainstream version of `su` and it didn't work, so I added some instrumentation to investigate why: https://git.kernel.org/pub/scm/libs/libcap/libcap.git/commit/?id=9f9602215ccf205cca1b0a495db9eae18d204265 It would seem that that version of `su` does not follow the PAM application writers guide: http://www.linux-pam.org/Linux-PAM-html/adg-interface-by-app-expected.html#adg-pam_end It basically doesn't call `pam_end()` from within the fork()ed code. That is this line is missing: ``` pam_end(pamh, PAM_SUCCESS | PAM_DATA_SILENT) ``` If the app adds that line, somewhere in the child code of the fork(), after the `setuid()` call and before the `exec*()` call, then the Ambient vector inheritance should work with the above config use of pam_cap.so: ``` auth required pam_cap.so keepcaps defer ``` For completeness, the resolution of this for the shadow suite: https://github.com/shadow-maint/shadow/pull/408#issuecomment-919673098 |