Arksine / moonraker

Web API Server for Klipper
https://moonraker.readthedocs.io
GNU General Public License v3.0
1.06k stars 408 forks source link

PolicyKit warnings; unable to manage services, restart system, or update packages #349

Closed Arksine closed 2 years ago

Arksine commented 2 years ago

This announcement is an effort to get ahead of a coming change that will certainly result in issues. PR #346 has been merged, and with it are some changes to Moonraker's default behavior.

It is desirable to move away from having Mooraker spawn "sudo" commands to collect system data, manage systemd services, reboot/shutdown the machine, and update packages. We can do this through the Linux D-Bus interface, however to do so we must add PolicyKit rules that grant Moonraker specific privileges. Currently Moonraker's update_manager isn't equipped to make these changes, and it would be unwise to automatically make such a change.

As such, after updating users will need to ssh into their machine (or open a terminal on Desktops) and run the following new script:

~/moonraker/scripts/set-policykit-rules.sh

Once the rules are installed Moonraker will restart and all necessary permissions should be available. Users of Debian and its derivatives (Raspberry Pi OS, Ubuntu, etc) should note that they ship with an older version of PolKit, which can only grant permissions on a per user or per group basis. This means that any DBus application run by the user will have the same privileges Moonraker has. For example, you can run systemctl restart klipper without sudo. Newer versions of PolKit (those included in Arch, etc) allow for more fine grained rules, only the Moonraker process will be granted such privileges.

If managing services and/or updating packages is not important to you, it is possible to disable this functionality rather than add the PolKit rules:

# moonraker.conf

# Disable service management.  The reboot/shutdown commands will
# fall back to the CLI calls, where password-less "sudo" access is
# necessary
[machine]
provider: none

# Disable Package Management
[update_manager]
enable_system_updates: False

Its also possible to use the CLI Fallbacks. Take note however that it is likely that these will be deprecated and removed in the future:

# moonraker.conf

# Use the CLI fallbacks to control system services and poll system state
[machine]
provider: systemd_cli

# Add the following to your [update_manager] section to disable PackageKit
# This will fall back to "apt" commands.
[update_manager]
enable_packagekit: False

One final note on package updates: To use the packagekit backend it is necessary to install packagekit. For most this would have been done on the last update. However, if the update manager fails to update packages its a good idea to ssh into the pi and run the install script with no options. This will update all dependencies and install new ones:

~/moonraker/scripts/install-moonraker.sh
StuSerious commented 2 years ago

Just a heads up that when installed through KIAUH using multiple instaces, set-policykit-rules.sh will fail to trigger a restart.

Be sure to restart your host if that is your case, that will still apply the changes made by the script.

Arksine commented 2 years ago

If you are running multiple instances then set-policykit-rules.sh will also fail to update the unit script for you. This isn't a huge deal on Debian and its derivatives, but on distros that run PolKit 0.120 it is necessary to add SupplementaryGroups=moonraker-admin to the unit.

That said, officially I don't encourage the use of multiple instances, as Moonraker isn't designed to be "instance aware".

alfrix commented 2 years ago

After having issues with users running install scripts with sudo, even though nobody told them to do so, i tried the same with this update in moonraker, i did sudo ./set-policykit-rules.sh and the script didn't work, then i ran the script as a regular user ./set-policykit-rules.sh and everything worked. For that reason i recommend to add a safeguard in the script to warn users something like:

if [ "$EUID" == 0 ]
    then echo_error "Plaease do not run this script as root"
    exit
fi
Arksine commented 2 years ago

Makes sense, IIRC the install script has this. I'll add it shortly. Thanks for the feedback.

dw-0 commented 2 years ago

Just a heads up that when installed through KIAUH using multiple instaces, set-policykit-rules.sh will fail to trigger a restart.

Be sure to restart your host if that is your case, that will still apply the changes made by the script.

Although i know that Arksine does not encurage the use of multi instances in the style of KIAUH creates them, i just wanted to give a quick update that i just pushed a fix to KIAUH adressing that issue (https://github.com/th33xitus/kiauh/commit/a14e321df9cca2d1d44b82edb5c22da482a6f53c) Unit files will now be updated correctly. That being said, the restart of the moonraker.service unit set-policykit-rules.sh fires will still be visible as failed though, but that can be ignored. KIAUH will restart all Moonraker services and should log confirm messages to the console as usual

StuSerious commented 2 years ago

i just pushed a fix to KIAUH adressing that issue (th33xitus/kiauh@a14e321)

image

tested and working! thanks! <3

miklschmidt commented 2 years ago

Oof this is a tough one @Arksine. If users update Moonraker they're locked out of their webinterface, so i can't fix it through RatOS scripting. It'll only work seamlessly if they update the RatOS module first. Would be nice with a transition period of a month (whatever the default package refresh interval is) or something where the policykit warnings are present, but policykit is still unused. Is that possible?

EDIT: My goal here is to avoid having to teach users SSH and linux commands.

miklschmidt commented 2 years ago

Makes sense, IIRC the install script has this. I'll add it shortly. Thanks for the feedback.

That safeguard will break my automated policykit rule installation. I had to patch the $USER variable temporarily and run the script as root to avoid sudo prompts. There are a lot of sudo prefixed commands in that script that i would prefer not to whitelist for passwordless usage. Basically it would make it impossible to fix this without the user having to interact with the commandline. I know that's not a goal of yours, but things like these are why klipper is scary and unapproachable to people.

miklschmidt commented 2 years ago

Btw (and sorry for all the ranting here šŸ˜‚), i will definitely transition to use policykit for RatOS scripts as well, it's definitely the right choice going forward, but it would be preferable if we could avoid breaking installations (for regular 3DP users) on the way there.

EDIT: Arksine/moonraker@13591d0a5b1324e2b44b5586189bdd63d757fc45 R.I.P my inbox šŸ˜‚

Arksine commented 2 years ago

Oof this is a tough one @Arksine. If users update Moonraker they're locked out of their webinterface, so i can't fix it through RatOS scripting.

@miklschmidt I'm not sure why this change would lock users out of the WebUI, it should simply result in warnings and an error if the user attempts to invoke an endpoint that requires those permissions. Users can configure the CLI fallbacks to restore access to these endpoints. The warnings won't show up in this scenario, however it may be possible to add a way to do this.

That safeguard will break my automated policykit rule installation. I had to patch the $USER variable temporarily and run the script as root to avoid sudo prompts. There are a lot of sudo prefixed commands in that script that i would prefer not to whitelist for passwordless usage. Basically it would make it impossible to fix this without the user having to interact with the commandline. I know that's not a goal of yours, but things like these are why klipper is scary and unapproachable to people.

If necessary I can add a command line parameter that disables the sudo check. Presumably all of the commands in the script would need to run without sudo access for this to work, as it would assume the that script is run by root.

miklschmidt commented 2 years ago

@Arksine True, you just can't restart services or access the update manager (which kills my automation). This was compounded with the recent pip/piwheel issue, sorry about that. I'll make a script that tries to patch moonraker.conf and enables the cli fallback, that way i can give people some time to get everything updated before switching back to policykit. It's just too late for me to do anything if people update moonraker first, because then they can't update RatOS.

If necessary I can add a command line parameter that disables the sudo check.

That would be nice, currently i'm copying and patching up the setup script to skip the check each time the RatOS or moonraker post-merge hook is run. Only if the rules aren't already present though.

Presumably all of the commands in the script would need to run without sudo access for this to work, as it would assume the that script is run by root.

Yeah, however it does work even with sudo if run as root, so don't go through the trouble for me :)

Thanks for bearing with me, it's been a tough weekend with all the pings and panic šŸ˜‚

Arksine commented 2 years ago

@miklschmidt All is good, been one of those days :smile:

I have released a workaround for the pip issue, https://github.com/Arksine/moonraker/issues/352#issuecomment-1025211166. It will require ssh, not ideal but it can get folks back up and running. We can track https://github.com/pypa/pip/pull/10847, when that is merged and released I believe that it users will be able to update Moonraker without issue.

I also added a --root option to set-policykit-rules.sh, which bypasses the check for root.

miklschmidt commented 2 years ago

@Arksine Awesome, i used that to fix one users broken installation 30 minutes ago, it does the trick. I made an announcement to the users earlier today to hold off on updating until further notice, that stopped the bulk of the carnage :D

Fixed the script to use the --root option. Should be good now if i somehow can make sure people update RatOS before they update moonraker once i tell them it's safe again.

I will follow along on the pip issue :)

ssjbardock commented 2 years ago

The recommended actions above to run set-policykit-rules.sh isn't working for me.

I've tried deleting the existing policy, as well as reinstalling moonraker, but still just getting the same errors moon1 moon2 .

Arksine commented 2 years ago

@ssjbardock If the polkit rules aren't working there is something about either the OS or the Moonraker installation that deviates from the standard. If you haven't been able to resolve this I would suggest creating a new issue. Make sure you attach moonraker.log and give details about your OS. Detail the process you used to install moonraker and any deviations from the instructions outlined in the documentation.

