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.

Sign up for GitHub

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

Jump to bottom

R4.1: qrexec protocol changes #4909

Closed
3 tasks
marmarek opened this issue Mar 25, 2019 · 3 comments · Fixed by QubesOS/qubes-core-qrexec#5
Closed
3 tasks

R4.1: qrexec protocol changes #4909

marmarek opened this issue Mar 25, 2019 · 3 comments · Fixed by QubesOS/qubes-core-qrexec#5
Assignees
@marmarek
Labels
C: core P: major Priority: major. Between "default" and "critical" in severity. release notes This issue should be mentioned in the release notes. T: enhancement Type: enhancement. A new feature that does not yet exist or improvement of existing functionality.
Milestone
Release 4.1

Comments

@marmarek
Copy link
Member

marmarek commented Mar 25, 2019 •
edited
Loading

Over the time, few qrexec protocol changes have accumulated. Lets implement them in R4.1, since qrexec will get some other related changes too (#4370)
Planned qrexec protocol changes for R4.1:

  • Improve handshake to allow backward compatibility, instead of only detecting incompatibility. Lets each side of connection advertise own max supported protocol version, then use min of those two numbers as the actual version in use. If either side does not support this version, drop the connection. This change should be also backported to R4.0 (and maybe even R3.2), to allow smooth upgrade.
  • Increase data chunk size. Even though ring buffer size may be much larger, data chunk is still at 4k, which affect performance.
  • Increase allowed length of requested qrexec service name - MSG_TRIGGER_SERVICE message. Preferably, at do not limit it at protocol level, use field length instead (similar as in MSG_EXEC_CMDLINE). For compatibility reasons, this should be a new message (available only in a new protocol version).
@marmarek marmarek added C: core P: major Priority: major. Between "default" and "critical" in severity. release notes This issue should be mentioned in the release notes. labels Mar 25, 2019
@marmarek marmarek added this to the Release 4.1 milestone Mar 25, 2019
@marmarek marmarek self-assigned this Mar 25, 2019
@andrewdavidwong andrewdavidwong added the T: enhancement Type: enhancement. A new feature that does not yet exist or improvement of existing functionality. label Mar 25, 2019
@marmarek marmarek mentioned this issue Mar 30, 2019
Merged
marmarek added a commit to QubesOS/qubes-doc that referenced this issue Mar 31, 2019
@marmarek
qrexec: update protocol version negotiation for R4.1
e3d0ee0 
QubesOS/qubes-issues#4909
marmarek added a commit to marmarek/qubes-core-admin-linux that referenced this issue Apr 5, 2019
@marmarek
qrexec: add version negotiation
61ec339 
Use lower version from (local, remote).

QubesOS/qubes-issues#4909
marmarek added a commit to marmarek/qubes-core-qrexec that referenced this issue Apr 5, 2019
@marmarek
qrexec: add version negotiation
1c0b958 
Use lower version from (local, remote).

QubesOS/qubes-issues#4909
marmarek added a commit to marmarek/qubes-core-agent-linux that referenced this issue Apr 8, 2019
@marmarek
qrexec: add version negotiation
1aeec02 
Use lower version from (local, remote).

QubesOS/qubes-issues#4909
@qubesos-bot qubesos-bot mentioned this issue Apr 13, 2019
Closed
marmarek added a commit to QubesOS/qubes-core-qrexec that referenced this issue Apr 13, 2019
@marmarek
qrexec: add version negotiation
8e653f9 
Use lower version from (local, remote).

QubesOS/qubes-issues#4909
marmarek added a commit to marmarek/qubes-core-qrexec that referenced this issue Apr 13, 2019
@marmarek
qrexec: add version negotiation
de34d98 
Use lower version from (local, remote).

QubesOS/qubes-issues#4909
marmarek added a commit to marmarek/qubes-core-qrexec that referenced this issue Apr 22, 2019
@marmarek
daemon: save negotiated protocol version
ccdb7cd 
- allow agent to be older than daemon
- save negotiated version for further use

QubesOS/qubes-issues#4909
marmarek added a commit to marmarek/qubes-core-qrexec that referenced this issue Apr 22, 2019
@marmarek
agent,daemon: increase data chunk size to 64k in protocol 3
7ad7b9f 
Vchan can hold (in current configuration) up to 64k of data in one go.
Do not force context switches more often than it's necessary - increase
max chunk size at qrexec protocol level too.
This should be safe against deadlocks, as qrexec agent/client will
adjust actual data chunk to available vchan buffer size anyway.

This change require protocol version bump (coming in subsequent commit),
as remote side validate the size. And since some VMs may be updated
later, support old value too - based on negotiated protocol version.

Make change in qrexec.h the way to produce compile error rather than
unexpected (potentially unsafe result): do not increase existing
MAX_DATA_CHUNK define, instead remove it and add MAX_DATA_CHUNK_V2 and
MAX_DATA_CHUNK_V3.

QubesOS/qubes-issues#4909
marmarek added a commit to marmarek/qubes-core-qrexec that referenced this issue Apr 22, 2019
@marmarek
Allow longer qrexec service names and arguments
5ee2af1 
Model new MSG_TRIGGER_SERVICE (vm->dom0) on MSG_EXEC_CMDLINE - put
service name at the end and make it arbitrary length, with length
calculated from header's 'len' field.
This also require adjusting qrexec-client-vm<->qrexec-agent protocol,
which means qrexec-agent needs to be restarted on update, before that
all calls will fail.

Make it new protocol message (MSG_TRIGGER_SERVICE3) and preserve the old
one for backward compatibility.

QubesOS/qubes-issues#4909
marmarek added a commit to marmarek/qubes-core-qrexec that referenced this issue Apr 22, 2019
@marmarek
Bump protocol version to 3
1c156a5 
QubesOS/qubes-issues#4909
@marmarek marmarek mentioned this issue Apr 22, 2019
Merged
marmarek added a commit to marmarek/qubes-core-qrexec that referenced this issue Apr 23, 2019
@marmarek
daemon: save negotiated protocol version
ca5ca6c 
- allow agent to be older than daemon
- save negotiated version for further use

QubesOS/qubes-issues#4909
marmarek added a commit to marmarek/qubes-core-qrexec that referenced this issue Apr 23, 2019
@marmarek
agent,daemon: increase data chunk size to 64k in protocol 3
5f134ad 
Vchan can hold (in current configuration) up to 64k of data in one go.
Do not force context switches more often than it's necessary - increase
max chunk size at qrexec protocol level too.
This should be safe against deadlocks, as qrexec agent/client will
adjust actual data chunk to available vchan buffer size anyway.

This change require protocol version bump (coming in subsequent commit),
as remote side validate the size. And since some VMs may be updated
later, support old value too - based on negotiated protocol version.

Make change in qrexec.h the way to produce compile error rather than
unexpected (potentially unsafe result): do not increase existing
MAX_DATA_CHUNK define, instead remove it and add MAX_DATA_CHUNK_V2 and
MAX_DATA_CHUNK_V3.

QubesOS/qubes-issues#4909
marmarek added a commit to marmarek/qubes-core-qrexec that referenced this issue Apr 23, 2019
@marmarek
Allow longer qrexec service names and arguments
5f35f5d 
Model new MSG_TRIGGER_SERVICE (vm->dom0) on MSG_EXEC_CMDLINE - put
service name at the end and make it arbitrary length, with length
calculated from header's 'len' field.
This also require adjusting qrexec-client-vm<->qrexec-agent protocol,
which means qrexec-agent needs to be restarted on update, before that
all calls will fail.

Make it new protocol message (MSG_TRIGGER_SERVICE3) and preserve the old
one for backward compatibility.

QubesOS/qubes-issues#4909
marmarek added a commit to marmarek/qubes-core-qrexec that referenced this issue Apr 23, 2019
@marmarek
Bump protocol version to 3
141f02c 
QubesOS/qubes-issues#4909
marmarek added a commit to marmarek/qubes-core-qrexec that referenced this issue Apr 23, 2019
@marmarek
Allow longer qrexec service names and arguments
b8869c2 
Model new MSG_TRIGGER_SERVICE (vm->dom0) on MSG_EXEC_CMDLINE - put
service name at the end and make it arbitrary length, with length
calculated from header's 'len' field.
This also require adjusting qrexec-client-vm<->qrexec-agent protocol,
which means qrexec-agent needs to be restarted on update, before that
all calls will fail.

Make it new protocol message (MSG_TRIGGER_SERVICE3) and preserve the old
one for backward compatibility.

QubesOS/qubes-issues#4909
marmarek added a commit to marmarek/qubes-core-qrexec that referenced this issue Apr 23, 2019
@marmarek
Bump protocol version to 3
d555fba 
QubesOS/qubes-issues#4909
marmarek added a commit to marmarek/qubes-core-qrexec that referenced this issue May 18, 2019
@marmarek
daemon: save negotiated protocol version
e124178 
- allow agent to be older than daemon
- save negotiated version for further use

QubesOS/qubes-issues#4909
marmarek added a commit to marmarek/qubes-core-qrexec that referenced this issue May 18, 2019
@marmarek
Add protocol version enum, with protocol changelog
19b0142 
QubesOS/qubes-issues#4909
marmarek added a commit to marmarek/qubes-core-qrexec that referenced this issue May 18, 2019
@marmarek
agent,daemon: increase data chunk size to 64k in protocol 3
5bc2ba6 
Vchan can hold (in current configuration) up to 64k of data in one go.
Do not force context switches more often than it's necessary - increase
max chunk size at qrexec protocol level too.
This should be safe against deadlocks, as qrexec agent/client will
adjust actual data chunk to available vchan buffer size anyway.

This change require protocol version bump (coming in subsequent commit),
as remote side validate the size. And since some VMs may be updated
later, support old value too - based on negotiated protocol version.

Make change in qrexec.h the way to produce compile error rather than
unexpected (potentially unsafe result): do not increase existing
MAX_DATA_CHUNK define, instead remove it and add MAX_DATA_CHUNK_V2 and
MAX_DATA_CHUNK_V3.

QubesOS/qubes-issues#4909
marmarek added a commit to marmarek/qubes-core-qrexec that referenced this issue May 18, 2019
@marmarek
Allow longer qrexec service names and arguments
30cca94 
Model new MSG_TRIGGER_SERVICE (vm->dom0) on MSG_EXEC_CMDLINE - put
service name at the end and make it arbitrary length, with length
calculated from header's 'len' field.
This also require adjusting qrexec-client-vm<->qrexec-agent protocol,
which means qrexec-agent needs to be restarted on update, before that
all calls will fail.

Make it new protocol message (MSG_TRIGGER_SERVICE3) and preserve the old
one for backward compatibility.

QubesOS/qubes-issues#4909
marmarek added a commit to marmarek/qubes-core-qrexec that referenced this issue May 18, 2019
@marmarek
Bump protocol version to 3
f7888b2 
QubesOS/qubes-issues#4909
marmarek added a commit to marmarek/qubes-core-qrexec that referenced this issue Jun 3, 2019
@marmarek
agent,daemon: increase data chunk size to 64k in protocol 3
cc4ddf5 
Vchan can hold (in current configuration) up to 64k of data in one go.
Do not force context switches more often than it's necessary - increase
max chunk size at qrexec protocol level too.
This should be safe against deadlocks, as qrexec agent/client will
adjust actual data chunk to available vchan buffer size anyway.

This change require protocol version bump (coming in subsequent commit),
as remote side validate the size. And since some VMs may be updated
later, support old value too - based on negotiated protocol version.

Make change in qrexec.h the way to produce compile error rather than
unexpected (potentially unsafe result): do not increase existing
MAX_DATA_CHUNK define, instead remove it and add MAX_DATA_CHUNK_V2 and
MAX_DATA_CHUNK_V3.

QubesOS/qubes-issues#4909
marmarek added a commit to marmarek/qubes-core-qrexec that referenced this issue Jun 3, 2019
@marmarek
Allow longer qrexec service names and arguments
93960fb 
Model new MSG_TRIGGER_SERVICE (vm->dom0) on MSG_EXEC_CMDLINE - put
service name at the end and make it arbitrary length, with length
calculated from header's 'len' field.
This also require adjusting qrexec-client-vm<->qrexec-agent protocol,
which means qrexec-agent needs to be restarted on update, before that
all calls will fail.

Make it new protocol message (MSG_TRIGGER_SERVICE3) and preserve the old
one for backward compatibility.

QubesOS/qubes-issues#4909
marmarek added a commit to marmarek/qubes-core-qrexec that referenced this issue Jun 3, 2019
@marmarek
Bump protocol version to 3
5ae77f4 
QubesOS/qubes-issues#4909
This was referenced Jun 8, 2019
Closed
Closed
@reynir reynir mentioned this issue Aug 20, 2019
Closed
@brendanhoar
Copy link

@marmarek

Increase data chunk size. Even though ring buffer size may be much larger, data chunk is still at 4k, which affect performance.

FYI, the above in the OP is a broken link.

Thanks,
Brendan

@marmarek
Copy link
Member Author

Thanks, updated the links to point at the old (R4.0) version.

@brendanhoar
Copy link

copy_file still using 4KB buffer?

https://github.com/QubesOS/qubes-linux-utils/blob/master/qrexec-lib/copy-file.c

char buf[4096];

@hannesm hannesm mentioned this issue Nov 13, 2020
Closed
marmarek added a commit to marmarek/qubes-core-agent-windows that referenced this issue Jan 27, 2024
@marmarek
qrexec: try service name without argument for long arguments
0ee6688 
Do not fail immediately if service name with the argument is too long
for the path name, but try the name without the argument anyway.

Part of QubesOS/qubes-issues#4909
marmarek added a commit to marmarek/qubes-core-agent-windows that referenced this issue Jan 27, 2024
@marmarek
qrexec: try service name without argument for long arguments
66572b3 
Do not fail immediately if service name with the argument is too long
for the path name, but try the name without the argument anyway.

While at it, fix missing error handling for StringCchPrintf.

Part of QubesOS/qubes-issues#4909
marmarek added a commit to marmarek/qubes-core-agent-windows that referenced this issue Jan 31, 2024
@marmarek
qrexec: try service name without argument for long arguments
5be4dff 
Do not fail immediately if service name with the argument is too long
for the path name, but try the name without the argument anyway.

While at it, fix missing error handling for StringCchPrintf.

Part of QubesOS/qubes-issues#4909
marmarek added a commit to marmarek/qubes-core-agent-windows that referenced this issue Jan 31, 2024
@marmarek
qrexec: try service name without argument for long arguments
6c87b12 
Do not fail immediately if service name with the argument is too long
for the path name, but try the name without the argument anyway.

While at it, fix missing error handling for StringCchPrintf.

Part of QubesOS/qubes-issues#4909
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@marmarek marmarek

Labels
C: core P: major Priority: major. Between "default" and "critical" in severity. release notes This issue should be mentioned in the release notes. T: enhancement Type: enhancement. A new feature that does not yet exist or improvement of existing functionality.
Projects
None yet
Milestone
Release 4.1
Development

Successfully merging a pull request may close this issue.

Protocol changes for R4.1 marmarek/qubes-core-qrexec
3 participants
@marmarek @brendanhoar @andrewdavidwong