Neon packages don't set file capabilities
Closed, ResolvedPublic

Description

It looks like Neon packages do not install files with the proper capabilities. E.g for kinit we can see in the build log:

-- Installing: /workspace/build/debian/tmp/usr/lib/x86_64-linux-gnu/libexec/kf5/start_kdeinit
04:03:39 -- Installing: /workspace/build/debian/tmp/usr/lib/x86_64-linux-gnu/libexec/kf5/start_kdeinit_wrapper
04:03:39 Failed to set capabilities on file `/usr/lib/x86_64-linux-gnu/libexec/kf5/start_kdeinit' (Invalid argument)
04:03:39 The value of the capability argument is not permitted for a file. Or the file is not a regular (non-symlink) file

And this can be confirmed by manually examining the installed file:

sudo getcap -v /usr/lib/x86_64-linux-gnu/libexec/kf5/start_kdeinit
/usr/lib/x86_64-linux-gnu/libexec/kf5/start_kdeinit

Similar problem exists for kwin_wayland. When installing through packages it does not have the capabilities, when installing manually we get:

sudo getcap /usr/bin/kwin_wayland 
/usr/bin/kwin_wayland = cap_sys_nice+ep
graesslin created this task.Oct 3 2017, 6:13 PM
graesslin added a subscriber: sitter.
sitter added a comment.Oct 3 2017, 9:40 PM

Well, looks to me like the capability setting thing on a cmake level is not properly handling effective install path versus the target install path (i.e. make install DESTDIR=./foo ought to set the cap in ./foo/.. not /...)

possible. I recently saw a patch somewhere about it...

kinit seems to honor DESTDIR:

  install( CODE "execute_process(
          COMMAND
                 ${SETCAP_EXECUTABLE}
                 CAP_SYS_RESOURCE=+ep
                 $ENV{DESTDIR}${CMAKE_INSTALL_FULL_LIBEXECDIR_KF5}/start_kdeinit)"
)

And I adjusted KWin to honor destdir, but still:

16:47:47 -- Installing: /workspace/build/debian/tmp/usr/bin/kwin_wayland
16:47:47 -- Set runtime path of "/workspace/build/debian/tmp/usr/bin/kwin_wayland" to ""
16:47:47 Failed to set capabilities on file `/usr/bin/kwin_wayland' (Invalid argument)
16:47:47 The value of the capability argument is not permitted for a file. Or the file is not a regular (non-symlink) file

Sorry, but I think that's a problem in the neon build setup.

sitter added a comment.Oct 6 2017, 9:09 AM

DESTDIR isn't (necessarily) set through the environment. make install DESTIDR=./foo is an entirely valid invocation and apparently what trips the cmake code up:

