search

flatpak / xdg-dbus-proxy

GNU Lesser General Public License v2.1
57 stars 21 forks source link

Add support for pipelined SASL handshakes #26

Closed refi64 closed 1 month ago

refi64 commented 2 years ago

sd-bus sends the BEGIN command in at the same time as the AUTH command, assuming that the authentication will succeed. This ends up confusing xdg-dbus-proxy, as it assumes that BEGIN is not sent at the same time as any other messages. As a result, the server's authentication replies end up interpreted as standard D-Bus messages, failing parsing and resulting in the client being unable to connect.

This changes the code to keep track of the number of authentication commands pipelined with BEGIN, then counts the responses from the server to know when the authentication phase of the connection has actually completed.

Fixes #21 (finally!)

Signed-off-by: Ryan Gonzalez ryan.gonzalez@collabora.com

META NOTES: This PR is built on top of #20, which is required to be able to move through the pipelined messages in the first place.

doraskayo commented 2 years ago

I tested this MR and it seemed to work well. It allowed me to run an app that relied upon sd-bus for its DBus communication inside the Flatpak sandbox.

It seems like this is fixing a relatively basic functionality which many apps potentially require, but it hasn't gotten any attention yet. Getting this reviewed and merged should be made a priority in my opinion.

alexlarsson commented 2 years ago

This doesn't seem right to me.

During the AUTH_WAITING_FOR_BEGIN we pass through buffers from the client to the bus as is, but keep all the from-bus data around until we get the BEGIN. Then we count how many responses we got before the BEGIN, and let through that many lines of data in the WAITING_FOR_BACKLOG phase. However, we may already have sent some buffers during the first phase, so we will not get that many sent lines.

This really needs to correctly handle lines sent from the client during the WAITING_FOR_BEGIN phase too.

bilelmoussaoui commented 1 month ago

A variant of this has been merged in #54