Gentoo Archives: gentoo-dev

From: Brian Harring <ferringb@×××××.com>
To: constanze@g.o
Cc: gentoo-dev@l.g.o
Subject: Re: [gentoo-dev] eclass for handling of file-based capabilities
Date: Sun, 06 Mar 2011 11:03:34
Message-Id: 20110306110151.GA5990@hrair
In Reply to: [gentoo-dev] eclass for handling of file-based capabilities by Constanze Hausner
On Sat, Mar 05, 2011 at 02:24:22PM +0100, Constanze Hausner wrote:
> fcaps() { > debug-print-function ${FUNCNAME} "$@" > debug-print "${FUNCNAME}: Trying to set capabilities for ${4}" > local uid_gid=$1 > local perms=$2 > export fallbackFileMode=$perms > local capset=$3 > local path=$4
That export of fallbackFileMode looks pointless... clarify please. Also, your arg count checking here means it's going to throw some weird ass errors if people invoke it incorrectly. You likely want a [ $# -ne 4 ] && die "wrong arg count" thrown in there..
> if [ $# -eq 5 ]; then > local set_mode=$5 > else > debug-print "${FUNCNAME}: no set-mode provided, setting it to ep" > #if there is no set_mode provided, it is set to true > local set_mode=1 > fi > > #set owner/group > debug-print "${FUNCNAME}: setting owner and group to ${uid_gid}" > chown $uid_gid $path > if [ $? -ne 0 ]; then > eerror "chown "$uid_gid" "$path" failed." > return 2 > fi > > #set file-mode including suid > debug-print "${FUNCNAME}: setting file-mode ${perms}, including suid" > chmod $perms $path
Arbitrary/user-controlled path's always need to be quoted...
> if [ $? -ne 0 ]; then > eerror "chmod "$perms" "$path" failed." > return 3 > fi > > #if filecaps is not enabled all is done > use !filecaps && return 0 > > #if libcap is not installed caps cannot be set > if [ ! -f "/sbin/setcap" ]; then > debug-print "${FUNCNAME}: libcap not installed, could not set caps" > return 4 > fi > > #Check for set mode > if [ $set_mode -eq 1 ]; then > debug-print "${FUNCNAME}: set-mode = ep" > local sets="=ep" > else > debug-print "${FUNCNAME}: set-mode = ei" > local sets="=ei" > fi
Shift this into the initial arg processing; eliminates the need for set_mode and is simpler to follow.
> > #set the capability > debug-print "${FUNCNAME}: setting capabilities"
lay off the debug-print's a bit...
> setcap "$capset""$sets" "$path" &> /dev/null > > #check if the capabilitiy got set correctly > debug-print "${FUNCNAME}: checking capabilities" > setcap -v "$capset""$sets" "$path" &> /dev/null > > local res=$? > > #if caps could be set, remove suid-bit > if [ $res -eq 0 ]; then > debug-print "${FUNCNAME}: caps were set, removing suid-bit" > chmod -s $path > else > debug-print "${FUNCNAME}: caps could not be set" > ewarn "setcap "$capset" "$path" failed." > ewarn "Check your kernel and filesystem." > ewarn "Fallback file-mode was set." > fi > > return $res > }
I'd take a different approach here; this code basically assumes that the PM knows of it- note the chmod -s. The use flag protection you tried adding, without some profile hacks, is user modifiable- meaning users can flip it on even if the PM doesn't support it. Or consider that the code above is purely doing it's thing during the install phase, specifically against whatever filesystem is used for building- while capabilities might be able to be set there, it's possible the final merge location won't support it. End result of that is you'll get a setuid stripped binary merged to the livefs lacking the caps- borkage. Or consider the inverse- the buildroot can't do capabilities, but the livefs could. You get the idea. Instead, write the code so the PM has to export a marker in some fashion to explicitly state "yes, I can do capabilities"- I'm specifically suggestining checking for a callback function exposed to the env. If that function isn't there, then the PM can't do it- end of story. If it is, the PM takes the args and will try to apply the capabilities at the correct time- stripping setuid/setgid if it succeeds. Please go that route; and please do not stick "portage" into the function name, something generic we can use for a later EAPI is better. Implementing it as I suggested has the nice side affect of not being limited by PMS also, although it's an approach that still requires planning for compatibility. Thanks- ~brian


Subject Author
Re: [gentoo-dev] eclass for handling of file-based capabilities Constanze Hausner <constanze@g.o>