rpm-ostree
rpm-ostree copied to clipboard
coreos
daemon: Unconditionally authorize uid 0 first
Today we only directly authorize uid 0 via bus credentials if we detect polkit is not available. However, it turns out there are at least two different errors that can occur if polkit isn't installed/running, and we were only checking one.
I looked at how systemd does this, and it unconditionally queries the credentials via dbus-{daemon,broker} first. Let's do the same.
Closes: https://github.com/coreos/rpm-ostree/issues/3554
This is a bigger change and would need careful review, perhaps we should ship https://github.com/coreos/rpm-ostree/pull/3556 now instead and do this later.
OK https://github.com/coreos/rpm-ostree/pull/3556 merged, let's ship that in the next release. I may try to (carefully) oxidize this stuff instead in the future rather than just shuffle C code around.
@cgwalters: PR needs rebase.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
![openshift-ci[bot] avatar](https://avatars.githubusercontent.com/in/91280?v=4 "Published by a pub.dev verified publisher"){kind=small}
@cgwalters: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:
| Test name | Commit | Details | Required | Rerun command |
|---|---|---|---|---|
| ci/prow/clang-analyzer | ff4697d3a136131017e84adce99389b134c9cfbd | link | true | /test clang-analyzer |
| ci/prow/images | ff4697d3a136131017e84adce99389b134c9cfbd | link | true | /test images |
| ci/prow/build-clang | ff4697d3a136131017e84adce99389b134c9cfbd | link | true | /test build-clang |
| ci/prow/fcos-e2e | ff4697d3a136131017e84adce99389b134c9cfbd | link | true | /test fcos-e2e |
Full PR test history. Your PR dashboard.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.
You have successfully added a new shellcheck configuration differential-shellcheck. As part of the setup process, we have scanned this repository and found no existing alerts. In the future, you will see all code scanning alerts on the repository Security tab.
![github-advanced-security[bot] avatar](https://avatars.githubusercontent.com/in/57789?v=4 "Published by a pub.dev verified publisher"){kind=small}
OK, I ended up reworking this more - there's more code motion, but we now have more common logic between the previously separate -os.cxx and -sysroot.cxx files. (e.g. the "am I on session bus" is shared directly too alongside checking uid0).
Also crucially, we now only call polkit_authority_get_sync if we didn't authorize via uid0 (or directly).
We may still eventually want the "retry loading polkit" logic from https://github.com/coreos/rpm-ostree/pull/4343 but it becomes less critical path. Now for example even with zincati we should allow having the daemon start up and allow e.g. RegisterClient to be invoked while polkit is still starting.
@cgwalters: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:
| Test name | Commit | Details | Required | Rerun command |
|---|---|---|---|---|
| ci/prow/clang-analyzer | ff4697d3a136131017e84adce99389b134c9cfbd | link | true | /test clang-analyzer |
| ci/prow/build-clang | ff4697d3a136131017e84adce99389b134c9cfbd | link | true | /test build-clang |
Full PR test history. Your PR dashboard.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.