From cbbdcd4f97bb19e5be6c11cf94397b38e869a0ee Mon Sep 17 00:00:00 2001 From: Gert Doering Date: Thu, 21 Jan 2021 14:39:29 +0100 Subject: [PATCH] Make OPENVPN_PLUGIN_ENABLE_PF failures FATAL Without this patch, if openpn is using a plugin that provides OPENVPN_PLUGIN_ENABLE_PF but then fails (returns OPENVPN_PLUGIN_FUNC_ERROR), OpenVPN will crash on a NULL pointer reference. The underlying cause is (likely) the refactoring work regarding CAS_SUCCEEDED etc., and that nobody adjusted the pf.c code accordingly (it tries to sent itself a SIGUSR1, which tries to tear down the client MI instance, but since it is not fully set up yet at this point, things explode). Full details on the call chain in Trac... Since we intend to remove pf in 2.6, but we still do not want OpenVPN to ever SIGSEGV, change the requirements for the plugins to "MUST SUCCEED", so if the plugin ENABLE_PF call fails, abort openvpn with a M_FATAL message. Trac: #1377 Signed-off-by: Gert Doering Acked-by: Arne Schwabe Message-Id: <20210121133929.20186-1-gert@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21464.html Signed-off-by: Gert Doering (cherry picked from commit 6a0c51baaa4d2b329183601ec35d3d16f127519e) --- src/openvpn/pf.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/openvpn/pf.c b/src/openvpn/pf.c index f9bbfb505..3f472ef44 100644 --- a/src/openvpn/pf.c +++ b/src/openvpn/pf.c @@ -639,8 +639,17 @@ pf_init_context(struct context *c) } if (!c->c2.pf.enabled) { - msg(M_WARN, "WARNING: failed to init PF plugin, rejecting client."); - register_signal(c, SIGUSR1, "plugin-pf-init-failed"); + /* At some point in openvpn history, this code just printed a + * warning and signalled itself (SIGUSR1, "plugin-pf-init-failed") + * to terminate the client instance. This got broken at one of + * the client auth state refactorings (leading to SIGSEGV crashes) + * and due to "pf will be removed anyway" reasons the easiest way + * to prevent crashes is to REQUIRE that plugins succeed - so if + * the plugin fails, we cleanly abort OpenVPN + * + * see also: https://community.openvpn.net/openvpn/ticket/1377 + */ + msg(M_FATAL, "FATAL: failed to init PF plugin, must succeed."); return; } } -- 2.47.2