Re: [Openvpn-devel] [PATCH 1/1] Exit management interface loop early on receiving 'remote MOD' message.
Hi, On Wed, Jul 03, 2019 at 03:50:41PM +0100, Daniel Kaldor wrote: > OpenVPN using management interface and running with > 'management-query-remote' in the config will wait for a 'remote MOD' > or 'remote ACCEPT' message before continuing with connection. > > Logs indicate that this stage of the connection process currently > takes ~1s to complete. > This seems to be because the management interface polling loop is > being run once unnecessarily after receiving 'remote MOD' message, and > is only exiting after 1s timeout is reached on final polling loop. > > This change exits the polling loop early, immediately after receiving > 'remote MOD' message and removes 1s delay in connection. This might actually have been fixed by commit d08b66f39cf80733a51e1607386dc4993faef09f Author: Vladislav Grishenko Date: Fri Oct 2 03:53:19 2020 +0500 Speedup TCP remote hosts connections For non-blocking TCP/Unix connection, OpenVPN checks was it established in loop and if not - sleeps or handles management for next one second. Since the first check is made right after the connection attempt, it will likely be always unsuccessful, causing redundant wait for one or more seconds: ... v3: teach management_sleep() to handle zero timeout and reject negative use 1s timeout for connection and 0s timeout for management events could you check if this solves the delays you are experiencing as well? If not, please re-send, taking Arne's comments into account. 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
[Openvpn-devel] [PATCH applied] Re: clean up / rewrite sample-plugins/defer/simple.c
Patch has been applied to the master, release/2.5 and release/2.4 branch. I have fixed the whitespace issue pointed out by Arne, and a few others that uncrustify wanted fixed (a few TABs have escaped, and one bracked was misaligned). commit 452e016cba977cb1c109e74977029b9c0de33de2 (master) commit 436a4cdd06ecdbab5ef73556431af776a984ad66 (release/2.5) commit 8007a8a347336fded2f71d84945bbf3e60e58a78 (release/2.4) Author: Gert Doering Date: Thu Jan 21 18:25:36 2021 +0100 clean up / rewrite sample-plugins/defer/simple.c Signed-off-by: Gert Doering Acked-by: Arne Schwabe Message-Id: <20210121172536.32500-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21466.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Make OPENVPN_PLUGIN_ENABLE_PF failures FATAL
Hi, (while technically in the wrong mail thread for the "should PF stay?" discussion, this is still interesting) On Fri, Jan 22, 2021 at 07:39:31AM +, tincanteksup wrote: > I agree that a VPN should focus on its task and not try to be a firewall. > > I do use the PF plugin but it is of little, if any, actual use, which is > not handled better elsewhere. Which PF plugin do you use? defer/simple? Or something else, which is not a Big Gaping Security Hole? > I do not pretend to understand the intricacies of the code but if > removing the packet filter plugin is relatively simple and clean then, > from a user point-of-view, it makes more sense to drop it. Less > complication overall. If you look into the code for places where you find ENABLE_PF or PLUGIN_PF, you can see that it really touches a LOT of places - and every single line of code increases the chance that it breaks on future changes, unless someone invests the time to write test rigs that test all these code paths (which gets increasingly complex with some features). Even testing all the "how is a packet forwarded or not?" paths might not have caught *this* problem, as it is basically the "I have enabled PF but the PF initialization failed" corner case which is often overlooked when building tests for "I have enabled PF and want to make sure PF works!" case... So, yes, ripping this out would make the code much simpler in some critical paths. OTOH pf can do nice things you can't easily do with a linux firewall, like "accept packets from this *other* client only, identified by common_name" (without having to know the actual IP address and subnets assigned to it). This is nice. But if it is not used, it's more "theoretically nice" and still can get kicked... 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
[Openvpn-devel] [PATCH applied] Re: Make OPENVPN_PLUGIN_ENABLE_PF failures FATAL
Patch has been applied to the master and release/2.5 branch. release/2.4 has different code and does not fail, so does not need the patch. commit 6a0c51baaa4d2b329183601ec35d3d16f127519e (master) commit cbbdcd4f97bb19e5be6c11cf94397b38e869a0ee (release/2.5) Author: Gert Doering Date: Thu Jan 21 14:39:29 2021 +0100 Make OPENVPN_PLUGIN_ENABLE_PF failures FATAL Signed-off-by: Gert Doering Acked-by: Arne Schwabe Message-Id: <20210121133929.20186-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21464.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Make OPENVPN_PLUGIN_ENABLE_PF failures FATAL
Hi, On Fri, Jan 22, 2021 at 08:02:42AM +0100, Arne Schwabe wrote: > I agree to make this "fixed" in a way that doesn't involve refactoring > of pf code that is later removed anyway. I don't think the refactoring > early affects this. It has been probably broken even earlier. So I got myself nerdsniped into understanding when and how this broke. Tested v2.4.10, and that works nicely when ENABLE_PF fails - it logs a warning, and that's it. No instance restart, no crash. The instance restart is introduced here: commit 492e42d35f141346fe21b3e984ed1bd86e5aac40 Author: Steffan Karger Date: Wed Nov 1 23:03:40 2017 +0100 pf: reject client if PF plugin is configured, but init fails This changes the behavior for pf plugins: instead of just not initializing the firewall rules and happily continuing, this now rejects the client in the case of an (unlikely) failure to initialize the pf. ... which, back then, actually worked. Sort of - it aborts the client connection, but since it does so very early (before TLS_FINAL), the client is never told, and times out eventually. Sat Jan 23 12:45:48 2021 us=726018 2001:608:0:814::f000:21 PLUGIN_CALL: plugin function PLUGIN_ENABLE_PF failed with status 1: /home/gert/openvpn-pftest.git/sample/sample-plugins/defer/simple.so Sat Jan 23 12:45:48 2021 us=726033 2001:608:0:814::f000:21 WARNING: failed to init PF plugin, rejecting client. So, git bisect time... and that leads to $ git bisect good c252dcc073155567c1982611ec6f065342909287 is the first bad commit commit c252dcc073155567c1982611ec6f065342909287 Author: Arne Schwabe Date: Fri Jul 3 11:55:06 2020 +0200 Remove did_open_context, defined and connection_established_flag ... which is somewhat nonobvious to me - but the call chain for the pf*context() stuff is sufficiently "out of the norm" that it could very well be, changing assumptions about "how do we know the state of a multi_instance" which work well for all well-behaved paths, but not for the pf stuff. That said, I think a "proper" fix might be to move the pf_init_context() call to "when the instance is fully initialized and TLS has succeeded" (so we can proper shut down the instance again). Somewhere close to the other plugin calls like "client-connect", and fail "as they do", if needed. But I do not intend to do that, just write up thoughts for reference if we ever ask ourselves "what was the result of that investigation?" :-) 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
