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 :|