vkhodygo commented 2 years ago

@Arksine Do you really need to mess with the permissions that much? Why don't you employ the --user flag of systemd instead? It seems to be working, at least partially, no extra users and groups, everything is stored where it's supposed to be, no writing to "/etc".

@StuSerious It seems that you might benefit from systemd templates for multiple instances.

Arksine commented 2 years ago

Moonraker requires permission to perform administrative tasks such as restarting another service, updating packages, and rebooting the system. That said, it is completely possible to disable this functionality in the configuration, in which case you will get no warnings. Your solution won't grant Moonraker permission to do these things, and I suspect it would break the aforementioned functionality.

vkhodygo commented 2 years ago

restarting another service, updating packages, and rebooting the system.

How exactly does this work?

Arksine commented 2 years ago

Moonraker connects to systemd over dbus issues commands. For updating packages it connects to packagekit over dbus. PolKit is used to grant Moonraker privileged access. If Moonraker is configured to use dbus for sysadmin the polkit rules must be applied, otherwise Moonraker will generate warnings. For this to work the user must be consistent and it must be defined in the rules (at least on Debian, other distros with newer versions of polkit may not be subject to this limitation).

The legacy implementation uses sudo commands and requires that the OS grant the Moonraker user (typically "Pi") "no password" sudo access.

vkhodygo commented 2 years ago

Hi @Arksine,

I did some research and it is possible to control services started with systemctl --user start without elevated privileges.

$ systemctl --user status klipper@drone.service

ā—‹ klipper@drone.service - 3D printer firmware with motion planning for the user drone:drone at hatchery
     Loaded: loaded (/usr/lib/systemd/user/klipper@.service; enabled; vendor preset: enabled)
     Active: inactive (dead) since Sat 2022-04-23 16:47:32 BST; 16s ago
    Process: 668 ExecStart=/usr/bin/python /usr/lib/klipper/klippy/klippy.py /home/drone/.config/klipper/printer.cfg -I /home/drone/.config/klipper/sock -a /home/drone/.config/klipper/ud_sock -l /home/drone/.config/klipper/klipper.log (code=killed, signal=TERM)
   Main PID: 668 (code=killed, signal=TERM)
        CPU: 684ms

Apr 23 15:49:15 hatchery systemd[548]: Started 3D printer firmware with motion planning for the user drone:drone at hatchery.
Apr 23 16:47:32 hatchery systemd[548]: Stopping 3D printer firmware with motion planning for the user drone:drone at hatchery...
Apr 23 16:47:32 hatchery systemd[548]: Stopped 3D printer firmware with motion planning for the user drone:drone at hatchery.
$ python
>>> from dbus.bus import BusConnection
>>> import dbus

>>> bus1 = BusConnection("unix:path=/run/user/1000/bus")
>>> systemd1 = bus1.get_object('org.freedesktop.systemd1', '/org/freedesktop/systemd1')

>>> manager = dbus.Interface(systemd1, 'org.freedesktop.systemd1.Manager')
>>> job = manager.RestartUnit('klipper@drone.service', 'fail')
$ systemctl --user status klipper@drone.service

ā— klipper@drone.service - 3D printer firmware with motion planning for the user drone:drone at hatchery
     Loaded: loaded (/usr/lib/systemd/user/klipper@.service; enabled; vendor preset: enabled)
     Active: active (running) since Sat 2022-04-23 16:52:05 BST; 5s ago
   Main PID: 6043 (python)
      Tasks: 2 (limit: 9376)
     Memory: 19.9M
        CPU: 417ms
     CGroup: /user.slice/user-1000.slice/user@1000.service/app.slice/app-klipper.slice/klipper@drone.service
             ā””ā”€6043 /usr/bin/python /usr/lib/klipper/klippy/klippy.py /home/drone/.config/klipper/printer.cfg -I /home/drone/.config/klipper/sock -a /home/drone/.config/klipper/ud_sock -l /home/drone/.config/klipper/klipper.log

Apr 23 16:52:05 hatchery systemd[548]: Started 3D printer firmware with motion planning for the user drone:drone at hatchery.

This should resolve the issue of service control. The only question is where to keep this .service file.

I still do not understand why you need to implement package update here though. Automatic Linux update is and was never recommended, since there might be changes requiring your approval. In addition, giving an app rights to control system packages doesn't look secure to me.

My position is that this is redundant and should be removed. We have package managers exactly for that. It would also make the process of distribution and code maintenance much easier.

Arksine commented 2 years ago

@vkhodygo I responded to your feedback in #422, as I believe that is the proper venue for this discussion rather than this issue.