Add support for ambient capabilities

[bjorn-fischer]

Caveat: This is for discussion, mostly. Still need to have a policy for bounded capabilities.

Enhance login(1) and su(1) to support ambient capabilities

How things work without ambient capabilities

POSIX capabilities have been supported by Linux for quite some time now. Most distributions allow to set inheritable capabilities:

Activate pam_cap.so (comes with PAM enabled libcap) in your PAM config...

auth		required	pam_env.so
auth		optional	pam_cap.so
auth		requisite	pam_faillock.so preauth
[...]

...add capabilities to users in /etc/security/capability.conf...

[...]
cap_net_raw      joeuser
[...]

...and enable that capability on an executable:

[βελλεροφων ~]# setcap cap_net_raw=ei /usr/bin/traceroute
[βελλεροφων ~]# getcap /usr/bin/traceroute
/usr/bin/traceroute cap_net_raw=ei
[βελλεροφων ~]#

Now joeuser can perform traceroutes that require raw sockets without using sudo, e.g.

traceroute -T -p 22 10.11.12.13

While this already works on most Linux distributions, you have to set the inherited capabilities on every applicable executable, and you will have to set it again after that executable has been updated. It may be the case that joeuser is your network admin and should be allowed to use raw sockets in any application. This is where ambient capabilities come into play.

Ambient Capabilities: Granting privileges on any application

Users with ambient capabilities can use these without having them set on an executable with setcap. If joeuser has the ambient capability cap_net_raw, he would be able to use raw sockets in any application, even in scripting languages.

Unfortunately ambient capabilities are more or less unsupported in current distributions. The main reason for this is lacking support in applications that grant access to Linux systems (like login(1) and su(1)). Those applications usually run with root privileges and drop these with the setuid() system call which clears all ambient capabilities. Generally this is a good idea, because there are many applications out there that rely on setuid() to drop all privileges.

To include support for ambient capabilities into login and su, the application has to set the "keep capabilities" flag and save the set of ambient capabilities before calling setuid(), and restore them afterwards. Since version 2.50 libcap's pam_cap.so module supports the "keepcaps" argument, which takes care of the "keep capabilities" flag. So each PAM enabled application just needs to save and restore the capabilities during setuid(). Old and new behavior can be controlled at runtime by changing the PAM configuration.

Signed-off-by: Björn Fischer bf@CeBiTec.Uni-Bielefeld.DE

[hallyn]

You seem to be saying that pam_cap.so by itself isn't able to give you the pI you specify in capabilities.conf, and therefore you need to set KEEPCAPS in login.c. Is that what you are finding?

I think it would be better to add ambient capability support to pam_cap.so and leave all that logic out of su and login, if possible.

[bjorn-fischer]

Leaving this to pam_cap.so entirely, will not work. Please, let me explain:

Ambient capabilities will not survive the setuid() system call. To support ambient capabilities, one has to save the ambient caps before calling setuid() and restore them after that. Calling setuid() is the most important thing the login and su applications do. There is no way to delegate this to pam_cap.so.

The keepcaps flag is different. This flag is thread specific and has to be set via prctl(). The flag must be set with root privileges and survives the setuid() call but it won't survive execve(). Without the keepcaps flag set it is not possible to restore the ambient caps after the setuid() call.

So, specifying ambient capabilities in capabilities.conf, will grant these capabilities to PAM clients up to the moment they call setuid().

With ambient caps not surviving setuid() and the keepcaps flag not surviving execve(), supporting ambient capabilities naturally fall in the scope of login and su.

[hallyn]

Yes, I see. Ideally there would be a pam_* helper called after the setuid() in which pam_cap.so could re-set the pA.

[AndrewGMorgan]

There seem like there are two aspects to this pull request:

1. embed keepcaps support into login
2. preserve the ambient vector across setuid()

For (1), is there some reason that the pam_cap.so "keepcaps" argument isn't sufficient?

For (2), In the proof of concept example I worked up here:

https://git.kernel.org/pub/scm/libs/libcap/libcap.git/tree/contrib/sucap

I used the cap_launch* abstraction to do all the setuid() etc. sequencing. Robustly setting things in the right order is actually quite complicated. Since my goal with that PoC was to explore how to avoid su being setuid-root and replace all the functionality with capabilities, that was good enough for my needs. However, linking the shadow suite against libcap to make pam_cap.so work seems a bit excessive - and is breaking the PAM abstraction. I've just filed:

https://bugzilla.kernel.org/show_bug.cgi?id=214377

to explore if pam_cap.so can actually do more. I don't mind hacking some more with that 'sucap' code to figure out the right structure for hiding all the relevant logic behind PAM's API. The 'sucap/su' binary is intended more as a proving ground for replacing setuid-root code with purely capable stuff, so I'm happy to use it for this sort of exploration.

[AndrewGMorgan]

So this will be included in libcap-2.58 (plus any bug fixes I accumulate before then):

• https://bugzilla.kernel.org/show_bug.cgi?id=214377#c1

Let me know if it works for you:

$ git clone https://git.kernel.org/pub/scm/libs/libcap/libcap.git/
$ cd libcap
$ make -C pam_cap
$ sudo cp pam_cap/pam_cap.so ....

[bjorn-fischer]

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¹.

This should not be changed, I think. The man page PAM_OPEN_SESSION(3) suggests that the calling application should have sufficient privileges to perform tasks like creating or mounting the user's home dir. Calling pam_open_session() after setuid() will obviously break pam_mkhomedir.so.

---

¹Seems to have quite a paper trail.

[AndrewGMorgan]

That is, indeed, a pity. I have one more idea before giving up on this.

[AndrewGMorgan]

My work here should be done. Specifically, comment 3 here:

• https://bugzilla.kernel.org/show_bug.cgi?id=214377#c3

Use the same instructions for downloading and building the module (or git pull if you just want to update what you already have) at HEAD as #408 (comment)

This attempt is based on the Linux-PAM documentation for application writers (I have some vague recollection of writing those words about 20 years ago...!), so if this doesn't work, I think there is some sort of problem with the application:

• http://www.linux-pam.org/Linux-PAM-html/adg-interface-by-app-expected.html#adg-pam_end
• Alt location: https://fossies.org/linux/Linux-PAM-docs/doc/adg/html/adg-interface-by-app-expected.html#adg-pam_end

This latest pam_cap.so version at HEAD, with the keepcaps and defer module arguments, relies on the fact that inside the application fork()'d process, but before exec*()ing the user shell, the application shuts down the forked pamh handle with a call like this:

pam_end(pamh, PAM_SUCCESS | PAM_DATA_SILENT)

If the app doesn't do this, then that's a deviation from that documentation, and that's a PAM API fix the app can make...

[AndrewGMorgan]

Indeed. Looking at

• https://github.com/shadow-maint/shadow/blob/master/src/su.c#L1100

the missing call to pam_end() described above should be inserted just below the } and above the #else in this code snippet:

	/* become the new user */
	if (change_uid (pw) != 0) {
		exit (1);
	}
#else				/* !USE_PAM */
	/* no limits if su from root (unless su must fake login's behavior) */
	if (!caller_is_root || fakelogin) {
		setup_limits (pw);
	}

[bjorn-fischer]

Great idea to put it into the cleanup code path. Confirmed to work with login and su.

So, thanks to Andrew's excellent work, my pull request basically collapses to:

--- a/src/login.c
+++ b/src/login.c
@@ -1288,6 +1288,7 @@ int main (int argc, char **argv)
                        env++;
                }
        }
+    (void) pam_end(pamh, PAM_SUCCESS | PAM_DATA_SILENT);
 #endif
 
        (void) setlocale (LC_ALL, "");
diff --git a/src/su.c b/src/su.c
index df4b70ea..ba2df98b 100644
--- a/src/su.c
+++ b/src/su.c
@@ -1098,6 +1098,7 @@ int main (int argc, char **argv)
        if (change_uid (pw) != 0) {
                exit (1);
        }
+    (void) pam_end(pamh, PAM_SUCCESS | PAM_DATA_SILENT);
 #else                          /* !USE_PAM */
        /* no limits if su from root (unless su must fake login's behavior) */
        if (!caller_is_root || fakelogin) {

[ikerexxe]

Taking into account that there's a commit that reverts another one I think that you should squash everything in one commit.

[bjorn-fischer]

How do I do that? Preferably git rebase -i or git revert?

[ikerexxe]

I'd use git rebase -i HEAD~3

For more information you can check: https://levelup.gitconnected.com/how-to-squash-git-commits-9a095c1bc1fc

[AndrewGMorgan]

There might be some issue left to explore...

Expanding the diff for that patch for su downwards, there is a pre-existing, commented out, call to pam_end() of the type I'm recommending ( ac938b2#diff-31aa13e6c93f9d326376aaccba38a7d07039da0cb207b758db141fb23ea14072L1159-L1164 ):

	/*
	 * PAM_DATA_SILENT is not supported by some modules, and
	 * there is no strong need to clean up the process space's
	 * memory since we will either call exec or exit.
	pam_end (pamh, PAM_SUCCESS | PAM_DATA_SILENT);
	 */

I don't know how old that comment is, but clearly there was some concern that it didn't work reliably at the time it was written. I'd argue that this is some sort of distributed non-compliance problem if it still exists. I plan to stick with this support expectation in pam_cap.so and libcap-2.58 and leave it up to PAM applications and modules to figure out a path to compliance.

[hallyn]

I don't know how old that comment is, but clearly there was some concern that it didn't work reliably at the time it was written. I'd argue that this is some sort of distributed non-compliance problem if it still exists. I plan to stick with this support expectation in pam_cap.so and libcap-2.58 and leave it up to PAM applications and modules to figure out a path to compliance.

Looks like it was introduced in 2007 by commit 0fd1ed4 to address https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=412061

[bjorn-fischer]

Does not look that well. pam_krb5 still does not implement PAM_DATA_SILENT, and does a full cache cleanup on pam_end() as it seems. Unfortunately I do not have access to a kerberized environment to test this, currently.

[AndrewGMorgan]

FWIW I just released libcap-2.58.

[AndrewGMorgan]

The pam-krb5 commit should have resolved this issue with that module.

[hallyn]

The pam-krb5 commit should have resolved this issue with that module.

Awesome.

Can you confirm (Björn) that, and explain why, the commented-out location is
wrong, and the two you added are correct? Then remove the commented out
block (~ lines 1159-1164), and we should be ready to merge.

[bjorn-fischer]

In fact, the added location of pam_end() is wrong as I missed a PAM call in set_environment(). I'll update the patch and force push after I tested the new version.

[bjorn-fischer]

Ok, I have used the patched version for some time now on my production workstation and laptop, and I explored a few different PAM setups at my university. Seems to work as intended. Anything left to do before merge?

[ZoltanFridrich]

I was trying this out and it did not work for me.

I took fedora version of libcap, patched it with keepcaps and defer commits from libcap upstream. Then I took shadow-utils patched with this PR and installed both these modified packages. I set /etc/security/capability.conf to ^cap_chown * and into /etc/pam.d/system-auth I added auth required pam_cap.so keepcaps defer. After I do su - bob he has no inheritable nor ambient caps set, when I remove defer from system-auth, then only the inheritable caps are set.

@bjorn-fischer @AndrewGMorgan Am I doing something wrong?

[bjorn-fischer]

[...]

I set /etc/security/capability.conf to ^cap_chown * and into /etc/pam.d/system-auth I added auth required pam_cap.so keepcaps defer. After I do su - bob he has no inheritable nor ambient caps set, when I remove defer from system-auth, then only the inheritable caps are set.

This usually happens if the application performing the login procedure misses the pam_end() call with PAM_DATA_SILENT: With defer no caps at all, and without defer the usual inheritable vector. It shows that at least the pam_cap.so part is working correctly. Can you instrument su.c so that it will print something to stderr just before and after the newly added pam_end() call (i.e. inside the USE_PAM clause)?

[bjorn-fischer]

@ZoltanFridrich When tracing /bin/su using strace, you should see something like this:

21166 clone(child_stack=NULL, flags=CLONE_CHILD_CLEARTID|CLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x7f8a18385e50) = 21167

This was the call to fork(), now looking for PID 21167:

[...]
21166 wait4(-1,  <unfinished ...>
21167 setuid(1000)                      = 0
21167 chdir("/home/bf")                 = 0
21167 capget({version=_LINUX_CAPABILITY_VERSION_3, pid=0}, NULL) = 0
21167 capget([...]) = 0
21167 capget({version=_LINUX_CAPABILITY_VERSION_3, pid=0}, NULL) = 0
[...]
21167 prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_IS_SET, CAP_CHOWN, 0, 0) = 0

After this, no set.*uid() should be called until execve("/bin/bash", [...]).

[AndrewGMorgan]

@ZoltanFridrich I suspect your pam_cap.so is failing to load at all. Make it required in your app config file and see if you can use su at all. It has some runtime dependencies on more recent libcap versions than are the default for Fedora. I've offered a revised libcap.spec file here (since the automatic builds appear to be failing of late):

https://bugzilla.redhat.com/show_bug.cgi?id=1919609#c40

[AndrewGMorgan]

I dug in a bit more. On Fedora-34, su is actually from the util-linux package:

$ rpm -qf `which su`
util-linux-2.36.2-1.fc34.x86_64

It too is non-compliant and the needed patch looks something like this:

--- util-linux-2.36.2/login-utils/su-common.c~  2021-02-12 03:32:01.749988723 -0800
+++ util-linux-2.36.2/login-utils/su-common.c   2021-11-19 20:00:36.006115931 -0800
@@ -1208,6 +1208,9 @@
        if (su->simulate_login && chdir(su->pwd->pw_dir) != 0)
                warn(_("warning: cannot change directory to %s"), su->pwd->pw_dir);
 
+       /* http://www.linux-pam.org/Linux-PAM-html/adg-interface-by-app-expected.html#adg-pam_end */
+       (void) pam_end(su->pamh, PAM_SUCCESS|PAM_DATA_SILENT);
+
        if (shell)
                run_shell(su, shell, command, argv + optind, max(0, argc - optind));

I've forwarded a copy of that patch to the author of the most recent Changelog entry here: https://mirrors.edge.kernel.org/pub/linux/utils/util-linux/v2.37/v2.37-ChangeLog

[ikerexxe]

Just in case, informing @karelzak maintainer of util-linux

[karelzak]

Thanks for your info.

It seems, the most important thing is PAM_DATA_SILENT -- I hope it's correctly implemented in PAM, because everywhere (in util-linux login and su) we keep the parent process running to clean up the session later. We have to be sure that after pam_end() in the child the PAM handler is still valid in the parent.

AndrewGMorgan and bjorn-fischer, please, please, can you also prepare PR for util-linux for login-utils/su-common.c and login-utils/login.c. I guess it's not only Fedora who uses login and su from util-linux. It's a trivial patch, but I'd like to be sure it's tested by someone who has experience with ambient capabilities. Thanks!

[AndrewGMorgan]

util-linux/util-linux#1515

[hallyn]

@AndrewGMorgan @bjorn-fischer so just to make sure, this patch is good to merge?

[AndrewGMorgan]

This looks good to me. However, the tabbing for the added code might need some attention to be consistent with the code it is being added to.

97f788f

[hallyn]

This looks good to me. However, the tabbing for the added code might need some attention to be consistent with the code it is being added to.

97f788f

Thanks, I went ahead and made that change, so am merging now.