OpenVPN / openvpn

OpenVPN is an open source VPN daemon
http://openvpn.net
Other
10.75k stars 2.99k forks source link

Fall-through vs fast-fail plugin execution #587

Open mc1arke opened 1 month ago

mc1arke commented 1 month ago

When executing plugins, OpenVPN currently iterates over all plugins and invokes each configured plugin, regardless of the result of the previous plugin execution. This could be potentially useful if there was an indicator of current state being passed onto subsequent plugin executions, but there's nothing visible in plugin calls as to what has already been run and what the outcome of any previous call has been, so it's not clear on why there's isn't a fast-fail with an abort of the loop when error == true.

Using the following plugin configuration as a multi-plugin use-case, with LDAP authentication being performed on user connect, then a subsequent multi-factor authentication challenge using Duo, if the LDAP authentication was to fail for an incorrect password but valid username, the Duo plugin would still be invoked and try to perform an MFA challenge through the likes of a push notification to a user's device.

plugin /usr/local/lib/openvpn-auth-ldap.so /etc/openvpn/server/demo/auth/ldap.conf
plugin /opt/duo/duo_openvpn.so --config-file=/etc/openvpn/server/demo/auth/duo.conf

Even where the user was to accept the MFA push from Duo, OpenVPN would (correctly) reject the authentication attempt for the first plugin call having failed, so the Duo push would have been unnecessary, and could potentially lead to MFA-challenge fatigue with a remote attacker repeatedly attempting connections with a valid username, or just a forced lock-out from repeated unacknowledged MFA calls even where the LDAP verification failed.

In the above scenario there's value in having OpenVPN abort plugin execution on first failure, but are there any use-cases where this behaviour wouldn't be desired? Would changing the flow in this area necessitate a new configuration flag (e.g. plugin-outcome-handling with options of fast-fail or fall-through, defaulting to fall-through for backwards compatibility), or would breaking out of the loop as soon as the error flag is set without any other conditions be a valid thing to do?