Skip to content

Commit

Permalink
cockpit-session: stop installing setuid root
Browse files Browse the repository at this point in the history
systemd spawns this for us now, so we don't need the setuid bit anymore.
Clean up the statoverride in the Debian packaging on upgrades.

However, that means that cockpit-ws cannot be run as
`cockpit-wsinstance` user outside of the unit any more. Adjust our tests
to run it as root instead.
  • Loading branch information
allisonkarlitskaya authored and martinpitt committed Nov 12, 2024
1 parent 2ab396a commit 809938f
Show file tree
Hide file tree
Showing 6 changed files with 9 additions and 18 deletions.
5 changes: 0 additions & 5 deletions src/session/Makefile-session.am
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,3 @@ cockpit_session_SOURCES = \
src/session/session-utils.h \
src/session/session.c \
$(NULL)

# set up cockpit-session to be setuid root, but only runnable by cockpit-session
install-exec-hook::
chown -f root:cockpit-wsinstance $(DESTDIR)$(libexecdir)/cockpit-session || true
chmod -f 4750 $(DESTDIR)$(libexecdir)/cockpit-session || true
9 changes: 3 additions & 6 deletions test/verify/check-connection
Original file line number Diff line number Diff line change
Expand Up @@ -1007,8 +1007,7 @@ until pgrep -f '^(/usr/[^ ]+/[^ /]*python[^ /]* )?/usr/bin/cockpit-bridge'; do s
m.spawn("socat TCP-LISTEN:9091,reuseaddr,fork TCP:localhost:9099", "socat.log")

# ws with plain --no-tls should fail after login with mismatching Origin (expected http, got https)
m.spawn(f"runuser -u cockpit-wsinstance -- {self.ws_executable} --no-tls -p 9099",
"ws-notls.log")
m.spawn(f"{self.ws_executable} --no-tls -p 9099", "ws-notls.log")
m.wait_for_cockpit_running(tls=True)

b.open(f"https://{b.address}:{b.port}/system")
Expand Down Expand Up @@ -1040,8 +1039,7 @@ until pgrep -f '^(/usr/[^ ]+/[^ /]*python[^ /]* )?/usr/bin/cockpit-bridge'; do s
self.allow_browser_errors("Error reading machine id")

# ws with --for-tls-proxy accepts only https origins, thus should work
m.spawn(f"runuser -u cockpit-wsinstance -- {self.ws_executable} --for-tls-proxy -p 9099 -a 127.0.0.1",
"ws-fortlsproxy.log")
m.spawn(f"{self.ws_executable} --for-tls-proxy -p 9099 -a 127.0.0.1", "ws-fortlsproxy.log")
m.wait_for_cockpit_running(tls=True)
b.open(f"https://{b.address}:{b.port}/system")
b.wait_visible("#login")
Expand Down Expand Up @@ -1395,8 +1393,7 @@ server {

def run_ws(extra_opts=""):
m.spawn(
f"runuser -u cockpit-wsinstance -- {self.libexecdir}/cockpit-ws "
f"--address=127.0.0.1 --for-tls-proxy {extra_opts}", "ws.log")
f"{self.libexecdir}/cockpit-ws --address=127.0.0.1 --for-tls-proxy {extra_opts}", "ws.log")
m.wait_for_cockpit_running()

def kill_ws():
Expand Down
2 changes: 0 additions & 2 deletions tools/arch/PKGBUILD
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,6 @@ package_cockpit() {
rm -rf "$pkgdir"/usr/{src,lib/firewalld}
install -Dm644 "$srcdir"/cockpit.pam "$pkgdir"/etc/pam.d/cockpit

echo "z /usr/lib/cockpit/cockpit-session - - cockpit-wsinstance -" >> "$pkgdir"/usr/lib/tmpfiles.d/cockpit-ws.conf

# remove unused plugins
rm -rf "$pkgdir"/usr/share/cockpit/{selinux,playground,sosreport} \
"$pkgdir"/usr/share/metainfo/org.cockpit_project.cockpit_{selinux,sosreport}.metainfo.xml
Expand Down
2 changes: 1 addition & 1 deletion tools/cockpit.spec
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ authentication via sssd/FreeIPA.
%{_libexecdir}/cockpit-desktop
%{_libexecdir}/cockpit-certificate-ensure
%{_libexecdir}/cockpit-certificate-helper
%attr(4750, root, cockpit-wsinstance) %{_libexecdir}/cockpit-session
%{_libexecdir}/cockpit-session
%{_datadir}/cockpit/branding
%{_datadir}/selinux/packages/%{selinuxtype}/%{name}.pp.bz2
%{_mandir}/man8/%{name}_session_selinux.8cockpit.*
Expand Down
7 changes: 5 additions & 2 deletions tools/debian/cockpit-ws.postinst
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@ set -e

#DEBHELPER#

if ! dpkg-statoverride --list /usr/lib/cockpit/cockpit-session >/dev/null; then
dpkg-statoverride --update --add root cockpit-wsinstance 4750 /usr/lib/cockpit/cockpit-session
# remove dpkg-statoverride on upgrade
if dpkg-statoverride --list /usr/lib/cockpit/cockpit-session >/dev/null; then
dpkg-statoverride --remove /usr/lib/cockpit/cockpit-session
chmod 755 /usr/lib/cockpit/cockpit-session
chgrp root /usr/lib/cockpit/cockpit-session
fi

# restart cockpit.service on package upgrades, if it's already running
Expand Down
2 changes: 0 additions & 2 deletions tools/debian/cockpit-ws.postrm
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,4 @@ if [ "$1" = purge ]; then
[ -L /etc/motd.d/cockpit ] && rm /etc/motd.d/cockpit || true
[ -L /etc/issue.d/cockpit.issue ] && rm /etc/issue.d/cockpit.issue || true
rm -f /etc/cockpit/disallowed-users

dpkg-statoverride --remove /usr/lib/cockpit/cockpit-session
fi

0 comments on commit 809938f

Please sign in to comment.