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

Logcontrol re-integration #516

Merged
merged 2 commits into from
Oct 31, 2024
Merged

Logcontrol re-integration #516

merged 2 commits into from
Oct 31, 2024

Conversation

jrybar-rh
Copy link
Member

Summary

Re-commit of 79d84d0 with additional authorization check.

Detailed description and/or reproducer

Setting log level via dbus api is now restricted to root only and setting log target is disabled completely.
Fixes #506 and #507

bluca and others added 2 commits October 29, 2024 13:31
Unlike sd-bus in libsystemd, gdbus in glib does not automatically
restrict changing properties to the root user. Check the credential
of the caller manually so that changes are restricted as expected.
Also add more user-friendly error messages to other error conditions
for a better user experience.
@jrybar-rh jrybar-rh merged commit 5a4ba7d into main Oct 31, 2024
8 checks passed
}

g_variant_get (value, "&s", &level);
polkit_backend_authority_set_log_level (level);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think error handling is missing here in the sense that it accepts anything without returning errors. For example here's what systemd-resolved does:

# busctl set-property org.freedesktop.resolve1 /org/freedesktop/LogControl1 org.freedesktop.LogControl1 LogLevel "s" yolo
Failed to set property LogLevel on interface org.freedesktop.LogControl1: Invalid log level 'yolo'
# echo $?
1

Here's what polkit does:

# busctl set-property org.freedesktop.PolicyKit1 /org/freedesktop/LogControl1 org.freedesktop.LogControl1 LogLevel "s" yolo
# echo $?
0

Copy link

@evverx evverx Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jrybar-rh since it's still half-implemented should a new issue be opened to at least document how it interacts with --no-debug or is it going to be added to #425 (or any other PR) later?

Edit: I opened #518 so as not to forget about it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or send pull request.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. I'll collect all the shortcomings and try to document them (though I'm not a native English speaker so I'm not the best person to write the documentation)

@jrybar-rh jrybar-rh deleted the logcontrol-integration branch November 20, 2024 09:23
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.

polkit crashes when the LogTarget D-Bus property is set
3 participants