On Fri, Oct 30, 2026 at 11:43:05AM +0100, Michal Prívozník via Devel wrote:
On 10/30/26 09:42, Erik Huelsmann wrote:
Hi Michal,
When I wrote Sys::Async::Virt 2 years ago, I extensively studied the SASL code in libvirt.
At the time, I thought I saw patters where the following sequence might be going on: INIT -> START -> STEP -> START
The second START would be issued after setting up the correct authentication data after an authentication had been challenged. Do you see the same pattern? If so, then the second START will already have the "started" boolean set to true, even when it hasn't. Using a state machine with a single "state" variable would prevent that.
Ah, if that's the case then my patch is incorrect, though the only thing needed is to drop some checks to allow START after STEP again. I have not seen that in my testing, though. And looking into remoteAuthSASL() I don't see a way that a client could call the sequence from your example. The only sequence possible is INIT -> START [-> STEP [-> STEP ...]].
That is certainly the only /intended/ sequence. If a "START"/"STEP" fails, then server should be dropping the client connection. There is no intended provision for retrying authentication a second time on the same connection. The client would be expected to open a new connection to retry auth. If we're not already, we should also be careful not to allow "INIT" again, if the client is in the middle of START/STEP and has not yet got to completion from the server's POV. A state machine like Erik suggests does sound somewhat appealing from this POV. With regards, Daniel -- |: https://berrange.com ~~ https://hachyderm.io/@berrange :| |: https://libvirt.org ~~ https://entangle-photo.org :| |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|