Support for source domain in RPC service#182
Conversation
| case $0 in (/*) cd "${0%/*}/";; (*/*) cd "./${0%/*}";; esac | ||
| if command -v dnf >/dev/null; then | ||
| sudo dnf install python3dist\({coverage,pytest,gbulb,pyinotify,pytest-asyncio}\) || : | ||
| sudo dnf install python3dist\({coverage,pytest,gbulb,pyinotify,pytest-asyncio}\) qubes-libvchan-xen-devel pam-devel || : |
There was a problem hiding this comment.
Keeping it here for now to not forget packages to install ..
fepitre
force-pushed
the
source-domain
branch
from
3de5847 to
2f0b1db
Compare
| } | ||
|
|
||
| if (info.version != QREXEC_PROTOCOL_VERSION) { | ||
| if (info.version < QREXEC_PROTOCOL_V3) { |
There was a problem hiding this comment.
Generally, it's not enough to just relax this check. You need to also save which version the daemon is using (V4 is just adding a message that isn't relevant for the qrexec-client, but still). And similarly change in handle_client_hello and also save which client is using what version.
Alternatively, you can keep using V3 for daemon<->client communication, since new message is not needed at this place (right?). This will be also easier to test. But relaxing the check is a good idea for the future anyway (but can be done in a separate PR if you prefer).
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025041703-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=2025031804-4.3&flavor=update
Failed tests14 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/132953#dependencies 12 fixed
Unstable testsDetailsPerformance TestsPerformance degradation:14 performance degradations
Remaining performance tests:58 tests
|
fepitre
force-pushed
the
source-domain
branch
3 times, most recently
from
edb805b to
809e50a
Compare
fepitre
force-pushed
the
source-domain
branch
2 times, most recently
from
d284843 to
d7869df
Compare
|
The update fails due to mismatch between qrexec-client and qrexec-daemon versions for already running VMs: See my earlier comment here about possible solution. |
I thought this were solved.. |
fepitre
force-pushed
the
source-domain
branch
2 times, most recently
from
d579e8d to
8d113b1
Compare
| self.send_message(MSG_HELLO, struct.pack("<L", QREXEC_PROTOCOL_VERSION)) | ||
| def handshake(self, protocol_version=None): | ||
| if not protocol_version: | ||
| protocol_version = QREXEC_CLIENT_PROTOCOL_VERSION |
There was a problem hiding this comment.
Wouldn't it make more sense to do the other way around? Default to version QREXEC_PROTOCOL_VERSION (set as 4?), only in cases where older version is required provide the parameter. I think you'll need to use the parameter in less places then.
And also, there is no need for =None and then the if - simply put default value in the function header.
There was a problem hiding this comment.
Yes multiple iterations and removing of code. I did it the way around.
|
Something went wrong with version negotiation between qrexec-daemon and qrexec-agent. With new daemon in dom0 and old agent in VM all is fine. But after updating agent in VM it fails: Hopefully #182 will fix it (and hopefully it won't result in merge conflict... |
| def test_handshake(self): | ||
| self.start_agent() | ||
|
|
||
| dom0 = self.connect_dom0() |
There was a problem hiding this comment.
This looks like unintentional removal
fepitre
force-pushed
the
source-domain
branch
2 times, most recently
from
366a0dc to
0364711
Compare
fepitre
changed the title
fepitre
force-pushed
the
source-domain
branch
2 times, most recently
from
bd0179f to
be3c98f
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #182 +/- ##
==========================================
- Coverage 78.78% 78.71% -0.08%
==========================================
Files 55 55
Lines 10160 10373 +213
==========================================
+ Hits 8005 8165 +160
- Misses 2155 2208 +53 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
fepitre
force-pushed
the
source-domain
branch
8 times, most recently
from
2e9bfd8 to
792c4c6
Compare
|
This is a good description of the whole process, but there should be also a section with just the information how transport rpc needs to be written. This needs to include specific information what such service gets and how (call arguments, env variables, qubesdb entries etc). And maybe a simple example with explanation of each step? |
Yes this is to be added. I can add either SSH example or a dummy version (understanding here on the same host) based on the core admin test? |
|
On Fri, Apr 18, 2025 at 12:46:18AM -0700, Frédéric Pierret wrote:
Yes this is to be added. I can add either SSH example or a dummy version (understanding here on the same host) based on the core admin test?
The SSH one is probably better, the dummy is confusing due to having the
same host playing two roles.
…--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
|
| # Forward the qrexec call via SSH | ||
| ssh "$remote_qube" qrexec-client-vm --source-qube="$QREXEC_REMOTE_DOMAIN" "$remote_qube" "$service" | ||
|
|
||
| Workflow Overview |
There was a problem hiding this comment.
IMO better to put description before the example. The description is (should be) what the service should do, and then example based on SSH. It should be possible to write also other services based on this description.
| Workflow Overview | ||
| """"""""""""""""" | ||
|
|
||
| 1. **Local-Qube to Local-Relay**: A Qubes RPC call is made from Local-Qube to Local-Relay with arguments of the form: |
There was a problem hiding this comment.
Add info that this is what the transport_rpc property is about.
|
|
||
| where `<service>` contains the original service with argument that has been processed by policies. | ||
|
|
||
| 2. **Destination lookup**: The RPC script extracts `Remote-Qube` and queries QubesDB on Local‑Relay to verify and translate it into the actual qube name in `Remote-QubesOS` (in this example it is assumed to be the same). |
There was a problem hiding this comment.
Include exact qubesdb path in this description.
| SSH Client Configuration Hint | ||
| +++++++++++++++++++++++++++++ | ||
|
|
||
| To ensure that any SSH connection to a given host actually lands on the corresponding `Remote‑Relay`, add an entry like the following to your ``/etc/ssh/ssh_config``: |
There was a problem hiding this comment.
Maybe better ~/.ssh/config? It will persist without extra bind-dirs then.
As for the hints - on the remote site /etc/ssh better be made persistent with bind-dirs (and the host key re-generated there), otherwise host key will be shared across all VMs based on the same template, so not a secret anymore...
There was a problem hiding this comment.
I know but for me it's up to the user to know that's configuring host key at system level (ofc we would document it in qubes-doc at the end). Also for home path, is RPC using user home?
There was a problem hiding this comment.
Services are called as the default user normally, so yes, it will be user.
There was a problem hiding this comment.
Actually, there may be a corner case when policy specify user=. Normally it would affect the final target, but here it would affect the transport rpc. I think that's undesired. IMO either using user= should be prohibited with RemoteVM, or maybe ignored (with a warning)?
There was a problem hiding this comment.
Yes let's ignore with a warning.
| fi | ||
|
|
||
| # Forward the qrexec call via SSH | ||
| ssh "$remote_qube" qrexec-client-vm --source-qube="$QREXEC_REMOTE_DOMAIN" "$remote_qube" "$service" |
There was a problem hiding this comment.
Theoretically, --source-qube="$QREXEC_REMOTE_DOMAIN" on remote side may need a similar lookup via qubesdb (on remote side) too. But IMO it's ok to simplify it here, just leave a comment that using QREXEC_REMOTE_DOMAIN directly here assumes the remote name matches the local one.
| HostName <Remote-Relay_ip> | ||
|
|
||
| Adjust the pattern, hostname, and jump host settings to match your environment. | ||
| This guarantees that SSH connections intended for a RemoteVM are transparently proxied through Remote‑Relay./ |
| Workflow Overview | ||
| ^^^^^^^^^^^^^^^^^ | ||
|
|
||
| 1. **Local-Qube to Local-Relay**: The `transport_rpc` property of `Remote‑Relay` is used to refer to RPC call to be made from Local-Qube to Local-Relay with arguments of the form: |
There was a problem hiding this comment.
of Remote-Relay? not Remote-Qube?
There was a problem hiding this comment.
Remote-Qube, yes. My bad, I start being fooled by all these prefix/suffix.
3 participants
QubesOS/qubes-issues/issues/9475
QubesOS/qubes-issues/issues/9015