โžœ make install DESTDIR=./foo
[  1%] Automatic moc for target kdeinit5
[  1%] Built target kdeinit5_automoc
[ 11%] Built target kdeinit5
[ 13%] Automatic moc for target kdeinit5_shutdown
[ 13%] Built target kdeinit5_shutdown_automoc
[ 19%] Built target kdeinit5_shutdown
[ 21%] Automatic moc for target kdeinit5_wrapper
[ 21%] Built target kdeinit5_wrapper_automoc
[ 26%] Built target kdeinit5_wrapper
[ 28%] Automatic moc for target kwrapper5
[ 28%] Built target kwrapper5_automoc
[ 34%] Built target kwrapper5
[ 36%] Automatic moc for target kdeinit_klauncher
[ 36%] Built target kdeinit_klauncher_automoc
[ 57%] Built target kdeinit_klauncher
[ 59%] Automatic moc for target klauncher
[ 59%] Built target klauncher_automoc
[ 65%] Built target klauncher
[ 67%] Automatic moc for target kshell5
[ 67%] Built target kshell5_automoc
[ 73%] Built target kshell5
[ 75%] Automatic moc for target start_kdeinit
[ 75%] Built target start_kdeinit_automoc
[ 80%] Built target start_kdeinit
[ 82%] Automatic moc for target start_kdeinit_wrapper
[ 82%] Built target start_kdeinit_wrapper_automoc
[ 88%] Built target start_kdeinit_wrapper
[ 90%] Built target docs-kdeinit5-kdeinit5-8
[ 92%] Automatic moc for target autostart_test
[ 92%] Built target autostart_test_automoc
[100%] Built target autostart_test
Install the project...
-- Install configuration: "Debug"
-- Installing: ./foo/usr/lib/x86_64-linux-gnu/cmake/KF5Init/KF5InitConfig.cmake
-- Installing: ./foo/usr/lib/x86_64-linux-gnu/cmake/KF5Init/KF5InitMacros.cmake
-- Installing: ./foo/usr/lib/x86_64-linux-gnu/cmake/KF5Init/KF5InitConfigVersion.cmake
-- Installing: ./foo/usr/lib/x86_64-linux-gnu/cmake/KF5Init/kde5init_dummy.cpp.in
-- Installing: ./foo/usr/bin/kdeinit5
-- Installing: ./foo/usr/bin/kdeinit5_shutdown
-- Installing: ./foo/usr/bin/kdeinit5_wrapper
-- Installing: ./foo/usr/bin/kwrapper5
-- Installing: ./foo/usr/lib/x86_64-linux-gnu/libkdeinit5_klauncher.so
-- Installing: ./foo/usr/lib/x86_64-linux-gnu/libexec/kf5/klauncher
-- Set runtime path of "./foo/usr/lib/x86_64-linux-gnu/libexec/kf5/klauncher" to ""
-- Installing: ./foo/usr/share/dbus-1/interfaces/kf5_org.kde.KLauncher.xml
-- Installing: ./foo/usr/bin/kshell5
-- Installing: ./foo/usr/lib/x86_64-linux-gnu/libexec/kf5/start_kdeinit
-- Installing: ./foo/usr/lib/x86_64-linux-gnu/libexec/kf5/start_kdeinit_wrapper
unable to set CAP_SETFCAP effective capability: Operation not permitted
-- Installing: ./foo/usr/share/man/man8/kdeinit5.8

Ah nvm, I was rambling nonesenese what actually happens is that make injects the var into its own environment. So that technically ought to be fine. But there's a bunch of problems nonetheless

  • a regular user doesn't get to setcap (which is why it doesn't work for me, and I wonder how this is meant to work with kdesrc-build, cause I don't see how). that may well need addressing by fatally failing if setcap fails and then maybe have -DKDE_NO_SETCAP=ON or something to disable it altogether when installing to $HOME.
  • neon builds happen in unprivileged subuid'd containers (i.e. they effectively run as a regular user, even though inside the container it looks like they are root), so they may not be able to setcap at all similar to a regular user as described above. not sure what to do about that, giving them setcap seems a bit "dangerous?" (I think I have a workaround that can deal with this somewhat by also largely bypassing the problems listed below)
  • after a quick duckduckwalk I ran into https://bugs.launchpad.net/ubuntu/+source/ubiquity/+bug/1302192 which describes a whole slew of problems with preserving capabilities across the various parts of the ubuntu (and probably debian) distro stack that copy files about in some form or fashion. fortunately, this seems dealt with so it probably is no longer a problem. BUT it may well be wise to send a mail to the distro packagers list telling them to make sure their stack can actually handle caps before adopting them.
  • there is indeed a bug in the way debs are built which applies to all of the debian world actually, make gets invoked with AM_UPDATE_INFO_DIR=no (originally introduced as a bit of a workaround for autohell being autohell and it breaking itself randomly) and cmake respects this as well and refuses to update its own makefiles accordingly. in practice this means DESTDIR is not applied to the setcap call. in fact, if you do make install DESTDIR=./foo AND then make install DESTDIR=./bar AM_UPDATE_INFO_DIR=no the second install will call setcap on the ./foo/.. path. I am entirely unsure how to resolve this given the flag comes from debian and affects everything and the kitchen sink. also, from a practical POV the AM_UPDATE_INFO_DIR setting has no adverse effect on autohell since there, DESTDIR is evaluated in make, it's only cmake that screws this up. so maybe one should consider it a cmake bug as DESTDIR must always be updated? I really do not know.

