Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!

Bug 460810

Summary: fcaps.eclass: pkg_postinst hook bypasses FEATURES=suidctl
Product: Portage Development Reporter: Bob's Your Uncle <garbage>
Component: UnclassifiedAssignee: Gentoo's Team for Core System packages <base-system>
Status: RESOLVED OBSOLETE    
Severity: normal CC: aranea, dev-portage, perfinion, pr.gentoo-acct
Priority: Normal    
Version: unspecified   
Hardware: All   
OS: Linux   
Whiteboard:
Package list:
Runtime testing required: ---
Bug Depends on: 467766, 548516    
Bug Blocks:    

Description Bob's Your Uncle 2013-03-08 18:46:05 UTC
innana ~ # grep FEATURES /etc/portage/make.conf
FEATURES="buildpkg suidctl"

casey@inanna ~ $ cat /etc/portage/suidctl.conf
/bin/passwd
/bin/su
/usr/bin/sudo
/usr/sbin/tcptraceroute
/usr/lib64/misc/glibc/pt_chown
#/bin/mount
#/bin/umount
#/usr/bin/crontab
#/usr/bin/mutt_dotlock
#/usr/bin/write
#/usr/sbin/exim
#/usr/bin/chage
#/usr/bin/chfn
#/usr/bin/chsh
#/usr/bin/expiry
#/usr/bin/gpasswd
#/usr/bin/newgrp
#/usr/bin/netselect
#/usr/bin/man
#/usr/lib64/misc/ssh-keysign
casey@inanna ~ $ sudo emerge -va1k iputils

These are the packages that would be merged, in order:

Calculating dependencies... done!
[binary   R    ] net-misc/iputils-20121221-r1  USE="-SECURITY_HAZARD -caps -doc -filecaps -gnutls -idn -ipv6 -ssl -static" 0 kB

Total: 1 package (1 reinstall, 1 binary), Size of downloads: 0 kB

Would you like to merge these packages? [Yes/No] Yes

>>> Emerging binary (1 of 1) net-misc/iputils-20121221-r1
 * iputils-20121221-r1.tbz2 MD5 SHA1 size ;-) ...                        [ ok ]
>>> Extracting info
>>> Extracting net-misc/iputils-20121221-r1

>>> Installing (1 of 1) net-misc/iputils-20121221-r1
 * checking 19 files for package collisions
>>> Merging net-misc/iputils-20121221-r1 to /
>>> Performing suid scan in /var/tmp/portage/net-misc/iputils-20121221-r1/image/
--- /usr/
--- /usr/share/
--- /usr/share/man/
--- /usr/share/man/man8/
>>> /usr/share/man/man8/tracepath.8
>>> /usr/share/man/man8/tftpd.8
>>> /usr/share/man/man8/rdisc.8
>>> /usr/share/man/man8/rarpd.8
>>> /usr/share/man/man8/ping.8
>>> /usr/share/man/man8/pg3.8
>>> /usr/share/man/man8/ninfod.8
>>> /usr/share/man/man8/clockdiff.8
>>> /usr/share/man/man8/arping.8
--- /usr/share/doc/
--- /usr/share/doc/iputils-20121221-r1/
>>> /usr/share/doc/iputils-20121221-r1/RELNOTES
>>> /usr/share/doc/iputils-20121221-r1/INSTALL
--- /usr/sbin/
>>> /usr/sbin/tracepath
>>> /usr/sbin/tftpd
>>> /usr/sbin/rdisc
>>> /usr/sbin/rarpd
>>> /usr/sbin/ipg
--- /usr/bin/
>>> /usr/bin/clockdiff
--- /bin/
>>> /bin/ping
>>> /bin/arping
>>> Safely unmerging already-installed instance...
No package files given... Grabbing a set.
--- replaced obj /usr/share/man/man8/tracepath.8
--- replaced obj /usr/share/man/man8/tftpd.8
--- replaced obj /usr/share/man/man8/rdisc.8
--- replaced obj /usr/share/man/man8/rarpd.8
--- replaced obj /usr/share/man/man8/ping.8
--- replaced obj /usr/share/man/man8/pg3.8
--- replaced obj /usr/share/man/man8/ninfod.8
--- replaced obj /usr/share/man/man8/clockdiff.8
--- replaced obj /usr/share/man/man8/arping.8
--- replaced dir /usr/share/man/man8
--- replaced dir /usr/share/man
--- replaced obj /usr/share/doc/iputils-20121221-r1/RELNOTES
--- replaced obj /usr/share/doc/iputils-20121221-r1/INSTALL
--- replaced dir /usr/share/doc/iputils-20121221-r1
--- replaced dir /usr/share/doc
--- replaced dir /usr/share
--- replaced obj /usr/sbin/tracepath
--- replaced obj /usr/sbin/tftpd
--- replaced obj /usr/sbin/rdisc
--- replaced obj /usr/sbin/rarpd
--- replaced obj /usr/sbin/ipg
--- replaced dir /usr/sbin
--- replaced obj /usr/bin/clockdiff
--- replaced dir /usr/bin
--- replaced dir /usr
--- replaced obj /bin/ping
--- replaced obj /bin/arping
--- replaced dir /bin
>>> Regenerating /etc/ld.so.cache...
>>> Original instance of package unmerged safely.
>>> net-misc/iputils-20121221-r1 merged.
>>> Auto-cleaning packages...

>>> No outdated packages were found on your system.

 * GNU info directory index is up-to-date.

casey@inanna ~ $ ls -l /bin/ping
-rws--x--x 1 root root 47360 Mar  8 08:47 /bin/ping


WTF?

Also if I type in /bin/ping manually into suidctl.conf, I do not see the "- /bin/ping is an approved suid file" message I would normally expect to.
Comment 1 Zac Medico gentoo-dev 2013-03-09 19:21:32 UTC
Currenty, you can avoid it by enabling USE="filecaps", and enabling extended attributes on you file system.

@base-system: Can the fcaps be done in src_install, so portage has a chance to apply suidctl?
Comment 2 Bob's Your Uncle 2013-03-10 18:56:52 UTC
I'd rather not USE something I don't otherwise need or want to workaround this - I don't understand why that should be - this is a SUID binary and the SUID scan should detect it, simple as that. My kernel does not have support for extended attributes at all. How is it that this feature makes it work?
Comment 3 Zac Medico gentoo-dev 2013-03-10 20:10:44 UTC
The iputils ebuild calls the fcaps eclass function in pkg_postist, operating on files which have already been installed. This prevents portage from being able to scan the files. Portage could scan the files if the the ebuild would instead call fcaps in src_install (which operates on the ${D} directory containing the files that portage scans).
Comment 4 SpanKY gentoo-dev 2013-04-28 04:28:14 UTC
(In reply to comment #1)

fcaps can't be done in src_* because portage doesn't save/restore the filesystem extended attributes that are necessary for the whole thing to work

we could probably add IUSE=+suid to fcaps.eclass and have it drop the set*id when that is not enabled
Comment 5 Zac Medico gentoo-dev 2013-04-28 04:55:53 UTC
(In reply to comment #4)
> fcaps can't be done in src_* because portage doesn't save/restore the
> filesystem extended attributes that are necessary for the whole thing to work

It should work if FEATURES=xattr is enabled.
Comment 6 SpanKY gentoo-dev 2013-04-28 16:41:36 UTC
(In reply to comment #5)

is that guaranteed by PMS yet ? :)
Comment 7 Pavel Labushev 2015-05-15 09:18:46 UTC
# ls -l /etc/portage/postsync.d/fcaps-eclass-hack
-rwx------ 1 root root 161 May 15 16:13 /etc/portage/postsync.d/fcaps-eclass-hack

# cat /etc/portage/postsync.d/fcaps-eclass-hack
#!/bin/sh
sed -i "s/mode=['\"]4711['\"]/mode='711'/" /usr/portage/eclass/fcaps.eclass
sed -i 's/chmod \${mode}/#chmod ${mode}/' /usr/portage/eclass/fcaps.eclass
Comment 8 SpanKY gentoo-dev 2015-07-13 07:41:24 UTC
*** Bug 553330 has been marked as a duplicate of this bug. ***
Comment 9 Mira Ressel 2016-01-08 22:19:26 UTC
I think the fcaps call could (and should) be moved to src_install without any relevant problems.

There are three cases in which might moving fcaps to src_install would cause problems: 

(1) The / fs supports xattrs, but the /var/tmp/portage fs doesn't. Pretty unlikely unless someone misconfigured their kernel.

(2) The copy operation during the merge step doesn't preserve xattrs. I haven't checked the other package managers, but for Portage this is pretty unlikely: With FEATURES=xattr the preservation is guaranteed, and otherwise is should still work in most cases (portage usually uses python's os.rename, which calls renameat(), which in turn preserves xattrs.

(3) There might be problems with binpkg's. I don't know if this is actually the case for Portage, however.

So, there are some rare problematic corner cases; but as the filecaps USE isn't enabled by default anywhere and users have to actively set it themselves, IMHO we can expect them to deal with any breakage that might ensue in those rare cases. Of course, an explanatory wiki entry wouldn't hurt (I could probably write one).

On the plus side, performing the proposed change would be a huge win for FEATURE=suidctl users; silently breaking the guarantees suidctl is supposed to provide is really not cool(TM). And perfinion has indicated that it'd be useful for some portage patch he's currently working on, too.

If there's interest in making this happen, I can prepare patches for the eclass and the ~15 affected ebuilds.
Comment 10 Mira Ressel 2016-01-08 22:38:06 UTC
Whoops, I'm sorry, turns out fcaps.eclass has USE=+filecaps, so filecaps is enabled by default. Of course, my previous argumentation doesn't apply anymore then. I'd suggest to disable the filecaps USE by default and proceed as previously proposed.