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

Create a serialization format for RPC calls #7084

Open
DemiMarie opened this issue Nov 24, 2021 · 7 comments
Open

Create a serialization format for RPC calls #7084

DemiMarie opened this issue Nov 24, 2021 · 7 comments
Labels
C: core P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. T: enhancement Type: enhancement. A new feature that does not yet exist or improvement of existing functionality.

Comments

@DemiMarie
Copy link

How to file a helpful issue

Qubes OS release (if applicable)

N/A

Brief summary

Qubes OS is full of stringly-typed RPC calls, with input validation done using regular expressions in Python. A strongly-typed serialization format, such as Protocol Buffers or Cap’n Proto, would be much cleaner.

Unfortunately, even the Rust implementation of Cap’n Proto uses a large amount of unsafe code. Options include a custom serialization format or auditing an existing implementation.

@DemiMarie DemiMarie added T: task Type: task. An action item that is neither a bug nor an enhancement. P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. labels Nov 24, 2021
@kalkin
Copy link
Member

kalkin commented Nov 24, 2021

I'm quite a fan of using D-Bus. It allows easier integration of services with the Desktop Environment. Originally I developed the UI tools speaking for managing the VM's with D-Bus integration.

@andrewdavidwong andrewdavidwong added C: core T: enhancement Type: enhancement. A new feature that does not yet exist or improvement of existing functionality. and removed T: task Type: task. An action item that is neither a bug nor an enhancement. labels Nov 24, 2021
@andrewdavidwong andrewdavidwong added this to the Release TBD milestone Nov 24, 2021
@DemiMarie
Copy link
Author

I'm quite a fan of using D-Bus. It allows easier integration of services with the Desktop Environment. Originally I developed the UI tools speaking for managing the VM's with D-Bus integration.

D-Bus is FAR too complicated for this use purpose.

@HW42
Copy link

HW42 commented Nov 25, 2021

Qubes OS is full of stringly-typed RPC calls, with input validation done using regular expressions in Python. A strongly-typed serialization format, such as Protocol Buffers or Cap’n Proto, would be much cleaner.

FWIW, I like those very simple and inspectable formats, so I'm not a fan of things like protobuf. If going that way I would look at something like trunnel that keeps the format more or less as simple as possible.

Also keep in mind that even with such "strongly-typed" formats a lot of validation can't be expressed directly in their declarations but needs to be checked separately (restricted charset in a string, non-trivial integer value limits, etc.). So maybe your concern would be better addressed by more formally documenting the used formats in human language to have something to check the implementation against. And a central library for common checks might make things nicer.

Unfortunately, even the Rust implementation of Cap’n Proto uses a large amount of unsafe code. Options include a custom serialization format or auditing an existing implementation.

That might make things actually worse. The current Python code should be at least memory safe (Yeah, I know this unsafe code is probably high quality, and Python's regex code is probably also C).

@DemiMarie
Copy link
Author

Qubes OS is full of stringly-typed RPC calls, with input validation done using regular expressions in Python. A strongly-typed serialization format, such as Protocol Buffers or Cap’n Proto, would be much cleaner.

FWIW, I like those very simple and inspectable formats, so I'm not a fan of things like protobuf. If going that way I would look at something like trunnel that keeps the format more or less as simple as possible.

Trunnel looks like a very good choice, with the caveats that it only supports C and big-endian, while we want Python/Rust and native-endian.

Also keep in mind that even with such "strongly-typed" formats a lot of validation can't be expressed directly in their declarations but needs to be checked separately (restricted charset in a string, non-trivial integer value limits, etc.). So maybe your concern would be better addressed by more formally documenting the used formats in human language to have something to check the implementation against. And a central library for common checks might make things nicer.

That is definitely a good idea.

Unfortunately, even the Rust implementation of Cap’n Proto uses a large amount of unsafe code. Options include a custom serialization format or auditing an existing implementation.

That might make things actually worse. The current Python code should be at least memory safe (Yeah, I know this unsafe code is probably high quality, and Python's regex code is probably also C).

Indeed, which is why I wouldn’t be okay with using the Rust Cap’n Proto implementation as it stands today. Most of the unsafe code should be able to be eliminated at the cost of substantial refactoring.

@marmarek
Copy link
Member

more formally documenting the used formats in human language

The canonical place for this is https://www.qubes-os.org/doc/vm-interface/#qubes-rpc (and https://www.qubes-os.org/doc/admin-api/ for Admin API). Admittedly, not very formal.

and native-endian.

This is actually a rather important point. At some point in the future I'd like to support qrexec calls between physical machines, if they'll have different architecture, we'll get all kind of funny issues about bitness and endianess.

@DemiMarie
Copy link
Author

and native-endian.

This is actually a rather important point. At some point in the future I'd like to support qrexec calls between physical machines, if they'll have different architecture, we'll get all kind of funny issues about bitness and endianess.

Some architectures (PPC64) support both big- and little- endian VMs on the same hardware, surprisingly enough. That said, I am still in favor of a little-endian default.

@greglook
Copy link

Out of curiosity, would something like CBOR be applicable? One of the explicit goals is to support very small/simple codec implementations, which seems in line with the desire to avoid complex parsing logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: core P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. T: enhancement Type: enhancement. A new feature that does not yet exist or improvement of existing functionality.
Projects
None yet
Development

No branches or pull requests

6 participants