Lastly, you need to be very careful here. As I've described in the "Plasma Wayland relocatability" thread on plasma-devel quite a while ago, while kinit wanted setuid/setcaps my analysis suggested that it doesn't actually need or use the capabilities (which is further supported by the fact that neon apparently didn't have the caps set at all and kinit worked fine). So, for all intents and purposes kwin is doing ground breaking work as the setcap in kinit was non-existent (at least as far as debian distros are concerned).

Oh, and maybe it would be wise to move this code to ECM so kinit and kwin both share the same bugs ;))

my analysis suggested that it doesn't actually need or use the capabilities

KWin needs capabilities. Or if you have better ideas on how to get what we need please tell me. KWin needs to change to real-time scheduler. This is just not possible for a process not having CAP_SYS_NICE. The minimum priority one can raise to is 0, which is just 1 below the minimum.

An idea: could you do the setcap call in the postinst script?

Interesting: D8064

This seems to actually make the setcap run on the correct path even with AM_UPDATE_INFO_DIR=no ๐ŸŒถ
So it should be applied to kwin as well I suppose (also, again, this should be shared code in ECM :P).

I understand that kwin actually needs the caps, hence why I think this needs an actual solution (cmake erroring on failed setcap) and communication to distros at large (so they make sure their stack works).

As far as neon is concerned a postinst hack would be the way to fix it that I had in mind, however given the aforementioned diff I now have hopes to make it work properly without hack.

I picked the patch for KWin and now we have:

18:08:53 /tooling/ci-tooling/lib/ci/build_binary.rb:100:in `system':  (RuntimeError)
18:08:53 
18:08:53 Unallowed call to: setcap ["CAP_SYS_NICE=+ep", "/workspace/build/debian/tmp/usr/bin/kwin_wayland"]
18:08:53 setcap must not be called. Build containers are run without a whole
18:08:53 bunch of privileges which makes setcap non functional!
18:08:53 Additionally, setcap uses xattrs which may not be available on the
18:08:53 installation file system. Instead you should introduce a postinst
18:08:53 call matching the setcap call with a fallback to setuid.

Btw. the build is (and was) red.

rikmills added a subscriber: rikmills.EditedOct 10 2017, 7:08 PM

I picked the patch for KWin

I ran a Kubuntu CI build with that on Artful.

The buildlog warning of "Failed to set capabilities on file" went away, however on installation it seems the capability is not there on the installed file.

So I presume post install script looks more the way to go.

I picked the patch for KWin

I ran a Kubuntu CI build with that on Artful.

The buildlog warning of "Failed to set capabilities on file" went away, however on installation it seems the capability is not there on the installed file.

As I've said above, the cap code needs improvements on the cmake side to actually fail the install if setcap comes back !0. That's not really related to the neon problem though, which indeed is now dealt with.

Erm please don't u+s kwin! That is nothing KWin handles and running KWin as root is NO, NO, NO! KWin does not have code to drop back to user.

@bshah could you please adapt the script to drop all the chmod u+s? It's really dangerous!

bshah added a comment.Oct 11 2017, 8:03 PM

Argh, sorry yeah, I didn't realize that chmod u+s, but at best I can fix it tomorrow morning..

bshah moved this task from Backlog to Review on the Neon board.Jul 18 2018, 3:12 AM

kwin_wayland on Neon no-longer does the chmod u+s so this can be closed.

jriddell closed this task as Resolved.Oct 3 2018, 9:23 AM
jriddell claimed this task.