diff mbox series

[Openvpn-devel,v8,5/5] client-connect: Implement deferred connect support for plugin API v2

Message ID 20200719173436.16431-5-arne@rfc2549.org
State Accepted
Headers
Series [Openvpn-devel,v8,1/5] Remove CAS_PARTIAL state |

Commit Message

Arne Schwabe July 19, 2020, 7:34 a.m. UTC 
  The V2 API is simpler than the V1 API since there is no passing of
data via files. This also means that with the current API the V2 API
cannot support async notify via files. Adding a file just for async
notify seems very hacky and when needed we should implement a better
option when async is needed for the plugin V2 API.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 include/openvpn-plugin.h.in |  3 ++-
 src/openvpn/multi.c         | 36 ++++++++++++++++++++++++------------
 src/openvpn/plugin.c        |  3 +++
 3 files changed, 29 insertions(+), 13 deletions(-)

Comments

Gert Doering July 20, 2020, 12:52 a.m. UTC | #1 
Acked-by: Gert Doering <gert@greenie.muc.de>

Stared-at-code, and whacked it from all sides - sync and 
async/deferred CLIENT_CONNECT and CLIENT_CONNECT_V2, with
"success", "failure" and "disabled" rejection:

Test sets succeeded: 5 5a 5b 5c 5v1 5v2 5v3 5w1 5w2 5w3 5w4 5x1 5x2 5x3 5x4.
Test sets failed: none.

(Full "full test" is still running, but I do not expect breakage
in other code paths)

I even tested this case:

2020-07-20 12:12:20 us=422277 cron2-freebsd-tc-amd64/194.97.140.21:50614 A plugin that defers from the OPENVPN_PLUGIN_CLIENT_CONNECT_V2 call must also declare support for OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER_V2

