Bug 214377 - Explore for a way for pam_cap.so to preserve the Ambient vector
Summary: Explore for a way for pam_cap.so to preserve the Ambient vector
Status: RESOLVED CODE_FIX
Alias: None
Product: Tools
Classification: Unclassified
Component: libcap (show other bugs)
Hardware: All Linux
: P1 enhancement
Assignee: Andrew G. Morgan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-09-12 23:28 UTC by Andrew G. Morgan
Modified: 2021-09-15 14:17 UTC (History)
0 users

See Also:
Kernel Version: n/a
Subsystem:
Regression: No
Bisected commit-id:


Attachments

Description Andrew G. Morgan 2021-09-12 23:28:44 UTC
Rather than hard-code some relationship between login and pam_cap.so to support raising the Ambient vector, see if there is some sort of mechanism for this to be abstracted via PAM's API and wholly implemented in pam_cap.so.

This issue was surfaced in:

   https://github.com/shadow-maint/shadow/pull/408
Comment 1 Andrew G. Morgan 2021-09-14 04:25:22 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).
Comment 2 Andrew G. Morgan 2021-09-14 14:00:07 UTC
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.
Comment 3 Andrew G. Morgan 2021-09-15 03:02:52 UTC
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.
Comment 4 Andrew G. Morgan 2021-09-15 04:06:26 UTC
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
```
Comment 5 Andrew G. Morgan 2021-09-15 14:17:44 UTC
For completeness, the resolution of this for the shadow suite:

https://github.com/shadow-maint/shadow/pull/408#issuecomment-919673098

Note You need to log in before you can comment on or make changes to this bug.