Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dbus_manager: system IPC over D-Bus #346

Merged
merged 23 commits into from
Jan 28, 2022
Merged

Conversation

Arksine
Copy link
Owner

@Arksine Arksine commented Jan 26, 2022

This adds D-Bus client support to Moonraker. Specifically, Moonraker uses this to communicate with 3 services:

  • systemd
  • systemd-logind
  • PackageKit

This brings several advantages:

  • Permissions are managed through PolicyKit. This means it is no longer necessary to run sudo_fix.sh, all we need to do is add the appropriate rules. This PR provides a new script, set-policykit-rules.sh, which adds the permissions Moonraker needs. This makes it possible for Moonraker to run privileged commands without going through distro specific hurdles to give the user "no password" sudo access. If Moonraker lacks any required PolicyKit permissions it will add a warning.
  • Launching subprocesses can be expensive. This isn't an issue for initializing state or issuing commands at request, however it can be a problem if we need to poll data. For example, Moonraker currently polls service state for detected services. With DBus this isn't necessary, Moonraker receives a signal when the service state changes.
  • With DBus available Moonraker can use PackageKit for system package updates. PackageKit has several backends so we aren't restricted to apt. That said, there may still be challenges for other distros. For example, Moonraker has no mechanism for installing or updating packages in Arch's AUR.

This series of commits adds initial support for the three DBus services above. CLI-based fallbacks may be configured for the machine and update_manager components, however going forward the default will be to use the DBus providers. The update_manager will gracefully fallback to the APT CLI provider if either dbus or PackageKit is not available.

If DBus or systemd isn't available the machine component will emit a warning and disable the service actions (effectively it falls back to the none provider type). This is likely only going to be an issue for containers, generally speaking both systemd and dbus will be available on modern linux distros.

I have done initial testing, however this is a significant change so I believe it is a good idea to give others an opportunity to test before merging. I also wanted to give client devs and helpers a heads up, as this change will likely lead to requests for help when users see warnings in their log requiring PolicyKit permissions.

Those who wish to test can begin by pulling this branch and installing the new dependencies:

~/moonraker-env/bin/pip install dbus-next
sudo apt install packagekit

You could also use install-moonraker.sh, however this will refresh the apt cache and update all system packages, so you wont be able to test applying updates via packagekit.

If you want to test PolicyKit warnings, proceed to restart moonraker, otherwise install the PolicyKit rules:

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

Since the dbus providers are now default no additional configuration is necessary to test them. However if you want to test the fallback implementations you could configure them like so.

# systemd cli (via systemctl)
[machine]
provider: systemd_cli

or

# No provider (service action APIs are disabled)
[machine]
provider: none

To test the apt cli fallback in the Update Manager:

[update_manager]
enable_packagekit: False

Rather than override the Process class instead create a custom
protocol that forwards data over callbacks.

Signed-off-by:  Eric Callahan <arksine.code@gmail.com>
Signed-off-by:  Eric Callahan <arksine.code@gmail.com>
This allows for users to execute a method directly rather than
create a command.  This is useful for fire once commands that
do not need to inspect the return code unless there is an error.

Signed-off-by:  Eric Callahan <arksine.code@gmail.com>
This allows other components to be register callbacks that will
be executed in the stat update timer. This is useful for methods
that wish to poll subprocess commands, as its desireable to
prevent multiple subprocesses from running simultaneously.

Signed-off-by: Eric Callahan <arksine.code@gmail.com>
Move all systemd cli calls to its own provider class, inherted from
a base provider class.  This is in preparation for multiple provider
implementations.

Signed-off-by:  Eric Callahan <arksine.code@gmail.com>
The DBus manager is a core component that connects to the
system bus, providing resources used to communicate with other
processes on the the system through the DBus interface.

Signed-off-by:  Eric Callahan <arksine.code@gmail.com>
Signed-off-by:  Eric Callahan <arksine.code@gmail.com>
Signed-off-by:  Eric Callahan <arksine.code@gmail.com>
Signed-off-by:  Eric Callahan <arksine.code@gmail.com>
This allows for the inclusion of 3rd party Python Source
within the Moonraker package.  Initially this includes
a python source file containing PackageKit enumerations.

Signed-off-by:  Eric Callahan <arksine.code@gmail.com>
This is added to distrubute the changes made to "enum-converter.py"
taken from the original PackageKit repo.

Signed-off-by:  Eric Callahan <arksine.code@gmail.com>
Signed-off-by:  Eric Callahan <arksine.code@gmail.com>
Signed-off-by:  Eric Callahan <arksine.code@gmail.com>
Signed-off-by:  Eric Callahan <arksine.code@gmail.com>
All requests to update, refresh, recover, or reinstall must acquire
the command lock. Given that the individual Deployment implementations
are not (and should not be) called from outside of a request the locks they
use to prevent unwanted re-entry are redundant, confusing, and could
potential result in a deadlock if used improperly.

Signed-off-by:  Eric Callahan <arksine.code@gmail.com>
When the "none"  provider is set service action APIs will be disabled
and return an error when called.  Service state tracking is also
disabled.

Signed-off-by:  Eric Callahan <arksine.code@gmail.com>
Signed-off-by:  Eric Callahan <arksine.code@gmail.com
Signed-off-by:  Eric Callahan <arksine.code@gmail.com>
If Klipper or its python executable is located at a custom path
this allows moonraker to instantiate its update on startup
rather than wait for Klipper to connect.  This also resolves an
issue where Klipper's update state is always refreshed on startup
when its located in a non-default path.