which I truly like ("this is an error in the plugin, point it 
out clearly and refuse cooperation")


The polling of the plugin status is a bit weird, as in "events are firing
much more often than I would have expected" - this is for a plugin with
25s delay (and the plugin handler logging the remaining time):

2020-07-20 12:30:26 PLUGIN sample-cc: OPENVPN_PLUGIN_CLIENT_CONNECT_V2
2020-07-20 12:30:26 PLUGIN sample-cc: env has UV_WANT_CC2_ASYNC=25 -> set up deferred handler
2020-07-20 12:30:26 PLUGIN_CALL: POST /root/t_server/tun-udp-p2mp-112-mask/sample-client-connect.so/PLUGIN_CLIENT_CONNECT status=2
2020-07-20 12:30:27 cron2-freebsd-tc-amd64/194.97.140.21:35288 PUSH: Received control message: 'PUSH_REQUEST'
2020-07-20 12:30:27 PLUGIN sample-cc: defer_v2: seconds left=24
2020-07-20 12:30:27 cron2-freebsd-tc-amd64/194.97.140.21:35288 PLUGIN_CALL: POST ...V2 status=2
2020-07-20 12:30:27 PLUGIN sample-cc: defer_v2: seconds left=24
2020-07-20 12:30:27 cron2-freebsd-tc-amd64/194.97.140.21:35288 PLUGIN_CALL: POST ...V2 status=2
2020-07-20 12:30:28 PLUGIN sample-cc: defer_v2: seconds left=23
2020-07-20 12:30:28 cron2-freebsd-tc-amd64/194.97.140.21:35288 PLUGIN_CALL: POST ...V2 status=2
2020-07-20 12:30:32 cron2-freebsd-tc-amd64/194.97.140.21:35288 PUSH: Received control message: 'PUSH_REQUEST'
2020-07-20 12:30:32 PLUGIN sample-cc: defer_v2: seconds left=19
2020-07-20 12:30:32 cron2-freebsd-tc-amd64/194.97.140.21:35288 PLUGIN_CALL: POST ...V2 status=2
2020-07-20 12:30:32 PLUGIN sample-cc: defer_v2: seconds left=19
2020-07-20 12:30:32 cron2-freebsd-tc-amd64/194.97.140.21:35288 PLUGIN_CALL: POST ...V2 status=2
2020-07-20 12:30:37 cron2-freebsd-tc-amd64/194.97.140.21:35288 PUSH: Received control message: 'PUSH_REQUEST'
2020-07-20 12:30:37 PLUGIN sample-cc: defer_v2: seconds left=14
2020-07-20 12:30:37 cron2-freebsd-tc-amd64/194.97.140.21:35288 PLUGIN_CALL: POST ...V2 status=2
2020-07-20 12:30:37 PLUGIN sample-cc: defer_v2: seconds left=14
2020-07-20 12:30:37 cron2-freebsd-tc-amd64/194.97.140.21:35288 PLUGIN_CALL: POST ...V2 status=2
2020-07-20 12:30:42 PLUGIN sample-cc: defer_v2: seconds left=9
2020-07-20 12:30:42 cron2-freebsd-tc-amd64/194.97.140.21:35288 PLUGIN_CALL: POST ...V2 status=2
2020-07-20 12:30:42 cron2-freebsd-tc-amd64/194.97.140.21:35288 PUSH: Received control message: 'PUSH_REQUEST'
2020-07-20 12:30:42 PLUGIN sample-cc: defer_v2: seconds left=9
2020-07-20 12:30:42 cron2-freebsd-tc-amd64/194.97.140.21:35288 PLUGIN_CALL: POST ...V2 status=2
2020-07-20 12:30:42 PLUGIN sample-cc: defer_v2: seconds left=9
2020-07-20 12:30:42 cron2-freebsd-tc-amd64/194.97.140.21:35288 PLUGIN_CALL: POST ...V2 status=2
2020-07-20 12:30:47 PLUGIN sample-cc: defer_v2: seconds left=4
2020-07-20 12:30:47 cron2-freebsd-tc-amd64/194.97.140.21:35288 PLUGIN_CALL: POST ...V2 status=2
2020-07-20 12:30:47 cron2-freebsd-tc-amd64/194.97.140.21:35288 PUSH: Received control message: 'PUSH_REQUEST'
2020-07-20 12:30:47 PLUGIN sample-cc: defer_v2: seconds left=4
2020-07-20 12:30:47 cron2-freebsd-tc-amd64/194.97.140.21:35288 PLUGIN_CALL: POST ...V2 status=2
2020-07-20 12:30:47 PLUGIN sample-cc: defer_v2: seconds left=4
2020-07-20 12:30:47 cron2-freebsd-tc-amd64/194.97.140.21:35288 PLUGIN_CALL: POST ...V2 status=2
2020-07-20 12:30:52 PLUGIN sample-cc: defer_v2: seconds left=-1
2020-07-20 12:30:52 PLUGIN sample-cc: env has UV_WANT_CC2_FAIL -> fail
2020-07-20 12:30:52 cron2-freebsd-tc-amd64/194.97.140.21:35288 PLUGIN_CALL: POST /root/t_server/tun-udp-p2mp-112-mask/sample-client-connect.so/PLUGIN_CLIENT_CONNECT_DEFER_V2 status=1
2020-07-20 12:30:52 cron2-freebsd-tc-amd64/194.97.140.21:35288 PLUGIN_CALL: plugin function PLUGIN_CLIENT_CONNECT_DEFER_V2 failed with status 1: /root/t_server/tun-udp-p2mp-112-mask/sample-client-connect.so
2020-07-20 12:30:52 cron2-freebsd-tc-amd64/194.97.140.21:35288 WARNING: client-connect-v2 plugin call failed
2020-07-20 12:30:52 PLUGIN sample-cc: OPENVPN_PLUGIN_CLIENT_DISCONNECT

Is this intentional?  Or more some sort of "huh? why?" effect?

(This is with just a single client connecting, so no events from 
other instances)


Your patch has been applied to the master branch.

commit aad16b6c48735c5449d9b3bd0689bfc0a0a9fff6
Author: Arne Schwabe
Date:   Sun Jul 19 19:34:36 2020 +0200

     client-connect: Implement deferred connect support for plugin API v2

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20200719173436.16431-5-arne@rfc2549.org>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20480.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/include/openvpn-plugin.h.in b/include/openvpn-plugin.h.in
index 99aa1678..38fbe097 100644
--- a/include/openvpn-plugin.h.in
+++ b/include/openvpn-plugin.h.in
@@ -130,7 +130,8 @@  extern "C" {
 #define OPENVPN_PLUGIN_ENABLE_PF                11
 #define OPENVPN_PLUGIN_ROUTE_PREDOWN            12
 #define OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER     13
-#define OPENVPN_PLUGIN_N                        14
+#define OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER_V2  14
+#define OPENVPN_PLUGIN_N                        15
 
 /*
  * Build a mask out of a set of plug-in types.
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index c66a4cea..06f8e6c1 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2112,36 +2112,48 @@  multi_client_connect_call_plugin_v2(struct multi_context *m,
                                     bool deferred,
                                     unsigned int *option_types_found)
 {
-    if (deferred)
-    {
-        return CC_RET_FAILED;
-    }
     enum client_connect_return ret = CC_RET_SKIPPED;
 #ifdef ENABLE_PLUGIN
     ASSERT(m);
     ASSERT(mi);
     ASSERT(option_types_found);
 
+    int call = deferred ? OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER_V2 :
+               OPENVPN_PLUGIN_CLIENT_CONNECT_V2;
     /* V2 callback, use a plugin_return struct for passing back return info */
-    if (plugin_defined(mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT_V2))
+    if (plugin_defined(mi->context.plugins, call))
     {
         struct plugin_return pr;
 
         plugin_return_init(&pr);
 
-        if (plugin_call(mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT_V2,
-                        NULL, &pr, mi->context.c2.es)
-            != OPENVPN_PLUGIN_FUNC_SUCCESS)
+        int plug_ret = plugin_call(mi->context.plugins, call,
+                                   NULL, &pr, mi->context.c2.es);
+        if (plug_ret == OPENVPN_PLUGIN_FUNC_SUCCESS)
         {
-            msg(M_WARN, "WARNING: client-connect-v2 plugin call failed");
-            ret = CC_RET_FAILED;
+            multi_client_connect_post_plugin(m, mi, &pr, option_types_found);
+            ret = CC_RET_SUCCEEDED;
+        }
+        else if (plug_ret == OPENVPN_PLUGIN_FUNC_DEFERRED)
+        {
+            ret = CC_RET_DEFERRED;
+            if (!(plugin_defined(mi->context.plugins,
+                                 OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER_V2)))
+            {
+                msg(M_WARN, "A plugin that defers from the "
+                    "OPENVPN_PLUGIN_CLIENT_CONNECT_V2 call must also "
+                    "declare support for "
+                    "OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER_V2");
+                ret = CC_RET_FAILED;
+            }
         }
         else
         {
-            multi_client_connect_post_plugin(m, mi, &pr, option_types_found);
-            ret = CC_RET_SUCCEEDED;
+            msg(M_WARN, "WARNING: client-connect-v2 plugin call failed");
+            ret = CC_RET_FAILED;
         }
 
+
         plugin_return_free(&pr);
     }
 #endif /* ifdef ENABLE_PLUGIN */
diff --git a/src/openvpn/plugin.c b/src/openvpn/plugin.c
index 9a6fa3cb..8b351c45 100644
--- a/src/openvpn/plugin.c
+++ b/src/openvpn/plugin.c
@@ -107,6 +107,9 @@  plugin_type_name(const int type)
         case OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER:
             return "PLUGIN_CLIENT_CONNECT_DEFER";
 
+        case OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER_V2:
+            return "PLUGIN_CLIENT_CONNECT_DEFER_V2";
+
         case OPENVPN_PLUGIN_CLIENT_DISCONNECT:
             return "PLUGIN_CLIENT_DISCONNECT";