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

Add more protocol support #46

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

DemiMarie
Copy link

Taken from my Fedora 39 system.

@DemiMarie DemiMarie force-pushed the more-protocols branch 3 times, most recently from b1c8aff to 5dce73a Compare July 12, 2024 19:36
@DemiMarie DemiMarie force-pushed the more-protocols branch 3 times, most recently from f77e676 to f90b56c Compare July 24, 2024 02:20
@DemiMarie DemiMarie force-pushed the more-protocols branch 3 times, most recently from f04b165 to 624c652 Compare August 4, 2024 04:42
@DemiMarie DemiMarie force-pushed the more-protocols branch 4 times, most recently from fefdf06 to 7501d55 Compare August 9, 2024 02:03
This will allow Proxy.t to appear in the connection record.
Wayland requires that badly behaved clients be disconnected with a fatal
protocol error.  Implement this.
These functions prevent the common mistake of sending an invalid error
code.
This logs all protocol errors sent at "warn" level.
A proxy may choose to then send the protocol error to its own client.
These will be used to proxy errors sent by the host compositor.
Bad data should cause protocol errors to be raised, not assertion
failures or Invalid_argument exceptions.
Wayland does not allow message lengths that exceed 4096, are less than
8, or are not a multiple of 4.  Currently, libwayland always enforces
that messages are at least 8 bytes and hangs the connection if messages
exceed the buffer size (4096 bytes by default on server, unlimited on
client).  Enforce the stricter rules that will (hopefully) be enforced
by future versions of libwayland.

See https://gitlab.freedesktop.org/wayland/wayland/-/merge_requests/416.
libwayland's API is incapable of handling them, and starting with
libwayland commit 6c4a695045155583a99f3fbce7bb745f79c2e726 they are
rejected entirely.  This commit will be included in libwayland 1.24 and
later.
Some Wayland protocol implementations enforce that messages must be as
small as possible, and libwayland will soon be one of these
implementations.  Therefore, correctly sizing serialized messages is
mandatory.  Unfortunately, the code did not count integer arguments
correctly: it allocated space for every argument, rather than only
integer arguments.  Therefore, messages with non-integer arguments were
over-allocated by 4 bytes for every non-integer arguemnt in the message.

To ensure that the test suite catches bugs like this, reject trailing
junk in ocaml-wayland too.  Historically, the check for trailing junk
was added to ocaml-wayland first.  When it was found to break the test
suite, the serialization bug was discovered and fixed.

See libwayland MR
https://gitlab.freedesktop.org/wayland/wayland/-/merge_requests/416.
ocaml-wayland can handle almost any Wayland message imaginable, so long
as message sizes do not exceed 2**16 - 1.  However, libwayland is much
more limited: it cannot handle messages with more than 20 arguments or
exceeding 4096 bytes.  To protect code using libwayland, ensure that
ocaml-wayland only generates messages that libwayland can handle.

In libwayland 1.23, messages exceeding 4096 bytes put servers into a
loop that consumes CPU.  libwayland 1.22 and before disconnect clients
with an unhelpful protocol error.  I have filed an MR [1] that fixes the
libwayland regression.  Messages with more than 20 arguments result in
errors from libwayland.

[1]: https://gitlab.freedesktop.org/wayland/wayland/-/merge_requests/409
This is a protocol requirement since Wayland commit
2bbd80c8df129ee6810f6ec6d8d379ed3190760b.
https://gitlab.freedesktop.org/wayland/wayland/-/merge_requests/422 adds
the same check to libwayland.
Taken from wayland-protocols commit
7d5a3a8b494ae44cd9651f9505e88a250082765e.
@DemiMarie DemiMarie force-pushed the more-protocols branch 3 times, most recently from 277fd70 to 0ad2436 Compare September 13, 2024 02:16
This mirrors the newly-added validation support in libwayland, but
unlike libwayland the validation happens automatically for every
message.  Message handlers do not need to validate enum values
themselves.
It should never return one that does not have a header.
This reminds server implementors that the data is in fact untrusted.
Qubes OS uses this extensively and it works very well.  This also
prevents the use of currying to pass unvalidated data directly from
clients to servers, which is convenient but bad for security.
It causes ocaml-wayland to generate wrong code.
This exposes various Wayland protocol constants to users, so that users
can avoid exceptions being thrown from functions that construct protocol
messages.
Events are trusted, and rejecting unknown event values means the
code won't be future proof against future compositor changes.
This is useful to prevent a client from being disconnected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant