Bug 646444 - Replace SETUID in spec file with the correct file capabilities.
Summary: Replace SETUID in spec file with the correct file capabilities.
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: iputils
Version: rawhide
Hardware: Unspecified
OS: Unspecified
low
medium
Target Milestone: ---
Assignee: Jiri Skala
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 455713 (view as bug list)
Depends On:
Blocks: 449984 rancid removesetuid
TreeView+ depends on / blocked
 
Reported: 2010-10-25 12:51 UTC by Daniel Walsh
Modified: 2020-01-27 12:11 UTC (History)
9 users (show)

Fixed In Version:
Clone Of: 646443
Environment:
Last Closed: 2010-10-27 08:50:42 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
patch to drop capabilities (2.65 KB, patch)
2010-11-04 13:17 UTC, Ludwig Nussel
no flags Details | Diff
updated patch (2.93 KB, patch)
2010-11-08 10:36 UTC, Ludwig Nussel
no flags Details | Diff

Description Daniel Walsh 2010-10-25 12:51:24 UTC
+++ This bug was initially created as a clone of Bug #646443 +++

Description of problem:

Please remove setuid setup of files in your package with file capabilities.

This is to satisfy the F15 feature.

https://fedoraproject.org/wiki/Features/RemoveSETUID

An example of how this was done for X is.


%if 0%{?fedora} < 15
%define Xorgperms %attr(4711, root, root)
%else
%define Xorgperms %attr(0711,root,root) %caps(cap_sys_admin,cap_sys_rawio,cap_dac_override=pe)
%endif

Comment 1 Terje Røsten 2010-10-31 07:22:32 UTC
This seems to break mock (when iputils is a dep) when using tmpfs as storage. 

Is file capabilities not supported on tmpfs?

Comment 2 Daniel Walsh 2010-11-01 13:16:55 UTC
I would figure if the tmpfs is mounted nosetuid this would be a problem.

Comment 3 Ludwig Nussel 2010-11-04 13:17:33 UTC
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.

Comment 4 Daniel Walsh 2010-11-04 15:01:24 UTC
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.

Comment 5 Ludwig Nussel 2010-11-08 10:36:48 UTC
Created attachment 458700 [details]
updated patch

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

Comment 6 Ludwig Nussel 2010-11-08 10:38:33 UTC
(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 :-)

Comment 7 Jiri Skala 2010-11-08 13:52:17 UTC
(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

Comment 8 Daniel Walsh 2010-11-08 20:22:10 UTC
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.

Comment 9 Ludwig Nussel 2010-11-09 08:20:53 UTC
(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 :-)

Comment 10 Peter Lemenkov 2020-01-27 12:11:13 UTC
*** Bug 455713 has been marked as a duplicate of this bug. ***


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