Hi, On Thu, Mar 10, 2022 at 01:57:01PM +0000, Pete Nelson wrote: > One of the behaviors that brought this to light was a user who had an LDAP > (non-deferred) plugin followed by a Duo MFA (deferred) plugin. He noted > that, even if the LDAP call returned failure, the Duo plugin was still > called. That would generate a push notification to his phone even though > the authentication was already destined to fail due to LDAP. It looks like > this patch returns failure when the first plugin fails, so would resolve > that scenario by not calling the Duo plugin at all if LDAP returned failure.
Yes, your report triggered this work. I think I misunderstood the original scenario (I thought it involved two plugins both going "deferred"), but this patch introduces a "short circuit" approach to plugin authentication where anything that fails will immediately abort all other plugin calls. So, it will fix that scenario(*). Now, if you have two deferred plugins, OpenVPN wouldn't know that "the deferred LDAP will fail in 0.1s", so would still fire up the DUO plugin. I thought about sequentializing them (the moment the first plugin goes "deferred", wait for that plugin to finish, and only then call the next one) but have not found a good way to do that in our current code. Might be worth having another look. (*) for that scenario ("first plugin is direct, fails, then do not call second plugin at all") the possible fix is much easier, though - in plugin.c, plugin_call_ssl(), just leave the inner for() loop whenever "error" becomes true. So line 810 would change from for (i = 0; i < n; ++i) into for (i = 0; i < n && !error; ++i) ... the existing behaviour is "run all plugins, always, and report if any of them failed". That change would it make "abort on the first failure, do not run the rest". Which sounds like it makes sense for authentication related, but maybe not so much for housekeeping (client-connect etc.)... to be discussed further :-) > Is the source for the "auth-multi" plugin you tested with available > anywhere? I could not find it, and had wanted something like it for my own > testing. It is part of the patchset that David has prepared for the "two plugins going deferred is not well defined" problem. It will hit the list "real soon now", but I'll send it to you unicast. It comes with detailed instructions how to run multiple instances that have their own "log id" and defined direct/deferred success/failure behaviour. gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de
signature.asc
Description: PGP signature
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel