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

polkit: create hook to check for polkit permissions #20283

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

chriswiggins
Copy link
Contributor

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

@chriswiggins
Copy link
Contributor Author

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

Copy link
Member

@martinpitt martinpitt left a 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.

pkg/lib/hooks.js Outdated Show resolved Hide resolved
pkg/lib/hooks.js Show resolved Hide resolved
Comment on lines 120 to 132
<NetworkPage privileged={superuser.allowed}
<NetworkPage privileged={superuser.allowed || polkitAllowed}
Copy link
Member

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.

pkg/lib/hooks.js Outdated Show resolved Hide resolved
@chriswiggins
Copy link
Contributor Author

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

@chriswiggins
Copy link
Contributor Author

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 org.freedesktop.systemd1.manage-units on a particular unit from a user session like so:

$ 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 cockpit-ws user. Is there anything typically running by cockpit that is running with uid 0? The reasoning behind this is to have a service that we can communicate with to do the policykit checks. Any pointers or hidden knowledge about how systemd/polkit works so we don't need to be doing the auth check as root?

@martinpitt
Copy link
Member

@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:

❱❱❱ 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

❱❱❱ pkcheck --action-id org.freedesktop.systemd1.manage-units -p $$
polkit\56retains_authorization_after_challenge=1
Authorization requires authentication and -u wasn't passed.

❱❱❱ pkcheck --action-id org.freedesktop.systemd1.manage-units -p $$ -u
polkit\56retains_authorization_after_challenge=1
Authorization requires authentication but no agent is available.

and indeed I just saw that in Cockpit's firewall page we do this wrong:

pkg/networkmanager/firewall-client.js:cockpit.spawn(['sh', '-c', 'pkcheck --action-id org.fedoraproject.FirewallD1.all --process $$ --allow-user-interaction 2>&1'], { superuser: "try" })

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.

Is there anything typically running by cockpit that is running with uid 0?

Sort of.. but irrelevant. You can't just ask for another process:

❱❱❱ pkcheck --action-id org.freedesktop.systemd1.manage-units -p 1
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() for subjects belonging to other identities

if that worked, it'd be rather pointless 😁 So what matters here is the process which does that pkcheck call, which is the bridge -- and that runs as the user. (And by definition there's no root bridge).

Thanks for pointing this out! This requires some more research whether even the existing firewalld code makes actual sense?

@chriswiggins
Copy link
Contributor Author

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?

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.

None yet

2 participants