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

Set correct ownership if qb_ipcs_connection_auth_set() has been used #381

Merged
merged 3 commits into from
Feb 12, 2020
Merged

Set correct ownership if qb_ipcs_connection_auth_set() has been used #381

merged 3 commits into from
Feb 12, 2020

Conversation

diabonas
Copy link
Contributor

@diabonas diabonas commented Feb 10, 2020

  • The temporary directory that is used since 6a4067c needs to be accessible by the group as well. This has been fixed on the version_1 branch (and in version 1.0.5) by 65d6fb3, but not on master. Cherry-pick this fix (as well as another related fix to make the patch apply cleanly) for master.
  • When qb_ipcs_connection_auth_set is used, the ownership of the temporary directory that is initially set in handle_new_connection must be updated as well, so do this at the beginning of qb_ipcs_shm_connect.

This pull request fixes USBGuard/usbguard#289: when running USBGuard without the CAP_DAC_OVERRIDE capability (as done by the provided system service or by running usbguard-daemon -C to drop unnecessary capabilities), the temporary directory cannot be accessed because its group ownership is set to the user of the IPC client despite USBGuard using qb_ipcs_connection_auth_set(conn, uid, 0, 0660) to set the gid to root. As a result, all IPC communication with non-root users is broken.

Fixes: #369

wferi and others added 3 commits February 10, 2020 10:57
And don't abort if we aren't permitted to chown() it.  The client might
still have the privileges to enter it.
When qb_ipcs_connection_auth_set() has been used, the ownership of the
temp directory initially set by handle_new_connection() must be updated
as well.
@knet-ci-bot
Copy link

Can one of the admins verify this patch?

@chrissie-c
Copy link
Contributor

Good catches. Thanks!

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