gets confused by sd-bus clients pipelining the SASL handshake #21
Comments
|
Working on a fix for this, #20 was sort of related in that I was also debugging sd-bus not working, but it's technically a separate issue. |
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 flatpak#21 (finally!) Signed-off-by: Ryan Gonzalez <[email protected]>
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 flatpak#21 (finally!) Signed-off-by: Ryan Gonzalez <[email protected]>
* Microsoft InTune is a device management/compliance checker that some organizations use for accessing corporate resources. When using an InTune enrolled device, Edge talks to the identity service using dbus for seamless login to corporate resources, and for conditional access enforcement. In order to support this, the msalsdk-dbusclient shared library must be present. This commit adds this library and its dependency (sd-bus) to the flatpak * NOTE: This won't work until properly flatpak/xdg-dbus-proxy#21 is fixed
* Microsoft InTune is a device management/compliance checker that some organizations use for accessing corporate resources. When using an InTune enrolled device, Edge talks to the identity service using dbus for seamless login to corporate resources, and for conditional access enforcement. In order to support this, the msalsdk-dbusclient shared library must be present. This commit adds this library and its dependency (sd-bus) to the flatpak * NOTE: This won't work until properly flatpak/xdg-dbus-proxy#21 is fixed
* Microsoft InTune is a device management/compliance checker that some organizations use for accessing corporate resources. When using an InTune enrolled device, Edge talks to the identity service using dbus for seamless login to corporate resources, and for conditional access enforcement. In order to support this, the msalsdk-dbusclient shared library must be present. This commit adds this library and its dependency (sd-bus) to the flatpak * NOTE: This won't work until properly flatpak/xdg-dbus-proxy#21 is fixed
* Microsoft InTune is a device management/compliance checker that some organizations use for accessing corporate resources. When using an InTune enrolled device, Edge talks to the identity service using dbus for seamless login to corporate resources, and for conditional access enforcement. In order to support this, the msalsdk-dbusclient shared library must be present. This commit adds this library and its dependency (sd-bus) to the flatpak * NOTE: This won't work until properly flatpak/xdg-dbus-proxy#21 is fixed
* Microsoft InTune is a device management/compliance checker that some organizations use for accessing corporate resources. When using an InTune enrolled device, Edge talks to the identity service using dbus for seamless login to corporate resources, and for conditional access enforcement. In order to support this, the msalsdk-dbusclient shared library must be present. This commit adds this library and its dependency (sd-bus) to the flatpak * NOTE: This won't work until properly flatpak/xdg-dbus-proxy#21 is fixed
* Microsoft InTune is a device management/compliance checker that some organizations use for accessing corporate resources. When using an InTune enrolled device, Edge talks to the identity service using dbus for seamless login to corporate resources, and for conditional access enforcement. In order to support this, the msalsdk-dbusclient shared library must be present. This commit adds this library and its dependency (sd-bus) to the flatpak * NOTE: This won't work until properly flatpak/xdg-dbus-proxy#21 is fixed
* Microsoft InTune is a device management/compliance checker that some organizations use for accessing corporate resources. When using an InTune enrolled device, Edge talks to the identity service using dbus for seamless login to corporate resources, and for conditional access enforcement. In order to support this, the msalsdk-dbusclient shared library must be present. This commit adds this library and its dependency (sd-bus) to the flatpak * NOTE: This won't work until properly flatpak/xdg-dbus-proxy#21 is fixed
* Microsoft InTune is a device management/compliance checker that some organizations use for accessing corporate resources. When using an InTune enrolled device, Edge talks to the identity service using dbus for seamless login to corporate resources, and for conditional access enforcement. In order to support this, the msalsdk-dbusclient shared library must be present. This commit adds this library and its dependency (sd-bus) to the flatpak * NOTE: This won't work until properly flatpak/xdg-dbus-proxy#21 is fixed
Since sd-dbus clients send the BEGIN before receiving OK, ensure that we look for both to come across the bus in either order. Fixes flatpak#21
Since sd-dbus clients send the BEGIN before receiving OK, ensure that we look for both to come across the bus in either order. Fixes flatpak#21
Since sd-dbus clients send the BEGIN before receiving OK, ensure that we look for both to come across the bus in either order. Fixes flatpak#21
Since sd-dbus clients send the BEGIN before receiving OK, ensure that we look for both to come across the bus in either order. Fixes flatpak#21
* Microsoft InTune is a device management/compliance checker that some organizations use for accessing corporate resources. When using an InTune enrolled device, Edge talks to the identity service using dbus for seamless login to corporate resources, and for conditional access enforcement. In order to support this, the msalsdk-dbusclient shared library must be present. This commit adds this library and its dependency (sd-bus) to the flatpak * NOTE: This won't work until properly flatpak/xdg-dbus-proxy#21 is fixed
Since sd-dbus clients send the BEGIN before receiving OK, ensure that we look for both to come across the bus in either order. Fixes flatpak#21
* Microsoft InTune is a device management/compliance checker that some organizations use for accessing corporate resources. When using an InTune enrolled device, Edge talks to the identity service using dbus for seamless login to corporate resources, and for conditional access enforcement. In order to support this, the msalsdk-dbusclient shared library must be present. This commit adds this library and its dependency (sd-bus) to the flatpak * NOTE: This won't work until properly flatpak/xdg-dbus-proxy#21 is fixed
* Microsoft InTune is a device management/compliance checker that some organizations use for accessing corporate resources. When using an InTune enrolled device, Edge talks to the identity service using dbus for seamless login to corporate resources, and for conditional access enforcement. In order to support this, the msalsdk-dbusclient shared library must be present. This commit adds this library and its dependency (sd-bus) to the flatpak * NOTE: This won't work until properly flatpak/xdg-dbus-proxy#21 is fixed
needed because of flatpak/xdg-dbus-proxy#21
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 flatpak#21 (finally!) Signed-off-by: Ryan Gonzalez <[email protected]>
|
Just FYI, zbus started pipelining the handshake in its latest releases (4.1.2 and 4.2) and this means zbus flatpak apps breaking. Seems @mardy already provided a fix a month ago but it was never reviewed. Can a maintainer have a look and merge if it makes sense? |
xdg-dbus-proxy can't handle pipelining[1], hence we need to handle `NEGOTIATE_UNIX_FD` command's response before sending out `BEGIN` command and `Hello` method call message. We should remote this as soon as flatpak is fixed and fix is available in major distros. [1] flatpak/xdg-dbus-proxy#21
xdg-dbus-proxy can't handle pipelining[1], hence we need to handle `NEGOTIATE_UNIX_FD` command's response before sending out `BEGIN` command and `Hello` method call message. We should remote this as soon as flatpak is fixed and fix is available in major distros. [1] flatpak/xdg-dbus-proxy#21
|
Btw while I added a workaround for this in zbus 4.2.1, I wanted to point out that I wouldn't want to keep this workaround forever in zbus. I'm thinking ideally only for 6 months (but could be much longer if i don't need to touch the client-side handshake part much). Also sdbus remains broken (because of Lennart not being as cooperative as me 😂) even though that probably just means you can't use busctl, which may not be a biggie. If other libs would also start pipelining handshakes (maybe some already do?), we'll have an even bigger problem. |
If that's the case, then Flatpak apps that use zbus are unlikely to be runnable on older LTS distros (Debian, Ubuntu LTS, RHEL family, SLES family, etc.) because any fix for this that might get merged into xdg-dbus-proxy will only fix future x-d-p and Flatpak versions, and will not somehow retroactively fix the older versions that already exist in LTS distros. |
Why not? If an issue breaks Flatpak completely against at least (that we know of) 2 separate D-Bus client implementations, then IMO it should be considered critical and hence backported. The same goes for #46, which I don't even have any idea how to workaround. |
Actually I'm not sure there is even backporting needed. At least Debian stable seems to already have xdg-dbus-proxy 0.1.4, Update: I thought 0.1.4 is very recently because it appears at the top in the git log but I only realized after writing the comment that it's only because this project hasn't been super active. |
|
That is just not how supporting multiple platforms and stables releases works, @zeenix. Interoperability requires some restraint on the embracing every latest cool idea that Lennart comes up with. These roundtrips are saving you a few microseconds at best. Is that really worth breaking the majority of existing flatpak users? |
I don't think this is new in sdbus. I think sdbus has been doing this for years afaik. I could be wrong though but he's been telling me I should do that for a long while now. :)
slomo did some very basic benchmarking and he saw that my workaround for flatpak, adds 13% increase in time needed to establish a single connection. I wouldn't call a 13% difference insignificant. Also to keep in mind that zbus isn't completely pipelining the handshake (yet). |
sd-bus has been doing it since mid 2020 or so at least, judging by when this issue report was opened. |
13% of a few microseconds is very much insignificant, in particular for a thing that happens once in an application's run. |
It's not a few microseconds even on high end machines.
That's true for typical applications but not always. If I could assume that's always the case, I also won't see any value in this optimisation. |
Ummm. sd-bus has been pipelining the auth logic since day one, i.e. March 2013, it never did it differently. I think it did that even before this flatpak dbus proxy hack existed, no? It's like more than a decade. I mean, I have no beef here, I don't think the audience for flatpak and for sd-bus really overlap that much, but just wanted to correct this here. And also, it does make a major difference to pipeline this, when we switched from old libdbus over to sd-bus it did shove multiple 100ms off our boot times, since we had so many tools going through this that we synced on. If you have many short-lived connections it's the only thing that makes D-Bus manageable at all I think, the roundtrips traditional dbus libraries did just were prohibitively expensive. If you care about performance at all for IPC, then it really is the roundtrips that kill everything. The time you spend on marshalling really never matter, it all comes down to roundtrips, hence pipelining is really what you have to do. (i figure gnome mostly uses gio/gdbus as dbus implementation. it's probably the worst implementation performance-wise you can use, because it moves dbus communication to a thread, and thus adds another level of roundtrips to the whole thing, tanking latencies in a way that I guess auth pipelining just can't really recover from). |
On systemd/systemd#16610, @refi64 writes:
sd-bus does this pipelining deliberately to reduce round-trips, and @poettering considers it to be an xdg-dbus-proxy bug that it gets confused by pipelining the SASL handshake.
It is not at all clear to me that the D-Bus Specification ever intended to allow pipelining the SASL handshake, and its current wording can be interpreted as forbidding pipelining the SASL handshake, but it does work in practice against the major server implementations (the reference libdbus, GDBus and sd-bus). Ideally xdg-dbus-proxy should only process as much text as it is ready to deal with (in practice this means only the next line), and defer the rest until its next return to the main loop.
#20 might be related to this.
The text was updated successfully, but these errors were encountered: