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
pgpsKYcI9z358.pgp
Description: PGP signature
