-
Notifications
You must be signed in to change notification settings - Fork 28
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
Rip out qubes-rpc-multiplexer #141
base: main
Are you sure you want to change the base?
Conversation
189d832
to
b4a2de9
Compare
3de2f4b
to
39f909f
Compare
39f909f
to
2291280
Compare
9f69888
to
a4de39f
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #141 +/- ##
==========================================
- Coverage 79.17% 79.13% -0.04%
==========================================
Files 54 55 +1
Lines 9953 10060 +107
==========================================
+ Hits 7880 7961 +81
- Misses 2073 2099 +26 ☔ View full report in Codecov by Sentry. |
b7502bc
to
b4d7ed6
Compare
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025010404-4.3&flavor=pull-requests Test run included the following:
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2024111705-4.3&flavor=update
Failed tests22 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/119126#dependencies 3 fixed
Unstable tests
|
Ugh, upgrade will require a bit more care. Right now, just after upgrading dom0, qrexec calls (to dom0) fails for currently running VMs:
Release upgrade script will need to handle this case. |
Maybe keep the script in repo, and remove only later (R4.4?) ? |
This still fails:
I haven't checked yet if it's update problem (I did merged #184 already, so it should work...), or wrong file context in policy in the first place. |
Manual |
This checks that the RPC multiplexer only allows calls with 2 or 4 arguments and sets environment variables $QREXEC_REQUESTED_TARGET_TYPE, $QREXEC_REQUESTED_TARGET, and $QREXEC_REQUESTED_TARGET_KEYWORD properly. Since it only affects the tests, it can safely be backported to R4.2.
No functional change.
b4164f1
to
e3996bf
Compare
if VERSION == "dom0" and dest == "dom0": | ||
# Invoke qubes-rpc-multiplexer directly. This will work for non-socket | ||
# services only. | ||
return [RPC_MULTIPLEXER, rpcname, "dom0"] | ||
|
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.
Well, this needs a replacement, otherwise calling dom0->dom0 services fail. For example policy editor crashes with:
subprocess.CalledProcessError: Command '['/usr/bin/qrexec-client', '-d', 'dom0', 'DEFAULT:QUBESRPC policy.List dom0']' returned non-zero exit status 1.
Several of the failures suggest there is something wrong with the environment, at least PATH (see resize-rootfs failing by not finding But there are also cases where I don't see what went wrong (which may mean missing error log somewhere):
nothing on stderr from this command and all the logs from sys-net at the time:
|
And just to not forget - this is still applicable |
Instead, directly execute the command from C. All variables with names beginning with QREXEC are stripped from the environment, except for QREXEC_SERVICE_PATH. This is a change in behavior compared to the current code. This is a backwards-incompatible change to exec_qubes_rpc_if_requested(), which now takes an extra argument. Therefore, it cannot be backported to R4.2. It also requires changing the SELinux policy so that the labels on /etc/qubes-rpc/ and /usr/local/etc/qubes-rpc/ (and their contents) are correct. Fixes: QubesOS/qubes-issues#9062
It carries no information, and various parts of the code must strip it. Just omit it from the command entirely. Whether a command is an RPC command should be determined by the service descriptor being non-NULL. Review with "git diff --ignore-space-change".
The previous two changes were ABI breaks.
e3996bf
to
b9fc2b7
Compare
Better solution: run a script at each boot that checks if the SELinux policy has changed since a relabel happened and relabels the priviate volume if it has. That is QubesOS/qubes-core-agent-linux#541 (draft because not tested yet). |
Currently that PR checks if full rootfs relabel happened since last /rw relabel, not if policy has changed. Most policy changes will not involve full relabel (rpm macros relabel only files affected by the update, using |
Indeed so, but it is easy to check if policy has changed by checking the timestamp on
What about using the |
That's why I said "similar", not "the same".
Maybe, but remember there are a bunch of corner cases, like |
Instead, directly execute the command from C.
All variables with names beginning with
QREXEC_
are stripped from the environment, except forQREXEC_SERVICE_PATH
. This is a change in behavior compared to the current code.This is a backwards-incompatible change to
exec_qubes_rpc_if_requested()
, which now takes an extra argument and is renamedexec_qubes_rpc()
. Therefore, it cannot be backported to R4.2.Fixes: QubesOS/qubes-issues#9062