-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
polkit: create hook to check for polkit permissions #20283
base: main
Are you sure you want to change the base?
Conversation
Further to the above, I'd also like to look into creating a hook to get really fine-grained permissions for the systemd services page too, so we can selectively check if a user is allowed to start/stop a service and enable/disable etc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @chriswiggins for taking a stab at this! This has been on our wishlists for quite some time (#11033).
Let's ponder the design a bit first.
<NetworkPage privileged={superuser.allowed} | ||
<NetworkPage privileged={superuser.allowed || polkitAllowed} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what I mean -- if you use superuser: try
, you don't have to explicitly check superuser
any more.
83d7421
to
dbab17c
Compare
Hey @martinpitt - I've made those changes you mentioned and am much happier with how it looks - thanks for the pointers! Let me know what you think, I'd be keen as part of this PR to add more polkit checks in around the place as there are more things we'd like to allow too |
Hey @martinpitt - more of an update as I have been trying to do the same thing in the systemd services page. Turns out we can't ask specifically if a user is allowed to say $ pkcheck --action-id org.freedesktop.systemd1.manage-units -p $$,0,1000 -d unit cockpit.service
Error checking for authorization org.freedesktop.systemd1.manage-units: GDBus.Error:org.freedesktop.PolicyKit1.Error.NotAuthorized: Only trusted callers (e.g. uid 0 or an action owner) can use CheckAuthorization() and pass details This is not due to anything other than PolicyKit requiring the user who is running the process to be the one that performs the check (as far as I can tell, that pkcheck succeeds as root) - unfortunately the PolicyKit docs are sparse 😅 This works on the NetworkManager page I think because we aren't asking for further detail On Yocto cockpit is running as root, but Ubuntu I see it running as a |
@chriswiggins sorry for the late reply! I've been swamped last week with other stuff. I suppose it makes sense to restrict the CheckAuthorization(), to avoid allowing unpriv users to brute-force through all possible actions/combinations? Kind of like normal users can't read /etc/sudoers? But anyway, that's what we have to live with then. I just tried this on my Fedora system, and it behaves the same way:
and indeed I just saw that in Cockpit's firewall page we do this wrong:
I think we need to take a look at this again -- if it merely requires user interaction (authentication as that user) without having that privilege, or general sudo privs, it's fine. If it fails wiht one of the above errors if you dont', then it's not worth doing this in the first place -- if we already can get root through sudo/pkexec, then we don't need the fine-grained permissions.
Sort of.. but irrelevant. You can't just ask for another process:
if that worked, it'd be rather pointless 😁 So what matters here is the process which does that Thanks for pointing this out! This requires some more research whether even the existing firewalld code makes actual sense? |
Yes a very tricky one indeed! What about we still add the ability to check for "high-level" polkit permissions for units (like I did in NetworkManager) and then the more fine-grained stuff can just fail if you don't have permission on that particular unit? I have an idea in my head - might draft some changes up tomorrow to show what I mean. Does Cockpit handle DBus auth requests like it handles sudo by popping a window up? |
Hey @martinpitt - looping back into this after a small hiatus! Would there be appetite for a very small helper utility that ran as root, which could run the pkcheck and then report back the result to the bridge? I'm thinking it is something that users could optionally include? What is the best way to work out a design for this? |
@chriswiggins Aside from suid helpers being a pain in the back to develop and maintain, I'm afraid I have to reject the mere idea. Tools like polkit or sudo are intentionally restrict what unprivileged users can find out about someone's privileges, as that would allow users to silently (no "not permitted" log entries) probe for possible attack vectors. If it's safe (and efficient) to enumerate all allowable PK actions for a user, this change should happen in polkit itself in my opinion. Adding such a suid root helper which is world-executable (so that you can call it in user sessions) circumvents these restrictions. Even if you route it through the web server and only make it executable by So I think the only possible way would be to put the UI into "fake admin" mode, expose all controls, and lots of them would fail with permission errors -- i.e. the user has to know what they are allowed to do. Not exactly a great experience, although it may help for some use cases. This is a hard problem, sorry 😢 |
I agree it's a hard problem! A fun one to dive into 😉 How about a dbus helper that we can call with something like I.e as an example: import subprocess
from pydbus import SystemBus
from gi.repository import GLib
class CockpitPermissionsHelper:
"""
D-Bus service to handle high-level permission checks.
"""
def CanPerformAction(self, action, user_pid):
# Map high-level action to specific polkit action
ACTION_MAP = {
"modify_network": [
"org.freedesktop.NetworkManager.reload",
"org.freedesktop.NetworkManager.checkpoint-rollback",
"org.freedesktop.NetworkManager.network-control",
"org.freedesktop.NetworkManager.settings.modify.global-dns",
"org.freedesktop.NetworkManager.settings.modify.hostname",
"org.freedesktop.NetworkManager.settings.modify.own",
"org.freedesktop.NetworkManager.settings.modify.system"
]
}
polkit_actions = ACTION_MAP.get(action)
if not polkit_actions:
return "Invalid action"
# Call pkcheck to check if the user is allowed to perform the action
allowed = False
for polkit_action in polkit_actions:
result = subprocess.run(['pkcheck', '--action-id', polkit_action, '--process', str(user_pid)], capture_output=True, text=True)
if result.returncode == 0:
allowed = True
break
return "yes" if allowed else "no"
def main():
# Connect to the system bus and export the service
bus = SystemBus()
bus.publish("org.cockpit.PermissionsHelper", CockpitPermissionsHelper())
# Run the main loop
loop = GLib.MainLoop()
loop.run()
if __name__ == "__main__":
main() |
@chriswiggins I feel like this quickly dives into a rabbit hole of an unmaintainable web of pk mappings, together with a combinatorial explosion of new tests, while it still undermines polkit's query restrictions 😢 |
This is a draft PR to understand whether or not something like this would be feasible to bring more polkit-based checks into Cockpit.
We have a use-case whereby we don't want to give a user sudo access, but we do want them to manage things such as the network interfaces.
This hook uses
pkcheck
to test permissions and return an array of booleans that the user can use to determine if permissions are allowed or not. This currently checks a wide-range of NetworkManager permissions as a catch-all however it can definitely be refined in the future, should the approach be acceptable.My thinking is that I'd like to move the
pkcheck
exec across to the polkit DBus interface, but baby steps first 😄Thanks for any feedback