Signed-off-by:  Eric Callahan <arksine.code@gmail.com>
Shallow clones don't report the tag in git describe, so use
git rev-list to extract the tag and prepend it to the version
string.

Signed-off-by:  Eric Callahan <arksine.code@gmail.com>
Signed-off-by:  Eric Callahan <arksine.code@gmail.com>
Add the "moonraker-admin" supplementary group to the service unit
file.  Check if polkit rules are available after installation, if not
advise the user that they may wish to run set-polkit-rules.sh.

Signed-off-by:  Eric Callahan <arksine.code@gmail.com>
@Arksine Arksine force-pushed the dev-systemd-dbus-20220112 branch from 2b0fe5b to a6766bf Compare January 28, 2022 00:43
Check for the moonraker-admin Supplementary group
in the moonraker service file and add it if necessary.

For PolKit versions > 0.106 make sure that the process
is launched with the moonraker-admin group before
granting any polkit permissions.

Signed-off-by:  Eric Callahan <arksine.code@gmail.com>
@Arksine Arksine force-pushed the dev-systemd-dbus-20220112 branch from a6766bf to 7b112f1 Compare January 28, 2022 01:36
@Arksine
Copy link
Owner Author

Arksine commented Jan 28, 2022

As an update, I made a few changes to install-moonraker.sh and set-policykit-rules.sh. I'll outline what I did and afterward explain my reasoning.

In the install script I now add a moonraker-admin group the system. Then a SupplementaryGroups=moonraker-admin entry is added to the service file:

#Systemd service file for moonraker
[Unit]
Description=API Server for Klipper
Requires=network-online.target
After=network-online.target

[Install]
WantedBy=multi-user.target

[Service]
Type=simple
User=$USER
SupplementaryGroups=moonraker-admin
RemainAfterExit=yes
WorkingDirectory=${SRCDIR}
ExecStart=${LAUNCH_CMD} -c ${CONFIG_PATH} -l ${LOG_PATH}
Restart=always
RestartSec=10

In set-policykit-rules.sh I check the unit for for the Supplemenatary Group line and add it if necessary. This way users don't have to re-run install-moonraker -f -c <configpath> -l <logpath>. I also modified the rules a bit for policykit versions 0.106 and up.

So why did I do this? I'll begin by stating that these changes don't impact distro's running PolKit 0.105...which is pretty much all Debian distros and its derivatives today. This is a forward thinking change, if a Debian user doesn't have the moonraker-admin group or the above SupplementaryGroup in the service file everything will still run ok.

That said, for PolKit 0.106 and up (which hopefully Debian will adopt in the future) this change is intended to restrict PolKit rules to the Moonraker process. On PolKit 0.105, any DBus application launched by the "pi" user will be granted the same privileges Moonraker has. For example you can run systemctl restart klipper without sudo. This isn't much different from password-less sudo, and its still more restrictive since it only applies to specific rules. That said, it would be ideal to restrict these privileges to Moonraker, and with with polkit 0.106 we can do this with a rule like the following:

// Allow Moonraker User to manage systemd units, reboot and shutdown
// the system
polkit.addRule(function(action, subject) {
    if ((action.id == "org.freedesktop.systemd1.manage-units" ||
         action.id == "org.freedesktop.login1.power-off" ||
         action.id == "org.freedesktop.login1.power-off-multiple-sessions" ||
         action.id == "org.freedesktop.login1.reboot" ||
         action.id == "org.freedesktop.login1.reboot-multiple-sessions" ||
         action.id.startsWith("org.freedesktop.packagekit.")) &&
        subject.user == "pi") {
        // Only allow processes with the "moonraker-admin" supplementary group
        // access
        var regex = "^Groups:.+?\\s1001[\\s\\0]";
        var cmdpath = "/proc/" + subject.pid.toString() + "/status";
        try {
            polkit.spawn(["grep", "-Po", regex, cmdpath]);
            return polkit.Result.YES;
        } catch (error) {
            return polkit.Result.NOT_HANDLED;
        }
    }
});

The action ids are privileges we are granting to the "pi" user, however we go a step further by using grep to make sure that the moonraker-admin group, gid 1001 in this example, is in the process supplementary groups. We don't add the user "pi" to the moonraker-admin group because we only want the Moonraker process to have these privileges.

@dw-0
Copy link
Contributor

dw-0 commented Jan 28, 2022

On PolKit 0.105, any DBus application launched by the "pi" user will be granted the same privileges Moonraker has.

action.id.startsWith("org.freedesktop.packagekit.")) &&
        subject.user == "pi") {

What happens if the user isn't named "Pi"

@Arksine
Copy link
Owner Author

Arksine commented Jan 28, 2022

In the above example users other than pi wont be granted authorization. The rule will implicitly return "NOT_HANDLED" and it will be passed off to the next rule on the system, which is typically to reject the auth unless they are root (or run as sudo).

The actual user is generated by the script, as is the GID for the moonraker-admin group. So if a user named "foo" runs set-policykit-rules.sh then the line will look like:

action.id.startsWith("org.freedesktop.packagekit.")) &&
        subject.user == "foo") {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants