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

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to