Page MenuHomeSolus
Create Task
Maniphest T10321

Meta-Task: Updating Polkit to v121 (request for testing)
Closed, ResolvedPublic
Actions

Description

What is happening:

We are switching Polkit from the polkit-noscript fork (based on Polkit 0.113) to upstream Polkit (v121 aka 0.121). Upstream Polkit needs a Javascript interpreter/runtime of which previously the only option was MozJS however v121 added the much lighter duktape as an option (which is what we will use). polkit-noscript was configured using keyrules files which are not supported by upstream Polkit thus we also need to ensure that every package that ships a keyrules file is updated to have equivalent .rules files.

Why:

  • polkit-noscript is unmaintained and rebasing the noscript patches upon a newer version of Polkit is likely to be a fair amount of work
  • Even once the patches are re-based polkit-noscript will continue to need maintenance in the future to keep it current with upstream polkit, or will need someone to attempt to upstream our patches
  • polkit-noscript contains several known bugs which will be resolved by switching to upstream polkit
  • polkit v0.113 is not tested against by software components. Typically they only test against upstream or 0.105 (Debian/Ubuntu). This is likely the cause of some of the known issues. Switching to upstream means we gain the benefit of our version of Polkit being tested against by those software components
  • A big reason that polkit-noscript existed in the first place was that MozJS is a very large package (670MB installed according to eopkg info) and that it does not have a stable API which meant that polkit had to constantly be updated to support newer versions of mozjs. Because GNOME also depends on mozjs it was likely that systems might require two separate versions of mozjs at once doubling the storage needs. Duktape is 286KB and follows semver for API changes.
  • Security issues in polkit can be resolved more quickly by just updating to the newly released upstream version of Polkit when CVEs are released. If need be we can also use patches more reliably because there's a greater chance they will apply cleanly.
  • Keyrules have no ecosystem support. Packages that ship rules files typically ship JS rules which previously would need be converted to the keyrules format manually. With upstream polkit we can just use the shipped .rules files as-is.

Changes:

  • Release notes for Polkit v121 are available at https://gitlab.freedesktop.org/polkit/polkit/-/blob/121/NEWS.md
  • All keyrules files were converted to equivalent .rules files.
  • Wheel to Sudo:
    • Many distributions use the wheel group for the admin group. Solus instead uses the sudo group for the same purpose. Many Solus packages however incorrectly still specified the wheel group in their rules file which meant that when a user in the sudo group attempted to perform the action allowed in the rules that said user would be prompted for their password (the intent behind the rules file would be that such an administrative user would be able to perform that action without further authenticating). All package-shipped rules files were checked for wheel/sudo and if the wheel group was specified the packages were modified so that sudo would be used instead.
  • All packages were modified so that the shipped rules file was named consistently according to the name of the actions shipped with that package. For instance gnome-control-center.rules was renamed to org.gnome.controlcenter.rules. Some packages like gvfs already shipped their rules file in this format.

Individual package changes (beyond those listed above):

  • corectrl
    • Package updated to v1.2.4
    • Previous keyrules file did not check for membership in the sudo group which meant that any user could start the corectrl helper. This however does not seem wise given what corectrl can do to a system so the check for the sudo group was added to the rules file.
  • blueman
    • Updated to v2.3.1
  • bolt
    • Updated to v0.9.2
  • libvirt
    • The old rule allowed any user in the libvirt group to connect to the libvirtd daemon (IE with virt-manager) without additional authentication. I added an additional check to allow users in the sudo group to connect as well which should make virt-manager a bit more useful out of the box

Testing:

Desktop Environments:

Each supported DE needs to be tested to ensure that the Polkit helper that that system uses still works appropriately

Individual Packages:

Each modified package additionally needs to be tested by someone familiar with that package to ensure that the package is still functional

  • blueman (tested by @Staudey)
  • bolt
  • budgie-control-center (tested by @Staudey)
  • corectrl (tested by @EbonJaeger)
  • duktape (IE test other revdeps of duktape)
  • flatpak (tested by @Staudey)
  • gnome-control-center (tested by @Staudey)
  • gvfs
  • libvirt (tested by @ReillyBrogan)
  • os-installer (??? might have to wait until we're testing new ISOs)
  • udisks

If there's anything else that should be tested please mention it in the comments.

How to switch:

Helper Script
Make sure you download the re-install helper script below and keep it on the system you are installing this on. You may need it if you need to restore the system.

How to switch back:

Helper Script

Related Objects

Event Timeline

ReillyBrogan created this task.Jul 21 2022, 5:54 PM
Herald added a project: Lacks Project. · View Herald TranscriptJul 21 2022, 5:54 PM
ReillyBrogan edited projects, added Software; removed Lacks Project.Jul 21 2022, 5:55 PM
ReillyBrogan mentioned this in D13356: polkit: Update to v121.
ReillyBrogan updated the task description. (Show Details)Jul 21 2022, 6:52 PM
ReillyBrogan added a comment.Jul 21 2022, 7:05 PM

Libvirt was tested by connecting virt-manager to it. I couldn't get virsh to work unauthenticated though, perhaps it's not polkit integrated?

EbonJaeger awarded a token.Jul 21 2022, 8:02 PM
EbonJaeger added a subscriber: EbonJaeger.
joebonrichie added a subscriber: joebonrichie.Jul 21 2022, 8:09 PM

Plasma okay - tested solus-sc and kate (editing /etc/ files) and tested wayland as well. All okay.

ReillyBrogan updated the task description. (Show Details)Jul 21 2022, 9:29 PM
In T10321#196935, @joebonrichie wrote:

kate (editing /etc/ files)

TIL that Kate can request root permissions through polkit

ReillyBrogan added a comment.EditedJul 21 2022, 9:39 PM

/etc/polkit/rules.d/kate.rules :

// This is probably not a good idea
polkit.addRule(function(action, subject) {
  if (action.id == "org.kde.ktexteditor.katetextbuffer.savefile" &&
      subject.isInGroup("sudo") &&
      subject.local == true &&
      subject.active == true) {
        return polkit.Result.AUTH_ADMIN_KEEP;
      }
});

Re-entering passwords is for squares.

ReillyBrogan renamed this task from Meta-Task: Updating Polkit to v121 to Meta-Task: Updating Polkit to v121 (request for testing).Jul 22 2022, 5:49 PM
ReillyBrogan added a comment.Jul 22 2022, 5:52 PM

If you are familiar with any of the individual packages listed please help test this. I just audited the rules files and made some modifications, most of those packages are not ones that I use or lack the hardware to test and it would be far better for someone familiar with that package to check that they still behave as expected.

EbonJaeger added a comment.Jul 22 2022, 6:38 PM

CoreCtrl continues to operate the same as before. Removing the rules for it makes you enter a password when starting it, putting it back skips the password. Looks good to me.

Staudey added a subscriber: Staudey.Jul 22 2022, 7:37 PM

flatpak, gnome-control-center, budgie-control-center and blueman work fine. Tested on a Budgie system.

Staudey updated the task description. (Show Details)Jul 22 2022, 7:38 PM
algent added a subscriber: algent.Jul 22 2022, 7:58 PM

No issues for me with flatpak budgie-control-center and corectrl.

ReillyBrogan added a comment.Jul 22 2022, 9:26 PM

@algent asked me some questions in IRC/Matrix and I figured my answers may be helpful to others trying to figure out how to test this (apologies for not being clear enough in the above post).

(Algent had a question about what a broken Polkit would look like)
"From the perspective of a user the result of a broken polkit would be that the polkit dialog for a given DE wouldn't appear or wouldn't cause the action to succeed when authentication succeeds.
Each DE uses a DE-specific Polkit authentication package which is why I wanted all of the DEs to be tested."

(on how to "test" that a given package is fine with this change)
"The other thing is that each modified package ships a .rules file which whitelists a set of actions that a given group (almost always sudo) can perform without re-authenticating. These actions typically correspond to something in the app, like budgie-control-center has (amongst several) the action org.freedesktop.hostname1.set-hostname which I assume means that budgie-control-center has somewhere in it that allows you to set the hostname. Typically this action requires admin escalation, however the rules file that the package ships allows any user in the sudo group to just change this without re-authentication. If any user not in the sudo group tries to perform this action they would be prompted for the password of an admin user (which Polkit is set to use the group sudo for)
That's the thing to keep an eye out for. Do the rules that are shipped with the package actually work in that sudo users (or whatever the rule mentions) are allowed to bypass re-authentication while non-sudo users are prompted to authenticate?
This is where the mentioned change from wheel to sudo comes into play. Previously many rules still referenced the wheel group which meant that when a user tried to do those actions they would be prompted for re-authentication, even though the original upstream intent was that admin users didn't need to re-authenticate in those cases. Presumably whoever created the keyrules file in the first place just converted the existing .rules file (shipped with the package) into the keyrules format without realizing that they should modify the group mentioned."

Staudey updated the task description. (Show Details)Jul 22 2022, 9:33 PM
kyrios123 updated the task description. (Show Details)Jul 23 2022, 11:58 AM
kyrios123 added a subscriber: kyrios123.
joebonrichie added a comment.Jul 24 2022, 10:11 AM

Once this is in unstable we'll need to cook up some testing ISOs. We'll also need to ensure updating from a fresh iso install to unstable with the new polkit is okay.

joebonrichie mentioned this in R2462:7cb4b521241f: polkit: Update to v121.Jul 25 2022, 9:38 AM
joebonrichie updated the task description. (Show Details)Jul 25 2022, 11:07 AM

os-installer tested via fresh plasma iso test

joebonrichie added a comment.Jul 25 2022, 11:18 AM

udisks okay (tested via dolphin)

Jul 25 11:57:38 b450-mortar udisksd[1000]: udisks daemon version 2.8.4 starting
Jul 25 11:57:38 b450-mortar udisksd[1000]: Acquired the name org.freedesktop.UDisks2 on the system message bus
Jul 25 12:17:10 b450-mortar udisksd[1000]: Mounted /dev/sdb1 at /run/media/ninya/ST31000524ASNTFS on behalf of uid 1000
Jul 25 12:17:10 b450-mortar ntfs-3g[3536]: Cmdline options: rw,nodev,nosuid,windows_names,uid=1000,gid=1000,uhelper=udisks2
joebonrichie updated the task description. (Show Details)Jul 25 2022, 11:19 AM
ReillyBrogan added a comment.Jul 29 2022, 7:13 PM

I'm going to close this. I think everything has been tested extensively at this point and the things that have not specifically been tested are fairly unlikely to specifically have issues that the other polkit-integrating apps didn't have.

ReillyBrogan closed this task as Resolved.Jul 29 2022, 7:13 PM
ReillyBrogan claimed this task.
Staudey added a comment.Jul 29 2022, 7:28 PM

Ha, was just looking at this task a few minutes ago and thought the same thing (with the additional thought of "the remaining tests will be performed by our users, if they want to or not" ?)

xulongwu4 added a subscriber: xulongwu4.Jul 30 2022, 3:45 PM
Copyright © 2015-2021 Solus Project. The Solus logo is Copyright © 2016-2021 Solus Project. All Rights Reserved.