Bug 646444

This seems to break mock (when iputils is a dep) when using tmpfs as storage. 

Is file capabilities not supported on tmpfs?

I would figure if the tmpfs is mounted nosetuid this would be a problem.

Created attachment 457784 [details]
patch to drop capabilities

Note that when running suid ping immediately drops root privileges after opening the raw socket. It does not drop capabilities though ie ping keeps running with CAP_NET_RAW even if it doesn't need it anymore. So blindly applying fscaps doesn't necessarily improve anything. I've just sent the attached patch upstream to fix that.

I think this is a  bug in the kernel, which I have opened, needing a capability in order to drop capabilities seems wrong.  Hopefully a combination of file capabilities along with allow apps to trop capabilities when they no longer are needed will make for a more secure system.

Created attachment 458700 [details]
updated patch

updated patch to also do the right thing if ping is still installed setuid

(In reply to comment #4)
> I think this is a  bug in the kernel, which I have opened, needing a capability
> in order to drop capabilities seems wrong.  Hopefully a combination of file
> capabilities along with allow apps to trop capabilities when they no longer are
> needed will make for a more secure system.

I don't understand what you are talking about. Dropping capabilities works just fine, ping just needs to be patched to actually do it :-)

(In reply to comment #5)
> Created attachment 458700 [details]
> updated patch
> 
> updated patch to also do the right thing if ping is still installed setuid

I don't see practical impact of moving dropping capabilities behind dropping setuid.
But I've noticed it would be fair to free caps structure:

caps = cap_init();
if (cap_set_proc(caps) < 0) {
	perror("ping: cap_set_proc");
	exit(-1);
}
cap_free(caps); // FREE IT

I am using capng, and My code looks like.

static int drop_capabilities(uid_t uid)
{
	capng_clear(CAPNG_SELECT_BOTH);
	if (capng_lock() < 0) 
		return -1;

	/* Change uid */
	if (setresuid(uid, uid, uid)) {
		fprintf(stderr, _("Error changing uid, aborting.\n"));
		return -1;
	}
	return capng_apply(CAPNG_SELECT_BOTH);
}


I think you need to do setresuid no matter what to make sure you drop the ability to get back to UID=0.

(In reply to comment #7)
> (In reply to comment #5)
> > Created attachment 458700 [details] [details]
> > updated patch
> > 
> > updated patch to also do the right thing if ping is still installed setuid
> 
> I don't see practical impact of moving dropping capabilities behind dropping
> setuid.

If you drop caps first setuid(getuid()) can't change the POSIX saved uid anymore (but returns success anyways!). Therefore a setuid(0) afterwards would succeed even though getuid() == geteuid(). The assumption is that both dropping setuid as well as dropping capabilities is needed in order to allow an unmodified ping binary to be used either with fscaps or setuid.

> But I've noticed it would be fair to free caps structure:

yes indeed :-)

*** Bug 455713 has been marked as a duplicate of this bug. ***