Fix temporary file leak

PATCH In review Samuel Thibault 4 participants 27 messages Oct 9, 2014 – Nov 30, 2016

lore

Share & tools

lore

Samuel Thibault

author

2014-10-09 21:40 UTC #0 lore raw

PATCH 1/1 Fix temporary file leak

Hello,

Our openvpn server got out of free inodes in /tmp, making it quite completely nonworking. This is due to some codepath in multi.c which does not remove its temporary file (when a plugin callback returns an error, or a client-connect script returns an error). Please see the attached patch which fixes this.

Samuel

src/openvpn/multi.c +9 -1

index 5910154..d0ed147 100644
@@ -1699,6 +1705,9 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 	    {
 	      msg (M_WARN, "WARNING: client-connect plugin call failed");
 	      cc_succeeded = false;
+	      if (!platform_unlink (dc_file))
+		msg (D_MULTI_ERRORS, "MULTI: problem deleting temporary file: %s",
+		     dc_file);
 	    }
 	  else
 	    {

@@ -1757,7 +1766,12 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 	      ++cc_succeeded_count;
 	    }
 	  else
-	    cc_succeeded = false;
+	    {
+	      cc_succeeded = false;
+	      if (!platform_unlink (dc_file))
+		msg (D_MULTI_ERRORS, "MULTI: problem deleting temporary file: %s",
+		     dc_file);
+	    }
         script_failed:
 	  argv_reset (&argv);
 	}

David Sommerseth

2014-10-10 10:13 UTC #1 lore raw

Quoted 33 lines · Samuel Thibault

Hello,
Our openvpn server got out of free inodes in /tmp, making it quite completely nonworking. This is due to some codepath in multi.c which does not remove its temporary file (when a plugin callback returns an error, or a client-connect script returns an error). Please see the attached patch which fixes this.
Samuel
patch
src/openvpn/multi.c +3 -0
index 5910154..d0ed147 100644
@@ -1699,6 +1705,9 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 	    {
 	      msg (M_WARN, "WARNING: client-connect plugin call failed");
 	      cc_succeeded = false;
+	      if (!platform_unlink (dc_file))
+		msg (D_MULTI_ERRORS, "MULTI: problem deleting temporary file: %s",
+		     dc_file);
 	    }
 	  else
 	    {
@@ -1757,7 +1766,12 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 	      ++cc_succeeded_count;
 	    }
 	  else

src/openvpn/multi.c multi_connection_established (struct multi_context *m, struct multi_instance *mi · line 1777

Quoted 10 lines

src/openvpn/multi.c +6 -1

-	    cc_succeeded = false;
+	    {
+	      cc_succeeded = false;
+	      if (!platform_unlink (dc_file))
+		msg (D_MULTI_ERRORS, "MULTI: problem deleting temporary file: %s",
+		     dc_file);
+	    }
         script_failed:
 	  argv_reset (&argv);
 	}

Good catch! But I'm not convinced about this approach, from a code quality perspective. Now we have the unlinking of these temp files two places. In multi_client_connect_post() if the plug-in(s) didn't fail, or in multi_connection_established() if the plug-in failed.

I think it would be better to move the unlink() code from multi_client_connect_post() into multi_connection_established(), where these temp files are created. This makes the code clearer and easier to understand. And you can have a label and goto statement in the error section, similar to what's already done (look for script_depr_failed:
and script_failed:).

Signature · 3 lines

kind regards,

David Sommerseth

Fabian Knittel

2014-10-10 22:09 UTC #2 lore raw

cover Refactor client-connect / add support for deferred handling

Hi,

a few years ago I wrote a patch-set concerning OpenVPN's client-connect code. The first part of the patch-set (patches 1 to 6) refactors and unifies the client-connect code. (This might be of interest in the current "Fix temporary file leak"-thread.)

The second part of the patch-set allows client-connect handling to proceed asynchronously, similar to how OpenVPN supports asynchronous (or deferred) authentication. Basically, the scripts or v1-plugins handling the client-connect event can now write back an additional status code that indicates deferred handling of the client-connect. This causes the OpenVPN server to continue with other things and to regularly re-read the status file. As soon as the status changes from "deferred" to "failed" or "succeeded", the client-connect processing for the connection is continued. (As mentioned before, the auth code does something very similar.)

The motivation for the deferred handling approach were relatively long running client-connect scripts (> 2s) intended for high-traffic servers. As the OpenVPN server completely blocks while synchronously waiting for client-connect scripts to complete, the asynchronous / deferred approach was needed.

The deferred script code path has been in production use for a few years now, although this freshly rebased patch-set has only seen light testing so far.

Feed-back would be very welcome.

The patches are also availabe on a Github branch:
https://github.com/fknittel/openvpn/tree/feat_deferred_client-connect

Cheers
Fabian

PS: See
https://github.com/fknittel/openvpn/wiki/Patch-set-%22deferred-client-connect%22
in case you're interested in ready-made Debian packages.

Fabian Knittel (9):
client-connect: Split multi_connection_established into separate
functions
client-connect: Properly indent all functions
client-connect: Refactor multi_client_connect_source_ccd
client-connect: Move multi_client_connect_setenv into early_setup
client-connect: Refactor to use return values instead of modifying a
passed-in flag
client-connect: Refactor client-connect handling to calling a bunch of
hooks in a loop
client-connect: Add CC_RET_DEFERRED and cope with deferred
client-connect
client-connect: Add deferred support to the client-connect script
handler
client-connect: Add deferred support to the client-connect plugin v1
handler

 src/openvpn/multi.c   | 831 ++++++++++++++++++++++++++++++++++++--------------
 src/openvpn/multi.h   |  32 +-
 src/openvpn/options.h |   6 +
 3 files changed, 635 insertions(+), 234 deletions(-)

Signature · 1 line

2.1.1

Fabian Knittel

2014-10-10 22:09 UTC #3 lore raw

PATCH 1/9 client-connect: Split multi_connection_established into separate functions

This patch splits up the multi_connection_established() function. Each new helper function does a specific job. Functions that do a similar job receive a similar calling interface.

The patch tries not to reindent code, so that the real changes are as clearly visible as possible. (A follow-up patch will only do indentation changes.)

Signed-off-by Fabian Knittel <[email protected]...>

---
 src/openvpn/multi.c   | 312 +++++++++++++++++++++++++++++++++++---------------
 src/openvpn/options.h |   6 +
 2 files changed, 224 insertions(+), 94 deletions(-)

src/openvpn/multi.c +218 ~~-94~~

index 5910154..ed10e20 100644
@@ -1446,7 +1446,6 @@ static void
 multi_client_connect_post (struct multi_context *m,
 			   struct multi_instance *mi,
 			   const char *dc_file,
-			   unsigned int option_permissions_mask,
 			   unsigned int *option_types_found)
 {
   /* Did script generate a dynamic config file? */

@@ -1455,7 +1454,7 @@ multi_client_connect_post (struct multi_context *m,
       options_server_import (&mi->context.options,
 			     dc_file,
 			     D_IMPORT_ERRORS|M_OPTERR,
-			     option_permissions_mask,
+			     CLIENT_CONNECT_OPT_MASK,
 			     option_types_found,
 			     mi->context.c2.es);

@@ -1483,7 +1482,6 @@ static void
 multi_client_connect_post_plugin (struct multi_context *m,
 				  struct multi_instance *mi,
 				  const struct plugin_return *pr,
-				  unsigned int option_permissions_mask,
 				  unsigned int *option_types_found)
 {
   struct plugin_return config;

@@ -1500,7 +1498,7 @@ multi_client_connect_post_plugin (struct multi_context *m,
 	    options_string_import (&mi->context.options,
 				   config.list[i]->value,
 				   D_IMPORT_ERRORS|M_OPTERR,
-				   option_permissions_mask,
+				   CLIENT_CONNECT_OPT_MASK,
 				   option_types_found,
 				   mi->context.c2.es);
 	}

@@ -1518,29 +1516,28 @@ multi_client_connect_post_plugin (struct multi_context *m,
 
 #endif
 
-#ifdef MANAGEMENT_DEF_AUTH
-
 /*
  * Called to load management-derived client-connect config
  */
 static void
 multi_client_connect_mda (struct multi_context *m,
 			  struct multi_instance *mi,
-			  const struct buffer_list *config,
-			  unsigned int option_permissions_mask,
-			  unsigned int *option_types_found)
+			  unsigned int *option_types_found,
+			  int *cc_succeeded,
+			  int *cc_succeeded_count)
 {
-  if (config)
+#ifdef MANAGEMENT_DEF_AUTH
+  if (mi->cc_config)
     {
       struct buffer_entry *be;
   
-      for (be = config->head; be != NULL; be = be->next)
+      for (be = mi->cc_config->head; be != NULL; be = be->next)
 	{
 	  const char *opt = BSTR(&be->buf);
 	  options_string_import (&mi->context.options,
 				 opt,
 				 D_IMPORT_ERRORS|M_OPTERR,
-				 option_permissions_mask,
+				 CLIENT_CONNECT_OPT_MASK,
 				 option_types_found,
 				 mi->context.c2.es);
 	}

@@ -1553,10 +1550,11 @@ multi_client_connect_mda (struct multi_context *m,
        */
       multi_select_virtual_addr (m, mi);
       multi_set_virtual_addr_env (m, mi);
-    }
-}
 
+      ++*cc_succeeded_count;
+    }
 #endif
+}
 
 static void
 multi_client_connect_setenv (struct multi_context *m,

@@ -1583,6 +1581,38 @@ multi_client_connect_setenv (struct multi_context *m,
   gc_free (&gc);
 }
 
+static void
+multi_client_connect_early_setup (struct multi_context *m,
+				  struct multi_instance *mi);
+static void
+multi_client_connect_source_ccd (struct multi_context *m,
+				 struct multi_instance *mi,
+				 unsigned int *option_types_found);
+static void
+multi_client_connect_call_plugin_v1 (struct multi_context *m,
+				     struct multi_instance *mi,
+				     unsigned int *option_types_found,
+				     int *cc_succeeded,
+				     int *cc_succeeded_count);
+static void
+multi_client_connect_call_plugin_v2 (struct multi_context *m,
+				     struct multi_instance *mi,
+				     unsigned int *option_types_found,
+				     int *cc_succeeded,
+				     int *cc_succeeded_count);
+static void
+multi_client_connect_call_script (struct multi_context *m,
+				  struct multi_instance *mi,
+				  unsigned int *option_types_found,
+				  int *cc_succeeded,
+				  int *cc_succeeded_count);
+static void
+multi_client_connect_late_setup (struct multi_context *m,
+				 struct multi_instance *mi,
+				 const unsigned int option_types_found,
+				 int cc_succeeded,
+				 const int cc_succeeded_count);
+
 /*
  * Called as soon as the SSL/TLS connection authenticates.
  *

@@ -1597,25 +1627,67 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 {
   if (tls_authentication_status (mi->context.c2.tls_multi, 0) == TLS_AUTHENTICATION_SUCCEEDED)
     {
-      struct gc_arena gc = gc_new ();
       unsigned int option_types_found = 0;
-
-      const unsigned int option_permissions_mask =
-	  OPT_P_INSTANCE
-	| OPT_P_INHERIT
-	| OPT_P_PUSH
-	| OPT_P_TIMER
-	| OPT_P_CONFIG
-	| OPT_P_ECHO
-	| OPT_P_COMP
-	| OPT_P_SOCKFLAGS;
-
       int cc_succeeded = true; /* client connect script status */
       int cc_succeeded_count = 0;
 
-      ASSERT (mi->context.c1.tuntap);
+      multi_client_connect_early_setup (m, mi);
+
+      multi_client_connect_source_ccd (m, mi, &option_types_found);
+
+      /*
+       * Select a virtual address from either --ifconfig-push in
+       * --client-config-dir file or --ifconfig-pool.
+       */
+      multi_select_virtual_addr (m, mi);
+
+      /* do --client-connect setenvs */
+      multi_client_connect_setenv (m, mi);
+
+      multi_client_connect_call_plugin_v1 (m, mi, &option_types_found,
+					   &cc_succeeded, &cc_succeeded_count);
+
+      multi_client_connect_call_plugin_v2 (m, mi, &option_types_found,
+					   &cc_succeeded, &cc_succeeded_count);
+
+      /*
+       * Run --client-connect script.
+       */
+      if (cc_succeeded)
+	{
+	  multi_client_connect_call_script (m, mi, &option_types_found,
+					    &cc_succeeded,
+					    &cc_succeeded_count);
+	}
 
-      /* lock down the common name and cert hashes so they can't change during future TLS renegotiations */
+      /*
+       * Check for client-connect script left by management interface client
+       */
+      if (cc_succeeded)
+	{
+	  multi_client_connect_mda (m, mi, &option_types_found,
+				    &cc_succeeded, &cc_succeeded_count);
+	}
+
+      multi_client_connect_late_setup (m, mi, option_types_found,
+				       cc_succeeded, cc_succeeded_count);
+
+      /* set flag so we don't get called again */
+      mi->connection_established_flag = true;
+    }
+
+  /*
+   * Reply now to client's PUSH_REQUEST query
+   */
+  mi->context.c2.push_reply_deferred = false;
+}
+
+static void
+multi_client_connect_early_setup (struct multi_context *m,
+				  struct multi_instance *mi)
+{
+      /* lock down the common name and cert hashes so they can't change during
+	 future TLS renegotiations */
       tls_lock_common_name (mi->context.c2.tls_multi);
       tls_lock_cert_hash_set (mi->context.c2.tls_multi);

@@ -1628,13 +1700,20 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 
       /* reset pool handle to null */
       mi->vaddr_handle = -1;
+}
 
-      /*
-       * Try to source a dynamic config file from the
-       * --client-config-dir directory.
-       */
+/*
+ * Try to source a dynamic config file from the
+ * --client-config-dir directory.
+ */
+static void
+multi_client_connect_source_ccd (struct multi_context *m,
+				 struct multi_instance *mi,
+				 unsigned int *option_types_found)
+{
       if (mi->context.options.client_config_dir)
 	{
+	  struct gc_arena gc = gc_new ();
 	  const char *ccd_file;
 	  
 	  ccd_file = gen_path (mi->context.options.client_config_dir,

@@ -1647,8 +1726,8 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 	      options_server_import (&mi->context.options,
 				     ccd_file,
 				     D_IMPORT_ERRORS|M_OPTERR,
-				     option_permissions_mask,
-				     &option_types_found,
+				     CLIENT_CONNECT_OPT_MASK,
+				     option_types_found,
 				     mi->context.c2.es);
 	    }
 	  else /* try default file */

@@ -1662,90 +1741,142 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 		  options_server_import (&mi->context.options,
 					 ccd_file,
 					 D_IMPORT_ERRORS|M_OPTERR,
-					 option_permissions_mask,
-					 &option_types_found,
+					 CLIENT_CONNECT_OPT_MASK,
+					 option_types_found,
 					 mi->context.c2.es);
 		}
 	    }
-	}
 
-      /*
-       * Select a virtual address from either --ifconfig-push in --client-config-dir file
-       * or --ifconfig-pool.
-       */
-      multi_select_virtual_addr (m, mi);
-
-      /* do --client-connect setenvs */
-      multi_client_connect_setenv (m, mi);
+	  gc_free (&gc);
+	}
+}
 
+/*
+ * Call client-connect plug-in.
+ *
+ * deprecated callback, use a file for passing back return info
+ */
+static void
+multi_client_connect_call_plugin_v1 (struct multi_context *m,
+				     struct multi_instance *mi,
+				     unsigned int *option_types_found,
+				     int *cc_succeeded,
+				     int *cc_succeeded_count)
+{
 #ifdef ENABLE_PLUGIN
-      /*
-       * Call client-connect plug-in.
-       */
+      ASSERT (m);
+      ASSERT (mi);
+      ASSERT (option_types_found);
+      ASSERT (cc_succeeded);
+      ASSERT (cc_succeeded_count);
 
-      /* deprecated callback, use a file for passing back return info */
       if (plugin_defined (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT))
 	{
+	  int plug_ret;
+	  struct gc_arena gc = gc_new ();
 	  struct argv argv = argv_new ();
 	  const char *dc_file = create_temp_file (mi->context.options.tmp_dir, "cc", &gc);
 
-          if( !dc_file ) {
-            cc_succeeded = false;
-            goto script_depr_failed;
-          }
+          if (!dc_file)
+	    {
+	      *cc_succeeded = false;
+	      goto script_depr_failed;
+	    }
 
 	  argv_printf (&argv, "%s", dc_file);
-	  if (plugin_call (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT, &argv, NULL, mi->context.c2.es) != OPENVPN_PLUGIN_FUNC_SUCCESS)
+
+	  plug_ret = plugin_call (mi->context.plugins,
+				  OPENVPN_PLUGIN_CLIENT_CONNECT,
+				  &argv, NULL, mi->context.c2.es);
+	  argv_reset (&argv);
+	  if (plug_ret != OPENVPN_PLUGIN_FUNC_SUCCESS)
 	    {
 	      msg (M_WARN, "WARNING: client-connect plugin call failed");
-	      cc_succeeded = false;
+	      *cc_succeeded = false;
 	    }
 	  else
 	    {
-	      multi_client_connect_post (m, mi, dc_file, option_permissions_mask, &option_types_found);
-	      ++cc_succeeded_count;
+	      multi_client_connect_post (m, mi, dc_file, option_types_found);
+	      ++*cc_succeeded_count;
 	    }
-        script_depr_failed:
-	  argv_reset (&argv);
+script_depr_failed:
+	  gc_free (&gc);
 	}
+#endif
+}
+
+/*
+ * Call client-connect plug-in.
+ *
+ * V2 callback, use a plugin_return struct for passing back return info
+ */
+static void
+multi_client_connect_call_plugin_v2 (struct multi_context *m,
+				     struct multi_instance *mi,
+				     unsigned int *option_types_found,
+				     int *cc_succeeded,
+				     int *cc_succeeded_count)
+{
+#ifdef ENABLE_PLUGIN
+      ASSERT (m);
+      ASSERT (mi);
+      ASSERT (option_types_found);
+      ASSERT (cc_succeeded);
+      ASSERT (cc_succeeded_count);
 
-      /* V2 callback, use a plugin_return struct for passing back return info */
       if (plugin_defined (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT_V2))
 	{
+	  int plug_ret;
 	  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)
+	  plug_ret = plugin_call (mi->context.plugins,
+				  OPENVPN_PLUGIN_CLIENT_CONNECT_V2,
+				  NULL, &pr, mi->context.c2.es);
+	  if (plug_ret != OPENVPN_PLUGIN_FUNC_SUCCESS)
 	    {
 	      msg (M_WARN, "WARNING: client-connect-v2 plugin call failed");
-	      cc_succeeded = false;
+	      *cc_succeeded = false;
 	    }
 	  else
 	    {
-	      multi_client_connect_post_plugin (m, mi, &pr, option_permissions_mask, &option_types_found);
-	      ++cc_succeeded_count;
+	      multi_client_connect_post_plugin (m, mi, &pr, option_types_found);
+	      ++*cc_succeeded_count;
 	    }
 
 	  plugin_return_free (&pr);
 	}
 #endif
+}
 
-      /*
-       * Run --client-connect script.
-       */
-      if (mi->context.options.client_connect_script && cc_succeeded)
+static void
+multi_client_connect_call_script (struct multi_context *m,
+				  struct multi_instance *mi,
+				  unsigned int *option_types_found,
+				  int *cc_succeeded,
+				  int *cc_succeeded_count)
+{
+      ASSERT (m);
+      ASSERT (mi);
+      ASSERT (option_types_found);
+      ASSERT (cc_succeeded);
+      ASSERT (cc_succeeded_count);
+
+      if (mi->context.options.client_connect_script)
 	{
+	  struct gc_arena gc = gc_new ();
 	  struct argv argv = argv_new ();
-	  const char *dc_file = NULL;
+	  const char *dc_file;
 
 	  setenv_str (mi->context.c2.es, "script_type", "client-connect");
 
 	  dc_file = create_temp_file (mi->context.options.tmp_dir, "cc", &gc);
-          if( !dc_file ) {
-            cc_succeeded = false;
-            goto script_failed;
-          }
+          if (!dc_file)
+	    {
+	      cc_succeeded = false;
+	      goto script_failed;
+	    }
 
 	  argv_printf (&argv, "%sc %s",
 		       mi->context.options.client_connect_script,

@@ -1753,25 +1884,26 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 
 	  if (openvpn_run_script (&argv, mi->context.c2.es, 0, "--client-connect"))
 	    {
-	      multi_client_connect_post (m, mi, dc_file, option_permissions_mask, &option_types_found);
+	      multi_client_connect_post (m, mi, dc_file, option_types_found);
 	      ++cc_succeeded_count;
 	    }
 	  else
 	    cc_succeeded = false;
-        script_failed:
+script_failed:
 	  argv_reset (&argv);
+	  gc_free (&gc);
 	}
+}
 
-      /*
-       * Check for client-connect script left by management interface client
-       */
-#ifdef MANAGEMENT_DEF_AUTH
-      if (cc_succeeded && mi->cc_config)
-	{
-	  multi_client_connect_mda (m, mi, mi->cc_config, option_permissions_mask, &option_types_found);
-	  ++cc_succeeded_count;
-	}
-#endif
+static void
+multi_client_connect_late_setup (struct multi_context *m,
+				 struct multi_instance *mi,
+				 const unsigned int option_types_found,
+				 int cc_succeeded,
+				 const int cc_succeeded_count)
+{
+      struct gc_arena gc = gc_new ();
+      ASSERT (mi->context.c1.tuntap);
 
       /*
        * Check for "disable" directive in client-config-dir file

@@ -1865,9 +1997,6 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 	  mi->context.c2.context_auth = cc_succeeded_count ? CAS_PARTIAL : CAS_FAILED;
 	}
 
-      /* set flag so we don't get called again */
-      mi->connection_established_flag = true;
-
       /* increment number of current authenticated clients */
       ++m->n_clients;
       update_mstat_n_clients(m->n_clients);

@@ -1879,14 +2008,9 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 #endif
 
       gc_free (&gc);
-    }
-
-  /*
-   * Reply now to client's PUSH_REQUEST query
-   */
-  mi->context.c2.push_reply_deferred = false;
 }
 
+
 /*
  * Add a mbuf buffer to a particular
  * instance.

src/openvpn/options.h +6 -0

index 5ba4f2f..65ed590 100644
@@ -636,6 +636,12 @@ struct options
 
 #define OPT_P_DEFAULT   (~(OPT_P_INSTANCE|OPT_P_PULL_MODE))
 
+#if P2MP_SERVER
+#define CLIENT_CONNECT_OPT_MASK (OPT_P_INSTANCE | OPT_P_INHERIT	| OPT_P_PUSH | \
+				 OPT_P_TIMER | OPT_P_CONFIG | OPT_P_ECHO | \
+				 OPT_P_COMP | OPT_P_SOCKFLAGS)
+#endif
+
 #if P2MP
 #define PULL_DEFINED(opt) ((opt)->pull)
 #if P2MP_SERVER

Signature · 1 line

2.1.1

Fabian Knittel

2014-10-10 22:09 UTC #4 lore raw

PATCH 2/9 client-connect: Properly indent all functions

In reply to Fabian Knittel

This patch adds proper indentation to all new helper functions. No functional changes are performed. Nothing is moved around.

Signed-off-by Fabian Knittel <[email protected]...>

---
 src/openvpn/multi.c | 408 ++++++++++++++++++++++++++--------------------------
 1 file changed, 204 insertions(+), 204 deletions(-)

src/openvpn/multi.c +204 ~~-204~~

index ed10e20..23b86a5 100644
@@ -1686,20 +1686,20 @@ static void
 multi_client_connect_early_setup (struct multi_context *m,
 				  struct multi_instance *mi)
 {
-      /* lock down the common name and cert hashes so they can't change during
-	 future TLS renegotiations */
-      tls_lock_common_name (mi->context.c2.tls_multi);
-      tls_lock_cert_hash_set (mi->context.c2.tls_multi);
+  /* lock down the common name and cert hashes so they can't change during
+     future TLS renegotiations */
+  tls_lock_common_name (mi->context.c2.tls_multi);
+  tls_lock_cert_hash_set (mi->context.c2.tls_multi);
 
-      /* generate a msg() prefix for this client instance */
-      generate_prefix (mi);
+  /* generate a msg() prefix for this client instance */
+  generate_prefix (mi);
 
-      /* delete instances of previous clients with same common-name */
-      if (!mi->context.options.duplicate_cn)
-	multi_delete_dup (m, mi);
+  /* delete instances of previous clients with same common-name */
+  if (!mi->context.options.duplicate_cn)
+    multi_delete_dup (m, mi);
 
-      /* reset pool handle to null */
-      mi->vaddr_handle = -1;
+  /* reset pool handle to null */
+  mi->vaddr_handle = -1;
 }
 
 /*

@@ -1711,16 +1711,31 @@ multi_client_connect_source_ccd (struct multi_context *m,
 				 struct multi_instance *mi,
 				 unsigned int *option_types_found)
 {
-      if (mi->context.options.client_config_dir)
+  if (mi->context.options.client_config_dir)
+    {
+      struct gc_arena gc = gc_new ();
+      const char *ccd_file;
+      
+      ccd_file = gen_path (mi->context.options.client_config_dir,
+			   tls_common_name (mi->context.c2.tls_multi, false),
+			   &gc);
+
+      /* try common-name file */
+      if (test_file (ccd_file))
+	{
+	  options_server_import (&mi->context.options,
+				 ccd_file,
+				 D_IMPORT_ERRORS|M_OPTERR,
+				 CLIENT_CONNECT_OPT_MASK,
+				 option_types_found,
+				 mi->context.c2.es);
+	}
+      else /* try default file */
 	{
-	  struct gc_arena gc = gc_new ();
-	  const char *ccd_file;
-	  
 	  ccd_file = gen_path (mi->context.options.client_config_dir,
-			       tls_common_name (mi->context.c2.tls_multi, false),
+			       CCD_DEFAULT,
 			       &gc);
 
-	  /* try common-name file */
 	  if (test_file (ccd_file))
 	    {
 	      options_server_import (&mi->context.options,

@@ -1730,25 +1745,10 @@ multi_client_connect_source_ccd (struct multi_context *m,
 				     option_types_found,
 				     mi->context.c2.es);
 	    }
-	  else /* try default file */
-	    {
-	      ccd_file = gen_path (mi->context.options.client_config_dir,
-				   CCD_DEFAULT,
-				   &gc);
-
-	      if (test_file (ccd_file))
-		{
-		  options_server_import (&mi->context.options,
-					 ccd_file,
-					 D_IMPORT_ERRORS|M_OPTERR,
-					 CLIENT_CONNECT_OPT_MASK,
-					 option_types_found,
-					 mi->context.c2.es);
-		}
-	    }
-
-	  gc_free (&gc);
 	}
+
+      gc_free (&gc);
+    }
 }
 
 /*

@@ -1764,44 +1764,44 @@ multi_client_connect_call_plugin_v1 (struct multi_context *m,
 				     int *cc_succeeded_count)
 {
 #ifdef ENABLE_PLUGIN
-      ASSERT (m);
-      ASSERT (mi);
-      ASSERT (option_types_found);
-      ASSERT (cc_succeeded);
-      ASSERT (cc_succeeded_count);
+  ASSERT (m);
+  ASSERT (mi);
+  ASSERT (option_types_found);
+  ASSERT (cc_succeeded);
+  ASSERT (cc_succeeded_count);
 
-      if (plugin_defined (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT))
-	{
-	  int plug_ret;
-	  struct gc_arena gc = gc_new ();
-	  struct argv argv = argv_new ();
-	  const char *dc_file = create_temp_file (mi->context.options.tmp_dir, "cc", &gc);
+  if (plugin_defined (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT))
+    {
+      int plug_ret;
+      struct gc_arena gc = gc_new ();
+      struct argv argv = argv_new ();
+      const char *dc_file = create_temp_file (mi->context.options.tmp_dir, "cc", &gc);
 
-          if (!dc_file)
-	    {
-	      *cc_succeeded = false;
-	      goto script_depr_failed;
-	    }
+      if (!dc_file)
+	{
+	  *cc_succeeded = false;
+	  goto script_depr_failed;
+	}
 
-	  argv_printf (&argv, "%s", dc_file);
+      argv_printf (&argv, "%s", dc_file);
 
-	  plug_ret = plugin_call (mi->context.plugins,
-				  OPENVPN_PLUGIN_CLIENT_CONNECT,
-				  &argv, NULL, mi->context.c2.es);
-	  argv_reset (&argv);
-	  if (plug_ret != OPENVPN_PLUGIN_FUNC_SUCCESS)
-	    {
-	      msg (M_WARN, "WARNING: client-connect plugin call failed");
-	      *cc_succeeded = false;
-	    }
-	  else
-	    {
-	      multi_client_connect_post (m, mi, dc_file, option_types_found);
-	      ++*cc_succeeded_count;
-	    }
-script_depr_failed:
-	  gc_free (&gc);
+      plug_ret = plugin_call (mi->context.plugins,
+			      OPENVPN_PLUGIN_CLIENT_CONNECT,
+			      &argv, NULL, mi->context.c2.es);
+      argv_reset (&argv);
+      if (plug_ret != OPENVPN_PLUGIN_FUNC_SUCCESS)
+	{
+	  msg (M_WARN, "WARNING: client-connect plugin call failed");
+	  *cc_succeeded = false;
 	}
+      else
+	{
+	  multi_client_connect_post (m, mi, dc_file, option_types_found);
+	  ++*cc_succeeded_count;
+	}
+script_depr_failed:
+      gc_free (&gc);
+    }
 #endif
 }

@@ -1818,35 +1818,35 @@ multi_client_connect_call_plugin_v2 (struct multi_context *m,
 				     int *cc_succeeded_count)
 {
 #ifdef ENABLE_PLUGIN
-      ASSERT (m);
-      ASSERT (mi);
-      ASSERT (option_types_found);
-      ASSERT (cc_succeeded);
-      ASSERT (cc_succeeded_count);
-
-      if (plugin_defined (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT_V2))
-	{
-	  int plug_ret;
-	  struct plugin_return pr;
+  ASSERT (m);
+  ASSERT (mi);
+  ASSERT (option_types_found);
+  ASSERT (cc_succeeded);
+  ASSERT (cc_succeeded_count);
 
-	  plugin_return_init (&pr);
+  if (plugin_defined (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT_V2))
+    {
+      int plug_ret;
+      struct plugin_return pr;
 
-	  plug_ret = plugin_call (mi->context.plugins,
-				  OPENVPN_PLUGIN_CLIENT_CONNECT_V2,
-				  NULL, &pr, mi->context.c2.es);
-	  if (plug_ret != OPENVPN_PLUGIN_FUNC_SUCCESS)
-	    {
-	      msg (M_WARN, "WARNING: client-connect-v2 plugin call failed");
-	      *cc_succeeded = false;
-	    }
-	  else
-	    {
-	      multi_client_connect_post_plugin (m, mi, &pr, option_types_found);
-	      ++*cc_succeeded_count;
-	    }
+      plugin_return_init (&pr);
 
-	  plugin_return_free (&pr);
+      plug_ret = plugin_call (mi->context.plugins,
+			      OPENVPN_PLUGIN_CLIENT_CONNECT_V2,
+			      NULL, &pr, mi->context.c2.es);
+      if (plug_ret != OPENVPN_PLUGIN_FUNC_SUCCESS)
+	{
+	  msg (M_WARN, "WARNING: client-connect-v2 plugin call failed");
+	  *cc_succeeded = false;
+	}
+      else
+	{
+	  multi_client_connect_post_plugin (m, mi, &pr, option_types_found);
+	  ++*cc_succeeded_count;
 	}
+
+      plugin_return_free (&pr);
+    }
 #endif
 }

@@ -1857,42 +1857,42 @@ multi_client_connect_call_script (struct multi_context *m,
 				  int *cc_succeeded,
 				  int *cc_succeeded_count)
 {
-      ASSERT (m);
-      ASSERT (mi);
-      ASSERT (option_types_found);
-      ASSERT (cc_succeeded);
-      ASSERT (cc_succeeded_count);
+  ASSERT (m);
+  ASSERT (mi);
+  ASSERT (option_types_found);
+  ASSERT (cc_succeeded);
+  ASSERT (cc_succeeded_count);
 
-      if (mi->context.options.client_connect_script)
-	{
-	  struct gc_arena gc = gc_new ();
-	  struct argv argv = argv_new ();
-	  const char *dc_file;
+  if (mi->context.options.client_connect_script)
+    {
+      struct gc_arena gc = gc_new ();
+      struct argv argv = argv_new ();
+      const char *dc_file;
 
-	  setenv_str (mi->context.c2.es, "script_type", "client-connect");
+      setenv_str (mi->context.c2.es, "script_type", "client-connect");
 
-	  dc_file = create_temp_file (mi->context.options.tmp_dir, "cc", &gc);
-          if (!dc_file)
-	    {
-	      cc_succeeded = false;
-	      goto script_failed;
-	    }
+      dc_file = create_temp_file (mi->context.options.tmp_dir, "cc", &gc);
+      if (!dc_file)
+	{
+	  cc_succeeded = false;
+	  goto script_failed;
+	}
 
-	  argv_printf (&argv, "%sc %s",
-		       mi->context.options.client_connect_script,
-		       dc_file);
+      argv_printf (&argv, "%sc %s",
+		   mi->context.options.client_connect_script,
+		   dc_file);
 
-	  if (openvpn_run_script (&argv, mi->context.c2.es, 0, "--client-connect"))
-	    {
-	      multi_client_connect_post (m, mi, dc_file, option_types_found);
-	      ++cc_succeeded_count;
-	    }
-	  else
-	    cc_succeeded = false;
-script_failed:
-	  argv_reset (&argv);
-	  gc_free (&gc);
+      if (openvpn_run_script (&argv, mi->context.c2.es, 0, "--client-connect"))
+	{
+	  multi_client_connect_post (m, mi, dc_file, option_types_found);
+	  ++cc_succeeded_count;
 	}
+      else
+	cc_succeeded = false;
+script_failed:
+      argv_reset (&argv);
+      gc_free (&gc);
+    }
 }
 
 static void

@@ -1902,112 +1902,112 @@ multi_client_connect_late_setup (struct multi_context *m,
 				 int cc_succeeded,
 				 const int cc_succeeded_count)
 {
-      struct gc_arena gc = gc_new ();
-      ASSERT (mi->context.c1.tuntap);
+  struct gc_arena gc = gc_new ();
+  ASSERT (mi->context.c1.tuntap);
 
+  /*
+   * Check for "disable" directive in client-config-dir file
+   * or config file generated by --client-connect script.
+   */
+  if (mi->context.options.disable)
+    {
+      msg (D_MULTI_ERRORS, "MULTI: client has been rejected due to 'disable' directive");
+      cc_succeeded = false;
+    }
+
+  if (cc_succeeded)
+    {
       /*
-       * Check for "disable" directive in client-config-dir file
-       * or config file generated by --client-connect script.
+       * Process sourced options.
        */
-      if (mi->context.options.disable)
+      do_deferred_options (&mi->context, option_types_found);
+
+      /*
+       * make sure we got ifconfig settings from somewhere
+       */
+      if (!mi->context.c2.push_ifconfig_defined)
 	{
-	  msg (D_MULTI_ERRORS, "MULTI: client has been rejected due to 'disable' directive");
-	  cc_succeeded = false;
+	  msg (D_MULTI_ERRORS, "MULTI: no dynamic or static remote --ifconfig address is available for %s",
+	       multi_instance_string (mi, false, &gc));
 	}
 
-      if (cc_succeeded)
+      /*
+       * make sure that ifconfig settings comply with constraints
+       */
+      if (!ifconfig_push_constraint_satisfied (&mi->context))
 	{
-	  /*
-	   * Process sourced options.
-	   */
-	  do_deferred_options (&mi->context, option_types_found);
+	  /* JYFIXME -- this should cause the connection to fail */
+	  msg (D_MULTI_ERRORS, "MULTI ERROR: primary virtual IP for %s (%s) violates tunnel network/netmask constraint (%s/%s)",
+	       multi_instance_string (mi, false, &gc),
+	       print_in_addr_t (mi->context.c2.push_ifconfig_local, 0, &gc),
+	       print_in_addr_t (mi->context.options.push_ifconfig_constraint_network, 0, &gc),
+	       print_in_addr_t (mi->context.options.push_ifconfig_constraint_netmask, 0, &gc));
+	}
 
-	  /*
-	   * make sure we got ifconfig settings from somewhere
-	   */
-	  if (!mi->context.c2.push_ifconfig_defined)
+      /*
+       * For routed tunnels, set up internal route to endpoint
+       * plus add all iroute routes.
+       */
+      if (TUNNEL_TYPE (mi->context.c1.tuntap) == DEV_TYPE_TUN)
+	{
+	  if (mi->context.c2.push_ifconfig_defined)
 	    {
-	      msg (D_MULTI_ERRORS, "MULTI: no dynamic or static remote --ifconfig address is available for %s",
-		   multi_instance_string (mi, false, &gc));
+	      multi_learn_in_addr_t (m, mi, mi->context.c2.push_ifconfig_local, -1, true);
+	      msg (D_MULTI_LOW, "MULTI: primary virtual IP for %s: %s",
+		   multi_instance_string (mi, false, &gc),
+		   print_in_addr_t (mi->context.c2.push_ifconfig_local, 0, &gc));
 	    }
 
-	  /*
-	   * make sure that ifconfig settings comply with constraints
-	   */
-	  if (!ifconfig_push_constraint_satisfied (&mi->context))
+	  if (mi->context.c2.push_ifconfig_ipv6_defined)
 	    {
-	      /* JYFIXME -- this should cause the connection to fail */
-	      msg (D_MULTI_ERRORS, "MULTI ERROR: primary virtual IP for %s (%s) violates tunnel network/netmask constraint (%s/%s)",
+	      multi_learn_in6_addr (m, mi, mi->context.c2.push_ifconfig_ipv6_local, -1, true);
+	      /* TODO: find out where addresses are "unlearned"!! */
+	      msg (D_MULTI_LOW, "MULTI: primary virtual IPv6 for %s: %s",
 		   multi_instance_string (mi, false, &gc),
-		   print_in_addr_t (mi->context.c2.push_ifconfig_local, 0, &gc),
-		   print_in_addr_t (mi->context.options.push_ifconfig_constraint_network, 0, &gc),
-		   print_in_addr_t (mi->context.options.push_ifconfig_constraint_netmask, 0, &gc));
+		   print_in6_addr (mi->context.c2.push_ifconfig_ipv6_local, 0, &gc));
 	    }
 
+	  /* add routes locally, pointing to new client, if
+	     --iroute options have been specified */
+	  multi_add_iroutes (m, mi);
+
 	  /*
-	   * For routed tunnels, set up internal route to endpoint
-	   * plus add all iroute routes.
+	   * iroutes represent subnets which are "owned" by a particular
+	   * client.  Therefore, do not actually push a route to a client
+	   * if it matches one of the client's iroutes.
 	   */
-	  if (TUNNEL_TYPE (mi->context.c1.tuntap) == DEV_TYPE_TUN)
-	    {
-	      if (mi->context.c2.push_ifconfig_defined)
-		{
-		  multi_learn_in_addr_t (m, mi, mi->context.c2.push_ifconfig_local, -1, true);
-		  msg (D_MULTI_LOW, "MULTI: primary virtual IP for %s: %s",
-		       multi_instance_string (mi, false, &gc),
-		       print_in_addr_t (mi->context.c2.push_ifconfig_local, 0, &gc));
-		}
-
-	      if (mi->context.c2.push_ifconfig_ipv6_defined)
-		{
-		  multi_learn_in6_addr (m, mi, mi->context.c2.push_ifconfig_ipv6_local, -1, true);
-		  /* TODO: find out where addresses are "unlearned"!! */
-		  msg (D_MULTI_LOW, "MULTI: primary virtual IPv6 for %s: %s",
-		       multi_instance_string (mi, false, &gc),
-		       print_in6_addr (mi->context.c2.push_ifconfig_ipv6_local, 0, &gc));
-		}
-
-	      /* add routes locally, pointing to new client, if
-		 --iroute options have been specified */
-	      multi_add_iroutes (m, mi);
-
-	      /*
-	       * iroutes represent subnets which are "owned" by a particular
-	       * client.  Therefore, do not actually push a route to a client
-	       * if it matches one of the client's iroutes.
-	       */
-	      remove_iroutes_from_push_route_list (&mi->context.options);
-	    }
-	  else if (mi->context.options.iroutes)
-	    {
-	      msg (D_MULTI_ERRORS, "MULTI: --iroute options rejected for %s -- iroute only works with tun-style tunnels",
-		   multi_instance_string (mi, false, &gc));
-	    }
-
-	  /* set our client's VPN endpoint for status reporting purposes */
-	  mi->reporting_addr = mi->context.c2.push_ifconfig_local;
-	  mi->reporting_addr_ipv6 = mi->context.c2.push_ifconfig_ipv6_local;
-
-	  /* set context-level authentication flag */
-	  mi->context.c2.context_auth = CAS_SUCCEEDED;
+	  remove_iroutes_from_push_route_list (&mi->context.options);
 	}
-      else
+      else if (mi->context.options.iroutes)
 	{
-	  /* set context-level authentication flag */
-	  mi->context.c2.context_auth = cc_succeeded_count ? CAS_PARTIAL : CAS_FAILED;
+	  msg (D_MULTI_ERRORS, "MULTI: --iroute options rejected for %s -- iroute only works with tun-style tunnels",
+	       multi_instance_string (mi, false, &gc));
 	}
 
-      /* increment number of current authenticated clients */
-      ++m->n_clients;
-      update_mstat_n_clients(m->n_clients);
-      --mi->n_clients_delta;
+      /* set our client's VPN endpoint for status reporting purposes */
+      mi->reporting_addr = mi->context.c2.push_ifconfig_local;
+      mi->reporting_addr_ipv6 = mi->context.c2.push_ifconfig_ipv6_local;
+
+      /* set context-level authentication flag */
+      mi->context.c2.context_auth = CAS_SUCCEEDED;
+    }
+  else
+    {
+      /* set context-level authentication flag */
+      mi->context.c2.context_auth = cc_succeeded_count ? CAS_PARTIAL : CAS_FAILED;
+    }
+
+  /* increment number of current authenticated clients */
+  ++m->n_clients;
+  update_mstat_n_clients(m->n_clients);
+  --mi->n_clients_delta;
 
 #ifdef MANAGEMENT_DEF_AUTH
-      if (management)
-	management_connection_established (management, &mi->context.c2.mda_context, mi->context.c2.es);
+  if (management)
+    management_connection_established (management, &mi->context.c2.mda_context, mi->context.c2.es);
 #endif
 
-      gc_free (&gc);
+  gc_free (&gc);
 }

Signature · 1 line

2.1.1

Fabian Knittel

2014-10-10 22:09 UTC #5 lore raw

PATCH 3/9 client-connect: Refactor multi_client_connect_source_ccd

In reply to Fabian Knittel

Refactor multi_client_connect_source_ccd(), so that options_server_import() (or the success path in general) is only entered in one place within the function.

Signed-off-by Fabian Knittel <[email protected]...>

---
 src/openvpn/multi.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

src/openvpn/multi.c +13 ~~-16~~

index 23b86a5..757e687 100644
@@ -1721,7 +1721,19 @@ multi_client_connect_source_ccd (struct multi_context *m,
 			   &gc);
 
       /* try common-name file */
-      if (test_file (ccd_file))
+      if (!test_file (ccd_file))
+	ccd_file = NULL;
+
+      if (!ccd_file)
+	{
+	  ccd_file = gen_path (mi->context.options.client_config_dir,
+			       CCD_DEFAULT, &gc);
+	  /* try default file */
+	  if (!test_file (ccd_file))
+	    ccd_file = NULL;
+	}
+
+      if (ccd_file)
 	{
 	  options_server_import (&mi->context.options,
 				 ccd_file,

@@ -1729,22 +1741,7 @@ multi_client_connect_source_ccd (struct multi_context *m,
 				 CLIENT_CONNECT_OPT_MASK,
 				 option_types_found,
 				 mi->context.c2.es);
-	}
-      else /* try default file */
-	{
-	  ccd_file = gen_path (mi->context.options.client_config_dir,
-			       CCD_DEFAULT,
-			       &gc);
 
-	  if (test_file (ccd_file))
-	    {
-	      options_server_import (&mi->context.options,
-				     ccd_file,
-				     D_IMPORT_ERRORS|M_OPTERR,
-				     CLIENT_CONNECT_OPT_MASK,
-				     option_types_found,
-				     mi->context.c2.es);
-	    }
 	}
 
       gc_free (&gc);

Signature · 1 line

2.1.1

Fabian Knittel

2014-10-10 22:09 UTC #6 lore raw

PATCH 4/9 client-connect: Move multi_client_connect_setenv into early_setup

In reply to Fabian Knittel

This patch moves multi_client_connect_setenv into multi_client_connect_early_setup and makes sure that every client-connect handling function updates the virtual address selection.

Background: This unifies how the client-connect handling functions work.

Signed-off-by Fabian Knittel <[email protected]...>

---
 src/openvpn/multi.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

src/openvpn/multi.c +10 -9

index 757e687..a34f985 100644
@@ -1635,15 +1635,6 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 
       multi_client_connect_source_ccd (m, mi, &option_types_found);
 
-      /*
-       * Select a virtual address from either --ifconfig-push in
-       * --client-config-dir file or --ifconfig-pool.
-       */
-      multi_select_virtual_addr (m, mi);
-
-      /* do --client-connect setenvs */
-      multi_client_connect_setenv (m, mi);
-
       multi_client_connect_call_plugin_v1 (m, mi, &option_types_found,
 					   &cc_succeeded, &cc_succeeded_count);

@@ -1700,6 +1691,10 @@ multi_client_connect_early_setup (struct multi_context *m,
 
   /* reset pool handle to null */
   mi->vaddr_handle = -1;
+
+  /* do --client-connect setenvs */
+  multi_select_virtual_addr (m, mi);
+  multi_client_connect_setenv (m, mi);
 }
 
 /*

@@ -1742,6 +1737,12 @@ multi_client_connect_source_ccd (struct multi_context *m,
 				 option_types_found,
 				 mi->context.c2.es);
 
+	  /*
+	   * Select a virtual address from either --ifconfig-push in
+	   * --client-config-dir file or --ifconfig-pool.
+	   */
+	  multi_select_virtual_addr (m, mi);
+	  multi_set_virtual_addr_env (m, mi);
 	}
 
       gc_free (&gc);

Signature · 1 line

2.1.1

Fabian Knittel

2014-10-10 22:09 UTC #7 lore raw

PATCH 5/9 client-connect: Refactor to use return values instead of modifying a passed-in flag

In reply to Fabian Knittel

This patch changes the way the client-connect helper functions communicate with the main function. Instead of updating cc_succeeded and cc_succeeded_count, they now return either CC_RET_SUCCEEDED, CC_RET_FAILED or CC_RET_SKIPPED.

In addition, the client-connect helpers are now called in completely identical ways. This is in preparation of handling the helpers as simple call-backs.

Signed-off-by Fabian Knittel <[email protected]...>

---
 src/openvpn/multi.c | 122 +++++++++++++++++++++++++++++++---------------------
 src/openvpn/multi.h |  11 ++++-
 2 files changed, 82 insertions(+), 51 deletions(-)

src/openvpn/multi.c +72 ~~-50~~

index a34f985..4ead41e 100644
@@ -1519,13 +1519,12 @@ multi_client_connect_post_plugin (struct multi_context *m,
 /*
  * Called to load management-derived client-connect config
  */
-static void
+static enum client_connect_return
 multi_client_connect_mda (struct multi_context *m,
 			  struct multi_instance *mi,
-			  unsigned int *option_types_found,
-			  int *cc_succeeded,
-			  int *cc_succeeded_count)
+			  unsigned int *option_types_found)
 {
+  enum client_connect_return ret = CC_RET_SKIPPED;
 #ifdef MANAGEMENT_DEF_AUTH
   if (mi->cc_config)
     {

@@ -1551,9 +1550,10 @@ multi_client_connect_mda (struct multi_context *m,
       multi_select_virtual_addr (m, mi);
       multi_set_virtual_addr_env (m, mi);
 
-      ++*cc_succeeded_count;
+      ret = CC_RET_SUCCEEDED;
     }
 #endif
+  return ret;
 }
 
 static void

@@ -1630,38 +1630,59 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
       unsigned int option_types_found = 0;
       int cc_succeeded = true; /* client connect script status */
       int cc_succeeded_count = 0;
+      enum client_connect_return ret;
 
       multi_client_connect_early_setup (m, mi);
 
-      multi_client_connect_source_ccd (m, mi, &option_types_found);
+      if (cc_succeeded)
+	{
+	  ret = multi_client_connect_source_ccd (m, mi, &option_types_found);
+	  if (ret == CC_RET_SUCCEEDED)
+	    ++cc_succeeded_count;
+	  else if (ret == CC_RET_FAILED)
+	    cc_succeeded = false;
+	}
 
-      multi_client_connect_call_plugin_v1 (m, mi, &option_types_found,
-					   &cc_succeeded, &cc_succeeded_count);
+      if (cc_succeeded)
+	{
+	  ret = multi_client_connect_call_plugin_v1 (m, mi,
+						     &option_types_found);
+	  if (ret == CC_RET_SUCCEEDED)
+	    ++cc_succeeded_count;
+	  else if (ret == CC_RET_FAILED)
+	    cc_succeeded = false;
+	}
 
-      multi_client_connect_call_plugin_v2 (m, mi, &option_types_found,
-					   &cc_succeeded, &cc_succeeded_count);
+      if (cc_succeeded)
+	{
+	  ret = multi_client_connect_call_plugin_v2 (m, mi,
+						     &option_types_found);
+	  if (ret == CC_RET_SUCCEEDED)
+	    ++cc_succeeded_count;
+	  else if (ret == CC_RET_FAILED)
+	    cc_succeeded = false;
+	}
 
-      /*
-       * Run --client-connect script.
-       */
       if (cc_succeeded)
 	{
-	  multi_client_connect_call_script (m, mi, &option_types_found,
-					    &cc_succeeded,
-					    &cc_succeeded_count);
+	  ret = multi_client_connect_call_script (m, mi, &option_types_found);
+	  if (ret == CC_RET_SUCCEEDED)
+	    ++cc_succeeded_count;
+	  else if (ret == CC_RET_FAILED)
+	    cc_succeeded = false;
 	}
 
-      /*
-       * Check for client-connect script left by management interface client
-       */
       if (cc_succeeded)
 	{
-	  multi_client_connect_mda (m, mi, &option_types_found,
-				    &cc_succeeded, &cc_succeeded_count);
+	  ret = multi_client_connect_mda (m, mi, &option_types_found);
+	  if (ret == CC_RET_SUCCEEDED)
+	    ++cc_succeeded_count;
+	  else if (ret == CC_RET_FAILED)
+	    cc_succeeded = false;
 	}
 
-      multi_client_connect_late_setup (m, mi, option_types_found,
-				       cc_succeeded, cc_succeeded_count);
+      multi_client_connect_late_setup (m, mi, option_types_found, cc_succeeded,
+				       cc_succeeded_count);
 
       /* set flag so we don't get called again */
       mi->connection_established_flag = true;

@@ -1701,11 +1722,13 @@ multi_client_connect_early_setup (struct multi_context *m,
  * Try to source a dynamic config file from the
  * --client-config-dir directory.
  */
-static void
+static enum client_connect_return
 multi_client_connect_source_ccd (struct multi_context *m,
 				 struct multi_instance *mi,
 				 unsigned int *option_types_found)
 {
+  enum client_connect_return ret = CC_RET_SKIPPED;
+
   if (mi->context.options.client_config_dir)
     {
       struct gc_arena gc = gc_new ();

@@ -1743,10 +1766,14 @@ multi_client_connect_source_ccd (struct multi_context *m,
 	   */
 	  multi_select_virtual_addr (m, mi);
 	  multi_set_virtual_addr_env (m, mi);
+
+	  ret = CC_RET_SUCCEEDED;
 	}
 
       gc_free (&gc);
     }
+
+  return ret;
 }
 
 /*

@@ -1754,19 +1781,16 @@ multi_client_connect_source_ccd (struct multi_context *m,
  *
  * deprecated callback, use a file for passing back return info
  */
-static void
+static enum client_connect_return
 multi_client_connect_call_plugin_v1 (struct multi_context *m,
 				     struct multi_instance *mi,
-				     unsigned int *option_types_found,
-				     int *cc_succeeded,
-				     int *cc_succeeded_count)
+				     unsigned int *option_types_found)
 {
+  enum client_connect_return ret = CC_RET_SKIPPED;
 #ifdef ENABLE_PLUGIN
   ASSERT (m);
   ASSERT (mi);
   ASSERT (option_types_found);
-  ASSERT (cc_succeeded);
-  ASSERT (cc_succeeded_count);
 
   if (plugin_defined (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT))
     {

@@ -1777,7 +1801,7 @@ multi_client_connect_call_plugin_v1 (struct multi_context *m,
 
       if (!dc_file)
 	{
-	  *cc_succeeded = false;
+	  ret = CC_RET_FAILED;
 	  goto script_depr_failed;
 	}

@@ -1790,17 +1814,18 @@ multi_client_connect_call_plugin_v1 (struct multi_context *m,
       if (plug_ret != OPENVPN_PLUGIN_FUNC_SUCCESS)
 	{
 	  msg (M_WARN, "WARNING: client-connect plugin call failed");
-	  *cc_succeeded = false;
+	  ret = CC_RET_FAILED;
 	}
       else
 	{
 	  multi_client_connect_post (m, mi, dc_file, option_types_found);
-	  ++*cc_succeeded_count;
+	  ret = CC_RET_SUCCEEDED;
 	}
 script_depr_failed:
       gc_free (&gc);
     }
 #endif
+  return ret;
 }
 
 /*

@@ -1808,19 +1833,16 @@ script_depr_failed:
  *
  * V2 callback, use a plugin_return struct for passing back return info
  */
-static void
+static enum client_connect_return
 multi_client_connect_call_plugin_v2 (struct multi_context *m,
 				     struct multi_instance *mi,
-				     unsigned int *option_types_found,
-				     int *cc_succeeded,
-				     int *cc_succeeded_count)
+				     unsigned int *option_types_found)
 {
+  enum client_connect_return ret = CC_RET_SKIPPED;
 #ifdef ENABLE_PLUGIN
   ASSERT (m);
   ASSERT (mi);
   ASSERT (option_types_found);
-  ASSERT (cc_succeeded);
-  ASSERT (cc_succeeded_count);
 
   if (plugin_defined (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT_V2))
     {

@@ -1835,31 +1857,29 @@ multi_client_connect_call_plugin_v2 (struct multi_context *m,
       if (plug_ret != OPENVPN_PLUGIN_FUNC_SUCCESS)
 	{
 	  msg (M_WARN, "WARNING: client-connect-v2 plugin call failed");
-	  *cc_succeeded = false;
+	  ret = CC_RET_FAILED;
 	}
       else
 	{
 	  multi_client_connect_post_plugin (m, mi, &pr, option_types_found);
-	  ++*cc_succeeded_count;
+	  ret = CC_RET_SUCCEEDED;
 	}
 
       plugin_return_free (&pr);
     }
 #endif
+  return ret;
 }
 
-static void
+static enum client_connect_return
 multi_client_connect_call_script (struct multi_context *m,
 				  struct multi_instance *mi,
-				  unsigned int *option_types_found,
-				  int *cc_succeeded,
-				  int *cc_succeeded_count)
+				  unsigned int *option_types_found)
 {
+  enum client_connect_return ret = CC_RET_SKIPPED;
   ASSERT (m);
   ASSERT (mi);
   ASSERT (option_types_found);
-  ASSERT (cc_succeeded);
-  ASSERT (cc_succeeded_count);
 
   if (mi->context.options.client_connect_script)
     {

@@ -1872,7 +1892,7 @@ multi_client_connect_call_script (struct multi_context *m,
       dc_file = create_temp_file (mi->context.options.tmp_dir, "cc", &gc);
       if (!dc_file)
 	{
-	  cc_succeeded = false;
+	  ret = CC_RET_FAILED;
 	  goto script_failed;
 	}

@@ -1883,14 +1903,16 @@ multi_client_connect_call_script (struct multi_context *m,
       if (openvpn_run_script (&argv, mi->context.c2.es, 0, "--client-connect"))
 	{
 	  multi_client_connect_post (m, mi, dc_file, option_types_found);
-	  ++cc_succeeded_count;
+	  ret = CC_RET_SUCCEEDED;
 	}
       else
-	cc_succeeded = false;
+	ret = CC_RET_FAILED;
+
 script_failed:
       argv_reset (&argv);
       gc_free (&gc);
     }
+  return ret;
 }
 
 static void

src/openvpn/multi.h +10 -1

index fc2ffb2..0adcd47 100644
@@ -105,7 +105,6 @@ struct multi_instance {
                                  *   for this VPN tunnel. */
 };
 
-
 /**
  * Main OpenVPN server state structure.
  *

@@ -169,6 +168,16 @@ struct multi_context {
   struct event_timeout stale_routes_check_et;
 };
 
+/**
+ * Return values used by the client connect call-back functions.
+ */
+enum client_connect_return
+{
+  CC_RET_FAILED,
+  CC_RET_SUCCEEDED,
+  CC_RET_SKIPPED
+};
+
 /*
  * Host route
  */

Signature · 1 line

2.1.1

Fabian Knittel

2014-10-10 22:09 UTC #8 lore raw

PATCH 6/9 client-connect: Refactor client-connect handling to calling a bunch of hooks in a loop

In reply to Fabian Knittel

This patch changes the calling of the client-connect functions into an array of hooks and a block of code that calls them in a loop.

Signed-off-by Fabian Knittel <[email protected]...>

---
 src/openvpn/multi.c | 57 +++++++++++++++--------------------------------------
 1 file changed, 16 insertions(+), 41 deletions(-)

src/openvpn/multi.c +16 ~~-41~~

index 4ead41e..7815146 100644
@@ -1627,6 +1627,18 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 {
   if (tls_authentication_status (mi->context.c2.tls_multi, 0) == TLS_AUTHENTICATION_SUCCEEDED)
     {
+      typedef enum client_connect_return_t (*multi_client_connect_handler)(struct multi_context *m, struct multi_instance *mi, unsigned int *option_types_found);
+
+      multi_client_connect_handler handlers[] = {
+	  multi_client_connect_source_ccd,
+	  multi_client_connect_call_plugin_v1,
+	  multi_client_connect_call_plugin_v2,
+	  multi_client_connect_call_script,
+	  multi_client_connect_mda,
+	  NULL
+      };
+
+      int cur_handler = 0;
       unsigned int option_types_found = 0;
       int cc_succeeded = true; /* client connect script status */
       int cc_succeeded_count = 0;

@@ -1634,51 +1646,14 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 
       multi_client_connect_early_setup (m, mi);
 
-      if (cc_succeeded)
+      while (succeeded && handlers[cur_handler])
 	{
-	  ret = multi_client_connect_source_ccd (m, mi, &option_types_found);
+	  ret = handlers[cur_handler] (m, mi, &option_types_found);
 	  if (ret == CC_RET_SUCCEEDED)
 	    ++cc_succeeded_count;
 	  else if (ret == CC_RET_FAILED)
-	    cc_succeeded = false;
-	}
-
-      if (cc_succeeded)
-	{
-	  ret = multi_client_connect_call_plugin_v1 (m, mi,
-						     &option_types_found);
-	  if (ret == CC_RET_SUCCEEDED)
-	    ++cc_succeeded_count;
-	  else if (ret == CC_RET_FAILED)
-	    cc_succeeded = false;
-	}
-
-      if (cc_succeeded)
-	{
-	  ret = multi_client_connect_call_plugin_v2 (m, mi,
-						     &option_types_found);
-	  if (ret == CC_RET_SUCCEEDED)
-	    ++cc_succeeded_count;
-	  else if (ret == CC_RET_FAILED)
-	    cc_succeeded = false;
-	}
-
-      if (cc_succeeded)
-	{
-	  ret = multi_client_connect_call_script (m, mi, &option_types_found);
-	  if (ret == CC_RET_SUCCEEDED)
-	    ++cc_succeeded_count;
-	  else if (ret == CC_RET_FAILED)
-	    cc_succeeded = false;
-	}
-
-      if (cc_succeeded)
-	{
-	  ret = multi_client_connect_mda (m, mi, &option_types_found);
-	  if (ret == CC_RET_SUCCEEDED)
-	    ++cc_succeeded_count;
-	  else if (ret == CC_RET_FAILED)
-	    cc_succeeded = false;
+	    succeeded = false;
+	  ++cur_handler;
 	}
 
       multi_client_connect_late_setup (m, mi, option_types_found, cc_succeeded,

Signature · 1 line

2.1.1

Fabian Knittel

2014-10-10 22:09 UTC #9 lore raw

PATCH 7/9 client-connect: Add CC_RET_DEFERRED and cope with deferred client-connect

In reply to Fabian Knittel

This patch moves the state, that was previously tracked within the multi_connection_established() function, into struct client_connect_state. The multi_connection_established() function can now be exited and re-entered as many times as necessary - without losing the client-connect handling state.

The patch also adds the new return value CC_RET_DEFERRED which indicates that the handler couldn't complete immediately, and needs to be called later. At that point multi_connection_established() will exit without indicating completion.

Each client-connect handler now has an (optional) additional call-back: The call-back for handling the deferred case. If the main call-back returns CC_RET_DEFERRED, the next call to the handler will be through the deferred call-back.

Signed-off-by Fabian Knittel <[email protected]...>

---
 src/openvpn/multi.c | 255 ++++++++++++++++++++++++++++++++++------------------
 src/openvpn/multi.h |  18 ++++
 2 files changed, 185 insertions(+), 88 deletions(-)

src/openvpn/multi.c +167 ~~-88~~

index 7815146..c044d67 100644
@@ -47,6 +47,9 @@
 
 /*#define MULTI_DEBUG_EVENT_LOOP*/
 
+static void delete_client_connect_state (struct multi_instance *mi);
+
+
 #ifdef MULTI_DEBUG_EVENT_LOOP
 static const char *
 id (struct multi_instance *mi)

@@ -573,6 +576,8 @@ multi_close_instance (struct multi_context *m,
   set_cc_config (mi, NULL);
 #endif
 
+  delete_client_connect_state (mi);
+
   multi_client_disconnect_script (m, mi);
 
   if (mi->did_open_context)

@@ -1583,94 +1588,6 @@ multi_client_connect_setenv (struct multi_context *m,
 
 static void
 multi_client_connect_early_setup (struct multi_context *m,
-				  struct multi_instance *mi);
-static void
-multi_client_connect_source_ccd (struct multi_context *m,
-				 struct multi_instance *mi,
-				 unsigned int *option_types_found);
-static void
-multi_client_connect_call_plugin_v1 (struct multi_context *m,
-				     struct multi_instance *mi,
-				     unsigned int *option_types_found,
-				     int *cc_succeeded,
-				     int *cc_succeeded_count);
-static void
-multi_client_connect_call_plugin_v2 (struct multi_context *m,
-				     struct multi_instance *mi,
-				     unsigned int *option_types_found,
-				     int *cc_succeeded,
-				     int *cc_succeeded_count);
-static void
-multi_client_connect_call_script (struct multi_context *m,
-				  struct multi_instance *mi,
-				  unsigned int *option_types_found,
-				  int *cc_succeeded,
-				  int *cc_succeeded_count);
-static void
-multi_client_connect_late_setup (struct multi_context *m,
-				 struct multi_instance *mi,
-				 const unsigned int option_types_found,
-				 int cc_succeeded,
-				 const int cc_succeeded_count);
-
-/*
- * Called as soon as the SSL/TLS connection authenticates.
- *
- * Instance-specific directives to be processed:
- *
- *   iroute start-ip end-ip
- *   ifconfig-push local remote-netmask
- *   push
- */
-static void
-multi_connection_established (struct multi_context *m, struct multi_instance *mi)
-{
-  if (tls_authentication_status (mi->context.c2.tls_multi, 0) == TLS_AUTHENTICATION_SUCCEEDED)
-    {
-      typedef enum client_connect_return_t (*multi_client_connect_handler)(struct multi_context *m, struct multi_instance *mi, unsigned int *option_types_found);
-
-      multi_client_connect_handler handlers[] = {
-	  multi_client_connect_source_ccd,
-	  multi_client_connect_call_plugin_v1,
-	  multi_client_connect_call_plugin_v2,
-	  multi_client_connect_call_script,
-	  multi_client_connect_mda,
-	  NULL
-      };
-
-      int cur_handler = 0;
-      unsigned int option_types_found = 0;
-      int cc_succeeded = true; /* client connect script status */
-      int cc_succeeded_count = 0;
-      enum client_connect_return ret;
-
-      multi_client_connect_early_setup (m, mi);
-
-      while (succeeded && handlers[cur_handler])
-	{
-	  ret = handlers[cur_handler] (m, mi, &option_types_found);
-	  if (ret == CC_RET_SUCCEEDED)
-	    ++cc_succeeded_count;
-	  else if (ret == CC_RET_FAILED)
-	    succeeded = false;
-	  ++cur_handler;
-	}
-
-      multi_client_connect_late_setup (m, mi, option_types_found, cc_succeeded,
-				       cc_succeeded_count);
-
-      /* set flag so we don't get called again */
-      mi->connection_established_flag = true;
-    }
-
-  /*
-   * Reply now to client's PUSH_REQUEST query
-   */
-  mi->context.c2.push_reply_deferred = false;
-}
-
-static void
-multi_client_connect_early_setup (struct multi_context *m,
 				  struct multi_instance *mi)
 {
   /* lock down the common name and cert hashes so they can't change during

@@ -2005,6 +1922,168 @@ multi_client_connect_late_setup (struct multi_context *m,
   gc_free (&gc);
 }
 
+static enum client_connect_return
+multi_client_connect_fail (struct multi_context *m, struct multi_instance *mi,
+			   unsigned int *option_types_found)
+{
+  /* Called null call-back.  This should never happen.  Fail loudly. */
+  return CC_RET_FAILED;
+}
+
+static void
+new_client_connect_state (struct multi_instance *mi)
+{
+  ASSERT (mi);
+  ALLOC_OBJ_CLEAR (mi->client_connect_state, struct client_connect_state);
+
+  mi->client_connect_state->succeeded = true;
+}
+
+static void
+delete_client_connect_state (struct multi_instance *mi)
+{
+  ASSERT (mi);
+  if (mi->client_connect_state)
+    {
+      free (mi->client_connect_state);
+      mi->client_connect_state = NULL;
+    }
+}
+
+typedef enum client_connect_return (*client_connect_handler)
+  (struct multi_context *m, struct multi_instance *mi,
+   unsigned int *option_types_found);
+
+struct client_connect_handlers
+{
+  client_connect_handler main;
+  client_connect_handler deferred;
+};
+
+static const struct client_connect_handlers client_connect_handlers[] = {
+  {
+    main: multi_client_connect_source_ccd,
+    deferred: multi_client_connect_fail
+  },
+  {
+    main: multi_client_connect_call_plugin_v1,
+    deferred: multi_client_connect_fail
+  },
+  {
+    main: multi_client_connect_call_plugin_v2,
+    deferred: multi_client_connect_fail
+  },
+  {
+    main: multi_client_connect_call_script,
+    deferred: multi_client_connect_fail
+  },
+  {
+    main: multi_client_connect_mda,
+    deferred: multi_client_connect_fail
+  },
+  {
+    /* End of list.  */
+  }
+};
+
+static enum client_connect_return
+multi_client_handle_single (client_connect_handler handler,
+			    struct multi_context *m,
+			    struct multi_instance *mi)
+{
+  struct client_connect_state *ccs = mi->client_connect_state;
+  enum client_connect_return ret;
+
+  ret = handler (m, mi, &ccs->option_types_found);
+  if (ret == CC_RET_SUCCEEDED)
+    ++ccs->succeeded_count;
+  else if (ret == CC_RET_FAILED)
+    ccs->succeeded = false;
+  else if (ret == CC_RET_DEFERRED)
+    {
+      /* Signal that we're deferring.  */
+      return ret;
+    }
+  return ret;
+}
+
+/*
+ * Called as soon as the SSL/TLS connection authenticates.
+ *
+ * Instance-specific directives to be processed:
+ *
+ *   iroute start-ip end-ip
+ *   ifconfig-push local remote-netmask
+ *   push
+ */
+static void
+multi_connection_established (struct multi_context *m, struct multi_instance *mi)
+{
+  if (tls_authentication_status (mi->context.c2.tls_multi, 0) == TLS_AUTHENTICATION_SUCCEEDED)
+    {
+      struct client_connect_state *ccs = mi->client_connect_state;
+      enum client_connect_return ret;
+
+      if (!ccs)
+	{
+	  /* This is the first time that we run client-connect handlers
+	     for this connection. */
+
+	  new_client_connect_state (mi);
+	  ccs = mi->client_connect_state;
+
+          multi_client_connect_early_setup (m, mi);
+	}
+      else
+	{
+	  /* We are returning from a deferred handler.  Check whether the
+	     handler has been completed yet.  */
+
+	  ASSERT (client_connect_handlers[ccs->cur_handler_idx].main);
+	  ASSERT (client_connect_handlers[ccs->cur_handler_idx].deferred);
+
+	  ret = multi_client_handle_single
+		  (client_connect_handlers[ccs->cur_handler_idx].deferred, m,
+		   mi);
+	  if (ret == CC_RET_DEFERRED)
+	    return;
+
+	  /* Proceed to next handler.  */
+	  ++ccs->cur_handler_idx;
+	}
+
+
+      /* Cycle through all (remaining) client-connect handlers until all
+	 have completed, one of them fails or one of them defers.  */
+      while (ccs->succeeded &&
+	     client_connect_handlers[ccs->cur_handler_idx].main)
+	{
+	  ret = multi_client_handle_single
+		  (client_connect_handlers[ccs->cur_handler_idx].main, m, mi);
+	  if (ret == CC_RET_DEFERRED)
+	    return;
+
+	  /* Proceed to next handler.  */
+	  ++ccs->cur_handler_idx;
+	}
+
+      /* Done with all client-connect handlers now.  */
+
+      multi_client_connect_late_setup (m, mi, ccs->option_types_found,
+				       ccs->succeeded, ccs->succeeded_count);
+
+      /* Clean-up.  */
+      delete_client_connect_state (mi);
+
+      /* set flag so we don't get called again */
+      mi->connection_established_flag = true;
+    }
+
+  /*
+   * Reply now to client's PUSH_REQUEST query
+   */
+  mi->context.c2.push_reply_deferred = false;
+}
 
 /*
  * Add a mbuf buffer to a particular

src/openvpn/multi.h +18 -0

index 0adcd47..16dd99d 100644
@@ -54,6 +54,22 @@ struct multi_reap
   time_t last_call;
 };
 
+/**
+ * Detached client connection state.  This is the state that is tracked while
+ * the client connect hooks are executed.
+ */
+struct client_connect_state
+{
+  /* Index of currently executed handler.  */
+  int cur_handler_idx;
+  /* Did all of the handlers succeed up to now?  */
+  bool succeeded;
+  /* How many handlers succeeded?  */
+  int succeeded_count;
+  /* Remember which option classes where processed for delayed option
+     handling. */
+  unsigned int option_types_found;
+};
 
 /**
  * Server-mode state structure for one single VPN tunnel.

@@ -103,6 +119,7 @@ struct multi_instance {
 
   struct context context;       /**< The context structure storing state
                                  *   for this VPN tunnel. */
+  struct client_connect_state *client_connect_state;
 };
 
 /**

@@ -175,6 +192,7 @@ enum client_connect_return
 {
   CC_RET_FAILED,
   CC_RET_SUCCEEDED,
+  CC_RET_DEFERRED,
   CC_RET_SKIPPED
 };

Signature · 1 line

2.1.1

Fabian Knittel

2014-10-10 22:09 UTC #10 lore raw

PATCH 8/9 client-connect: Add deferred support to the client-connect script handler

In reply to Fabian Knittel

This patch introduces the concept of a return value file for the client-connect handlers. (This is very similar to the auth value file used during deferred authentication.) The file name is stored in the client_connect_state struct.

In addition, the patch also allows the storage of the client config file name in struct client_connect_state.

Both changes are used by the client-connect script handler to support deferred client-connection handling. The deferred return value file (deferred_ret_file) is passed to the actual script via the environment. If the script succeeds and writes the value for deferral into the deferred_ret_file, the handler knows to indicate deferral. Later on, the deferred handler checks whether the value of the deferred_ret_file has been updated to success or failure.

Signed-off-by Fabian Knittel <[email protected]...>

---
 src/openvpn/multi.c | 177 +++++++++++++++++++++++++++++++++++++++++++++++++---
 src/openvpn/multi.h |   3 +
 2 files changed, 171 insertions(+), 9 deletions(-)

src/openvpn/multi.c +168 -9

index c044d67..a3d94fa 100644
@@ -1610,6 +1610,125 @@ multi_client_connect_early_setup (struct multi_context *m,
   multi_client_connect_setenv (m, mi);
 }
 
+
+static void
+ccs_delete_deferred_ret_file (struct multi_instance *mi)
+{
+  struct client_connect_state *ccs = mi->client_connect_state;
+  if (ccs->deferred_ret_file)
+    {
+      setenv_del (mi->context.c2.es, "client_connect_deferred_file");
+      platform_unlink (ccs->deferred_ret_file);
+      free (ccs->deferred_ret_file);
+      ccs->deferred_ret_file = NULL;
+    }
+}
+
+static bool
+ccs_gen_deferred_ret_file (struct multi_instance *mi)
+{
+  struct client_connect_state *ccs = mi->client_connect_state;
+  struct gc_arena gc = gc_new ();
+  const char *fn;
+
+  if (ccs->deferred_ret_file)
+    ccs_delete_deferred_ret_file (mi);
+
+  fn = create_temp_file (mi->context.options.tmp_dir, "ccr", &gc);
+  if (!fn)
+    {
+      gc_free (&gc);
+      return false;
+    }
+  ccs->deferred_ret_file = string_alloc (fn, NULL);
+
+  setenv_str (mi->context.c2.es, "client_connect_deferred_file",
+	      ccs->deferred_ret_file);
+
+  gc_free (&gc);
+  return true;
+}
+
+/*
+ * Tests whether the deferred return value file exists and returns the
+ * contained return value.
+ *
+ * Returns CC_RET_SKIPPED if the file doesn't exist or is empty.
+ * Returns CC_RET_DEFERRED, CC_RET_SUCCEEDED or CC_RET_FAILED depending on
+ * the value stored in the file.
+ */
+static enum client_connect_return
+ccs_test_deferred_ret_file (struct multi_instance *mi)
+{
+  struct client_connect_state *ccs = mi->client_connect_state;
+  enum client_connect_return ret = CC_RET_SKIPPED;
+  FILE *fp = fopen (ccs->deferred_ret_file, "r");
+  if (fp)
+    {
+      const int c = fgetc (fp);
+      switch (c)
+	{
+	case '0':
+	  ret = CC_RET_FAILED;
+	  break;
+	case '1':
+	  ret = CC_RET_SUCCEEDED;
+	  break;
+	case '2':
+	  ret = CC_RET_DEFERRED;
+	  break;
+	case EOF:
+	  ret = CC_RET_SKIPPED;
+	  break;
+	default:
+	  /* We received an unknown/unexpected value.  Assume failure. */
+	  ret = CC_RET_FAILED;
+	}
+      fclose (fp);
+    }
+  return ret;
+}
+
+
+static void
+ccs_delete_config_file (struct multi_instance *mi)
+{
+  struct client_connect_state *ccs = mi->client_connect_state;
+  if (ccs->config_file)
+    {
+      setenv_del (mi->context.c2.es, "client_connect_config_file");
+      platform_unlink (ccs->config_file);
+      free (ccs->config_file);
+      ccs->config_file = NULL;
+    }
+}
+
+static bool
+ccs_gen_config_file (struct multi_instance *mi)
+{
+  struct client_connect_state *ccs = mi->client_connect_state;
+  struct gc_arena gc = gc_new ();
+  const char *fn;
+
+  if (ccs->config_file)
+    ccs_delete_config_file (mi);
+
+  fn = create_temp_file (mi->context.options.tmp_dir, "cc", &gc);
+  if (!fn)
+    {
+      gc_free (&gc);
+      return false;
+    }
+  ccs->config_file = string_alloc (fn, NULL);
+
+  setenv_str (mi->context.c2.es, "client_connect_config_file",
+	      ccs->config_file);
+
+  gc_free (&gc);
+  return true;
+}
+
+
 /*
  * Try to source a dynamic config file from the
  * --client-config-dir directory.

@@ -1775,14 +1894,14 @@ multi_client_connect_call_script (struct multi_context *m,
 
   if (mi->context.options.client_connect_script)
     {
-      struct gc_arena gc = gc_new ();
+      struct client_connect_state *ccs = mi->client_connect_state;
       struct argv argv = argv_new ();
-      const char *dc_file;
+      ASSERT (ccs);
 
       setenv_str (mi->context.c2.es, "script_type", "client-connect");
 
-      dc_file = create_temp_file (mi->context.options.tmp_dir, "cc", &gc);
-      if (!dc_file)
+      if (!ccs_gen_config_file (mi) ||
+	  !ccs_gen_deferred_ret_file (mi))
 	{
 	  ret = CC_RET_FAILED;
 	  goto script_failed;

@@ -1790,19 +1909,56 @@ multi_client_connect_call_script (struct multi_context *m,
 
       argv_printf (&argv, "%sc %s",
 		   mi->context.options.client_connect_script,
-		   dc_file);
+		   ccs->config_file);
 
       if (openvpn_run_script (&argv, mi->context.c2.es, 0, "--client-connect"))
 	{
-	  multi_client_connect_post (m, mi, dc_file, option_types_found);
-	  ret = CC_RET_SUCCEEDED;
+	  if (ccs_test_deferred_ret_file (mi) == CC_RET_DEFERRED)
+	    ret = CC_RET_DEFERRED;
+	  else
+	    {
+	      multi_client_connect_post (m, mi, ccs->config_file,
+					 option_types_found);
+	      ret = CC_RET_SUCCEEDED;
+	    }
 	}
       else
 	ret = CC_RET_FAILED;
 
 script_failed:
+      if (ret != CC_RET_DEFERRED)
+	{
+	  ccs_delete_config_file (mi);
+	  ccs_delete_deferred_ret_file (mi);
+	}
+
       argv_reset (&argv);
-      gc_free (&gc);
+    }
+  return ret;
+}
+
+static enum client_connect_return
+multi_client_handle_deferred (struct multi_context *m,
+			      struct multi_instance *mi,
+			      unsigned int *option_types_found)
+{
+  struct client_connect_state *ccs = mi->client_connect_state;
+  enum client_connect_return ret = CC_RET_SKIPPED;
+  ASSERT (option_types_found);
+  ASSERT (ccs);
+
+  ret = ccs_test_deferred_ret_file (mi);
+
+  if (ret == CC_RET_SKIPPED)
+    /* Skipped and deferred are equivalent in this context. */
+    ret = CC_RET_DEFERRED;
+
+  if (ret != CC_RET_DEFERRED)
+    {
+      ccs_delete_deferred_ret_file (mi);
+
+      multi_client_connect_post (m, mi, ccs->config_file, option_types_found);
+      ccs_delete_config_file (mi);
     }
   return ret;
 }

@@ -1945,6 +2101,9 @@ delete_client_connect_state (struct multi_instance *mi)
   ASSERT (mi);
   if (mi->client_connect_state)
     {
+      ccs_delete_deferred_ret_file (mi);
+      ccs_delete_config_file (mi);
+
       free (mi->client_connect_state);
       mi->client_connect_state = NULL;
     }

@@ -1975,7 +2134,7 @@ static const struct client_connect_handlers client_connect_handlers[] = {
   },
   {
     main: multi_client_connect_call_script,
-    deferred: multi_client_connect_fail
+    deferred: multi_client_handle_deferred
   },
   {
     main: multi_client_connect_mda,

src/openvpn/multi.h +3 -0

index 16dd99d..137228d 100644
@@ -69,6 +69,9 @@ struct client_connect_state
   /* Remember which option classes where processed for delayed option
      handling. */
   unsigned int option_types_found;
+
+  char *deferred_ret_file;
+  char *config_file;
 };
 
 /**

Signature · 1 line

2.1.1

Fabian Knittel

2014-10-10 22:09 UTC #11 lore raw

PATCH 9/9 client-connect: Add deferred support to the client-connect plugin v1 handler

In reply to Fabian Knittel

Uses the infrastructure provided and used in the previous patch to provide deferral support to the v1 client-connect plugin handler as well.

Signed-off-by Fabian Knittel <[email protected]...>

---
 src/openvpn/multi.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

src/openvpn/multi.c +19 ~~-11~~

index a3d94fa..c888e39 100644
@@ -1799,6 +1799,7 @@ multi_client_connect_call_plugin_v1 (struct multi_context *m,
 {
   enum client_connect_return ret = CC_RET_SKIPPED;
 #ifdef ENABLE_PLUGIN
+  struct client_connect_state *ccs = mi->client_connect_state;
   ASSERT (m);
   ASSERT (mi);
   ASSERT (option_types_found);

@@ -1806,34 +1807,41 @@ multi_client_connect_call_plugin_v1 (struct multi_context *m,
   if (plugin_defined (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT))
     {
       int plug_ret;
-      struct gc_arena gc = gc_new ();
       struct argv argv = argv_new ();
-      const char *dc_file = create_temp_file (mi->context.options.tmp_dir, "cc", &gc);
 
-      if (!dc_file)
+      if (!ccs_gen_config_file (mi) ||
+	  !ccs_gen_deferred_ret_file (mi))
 	{
 	  ret = CC_RET_FAILED;
 	  goto script_depr_failed;
 	}
 
-      argv_printf (&argv, "%s", dc_file);
+      argv_printf (&argv, "%s", ccs->config_file);
 
       plug_ret = plugin_call (mi->context.plugins,
 			      OPENVPN_PLUGIN_CLIENT_CONNECT,
 			      &argv, NULL, mi->context.c2.es);
       argv_reset (&argv);
-      if (plug_ret != OPENVPN_PLUGIN_FUNC_SUCCESS)
+      if (plug_ret == OPENVPN_PLUGIN_FUNC_SUCCESS)
 	{
-	  msg (M_WARN, "WARNING: client-connect plugin call failed");
-	  ret = CC_RET_FAILED;
+	  multi_client_connect_post (m, mi, ccs->config_file,
+				     option_types_found);
+	  ret = CC_RET_SUCCEEDED;
 	}
+      else if (plug_ret == OPENVPN_PLUGIN_FUNC_DEFERRED)
+	ret = CC_RET_DEFERRED;
       else
 	{
-	  multi_client_connect_post (m, mi, dc_file, option_types_found);
-	  ret = CC_RET_SUCCEEDED;
+	  msg (M_WARN, "WARNING: client-connect plugin call failed");
+	  ret = CC_RET_FAILED;
 	}
+
 script_depr_failed:
-      gc_free (&gc);
+      if (ret != CC_RET_DEFERRED)
+	{
+	  ccs_delete_config_file (mi);
+	  ccs_delete_deferred_ret_file (mi);
+	}
     }
 #endif
   return ret;

@@ -2126,7 +2134,7 @@ static const struct client_connect_handlers client_connect_handlers[] = {
   },
   {
     main: multi_client_connect_call_plugin_v1,
-    deferred: multi_client_connect_fail
+    deferred: multi_client_handle_deferred
   },
   {
     main: multi_client_connect_call_plugin_v2,

Signature · 1 line

2.1.1

Samuel Thibault

author

2014-10-12 19:59 UTC #23 lore raw

In reply to David Sommerseth

Quoted 4 lines · David Sommerseth, le Fri 10 Oct 2014 12:13:42 +0200, a écrit :

I think it would be better to move the unlink() code from multi_client_connect_post() into multi_connection_established(), where these temp files are created. This makes the code clearer and easier to understand.

Right, how about this?

Samuel

src/openvpn/multi.c +10 -4

index 5910154..25e7384 100644
@@ -1459,10 +1465,6 @@ multi_client_connect_post (struct multi_context *m,
 			     option_types_found,
 			     mi->context.c2.es);
 
-      if (!platform_unlink (dc_file))
-	msg (D_MULTI_ERRORS, "MULTI: problem deleting temporary file: %s",
-	     dc_file);
-
       /*
        * If the --client-connect script generates a config file
        * with an --ifconfig-push directive, it will override any

@@ -1705,6 +1707,11 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 	      multi_client_connect_post (m, mi, dc_file, option_permissions_mask, &option_types_found);
 	      ++cc_succeeded_count;
 	    }
+
+	  if (!platform_unlink (dc_file))
+	    msg (D_MULTI_ERRORS, "MULTI: problem deleting temporary file: %s",
+		 dc_file);
+
         script_depr_failed:
 	  argv_reset (&argv);
 	}

@@ -1758,6 +1765,11 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 	    }
 	  else
 	    cc_succeeded = false;
+
+	  if (!platform_unlink (dc_file))
+	    msg (D_MULTI_ERRORS, "MULTI: problem deleting temporary file: %s",
+		 dc_file);
+
         script_failed:
 	  argv_reset (&argv);
 	}

David Sommerseth

2014-10-13 09:37 UTC #24 lore raw

Re: [Openvpn-devel] [PATCH applied] Ensure that client-connect files are always deleted

From: David Sommerseth <[email protected]...>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Your patch has been applied to the master branch.

commit 5c2a3b5d29d10db3dbd8143ac2b671ce2eed1707

Author: Samuel Thibault
Date:   Thu Oct 9 23:40:49 2014 +0200

     Ensure that client-connect files are always deleted

     Signed-off-by: David Sommerseth <[email protected]...>
     Acked-by: David Sommerseth <[email protected]...>
     Message-Id: [email protected]...
     URL: http://thread.gmane.org/gmane.network.openvpn.devel/9104/focus=9118

- --
kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iEYEARECAAYFAlQ7nU4ACgkQDC186MBRfrrtSwCfZPcUtiimf14tCeWirUPm7CPv 8/AAniDygdcUoGAXcPYvgiQE5WfMiOxG
=zi1V
-----END PGP SIGNATURE-----

David Sommerseth

2014-10-14 16:15 UTC #25 lore raw

In reply to Samuel Thibault

Quoted 9 lines · Samuel Thibault

Quoted 4 lines · David Sommerseth, le Fri 10 Oct 2014 12:13:42 +0200, a écrit :
I think it would be better to move the unlink() code from multi_client_connect_post() into multi_connection_established(), where these temp files are created. This makes the code clearer and easier to understand.
Right, how about this?
Samuel

Okay, so I messed up the push. But it's fixed now, and it really got pushed out this time :)

Your patch has been applied to the master branch.

commit 7da9d40243e0743e2d050ceb6ae34e467dd58973

Author: Samuel Thibault
Date:   Thu Oct 9 23:40:49 2014 +0200

     Ensure that client-connect files are always deleted

     Signed-off-by: David Sommerseth <[email protected]...>
     Acked-by: David Sommerseth <[email protected]...>
     Message-Id: [email protected]...
     URL:

<http://thread.gmane.org/gmane.network.openvpn.devel/9104/focus=9118>

Signature · 3 lines

kind regards,

David Sommerseth

David Sommerseth

2014-10-20 08:47 UTC #26 lore raw

Quoted 20 lines · David Sommerseth

Quoted 9 lines · Samuel Thibault
Quoted 4 lines · David Sommerseth, le Fri 10 Oct 2014 12:13:42 +0200, a écrit :
I think it would be better to move the unlink() code from multi_client_connect_post() into multi_connection_established(), where these temp files are created. This makes the code clearer and easier to understand.
Right, how about this?
Samuel
Okay, so I messed up the push. But it's fixed now, and it really got pushed out this time :)
Your patch has been applied to the master branch.
commit 7da9d40243e0743e2d050ceb6ae34e467dd58973
Author: Samuel Thibault
Date:   Thu Oct 9 23:40:49 2014 +0200

Quoted 7 lines

     Ensure that client-connect files are always deleted

     Signed-off-by: David Sommerseth <[email protected]...>
     Acked-by: David Sommerseth <[email protected]...>
     Message-Id: [email protected]...
     URL:

<http://thread.gmane.org/gmane.network.openvpn.devel/9104/focus=9118>

We decided to include this in the coming 2.3 release as well. So the patch can be found in the release/2.3 branch as commit 10cbaea91d7d3774 as well.

Signature · 3 lines

kind regards,

David Sommerseth

Fabian Knittel

2015-01-17 12:30 UTC #12 lore raw

v2 cover Refactor client-connect / add support for deferred handling

In reply to Fabian Knittel

Hi,

changes from v1:
- rebased to the current master,
- reports errors during file delete

Introduction to the patch-set:

A few years ago I wrote a patch-set concerning OpenVPN's client-connect code. The first part of the patch-set (patches 1 to 6) refactors and unifies the client-connect code.

The deferred script code path has been in production use for a few years now, although this freshly rebased patch-set has only seen light testing so far.

Feed-back would be very welcome.

The patches are also availabe on a Github branch:
https://github.com/fknittel/openvpn/tree/feat_deferred_client-connect

Cheers
Fabian

PS: See
https://github.com/fknittel/openvpn/wiki/Patch-set-%22deferred-client-connect%22
in case you're interested in ready-made Debian packages.

 src/openvpn/multi.c   | 839 +++++++++++++++++++++++++++++++++++---------------
 src/openvpn/multi.h   |  32 +-
 src/openvpn/options.h |   6 +
 3 files changed, 636 insertions(+), 241 deletions(-)

Signature · 1 line

2.1.4

Fabian Knittel

2015-01-17 12:30 UTC #13 lore raw

PATCH v2 1/9 client-connect: Split multi_connection_established into separate functions

This patch splits up the multi_connection_established() function. Each new helper function does a specific job. Functions that do a similar job receive a similar calling interface.

The patch tries not to reindent code, so that the real changes are as clearly visible as possible. (A follow-up patch will only do indentation changes.)

Signed-off-by Fabian Knittel <[email protected]...>

---
 src/openvpn/multi.c   | 322 ++++++++++++++++++++++++++++++++++----------------
 src/openvpn/options.h |   6 +
 2 files changed, 224 insertions(+), 104 deletions(-)

src/openvpn/multi.c +218 ~~-104~~

index 6ddfbb5..c24b203 100644
@@ -1461,7 +1461,6 @@ static void
 multi_client_connect_post (struct multi_context *m,
 			   struct multi_instance *mi,
 			   const char *dc_file,
-			   unsigned int option_permissions_mask,
 			   unsigned int *option_types_found)
 {
   /* Did script generate a dynamic config file? */

@@ -1470,7 +1469,7 @@ multi_client_connect_post (struct multi_context *m,
       options_server_import (&mi->context.options,
 			     dc_file,
 			     D_IMPORT_ERRORS|M_OPTERR,
-			     option_permissions_mask,
+			     CLIENT_CONNECT_OPT_MASK,
 			     option_types_found,
 			     mi->context.c2.es);

@@ -1494,7 +1493,6 @@ static void
 multi_client_connect_post_plugin (struct multi_context *m,
 				  struct multi_instance *mi,
 				  const struct plugin_return *pr,
-				  unsigned int option_permissions_mask,
 				  unsigned int *option_types_found)
 {
   struct plugin_return config;

@@ -1511,7 +1509,7 @@ multi_client_connect_post_plugin (struct multi_context *m,
 	    options_string_import (&mi->context.options,
 				   config.list[i]->value,
 				   D_IMPORT_ERRORS|M_OPTERR,
-				   option_permissions_mask,
+				   CLIENT_CONNECT_OPT_MASK,
 				   option_types_found,
 				   mi->context.c2.es);
 	}

@@ -1529,29 +1527,28 @@ multi_client_connect_post_plugin (struct multi_context *m,
 
 #endif
 
-#ifdef MANAGEMENT_DEF_AUTH
-
 /*
  * Called to load management-derived client-connect config
  */
 static void
 multi_client_connect_mda (struct multi_context *m,
 			  struct multi_instance *mi,
-			  const struct buffer_list *config,
-			  unsigned int option_permissions_mask,
-			  unsigned int *option_types_found)
+			  unsigned int *option_types_found,
+			  int *cc_succeeded,
+			  int *cc_succeeded_count)
 {
-  if (config)
+#ifdef MANAGEMENT_DEF_AUTH
+  if (mi->cc_config)
     {
       struct buffer_entry *be;
   
-      for (be = config->head; be != NULL; be = be->next)
+      for (be = mi->cc_config->head; be != NULL; be = be->next)
 	{
 	  const char *opt = BSTR(&be->buf);
 	  options_string_import (&mi->context.options,
 				 opt,
 				 D_IMPORT_ERRORS|M_OPTERR,
-				 option_permissions_mask,
+				 CLIENT_CONNECT_OPT_MASK,
 				 option_types_found,
 				 mi->context.c2.es);
 	}

@@ -1564,10 +1561,11 @@ multi_client_connect_mda (struct multi_context *m,
        */
       multi_select_virtual_addr (m, mi);
       multi_set_virtual_addr_env (m, mi);
-    }
-}
 
+      ++*cc_succeeded_count;
+    }
 #endif
+}
 
 static void
 multi_client_connect_setenv (struct multi_context *m,

@@ -1594,6 +1592,38 @@ multi_client_connect_setenv (struct multi_context *m,
   gc_free (&gc);
 }
 
+static void
+multi_client_connect_early_setup (struct multi_context *m,
+				  struct multi_instance *mi);
+static void
+multi_client_connect_source_ccd (struct multi_context *m,
+				 struct multi_instance *mi,
+				 unsigned int *option_types_found);
+static void
+multi_client_connect_call_plugin_v1 (struct multi_context *m,
+				     struct multi_instance *mi,
+				     unsigned int *option_types_found,
+				     int *cc_succeeded,
+				     int *cc_succeeded_count);
+static void
+multi_client_connect_call_plugin_v2 (struct multi_context *m,
+				     struct multi_instance *mi,
+				     unsigned int *option_types_found,
+				     int *cc_succeeded,
+				     int *cc_succeeded_count);
+static void
+multi_client_connect_call_script (struct multi_context *m,
+				  struct multi_instance *mi,
+				  unsigned int *option_types_found,
+				  int *cc_succeeded,
+				  int *cc_succeeded_count);
+static void
+multi_client_connect_late_setup (struct multi_context *m,
+				 struct multi_instance *mi,
+				 const unsigned int option_types_found,
+				 int cc_succeeded,
+				 const int cc_succeeded_count);
+
 /*
  * Called as soon as the SSL/TLS connection authenticates.
  *

@@ -1608,25 +1638,67 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 {
   if (tls_authentication_status (mi->context.c2.tls_multi, 0) == TLS_AUTHENTICATION_SUCCEEDED)
     {
-      struct gc_arena gc = gc_new ();
       unsigned int option_types_found = 0;
-
-      const unsigned int option_permissions_mask =
-	  OPT_P_INSTANCE
-	| OPT_P_INHERIT
-	| OPT_P_PUSH
-	| OPT_P_TIMER
-	| OPT_P_CONFIG
-	| OPT_P_ECHO
-	| OPT_P_COMP
-	| OPT_P_SOCKFLAGS;
-
       int cc_succeeded = true; /* client connect script status */
       int cc_succeeded_count = 0;
 
-      ASSERT (mi->context.c1.tuntap);
+      multi_client_connect_early_setup (m, mi);
+
+      multi_client_connect_source_ccd (m, mi, &option_types_found);
+
+      /*
+       * Select a virtual address from either --ifconfig-push in
+       * --client-config-dir file or --ifconfig-pool.
+       */
+      multi_select_virtual_addr (m, mi);
+
+      /* do --client-connect setenvs */
+      multi_client_connect_setenv (m, mi);
+
+      multi_client_connect_call_plugin_v1 (m, mi, &option_types_found,
+					   &cc_succeeded, &cc_succeeded_count);
+
+      multi_client_connect_call_plugin_v2 (m, mi, &option_types_found,
+					   &cc_succeeded, &cc_succeeded_count);
+
+      /*
+       * Run --client-connect script.
+       */
+      if (cc_succeeded)
+	{
+	  multi_client_connect_call_script (m, mi, &option_types_found,
+					    &cc_succeeded,
+					    &cc_succeeded_count);
+	}
+
+      /*
+       * Check for client-connect script left by management interface client
+       */
+      if (cc_succeeded)
+	{
+	  multi_client_connect_mda (m, mi, &option_types_found,
+				    &cc_succeeded, &cc_succeeded_count);
+	}
+
+      multi_client_connect_late_setup (m, mi, option_types_found,
+				       cc_succeeded, cc_succeeded_count);
+
+      /* set flag so we don't get called again */
+      mi->connection_established_flag = true;
+    }
+
+  /*
+   * Reply now to client's PUSH_REQUEST query
+   */
+  mi->context.c2.push_reply_deferred = false;
+}
 
-      /* lock down the common name and cert hashes so they can't change during future TLS renegotiations */
+static void
+multi_client_connect_early_setup (struct multi_context *m,
+				  struct multi_instance *mi)
+{
+      /* lock down the common name and cert hashes so they can't change during
+	 future TLS renegotiations */
       tls_lock_common_name (mi->context.c2.tls_multi);
       tls_lock_cert_hash_set (mi->context.c2.tls_multi);

@@ -1639,13 +1711,20 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 
       /* reset pool handle to null */
       mi->vaddr_handle = -1;
+}
 
-      /*
-       * Try to source a dynamic config file from the
-       * --client-config-dir directory.
-       */
+/*
+ * Try to source a dynamic config file from the
+ * --client-config-dir directory.
+ */
+static void
+multi_client_connect_source_ccd (struct multi_context *m,
+				 struct multi_instance *mi,
+				 unsigned int *option_types_found)
+{
       if (mi->context.options.client_config_dir)
 	{
+	  struct gc_arena gc = gc_new ();
 	  const char *ccd_file;
 	  
 	  ccd_file = gen_path (mi->context.options.client_config_dir,

@@ -1658,8 +1737,8 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 	      options_server_import (&mi->context.options,
 				     ccd_file,
 				     D_IMPORT_ERRORS|M_OPTERR,
-				     option_permissions_mask,
-				     &option_types_found,
+				     CLIENT_CONNECT_OPT_MASK,
+				     option_types_found,
 				     mi->context.c2.es);
 	    }
 	  else /* try default file */

@@ -1673,95 +1752,142 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 		  options_server_import (&mi->context.options,
 					 ccd_file,
 					 D_IMPORT_ERRORS|M_OPTERR,
-					 option_permissions_mask,
-					 &option_types_found,
+					 CLIENT_CONNECT_OPT_MASK,
+					 option_types_found,
 					 mi->context.c2.es);
 		}
 	    }
-	}
 
-      /*
-       * Select a virtual address from either --ifconfig-push in --client-config-dir file
-       * or --ifconfig-pool.
-       */
-      multi_select_virtual_addr (m, mi);
-
-      /* do --client-connect setenvs */
-      multi_client_connect_setenv (m, mi);
+	  gc_free (&gc);
+	}
+}
 
+/*
+ * Call client-connect plug-in.
+ *
+ * deprecated callback, use a file for passing back return info
+ */
+static void
+multi_client_connect_call_plugin_v1 (struct multi_context *m,
+				     struct multi_instance *mi,
+				     unsigned int *option_types_found,
+				     int *cc_succeeded,
+				     int *cc_succeeded_count)
+{
 #ifdef ENABLE_PLUGIN
-      /*
-       * Call client-connect plug-in.
-       */
+      ASSERT (m);
+      ASSERT (mi);
+      ASSERT (option_types_found);
+      ASSERT (cc_succeeded);
+      ASSERT (cc_succeeded_count);
 
-      /* deprecated callback, use a file for passing back return info */
       if (plugin_defined (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT))
 	{
+	  int plug_ret;
+	  struct gc_arena gc = gc_new ();
 	  struct argv argv = argv_new ();
 	  const char *dc_file = create_temp_file (mi->context.options.tmp_dir, "cc", &gc);
 
-          if( !dc_file ) {
-            cc_succeeded = false;
-            goto script_depr_failed;
-          }
+          if (!dc_file)
+	    {
+	      *cc_succeeded = false;
+	      goto script_depr_failed;
+	    }
 
 	  argv_printf (&argv, "%s", dc_file);
-	  if (plugin_call (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT, &argv, NULL, mi->context.c2.es) != OPENVPN_PLUGIN_FUNC_SUCCESS)
+
+	  plug_ret = plugin_call (mi->context.plugins,
+				  OPENVPN_PLUGIN_CLIENT_CONNECT,
+				  &argv, NULL, mi->context.c2.es);
+	  argv_reset (&argv);
+	  if (plug_ret != OPENVPN_PLUGIN_FUNC_SUCCESS)
 	    {
 	      msg (M_WARN, "WARNING: client-connect plugin call failed");
-	      cc_succeeded = false;
+	      *cc_succeeded = false;
 	    }
 	  else
 	    {
-	      multi_client_connect_post (m, mi, dc_file, option_permissions_mask, &option_types_found);
-	      ++cc_succeeded_count;
+	      multi_client_connect_post (m, mi, dc_file, option_types_found);
+	      ++*cc_succeeded_count;
 	    }
-
-	  if (!platform_unlink (dc_file))
-	    msg (D_MULTI_ERRORS, "MULTI: problem deleting temporary file: %s",
-		 dc_file);
-
-        script_depr_failed:
-	  argv_reset (&argv);
+script_depr_failed:
+	  gc_free (&gc);
 	}
+#endif
+}
+
+/*
+ * Call client-connect plug-in.
+ *
+ * V2 callback, use a plugin_return struct for passing back return info
+ */
+static void
+multi_client_connect_call_plugin_v2 (struct multi_context *m,
+				     struct multi_instance *mi,
+				     unsigned int *option_types_found,
+				     int *cc_succeeded,
+				     int *cc_succeeded_count)
+{
+#ifdef ENABLE_PLUGIN
+      ASSERT (m);
+      ASSERT (mi);
+      ASSERT (option_types_found);
+      ASSERT (cc_succeeded);
+      ASSERT (cc_succeeded_count);
 
-      /* V2 callback, use a plugin_return struct for passing back return info */
       if (plugin_defined (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT_V2))
 	{
+	  int plug_ret;
 	  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)
+	  plug_ret = plugin_call (mi->context.plugins,
+				  OPENVPN_PLUGIN_CLIENT_CONNECT_V2,
+				  NULL, &pr, mi->context.c2.es);
+	  if (plug_ret != OPENVPN_PLUGIN_FUNC_SUCCESS)
 	    {
 	      msg (M_WARN, "WARNING: client-connect-v2 plugin call failed");
-	      cc_succeeded = false;
+	      *cc_succeeded = false;
 	    }
 	  else
 	    {
-	      multi_client_connect_post_plugin (m, mi, &pr, option_permissions_mask, &option_types_found);
-	      ++cc_succeeded_count;
+	      multi_client_connect_post_plugin (m, mi, &pr, option_types_found);
+	      ++*cc_succeeded_count;
 	    }
 
 	  plugin_return_free (&pr);
 	}
 #endif
+}
 
-      /*
-       * Run --client-connect script.
-       */
-      if (mi->context.options.client_connect_script && cc_succeeded)
+static void
+multi_client_connect_call_script (struct multi_context *m,
+				  struct multi_instance *mi,
+				  unsigned int *option_types_found,
+				  int *cc_succeeded,
+				  int *cc_succeeded_count)
+{
+      ASSERT (m);
+      ASSERT (mi);
+      ASSERT (option_types_found);
+      ASSERT (cc_succeeded);
+      ASSERT (cc_succeeded_count);
+
+      if (mi->context.options.client_connect_script)
 	{
+	  struct gc_arena gc = gc_new ();
 	  struct argv argv = argv_new ();
-	  const char *dc_file = NULL;
+	  const char *dc_file;
 
 	  setenv_str (mi->context.c2.es, "script_type", "client-connect");
 
 	  dc_file = create_temp_file (mi->context.options.tmp_dir, "cc", &gc);
-          if( !dc_file ) {
-            cc_succeeded = false;
-            goto script_failed;
-          }
+          if (!dc_file)
+	    {
+	      cc_succeeded = false;
+	      goto script_failed;
+	    }
 
 	  argv_printf (&argv, "%sc %s",
 		       mi->context.options.client_connect_script,

@@ -1769,30 +1895,26 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 
 	  if (openvpn_run_script (&argv, mi->context.c2.es, 0, "--client-connect"))
 	    {
-	      multi_client_connect_post (m, mi, dc_file, option_permissions_mask, &option_types_found);
+	      multi_client_connect_post (m, mi, dc_file, option_types_found);
 	      ++cc_succeeded_count;
 	    }
 	  else
 	    cc_succeeded = false;
-
-	  if (!platform_unlink (dc_file))
-	    msg (D_MULTI_ERRORS, "MULTI: problem deleting temporary file: %s",
-		 dc_file);
-
-        script_failed:
+script_failed:
 	  argv_reset (&argv);
+	  gc_free (&gc);
 	}
+}
 
-      /*
-       * Check for client-connect script left by management interface client
-       */
-#ifdef MANAGEMENT_DEF_AUTH
-      if (cc_succeeded && mi->cc_config)
-	{
-	  multi_client_connect_mda (m, mi, mi->cc_config, option_permissions_mask, &option_types_found);
-	  ++cc_succeeded_count;
-	}
-#endif
+static void
+multi_client_connect_late_setup (struct multi_context *m,
+				 struct multi_instance *mi,
+				 const unsigned int option_types_found,
+				 int cc_succeeded,
+				 const int cc_succeeded_count)
+{
+      struct gc_arena gc = gc_new ();
+      ASSERT (mi->context.c1.tuntap);
 
       /*
        * Check for "disable" directive in client-config-dir file

@@ -1886,9 +2008,6 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 	  mi->context.c2.context_auth = cc_succeeded_count ? CAS_PARTIAL : CAS_FAILED;
 	}
 
-      /* set flag so we don't get called again */
-      mi->connection_established_flag = true;
-
       /* increment number of current authenticated clients */
       ++m->n_clients;
       update_mstat_n_clients(m->n_clients);

@@ -1900,14 +2019,9 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 #endif
 
       gc_free (&gc);
-    }
-
-  /*
-   * Reply now to client's PUSH_REQUEST query
-   */
-  mi->context.c2.push_reply_deferred = false;
 }
 
+
 /*
  * Add a mbuf buffer to a particular
  * instance.

src/openvpn/options.h +6 -0

index 7a8b21e..73d1caa 100644
@@ -636,6 +636,12 @@ struct options
 
 #define OPT_P_DEFAULT   (~(OPT_P_INSTANCE|OPT_P_PULL_MODE))
 
+#if P2MP_SERVER
+#define CLIENT_CONNECT_OPT_MASK (OPT_P_INSTANCE | OPT_P_INHERIT	| OPT_P_PUSH | \
+				 OPT_P_TIMER | OPT_P_CONFIG | OPT_P_ECHO | \
+				 OPT_P_COMP | OPT_P_SOCKFLAGS)
+#endif
+
 #if P2MP
 #define PULL_DEFINED(opt) ((opt)->pull)
 #if P2MP_SERVER

Signature · 1 line

2.1.4

Fabian Knittel

2015-01-17 12:30 UTC #14 lore raw

PATCH v2 2/9 client-connect: Properly indent all functions

In reply to Fabian Knittel

This patch adds proper indentation to all new helper functions. No functional changes are performed. Nothing is moved around.

Signed-off-by Fabian Knittel <[email protected]...>

---
 src/openvpn/multi.c | 408 ++++++++++++++++++++++++++--------------------------
 1 file changed, 204 insertions(+), 204 deletions(-)

src/openvpn/multi.c +204 ~~-204~~

index c24b203..80bd5a3 100644
@@ -1697,20 +1697,20 @@ static void
 multi_client_connect_early_setup (struct multi_context *m,
 				  struct multi_instance *mi)
 {
-      /* lock down the common name and cert hashes so they can't change during
-	 future TLS renegotiations */
-      tls_lock_common_name (mi->context.c2.tls_multi);
-      tls_lock_cert_hash_set (mi->context.c2.tls_multi);
+  /* lock down the common name and cert hashes so they can't change during
+     future TLS renegotiations */
+  tls_lock_common_name (mi->context.c2.tls_multi);
+  tls_lock_cert_hash_set (mi->context.c2.tls_multi);
 
-      /* generate a msg() prefix for this client instance */
-      generate_prefix (mi);
+  /* generate a msg() prefix for this client instance */
+  generate_prefix (mi);
 
-      /* delete instances of previous clients with same common-name */
-      if (!mi->context.options.duplicate_cn)
-	multi_delete_dup (m, mi);
+  /* delete instances of previous clients with same common-name */
+  if (!mi->context.options.duplicate_cn)
+    multi_delete_dup (m, mi);
 
-      /* reset pool handle to null */
-      mi->vaddr_handle = -1;
+  /* reset pool handle to null */
+  mi->vaddr_handle = -1;
 }
 
 /*

@@ -1722,16 +1722,31 @@ multi_client_connect_source_ccd (struct multi_context *m,
 				 struct multi_instance *mi,
 				 unsigned int *option_types_found)
 {
-      if (mi->context.options.client_config_dir)
+  if (mi->context.options.client_config_dir)
+    {
+      struct gc_arena gc = gc_new ();
+      const char *ccd_file;
+      
+      ccd_file = gen_path (mi->context.options.client_config_dir,
+			   tls_common_name (mi->context.c2.tls_multi, false),
+			   &gc);
+
+      /* try common-name file */
+      if (test_file (ccd_file))
+	{
+	  options_server_import (&mi->context.options,
+				 ccd_file,
+				 D_IMPORT_ERRORS|M_OPTERR,
+				 CLIENT_CONNECT_OPT_MASK,
+				 option_types_found,
+				 mi->context.c2.es);
+	}
+      else /* try default file */
 	{
-	  struct gc_arena gc = gc_new ();
-	  const char *ccd_file;
-	  
 	  ccd_file = gen_path (mi->context.options.client_config_dir,
-			       tls_common_name (mi->context.c2.tls_multi, false),
+			       CCD_DEFAULT,
 			       &gc);
 
-	  /* try common-name file */
 	  if (test_file (ccd_file))
 	    {
 	      options_server_import (&mi->context.options,

@@ -1741,25 +1756,10 @@ multi_client_connect_source_ccd (struct multi_context *m,
 				     option_types_found,
 				     mi->context.c2.es);
 	    }
-	  else /* try default file */
-	    {
-	      ccd_file = gen_path (mi->context.options.client_config_dir,
-				   CCD_DEFAULT,
-				   &gc);
-
-	      if (test_file (ccd_file))
-		{
-		  options_server_import (&mi->context.options,
-					 ccd_file,
-					 D_IMPORT_ERRORS|M_OPTERR,
-					 CLIENT_CONNECT_OPT_MASK,
-					 option_types_found,
-					 mi->context.c2.es);
-		}
-	    }
-
-	  gc_free (&gc);
 	}
+
+      gc_free (&gc);
+    }
 }
 
 /*

@@ -1775,44 +1775,44 @@ multi_client_connect_call_plugin_v1 (struct multi_context *m,
 				     int *cc_succeeded_count)
 {
 #ifdef ENABLE_PLUGIN
-      ASSERT (m);
-      ASSERT (mi);
-      ASSERT (option_types_found);
-      ASSERT (cc_succeeded);
-      ASSERT (cc_succeeded_count);
+  ASSERT (m);
+  ASSERT (mi);
+  ASSERT (option_types_found);
+  ASSERT (cc_succeeded);
+  ASSERT (cc_succeeded_count);
 
-      if (plugin_defined (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT))
-	{
-	  int plug_ret;
-	  struct gc_arena gc = gc_new ();
-	  struct argv argv = argv_new ();
-	  const char *dc_file = create_temp_file (mi->context.options.tmp_dir, "cc", &gc);
+  if (plugin_defined (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT))
+    {
+      int plug_ret;
+      struct gc_arena gc = gc_new ();
+      struct argv argv = argv_new ();
+      const char *dc_file = create_temp_file (mi->context.options.tmp_dir, "cc", &gc);
 
-          if (!dc_file)
-	    {
-	      *cc_succeeded = false;
-	      goto script_depr_failed;
-	    }
+      if (!dc_file)
+	{
+	  *cc_succeeded = false;
+	  goto script_depr_failed;
+	}
 
-	  argv_printf (&argv, "%s", dc_file);
+      argv_printf (&argv, "%s", dc_file);
 
-	  plug_ret = plugin_call (mi->context.plugins,
-				  OPENVPN_PLUGIN_CLIENT_CONNECT,
-				  &argv, NULL, mi->context.c2.es);
-	  argv_reset (&argv);
-	  if (plug_ret != OPENVPN_PLUGIN_FUNC_SUCCESS)
-	    {
-	      msg (M_WARN, "WARNING: client-connect plugin call failed");
-	      *cc_succeeded = false;
-	    }
-	  else
-	    {
-	      multi_client_connect_post (m, mi, dc_file, option_types_found);
-	      ++*cc_succeeded_count;
-	    }
-script_depr_failed:
-	  gc_free (&gc);
+      plug_ret = plugin_call (mi->context.plugins,
+			      OPENVPN_PLUGIN_CLIENT_CONNECT,
+			      &argv, NULL, mi->context.c2.es);
+      argv_reset (&argv);
+      if (plug_ret != OPENVPN_PLUGIN_FUNC_SUCCESS)
+	{
+	  msg (M_WARN, "WARNING: client-connect plugin call failed");
+	  *cc_succeeded = false;
 	}
+      else
+	{
+	  multi_client_connect_post (m, mi, dc_file, option_types_found);
+	  ++*cc_succeeded_count;
+	}
+script_depr_failed:
+      gc_free (&gc);
+    }
 #endif
 }

@@ -1829,35 +1829,35 @@ multi_client_connect_call_plugin_v2 (struct multi_context *m,
 				     int *cc_succeeded_count)
 {
 #ifdef ENABLE_PLUGIN
-      ASSERT (m);
-      ASSERT (mi);
-      ASSERT (option_types_found);
-      ASSERT (cc_succeeded);
-      ASSERT (cc_succeeded_count);
-
-      if (plugin_defined (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT_V2))
-	{
-	  int plug_ret;
-	  struct plugin_return pr;
+  ASSERT (m);
+  ASSERT (mi);
+  ASSERT (option_types_found);
+  ASSERT (cc_succeeded);
+  ASSERT (cc_succeeded_count);
 
-	  plugin_return_init (&pr);
+  if (plugin_defined (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT_V2))
+    {
+      int plug_ret;
+      struct plugin_return pr;
 
-	  plug_ret = plugin_call (mi->context.plugins,
-				  OPENVPN_PLUGIN_CLIENT_CONNECT_V2,
-				  NULL, &pr, mi->context.c2.es);
-	  if (plug_ret != OPENVPN_PLUGIN_FUNC_SUCCESS)
-	    {
-	      msg (M_WARN, "WARNING: client-connect-v2 plugin call failed");
-	      *cc_succeeded = false;
-	    }
-	  else
-	    {
-	      multi_client_connect_post_plugin (m, mi, &pr, option_types_found);
-	      ++*cc_succeeded_count;
-	    }
+      plugin_return_init (&pr);
 
-	  plugin_return_free (&pr);
+      plug_ret = plugin_call (mi->context.plugins,
+			      OPENVPN_PLUGIN_CLIENT_CONNECT_V2,
+			      NULL, &pr, mi->context.c2.es);
+      if (plug_ret != OPENVPN_PLUGIN_FUNC_SUCCESS)
+	{
+	  msg (M_WARN, "WARNING: client-connect-v2 plugin call failed");
+	  *cc_succeeded = false;
+	}
+      else
+	{
+	  multi_client_connect_post_plugin (m, mi, &pr, option_types_found);
+	  ++*cc_succeeded_count;
 	}
+
+      plugin_return_free (&pr);
+    }
 #endif
 }

@@ -1868,42 +1868,42 @@ multi_client_connect_call_script (struct multi_context *m,
 				  int *cc_succeeded,
 				  int *cc_succeeded_count)
 {
-      ASSERT (m);
-      ASSERT (mi);
-      ASSERT (option_types_found);
-      ASSERT (cc_succeeded);
-      ASSERT (cc_succeeded_count);
+  ASSERT (m);
+  ASSERT (mi);
+  ASSERT (option_types_found);
+  ASSERT (cc_succeeded);
+  ASSERT (cc_succeeded_count);
 
-      if (mi->context.options.client_connect_script)
-	{
-	  struct gc_arena gc = gc_new ();
-	  struct argv argv = argv_new ();
-	  const char *dc_file;
+  if (mi->context.options.client_connect_script)
+    {
+      struct gc_arena gc = gc_new ();
+      struct argv argv = argv_new ();
+      const char *dc_file;
 
-	  setenv_str (mi->context.c2.es, "script_type", "client-connect");
+      setenv_str (mi->context.c2.es, "script_type", "client-connect");
 
-	  dc_file = create_temp_file (mi->context.options.tmp_dir, "cc", &gc);
-          if (!dc_file)
-	    {
-	      cc_succeeded = false;
-	      goto script_failed;
-	    }
+      dc_file = create_temp_file (mi->context.options.tmp_dir, "cc", &gc);
+      if (!dc_file)
+	{
+	  cc_succeeded = false;
+	  goto script_failed;
+	}
 
-	  argv_printf (&argv, "%sc %s",
-		       mi->context.options.client_connect_script,
-		       dc_file);
+      argv_printf (&argv, "%sc %s",
+		   mi->context.options.client_connect_script,
+		   dc_file);
 
-	  if (openvpn_run_script (&argv, mi->context.c2.es, 0, "--client-connect"))
-	    {
-	      multi_client_connect_post (m, mi, dc_file, option_types_found);
-	      ++cc_succeeded_count;
-	    }
-	  else
-	    cc_succeeded = false;
-script_failed:
-	  argv_reset (&argv);
-	  gc_free (&gc);
+      if (openvpn_run_script (&argv, mi->context.c2.es, 0, "--client-connect"))
+	{
+	  multi_client_connect_post (m, mi, dc_file, option_types_found);
+	  ++cc_succeeded_count;
 	}
+      else
+	cc_succeeded = false;
+script_failed:
+      argv_reset (&argv);
+      gc_free (&gc);
+    }
 }
 
 static void

@@ -1913,112 +1913,112 @@ multi_client_connect_late_setup (struct multi_context *m,
 				 int cc_succeeded,
 				 const int cc_succeeded_count)
 {
-      struct gc_arena gc = gc_new ();
-      ASSERT (mi->context.c1.tuntap);
+  struct gc_arena gc = gc_new ();
+  ASSERT (mi->context.c1.tuntap);
 
+  /*
+   * Check for "disable" directive in client-config-dir file
+   * or config file generated by --client-connect script.
+   */
+  if (mi->context.options.disable)
+    {
+      msg (D_MULTI_ERRORS, "MULTI: client has been rejected due to 'disable' directive");
+      cc_succeeded = false;
+    }
+
+  if (cc_succeeded)
+    {
       /*
-       * Check for "disable" directive in client-config-dir file
-       * or config file generated by --client-connect script.
+       * Process sourced options.
        */
-      if (mi->context.options.disable)
+      do_deferred_options (&mi->context, option_types_found);
+
+      /*
+       * make sure we got ifconfig settings from somewhere
+       */
+      if (!mi->context.c2.push_ifconfig_defined)
 	{
-	  msg (D_MULTI_ERRORS, "MULTI: client has been rejected due to 'disable' directive");
-	  cc_succeeded = false;
+	  msg (D_MULTI_ERRORS, "MULTI: no dynamic or static remote --ifconfig address is available for %s",
+	       multi_instance_string (mi, false, &gc));
 	}
 
-      if (cc_succeeded)
+      /*
+       * make sure that ifconfig settings comply with constraints
+       */
+      if (!ifconfig_push_constraint_satisfied (&mi->context))
 	{
-	  /*
-	   * Process sourced options.
-	   */
-	  do_deferred_options (&mi->context, option_types_found);
+	  /* JYFIXME -- this should cause the connection to fail */
+	  msg (D_MULTI_ERRORS, "MULTI ERROR: primary virtual IP for %s (%s) violates tunnel network/netmask constraint (%s/%s)",
+	       multi_instance_string (mi, false, &gc),
+	       print_in_addr_t (mi->context.c2.push_ifconfig_local, 0, &gc),
+	       print_in_addr_t (mi->context.options.push_ifconfig_constraint_network, 0, &gc),
+	       print_in_addr_t (mi->context.options.push_ifconfig_constraint_netmask, 0, &gc));
+	}
 
-	  /*
-	   * make sure we got ifconfig settings from somewhere
-	   */
-	  if (!mi->context.c2.push_ifconfig_defined)
+      /*
+       * For routed tunnels, set up internal route to endpoint
+       * plus add all iroute routes.
+       */
+      if (TUNNEL_TYPE (mi->context.c1.tuntap) == DEV_TYPE_TUN)
+	{
+	  if (mi->context.c2.push_ifconfig_defined)
 	    {
-	      msg (D_MULTI_ERRORS, "MULTI: no dynamic or static remote --ifconfig address is available for %s",
-		   multi_instance_string (mi, false, &gc));
+	      multi_learn_in_addr_t (m, mi, mi->context.c2.push_ifconfig_local, -1, true);
+	      msg (D_MULTI_LOW, "MULTI: primary virtual IP for %s: %s",
+		   multi_instance_string (mi, false, &gc),
+		   print_in_addr_t (mi->context.c2.push_ifconfig_local, 0, &gc));
 	    }
 
-	  /*
-	   * make sure that ifconfig settings comply with constraints
-	   */
-	  if (!ifconfig_push_constraint_satisfied (&mi->context))
+	  if (mi->context.c2.push_ifconfig_ipv6_defined)
 	    {
-	      /* JYFIXME -- this should cause the connection to fail */
-	      msg (D_MULTI_ERRORS, "MULTI ERROR: primary virtual IP for %s (%s) violates tunnel network/netmask constraint (%s/%s)",
+	      multi_learn_in6_addr (m, mi, mi->context.c2.push_ifconfig_ipv6_local, -1, true);
+	      /* TODO: find out where addresses are "unlearned"!! */
+	      msg (D_MULTI_LOW, "MULTI: primary virtual IPv6 for %s: %s",
 		   multi_instance_string (mi, false, &gc),
-		   print_in_addr_t (mi->context.c2.push_ifconfig_local, 0, &gc),
-		   print_in_addr_t (mi->context.options.push_ifconfig_constraint_network, 0, &gc),
-		   print_in_addr_t (mi->context.options.push_ifconfig_constraint_netmask, 0, &gc));
+		   print_in6_addr (mi->context.c2.push_ifconfig_ipv6_local, 0, &gc));
 	    }
 
+	  /* add routes locally, pointing to new client, if
+	     --iroute options have been specified */
+	  multi_add_iroutes (m, mi);
+
 	  /*
-	   * For routed tunnels, set up internal route to endpoint
-	   * plus add all iroute routes.
+	   * iroutes represent subnets which are "owned" by a particular
+	   * client.  Therefore, do not actually push a route to a client
+	   * if it matches one of the client's iroutes.
 	   */
-	  if (TUNNEL_TYPE (mi->context.c1.tuntap) == DEV_TYPE_TUN)
-	    {
-	      if (mi->context.c2.push_ifconfig_defined)
-		{
-		  multi_learn_in_addr_t (m, mi, mi->context.c2.push_ifconfig_local, -1, true);
-		  msg (D_MULTI_LOW, "MULTI: primary virtual IP for %s: %s",
-		       multi_instance_string (mi, false, &gc),
-		       print_in_addr_t (mi->context.c2.push_ifconfig_local, 0, &gc));
-		}
-
-	      if (mi->context.c2.push_ifconfig_ipv6_defined)
-		{
-		  multi_learn_in6_addr (m, mi, mi->context.c2.push_ifconfig_ipv6_local, -1, true);
-		  /* TODO: find out where addresses are "unlearned"!! */
-		  msg (D_MULTI_LOW, "MULTI: primary virtual IPv6 for %s: %s",
-		       multi_instance_string (mi, false, &gc),
-		       print_in6_addr (mi->context.c2.push_ifconfig_ipv6_local, 0, &gc));
-		}
-
-	      /* add routes locally, pointing to new client, if
-		 --iroute options have been specified */
-	      multi_add_iroutes (m, mi);
-
-	      /*
-	       * iroutes represent subnets which are "owned" by a particular
-	       * client.  Therefore, do not actually push a route to a client
-	       * if it matches one of the client's iroutes.
-	       */
-	      remove_iroutes_from_push_route_list (&mi->context.options);
-	    }
-	  else if (mi->context.options.iroutes)
-	    {
-	      msg (D_MULTI_ERRORS, "MULTI: --iroute options rejected for %s -- iroute only works with tun-style tunnels",
-		   multi_instance_string (mi, false, &gc));
-	    }
-
-	  /* set our client's VPN endpoint for status reporting purposes */
-	  mi->reporting_addr = mi->context.c2.push_ifconfig_local;
-	  mi->reporting_addr_ipv6 = mi->context.c2.push_ifconfig_ipv6_local;
-
-	  /* set context-level authentication flag */
-	  mi->context.c2.context_auth = CAS_SUCCEEDED;
+	  remove_iroutes_from_push_route_list (&mi->context.options);
 	}
-      else
+      else if (mi->context.options.iroutes)
 	{
-	  /* set context-level authentication flag */
-	  mi->context.c2.context_auth = cc_succeeded_count ? CAS_PARTIAL : CAS_FAILED;
+	  msg (D_MULTI_ERRORS, "MULTI: --iroute options rejected for %s -- iroute only works with tun-style tunnels",
+	       multi_instance_string (mi, false, &gc));
 	}
 
-      /* increment number of current authenticated clients */
-      ++m->n_clients;
-      update_mstat_n_clients(m->n_clients);
-      --mi->n_clients_delta;
+      /* set our client's VPN endpoint for status reporting purposes */
+      mi->reporting_addr = mi->context.c2.push_ifconfig_local;
+      mi->reporting_addr_ipv6 = mi->context.c2.push_ifconfig_ipv6_local;
+
+      /* set context-level authentication flag */
+      mi->context.c2.context_auth = CAS_SUCCEEDED;
+    }
+  else
+    {
+      /* set context-level authentication flag */
+      mi->context.c2.context_auth = cc_succeeded_count ? CAS_PARTIAL : CAS_FAILED;
+    }
+
+  /* increment number of current authenticated clients */
+  ++m->n_clients;
+  update_mstat_n_clients(m->n_clients);
+  --mi->n_clients_delta;
 
 #ifdef MANAGEMENT_DEF_AUTH
-      if (management)
-	management_connection_established (management, &mi->context.c2.mda_context, mi->context.c2.es);
+  if (management)
+    management_connection_established (management, &mi->context.c2.mda_context, mi->context.c2.es);
 #endif
 
-      gc_free (&gc);
+  gc_free (&gc);
 }

Signature · 1 line

2.1.4

Fabian Knittel

2015-01-17 12:30 UTC #15 lore raw

PATCH v2 3/9 client-connect: Refactor multi_client_connect_source_ccd

In reply to Fabian Knittel

Refactor multi_client_connect_source_ccd(), so that options_server_import() (or the success path in general) is only entered in one place within the function.

Signed-off-by Fabian Knittel <[email protected]...>

---
 src/openvpn/multi.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

src/openvpn/multi.c +13 ~~-16~~

index 80bd5a3..8cc61d7 100644
@@ -1732,7 +1732,19 @@ multi_client_connect_source_ccd (struct multi_context *m,
 			   &gc);
 
       /* try common-name file */
-      if (test_file (ccd_file))
+      if (!test_file (ccd_file))
+	ccd_file = NULL;
+
+      if (!ccd_file)
+	{
+	  ccd_file = gen_path (mi->context.options.client_config_dir,
+			       CCD_DEFAULT, &gc);
+	  /* try default file */
+	  if (!test_file (ccd_file))
+	    ccd_file = NULL;
+	}
+
+      if (ccd_file)
 	{
 	  options_server_import (&mi->context.options,
 				 ccd_file,

@@ -1740,22 +1752,7 @@ multi_client_connect_source_ccd (struct multi_context *m,
 				 CLIENT_CONNECT_OPT_MASK,
 				 option_types_found,
 				 mi->context.c2.es);
-	}
-      else /* try default file */
-	{
-	  ccd_file = gen_path (mi->context.options.client_config_dir,
-			       CCD_DEFAULT,
-			       &gc);
 
-	  if (test_file (ccd_file))
-	    {
-	      options_server_import (&mi->context.options,
-				     ccd_file,
-				     D_IMPORT_ERRORS|M_OPTERR,
-				     CLIENT_CONNECT_OPT_MASK,
-				     option_types_found,
-				     mi->context.c2.es);
-	    }
 	}
 
       gc_free (&gc);

Signature · 1 line

2.1.4

Fabian Knittel

2015-01-17 12:30 UTC #16 lore raw

PATCH v2 4/9 client-connect: Move multi_client_connect_setenv into early_setup

In reply to Fabian Knittel

This patch moves multi_client_connect_setenv into multi_client_connect_early_setup and makes sure that every client-connect handling function updates the virtual address selection.

Background: This unifies how the client-connect handling functions work.

Signed-off-by Fabian Knittel <[email protected]...>

---
 src/openvpn/multi.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

src/openvpn/multi.c +10 -9

index 8cc61d7..e9fbdc6 100644
@@ -1646,15 +1646,6 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 
       multi_client_connect_source_ccd (m, mi, &option_types_found);
 
-      /*
-       * Select a virtual address from either --ifconfig-push in
-       * --client-config-dir file or --ifconfig-pool.
-       */
-      multi_select_virtual_addr (m, mi);
-
-      /* do --client-connect setenvs */
-      multi_client_connect_setenv (m, mi);
-
       multi_client_connect_call_plugin_v1 (m, mi, &option_types_found,
 					   &cc_succeeded, &cc_succeeded_count);

@@ -1711,6 +1702,10 @@ multi_client_connect_early_setup (struct multi_context *m,
 
   /* reset pool handle to null */
   mi->vaddr_handle = -1;
+
+  /* do --client-connect setenvs */
+  multi_select_virtual_addr (m, mi);
+  multi_client_connect_setenv (m, mi);
 }
 
 /*

@@ -1753,6 +1748,12 @@ multi_client_connect_source_ccd (struct multi_context *m,
 				 option_types_found,
 				 mi->context.c2.es);
 
+	  /*
+	   * Select a virtual address from either --ifconfig-push in
+	   * --client-config-dir file or --ifconfig-pool.
+	   */
+	  multi_select_virtual_addr (m, mi);
+	  multi_set_virtual_addr_env (m, mi);
 	}
 
       gc_free (&gc);

Signature · 1 line

2.1.4

Fabian Knittel

2015-01-17 12:30 UTC #17 lore raw

PATCH v2 5/9 client-connect: Refactor to use return values instead of modifying a passed-in flag

In reply to Fabian Knittel

In addition, the client-connect helpers are now called in completely identical ways. This is in preparation of handling the helpers as simple call-backs.

Signed-off-by Fabian Knittel <[email protected]...>

---
 src/openvpn/multi.c | 122 +++++++++++++++++++++++++++++++---------------------
 src/openvpn/multi.h |  11 ++++-
 2 files changed, 82 insertions(+), 51 deletions(-)

src/openvpn/multi.c +72 ~~-50~~

index e9fbdc6..072a5f6 100644
@@ -1530,13 +1530,12 @@ multi_client_connect_post_plugin (struct multi_context *m,
 /*
  * Called to load management-derived client-connect config
  */
-static void
+static enum client_connect_return
 multi_client_connect_mda (struct multi_context *m,
 			  struct multi_instance *mi,
-			  unsigned int *option_types_found,
-			  int *cc_succeeded,
-			  int *cc_succeeded_count)
+			  unsigned int *option_types_found)
 {
+  enum client_connect_return ret = CC_RET_SKIPPED;
 #ifdef MANAGEMENT_DEF_AUTH
   if (mi->cc_config)
     {

@@ -1562,9 +1561,10 @@ multi_client_connect_mda (struct multi_context *m,
       multi_select_virtual_addr (m, mi);
       multi_set_virtual_addr_env (m, mi);
 
-      ++*cc_succeeded_count;
+      ret = CC_RET_SUCCEEDED;
     }
 #endif
+  return ret;
 }
 
 static void

@@ -1641,38 +1641,59 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
       unsigned int option_types_found = 0;
       int cc_succeeded = true; /* client connect script status */
       int cc_succeeded_count = 0;
+      enum client_connect_return ret;
 
       multi_client_connect_early_setup (m, mi);
 
-      multi_client_connect_source_ccd (m, mi, &option_types_found);
+      if (cc_succeeded)
+	{
+	  ret = multi_client_connect_source_ccd (m, mi, &option_types_found);
+	  if (ret == CC_RET_SUCCEEDED)
+	    ++cc_succeeded_count;
+	  else if (ret == CC_RET_FAILED)
+	    cc_succeeded = false;
+	}
 
-      multi_client_connect_call_plugin_v1 (m, mi, &option_types_found,
-					   &cc_succeeded, &cc_succeeded_count);
+      if (cc_succeeded)
+	{
+	  ret = multi_client_connect_call_plugin_v1 (m, mi,
+						     &option_types_found);
+	  if (ret == CC_RET_SUCCEEDED)
+	    ++cc_succeeded_count;
+	  else if (ret == CC_RET_FAILED)
+	    cc_succeeded = false;
+	}
 
-      multi_client_connect_call_plugin_v2 (m, mi, &option_types_found,
-					   &cc_succeeded, &cc_succeeded_count);
+      if (cc_succeeded)
+	{
+	  ret = multi_client_connect_call_plugin_v2 (m, mi,
+						     &option_types_found);
+	  if (ret == CC_RET_SUCCEEDED)
+	    ++cc_succeeded_count;
+	  else if (ret == CC_RET_FAILED)
+	    cc_succeeded = false;
+	}
 
-      /*
-       * Run --client-connect script.
-       */
       if (cc_succeeded)
 	{
-	  multi_client_connect_call_script (m, mi, &option_types_found,
-					    &cc_succeeded,
-					    &cc_succeeded_count);
+	  ret = multi_client_connect_call_script (m, mi, &option_types_found);
+	  if (ret == CC_RET_SUCCEEDED)
+	    ++cc_succeeded_count;
+	  else if (ret == CC_RET_FAILED)
+	    cc_succeeded = false;
 	}
 
-      /*
-       * Check for client-connect script left by management interface client
-       */
       if (cc_succeeded)
 	{
-	  multi_client_connect_mda (m, mi, &option_types_found,
-				    &cc_succeeded, &cc_succeeded_count);
+	  ret = multi_client_connect_mda (m, mi, &option_types_found);
+	  if (ret == CC_RET_SUCCEEDED)
+	    ++cc_succeeded_count;
+	  else if (ret == CC_RET_FAILED)
+	    cc_succeeded = false;
 	}
 
-      multi_client_connect_late_setup (m, mi, option_types_found,
-				       cc_succeeded, cc_succeeded_count);
+      multi_client_connect_late_setup (m, mi, option_types_found, cc_succeeded,
+				       cc_succeeded_count);
 
       /* set flag so we don't get called again */
       mi->connection_established_flag = true;

@@ -1712,11 +1733,13 @@ multi_client_connect_early_setup (struct multi_context *m,
  * Try to source a dynamic config file from the
  * --client-config-dir directory.
  */
-static void
+static enum client_connect_return
 multi_client_connect_source_ccd (struct multi_context *m,
 				 struct multi_instance *mi,
 				 unsigned int *option_types_found)
 {
+  enum client_connect_return ret = CC_RET_SKIPPED;
+
   if (mi->context.options.client_config_dir)
     {
       struct gc_arena gc = gc_new ();

@@ -1754,10 +1777,14 @@ multi_client_connect_source_ccd (struct multi_context *m,
 	   */
 	  multi_select_virtual_addr (m, mi);
 	  multi_set_virtual_addr_env (m, mi);
+
+	  ret = CC_RET_SUCCEEDED;
 	}
 
       gc_free (&gc);
     }
+
+  return ret;
 }
 
 /*

@@ -1765,19 +1792,16 @@ multi_client_connect_source_ccd (struct multi_context *m,
  *
  * deprecated callback, use a file for passing back return info
  */
-static void
+static enum client_connect_return
 multi_client_connect_call_plugin_v1 (struct multi_context *m,
 				     struct multi_instance *mi,
-				     unsigned int *option_types_found,
-				     int *cc_succeeded,
-				     int *cc_succeeded_count)
+				     unsigned int *option_types_found)
 {
+  enum client_connect_return ret = CC_RET_SKIPPED;
 #ifdef ENABLE_PLUGIN
   ASSERT (m);
   ASSERT (mi);
   ASSERT (option_types_found);
-  ASSERT (cc_succeeded);
-  ASSERT (cc_succeeded_count);
 
   if (plugin_defined (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT))
     {

@@ -1788,7 +1812,7 @@ multi_client_connect_call_plugin_v1 (struct multi_context *m,
 
       if (!dc_file)
 	{
-	  *cc_succeeded = false;
+	  ret = CC_RET_FAILED;
 	  goto script_depr_failed;
 	}

@@ -1801,17 +1825,18 @@ multi_client_connect_call_plugin_v1 (struct multi_context *m,
       if (plug_ret != OPENVPN_PLUGIN_FUNC_SUCCESS)
 	{
 	  msg (M_WARN, "WARNING: client-connect plugin call failed");
-	  *cc_succeeded = false;
+	  ret = CC_RET_FAILED;
 	}
       else
 	{
 	  multi_client_connect_post (m, mi, dc_file, option_types_found);
-	  ++*cc_succeeded_count;
+	  ret = CC_RET_SUCCEEDED;
 	}
 script_depr_failed:
       gc_free (&gc);
     }
 #endif
+  return ret;
 }
 
 /*

@@ -1819,19 +1844,16 @@ script_depr_failed:
  *
  * V2 callback, use a plugin_return struct for passing back return info
  */
-static void
+static enum client_connect_return
 multi_client_connect_call_plugin_v2 (struct multi_context *m,
 				     struct multi_instance *mi,
-				     unsigned int *option_types_found,
-				     int *cc_succeeded,
-				     int *cc_succeeded_count)
+				     unsigned int *option_types_found)
 {
+  enum client_connect_return ret = CC_RET_SKIPPED;
 #ifdef ENABLE_PLUGIN
   ASSERT (m);
   ASSERT (mi);
   ASSERT (option_types_found);
-  ASSERT (cc_succeeded);
-  ASSERT (cc_succeeded_count);
 
   if (plugin_defined (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT_V2))
     {

@@ -1846,31 +1868,29 @@ multi_client_connect_call_plugin_v2 (struct multi_context *m,
       if (plug_ret != OPENVPN_PLUGIN_FUNC_SUCCESS)
 	{
 	  msg (M_WARN, "WARNING: client-connect-v2 plugin call failed");
-	  *cc_succeeded = false;
+	  ret = CC_RET_FAILED;
 	}
       else
 	{
 	  multi_client_connect_post_plugin (m, mi, &pr, option_types_found);
-	  ++*cc_succeeded_count;
+	  ret = CC_RET_SUCCEEDED;
 	}
 
       plugin_return_free (&pr);
     }
 #endif
+  return ret;
 }
 
-static void
+static enum client_connect_return
 multi_client_connect_call_script (struct multi_context *m,
 				  struct multi_instance *mi,
-				  unsigned int *option_types_found,
-				  int *cc_succeeded,
-				  int *cc_succeeded_count)
+				  unsigned int *option_types_found)
 {
+  enum client_connect_return ret = CC_RET_SKIPPED;
   ASSERT (m);
   ASSERT (mi);
   ASSERT (option_types_found);
-  ASSERT (cc_succeeded);
-  ASSERT (cc_succeeded_count);
 
   if (mi->context.options.client_connect_script)
     {

@@ -1883,7 +1903,7 @@ multi_client_connect_call_script (struct multi_context *m,
       dc_file = create_temp_file (mi->context.options.tmp_dir, "cc", &gc);
       if (!dc_file)
 	{
-	  cc_succeeded = false;
+	  ret = CC_RET_FAILED;
 	  goto script_failed;
 	}

@@ -1894,14 +1914,16 @@ multi_client_connect_call_script (struct multi_context *m,
       if (openvpn_run_script (&argv, mi->context.c2.es, 0, "--client-connect"))
 	{
 	  multi_client_connect_post (m, mi, dc_file, option_types_found);
-	  ++cc_succeeded_count;
+	  ret = CC_RET_SUCCEEDED;
 	}
       else
-	cc_succeeded = false;
+	ret = CC_RET_FAILED;
+
 script_failed:
       argv_reset (&argv);
       gc_free (&gc);
     }
+  return ret;
 }
 
 static void

src/openvpn/multi.h +10 -1

index 32b89d2..4d61085 100644
@@ -107,7 +107,6 @@ struct multi_instance {
                                  *   for this VPN tunnel. */
 };
 
-
 /**
  * Main OpenVPN server state structure.
  *

@@ -174,6 +173,16 @@ struct multi_context {
   struct event_timeout stale_routes_check_et;
 };
 
+/**
+ * Return values used by the client connect call-back functions.
+ */
+enum client_connect_return
+{
+  CC_RET_FAILED,
+  CC_RET_SUCCEEDED,
+  CC_RET_SKIPPED
+};
+
 /*
  * Host route
  */

Signature · 1 line

2.1.4

Fabian Knittel

2015-01-17 12:30 UTC #18 lore raw

PATCH v2 6/9 client-connect: Refactor client-connect handling to calling a bunch of hooks in a loop

In reply to Fabian Knittel

This patch changes the calling of the client-connect functions into an array of hooks and a block of code that calls them in a loop.

Signed-off-by Fabian Knittel <[email protected]...>

---
 src/openvpn/multi.c | 57 +++++++++++++++--------------------------------------
 1 file changed, 16 insertions(+), 41 deletions(-)

src/openvpn/multi.c +16 ~~-41~~

index 072a5f6..648b026 100644
@@ -1638,6 +1638,18 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 {
   if (tls_authentication_status (mi->context.c2.tls_multi, 0) == TLS_AUTHENTICATION_SUCCEEDED)
     {
+      typedef enum client_connect_return_t (*multi_client_connect_handler)(struct multi_context *m, struct multi_instance *mi, unsigned int *option_types_found);
+
+      multi_client_connect_handler handlers[] = {
+	  multi_client_connect_source_ccd,
+	  multi_client_connect_call_plugin_v1,
+	  multi_client_connect_call_plugin_v2,
+	  multi_client_connect_call_script,
+	  multi_client_connect_mda,
+	  NULL
+      };
+
+      int cur_handler = 0;
       unsigned int option_types_found = 0;
       int cc_succeeded = true; /* client connect script status */
       int cc_succeeded_count = 0;

@@ -1645,51 +1657,14 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mi
 
       multi_client_connect_early_setup (m, mi);
 
-      if (cc_succeeded)
+      while (succeeded && handlers[cur_handler])
 	{
-	  ret = multi_client_connect_source_ccd (m, mi, &option_types_found);
+	  ret = handlers[cur_handler] (m, mi, &option_types_found);
 	  if (ret == CC_RET_SUCCEEDED)
 	    ++cc_succeeded_count;
 	  else if (ret == CC_RET_FAILED)
-	    cc_succeeded = false;
-	}
-
-      if (cc_succeeded)
-	{
-	  ret = multi_client_connect_call_plugin_v1 (m, mi,
-						     &option_types_found);
-	  if (ret == CC_RET_SUCCEEDED)
-	    ++cc_succeeded_count;
-	  else if (ret == CC_RET_FAILED)
-	    cc_succeeded = false;
-	}
-
-      if (cc_succeeded)
-	{
-	  ret = multi_client_connect_call_plugin_v2 (m, mi,
-						     &option_types_found);
-	  if (ret == CC_RET_SUCCEEDED)
-	    ++cc_succeeded_count;
-	  else if (ret == CC_RET_FAILED)
-	    cc_succeeded = false;
-	}
-
-      if (cc_succeeded)
-	{
-	  ret = multi_client_connect_call_script (m, mi, &option_types_found);
-	  if (ret == CC_RET_SUCCEEDED)
-	    ++cc_succeeded_count;
-	  else if (ret == CC_RET_FAILED)
-	    cc_succeeded = false;
-	}
-
-      if (cc_succeeded)
-	{
-	  ret = multi_client_connect_mda (m, mi, &option_types_found);
-	  if (ret == CC_RET_SUCCEEDED)
-	    ++cc_succeeded_count;
-	  else if (ret == CC_RET_FAILED)
-	    cc_succeeded = false;
+	    succeeded = false;
+	  ++cur_handler;
 	}
 
       multi_client_connect_late_setup (m, mi, option_types_found, cc_succeeded,

Signature · 1 line

2.1.4

Fabian Knittel

2015-01-17 12:30 UTC #19 lore raw

PATCH v2 7/9 client-connect: Add CC_RET_DEFERRED and cope with deferred client-connect

In reply to Fabian Knittel

Signed-off-by Fabian Knittel <[email protected]...>

---
 src/openvpn/multi.c | 255 ++++++++++++++++++++++++++++++++++------------------
 src/openvpn/multi.h |  18 ++++
 2 files changed, 185 insertions(+), 88 deletions(-)

src/openvpn/multi.c +167 ~~-88~~

index 648b026..7079946 100644
@@ -48,6 +48,9 @@
 
 /*#define MULTI_DEBUG_EVENT_LOOP*/
 
+static void delete_client_connect_state (struct multi_instance *mi);
+
+
 #ifdef MULTI_DEBUG_EVENT_LOOP
 static const char *
 id (struct multi_instance *mi)

@@ -584,6 +587,8 @@ multi_close_instance (struct multi_context *m,
   set_cc_config (mi, NULL);
 #endif
 
+  delete_client_connect_state (mi);
+
   multi_client_disconnect_script (m, mi);
 
   if (mi->did_open_context)

@@ -1594,94 +1599,6 @@ multi_client_connect_setenv (struct multi_context *m,
 
 static void
 multi_client_connect_early_setup (struct multi_context *m,
-				  struct multi_instance *mi);
-static void
-multi_client_connect_source_ccd (struct multi_context *m,
-				 struct multi_instance *mi,
-				 unsigned int *option_types_found);
-static void
-multi_client_connect_call_plugin_v1 (struct multi_context *m,
-				     struct multi_instance *mi,
-				     unsigned int *option_types_found,
-				     int *cc_succeeded,
-				     int *cc_succeeded_count);
-static void
-multi_client_connect_call_plugin_v2 (struct multi_context *m,
-				     struct multi_instance *mi,
-				     unsigned int *option_types_found,
-				     int *cc_succeeded,
-				     int *cc_succeeded_count);
-static void
-multi_client_connect_call_script (struct multi_context *m,
-				  struct multi_instance *mi,
-				  unsigned int *option_types_found,
-				  int *cc_succeeded,
-				  int *cc_succeeded_count);
-static void
-multi_client_connect_late_setup (struct multi_context *m,
-				 struct multi_instance *mi,
-				 const unsigned int option_types_found,
-				 int cc_succeeded,
-				 const int cc_succeeded_count);
-
-/*
- * Called as soon as the SSL/TLS connection authenticates.
- *
- * Instance-specific directives to be processed:
- *
- *   iroute start-ip end-ip
- *   ifconfig-push local remote-netmask
- *   push
- */
-static void
-multi_connection_established (struct multi_context *m, struct multi_instance *mi)
-{
-  if (tls_authentication_status (mi->context.c2.tls_multi, 0) == TLS_AUTHENTICATION_SUCCEEDED)
-    {
-      typedef enum client_connect_return_t (*multi_client_connect_handler)(struct multi_context *m, struct multi_instance *mi, unsigned int *option_types_found);
-
-      multi_client_connect_handler handlers[] = {
-	  multi_client_connect_source_ccd,
-	  multi_client_connect_call_plugin_v1,
-	  multi_client_connect_call_plugin_v2,
-	  multi_client_connect_call_script,
-	  multi_client_connect_mda,
-	  NULL
-      };
-
-      int cur_handler = 0;
-      unsigned int option_types_found = 0;
-      int cc_succeeded = true; /* client connect script status */
-      int cc_succeeded_count = 0;
-      enum client_connect_return ret;
-
-      multi_client_connect_early_setup (m, mi);
-
-      while (succeeded && handlers[cur_handler])
-	{
-	  ret = handlers[cur_handler] (m, mi, &option_types_found);
-	  if (ret == CC_RET_SUCCEEDED)
-	    ++cc_succeeded_count;
-	  else if (ret == CC_RET_FAILED)
-	    succeeded = false;
-	  ++cur_handler;
-	}
-
-      multi_client_connect_late_setup (m, mi, option_types_found, cc_succeeded,
-				       cc_succeeded_count);
-
-      /* set flag so we don't get called again */
-      mi->connection_established_flag = true;
-    }
-
-  /*
-   * Reply now to client's PUSH_REQUEST query
-   */
-  mi->context.c2.push_reply_deferred = false;
-}
-
-static void
-multi_client_connect_early_setup (struct multi_context *m,
 				  struct multi_instance *mi)
 {
   /* lock down the common name and cert hashes so they can't change during

@@ -2016,6 +1933,168 @@ multi_client_connect_late_setup (struct multi_context *m,
   gc_free (&gc);
 }
 
+static enum client_connect_return
+multi_client_connect_fail (struct multi_context *m, struct multi_instance *mi,
+			   unsigned int *option_types_found)
+{
+  /* Called null call-back.  This should never happen. */
+  return CC_RET_FAILED;
+}
+
+static void
+new_client_connect_state (struct multi_instance *mi)
+{
+  ASSERT (mi);
+  ALLOC_OBJ_CLEAR (mi->client_connect_state, struct client_connect_state);
+
+  mi->client_connect_state->succeeded = true;
+}
+
+static void
+delete_client_connect_state (struct multi_instance *mi)
+{
+  ASSERT (mi);
+  if (mi->client_connect_state)
+    {
+      free (mi->client_connect_state);
+      mi->client_connect_state = NULL;
+    }
+}
+
+typedef enum client_connect_return (*client_connect_handler)
+  (struct multi_context *m, struct multi_instance *mi,
+   unsigned int *option_types_found);
+
+struct client_connect_handlers
+{
+  client_connect_handler main;
+  client_connect_handler deferred;
+};
+
+static const struct client_connect_handlers client_connect_handlers[] = {
+  {
+    main: multi_client_connect_source_ccd,
+    deferred: multi_client_connect_fail
+  },
+  {
+    main: multi_client_connect_call_plugin_v1,
+    deferred: multi_client_connect_fail
+  },
+  {
+    main: multi_client_connect_call_plugin_v2,
+    deferred: multi_client_connect_fail
+  },
+  {
+    main: multi_client_connect_call_script,
+    deferred: multi_client_connect_fail
+  },
+  {
+    main: multi_client_connect_mda,
+    deferred: multi_client_connect_fail
+  },
+  {
+    /* End of list.  */
+  }
+};
+
+static enum client_connect_return
+multi_client_handle_single (client_connect_handler handler,
+			    struct multi_context *m,
+			    struct multi_instance *mi)
+{
+  struct client_connect_state *ccs = mi->client_connect_state;
+  enum client_connect_return ret;
+
+  ret = handler (m, mi, &ccs->option_types_found);
+  if (ret == CC_RET_SUCCEEDED)
+    ++ccs->succeeded_count;
+  else if (ret == CC_RET_FAILED)
+    ccs->succeeded = false;
+  else if (ret == CC_RET_DEFERRED)
+    {
+      /* Signal that we're deferring.  */
+      return ret;
+    }
+  return ret;
+}
+
+/*
+ * Called as soon as the SSL/TLS connection authenticates.
+ *
+ * Instance-specific directives to be processed:
+ *
+ *   iroute start-ip end-ip
+ *   ifconfig-push local remote-netmask
+ *   push
+ */
+static void
+multi_connection_established (struct multi_context *m, struct multi_instance *mi)
+{
+  if (tls_authentication_status (mi->context.c2.tls_multi, 0) == TLS_AUTHENTICATION_SUCCEEDED)
+    {
+      struct client_connect_state *ccs = mi->client_connect_state;
+      enum client_connect_return ret;
+
+      if (!ccs)
+	{
+	  /* This is the first time that we run client-connect handlers
+	     for this connection. */
+
+	  new_client_connect_state (mi);
+	  ccs = mi->client_connect_state;
+
+          multi_client_connect_early_setup (m, mi);
+	}
+      else
+	{
+	  /* We are returning from a deferred handler.  Check whether the
+	     handler has been completed yet.  */
+
+	  ASSERT (client_connect_handlers[ccs->cur_handler_idx].main);
+	  ASSERT (client_connect_handlers[ccs->cur_handler_idx].deferred);
+
+	  ret = multi_client_handle_single
+		  (client_connect_handlers[ccs->cur_handler_idx].deferred, m,
+		   mi);
+	  if (ret == CC_RET_DEFERRED)
+	    return;
+
+	  /* Proceed to next handler.  */
+	  ++ccs->cur_handler_idx;
+	}
+
+
+      /* Cycle through all (remaining) client-connect handlers until all
+	 have completed, one of them fails or one of them defers.  */
+      while (ccs->succeeded &&
+	     client_connect_handlers[ccs->cur_handler_idx].main)
+	{
+	  ret = multi_client_handle_single
+		  (client_connect_handlers[ccs->cur_handler_idx].main, m, mi);
+	  if (ret == CC_RET_DEFERRED)
+	    return;
+
+	  /* Proceed to next handler.  */
+	  ++ccs->cur_handler_idx;
+	}
+
+      /* Done with all client-connect handlers now.  */
+
+      multi_client_connect_late_setup (m, mi, ccs->option_types_found,
+				       ccs->succeeded, ccs->succeeded_count);
+
+      /* Clean-up.  */
+      delete_client_connect_state (mi);
+
+      /* set flag so we don't get called again */
+      mi->connection_established_flag = true;
+    }
+
+  /*
+   * Reply now to client's PUSH_REQUEST query
+   */
+  mi->context.c2.push_reply_deferred = false;
+}
 
 /*
  * Add a mbuf buffer to a particular

src/openvpn/multi.h +18 -0

index 4d61085..e493c7d 100644
@@ -56,6 +56,22 @@ struct multi_reap
   time_t last_call;
 };
 
+/**
+ * Detached client connection state.  This is the state that is tracked while
+ * the client connect hooks are executed.
+ */
+struct client_connect_state
+{
+  /* Index of currently executed handler.  */
+  int cur_handler_idx;
+  /* Did all of the handlers succeed up to now?  */
+  bool succeeded;
+  /* How many handlers succeeded?  */
+  int succeeded_count;
+  /* Remember which option classes where processed for delayed option
+     handling. */
+  unsigned int option_types_found;
+};
 
 /**
  * Server-mode state structure for one single VPN tunnel.

@@ -105,6 +121,7 @@ struct multi_instance {
 
   struct context context;       /**< The context structure storing state
                                  *   for this VPN tunnel. */
+  struct client_connect_state *client_connect_state;
 };
 
 /**

@@ -180,6 +197,7 @@ enum client_connect_return
 {
   CC_RET_FAILED,
   CC_RET_SUCCEEDED,
+  CC_RET_DEFERRED,
   CC_RET_SKIPPED
 };

Signature · 1 line

2.1.4

Fabian Knittel

2015-01-17 12:30 UTC #20 lore raw

PATCH v2 8/9 client-connect: Add deferred support to the client-connect script handler

In reply to Fabian Knittel

In addition, the patch also allows the storage of the client config file name in struct client_connect_state.

Signed-off-by Fabian Knittel <[email protected]...>

---
 src/openvpn/multi.c | 185 ++++++++++++++++++++++++++++++++++++++++++++++++----
 src/openvpn/multi.h |   3 +
 2 files changed, 177 insertions(+), 11 deletions(-)

src/openvpn/multi.c +174 ~~-11~~

index 7079946..68a7248 100644
@@ -1621,6 +1621,129 @@ multi_client_connect_early_setup (struct multi_context *m,
   multi_client_connect_setenv (m, mi);
 }
 
+
+static void
+ccs_delete_deferred_ret_file (struct multi_instance *mi)
+{
+  struct client_connect_state *ccs = mi->client_connect_state;
+  if (ccs->deferred_ret_file)
+    {
+      setenv_del (mi->context.c2.es, "client_connect_deferred_file");
+      if (!platform_unlink (ccs->deferred_ret_file))
+           msg (D_MULTI_ERRORS, "MULTI: problem deleting temporary file: %s",
+                ccs->deferred_ret_file);
+      free (ccs->deferred_ret_file);
+      ccs->deferred_ret_file = NULL;
+    }
+}
+
+static bool
+ccs_gen_deferred_ret_file (struct multi_instance *mi)
+{
+  struct client_connect_state *ccs = mi->client_connect_state;
+  struct gc_arena gc = gc_new ();
+  const char *fn;
+
+  if (ccs->deferred_ret_file)
+    ccs_delete_deferred_ret_file (mi);
+
+  fn = create_temp_file (mi->context.options.tmp_dir, "ccr", &gc);
+  if (!fn)
+    {
+      gc_free (&gc);
+      return false;
+    }
+  ccs->deferred_ret_file = string_alloc (fn, NULL);
+
+  setenv_str (mi->context.c2.es, "client_connect_deferred_file",
+	      ccs->deferred_ret_file);
+
+  gc_free (&gc);
+  return true;
+}
+
+/*
+ * Tests whether the deferred return value file exists and returns the
+ * contained return value.
+ *
+ * Returns CC_RET_SKIPPED if the file doesn't exist or is empty.
+ * Returns CC_RET_DEFERRED, CC_RET_SUCCEEDED or CC_RET_FAILED depending on
+ * the value stored in the file.
+ */
+static enum client_connect_return
+ccs_test_deferred_ret_file (struct multi_instance *mi)
+{
+  struct client_connect_state *ccs = mi->client_connect_state;
+  enum client_connect_return ret = CC_RET_SKIPPED;
+  FILE *fp = fopen (ccs->deferred_ret_file, "r");
+  if (fp)
+    {
+      const int c = fgetc (fp);
+      switch (c)
+	{
+	case '0':
+	  ret = CC_RET_FAILED;
+	  break;
+	case '1':
+	  ret = CC_RET_SUCCEEDED;
+	  break;
+	case '2':
+	  ret = CC_RET_DEFERRED;
+	  break;
+	case EOF:
+	  ret = CC_RET_SKIPPED;
+	  break;
+	default:
+	  /* We received an unknown/unexpected value.  Assume failure. */
+	  ret = CC_RET_FAILED;
+	}
+      fclose (fp);
+    }
+  return ret;
+}
+
+
+static void
+ccs_delete_config_file (struct multi_instance *mi)
+{
+  struct client_connect_state *ccs = mi->client_connect_state;
+  if (ccs->config_file)
+    {
+      setenv_del (mi->context.c2.es, "client_connect_config_file");
+      if (!platform_unlink (ccs->config_file))
+           msg (D_MULTI_ERRORS, "MULTI: problem deleting temporary file: %s",
+                ccs->config_file);
+      free (ccs->config_file);
+      ccs->config_file = NULL;
+    }
+}
+
+static bool
+ccs_gen_config_file (struct multi_instance *mi)
+{
+  struct client_connect_state *ccs = mi->client_connect_state;
+  struct gc_arena gc = gc_new ();
+  const char *fn;
+
+  if (ccs->config_file)
+    ccs_delete_config_file (mi);
+
+  fn = create_temp_file (mi->context.options.tmp_dir, "cc", &gc);
+  if (!fn)
+    {
+      gc_free (&gc);
+      return false;
+    }
+  ccs->config_file = string_alloc (fn, NULL);
+
+  setenv_str (mi->context.c2.es, "client_connect_config_file",
+	      ccs->config_file);
+
+  gc_free (&gc);
+  return true;
+}
+
+
 /*
  * Try to source a dynamic config file from the
  * --client-config-dir directory.

@@ -1786,34 +1909,71 @@ multi_client_connect_call_script (struct multi_context *m,
 
   if (mi->context.options.client_connect_script)
     {
-      struct gc_arena gc = gc_new ();
+      struct client_connect_state *ccs = mi->client_connect_state;
       struct argv argv = argv_new ();
-      const char *dc_file;
+      ASSERT (ccs);
 
       setenv_str (mi->context.c2.es, "script_type", "client-connect");
 
-      dc_file = create_temp_file (mi->context.options.tmp_dir, "cc", &gc);
-      if (!dc_file)
+      if (!ccs_gen_config_file (mi) ||
+	  !ccs_gen_deferred_ret_file (mi))
 	{
 	  ret = CC_RET_FAILED;
-	  goto script_failed;
+	  goto cleanup;
 	}
 
       argv_printf (&argv, "%sc %s",
 		   mi->context.options.client_connect_script,
-		   dc_file);
+		   ccs->config_file);
 
       if (openvpn_run_script (&argv, mi->context.c2.es, 0, "--client-connect"))
 	{
-	  multi_client_connect_post (m, mi, dc_file, option_types_found);
-	  ret = CC_RET_SUCCEEDED;
+	  if (ccs_test_deferred_ret_file (mi) == CC_RET_DEFERRED)
+	    ret = CC_RET_DEFERRED;
+	  else
+	    {
+	      multi_client_connect_post (m, mi, ccs->config_file,
+					 option_types_found);
+	      ret = CC_RET_SUCCEEDED;
+	    }
 	}
       else
 	ret = CC_RET_FAILED;
 
-script_failed:
+cleanup:
+      if (ret != CC_RET_DEFERRED)
+	{
+	  ccs_delete_config_file (mi);
+	  ccs_delete_deferred_ret_file (mi);
+	}
+
       argv_reset (&argv);
-      gc_free (&gc);
+    }
+  return ret;
+}
+
+static enum client_connect_return
+multi_client_handle_deferred (struct multi_context *m,
+			      struct multi_instance *mi,
+			      unsigned int *option_types_found)
+{
+  struct client_connect_state *ccs = mi->client_connect_state;
+  enum client_connect_return ret = CC_RET_SKIPPED;
+  ASSERT (option_types_found);
+  ASSERT (ccs);
+
+  ret = ccs_test_deferred_ret_file (mi);
+
+  if (ret == CC_RET_SKIPPED)
+    /* Skipped and deferred are equivalent in this context. */
+    ret = CC_RET_DEFERRED;
+
+  if (ret != CC_RET_DEFERRED)
+    {
+      ccs_delete_deferred_ret_file (mi);
+
+      multi_client_connect_post (m, mi, ccs->config_file, option_types_found);
+      ccs_delete_config_file (mi);
     }
   return ret;
 }

@@ -1956,6 +2116,9 @@ delete_client_connect_state (struct multi_instance *mi)
   ASSERT (mi);
   if (mi->client_connect_state)
     {
+      ccs_delete_deferred_ret_file (mi);
+      ccs_delete_config_file (mi);
+
       free (mi->client_connect_state);
       mi->client_connect_state = NULL;
     }

@@ -1986,7 +2149,7 @@ static const struct client_connect_handlers client_connect_handlers[] = {
   },
   {
     main: multi_client_connect_call_script,
-    deferred: multi_client_connect_fail
+    deferred: multi_client_handle_deferred
   },
   {
     main: multi_client_connect_mda,

src/openvpn/multi.h +3 -0

index e493c7d..9244739 100644
@@ -71,6 +71,9 @@ struct client_connect_state
   /* Remember which option classes where processed for delayed option
      handling. */
   unsigned int option_types_found;
+
+  char *deferred_ret_file;
+  char *config_file;
 };
 
 /**

Signature · 1 line

2.1.4

Fabian Knittel

2015-01-17 12:30 UTC #21 lore raw

PATCH v2 9/9 client-connect: Add deferred support to the client-connect plugin v1 handler

In reply to Fabian Knittel

Uses the infrastructure provided and used in the previous patch to provide deferral support to the v1 client-connect plugin handler as well.

Signed-off-by Fabian Knittel <[email protected]...>

---
 src/openvpn/multi.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

src/openvpn/multi.c +20 ~~-12~~

index 68a7248..ab73034 100644
@@ -1814,6 +1814,7 @@ multi_client_connect_call_plugin_v1 (struct multi_context *m,
 {
   enum client_connect_return ret = CC_RET_SKIPPED;
 #ifdef ENABLE_PLUGIN
+  struct client_connect_state *ccs = mi->client_connect_state;
   ASSERT (m);
   ASSERT (mi);
   ASSERT (option_types_found);

@@ -1821,34 +1822,41 @@ multi_client_connect_call_plugin_v1 (struct multi_context *m,
   if (plugin_defined (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT))
     {
       int plug_ret;
-      struct gc_arena gc = gc_new ();
       struct argv argv = argv_new ();
-      const char *dc_file = create_temp_file (mi->context.options.tmp_dir, "cc", &gc);
 
-      if (!dc_file)
+      if (!ccs_gen_config_file (mi) ||
+	  !ccs_gen_deferred_ret_file (mi))
 	{
 	  ret = CC_RET_FAILED;
-	  goto script_depr_failed;
+	  goto cleanup;
 	}
 
-      argv_printf (&argv, "%s", dc_file);
+      argv_printf (&argv, "%s", ccs->config_file);
 
       plug_ret = plugin_call (mi->context.plugins,
 			      OPENVPN_PLUGIN_CLIENT_CONNECT,
 			      &argv, NULL, mi->context.c2.es);
       argv_reset (&argv);
-      if (plug_ret != OPENVPN_PLUGIN_FUNC_SUCCESS)
+      if (plug_ret == OPENVPN_PLUGIN_FUNC_SUCCESS)
+	{
+	  multi_client_connect_post (m, mi, ccs->config_file,
+				     option_types_found);
+	  ret = CC_RET_SUCCEEDED;
+	}
+      else if (plug_ret == OPENVPN_PLUGIN_FUNC_DEFERRED)
+	ret = CC_RET_DEFERRED;
+      else
 	{
 	  msg (M_WARN, "WARNING: client-connect plugin call failed");
 	  ret = CC_RET_FAILED;
 	}
-      else
+
+cleanup:
+      if (ret != CC_RET_DEFERRED)
 	{
-	  multi_client_connect_post (m, mi, dc_file, option_types_found);
-	  ret = CC_RET_SUCCEEDED;
+	  ccs_delete_config_file (mi);
+	  ccs_delete_deferred_ret_file (mi);
 	}
-script_depr_failed:
-      gc_free (&gc);
     }
 #endif
   return ret;

@@ -2141,7 +2149,7 @@ static const struct client_connect_handlers client_connect_handlers[] = {
   },
   {
     main: multi_client_connect_call_plugin_v1,
-    deferred: multi_client_connect_fail
+    deferred: multi_client_handle_deferred
   },
   {
     main: multi_client_connect_call_plugin_v2,

Signature · 1 line

2.1.4

Heikki Hannikainen

2016-11-30 12:13 UTC #22 lore raw

Re: [Openvpn-devel] [PATCH v2 9/9] client-connect: Add deferred support to the client-connect plugin v1 handler

Fabian,

Have you by any chance ported this patch set forward to the current OpenVPN master / 2.4. beta?

We have used a variation of it, and gained a noticeable performance boost,
for a long time. Thanks to the deferred client-connect handling the
openvpn server does not stop packet processing while a client-connect
handler plugin is doing what it needs to do, reducing jitter and packet loss for other clients.

It'd be great to have this merged in the master tree at some point.

Quoted 97 lines · Fabian Knittel

Uses the infrastructure provided and used in the previous patch to provide deferral support to the v1 client-connect plugin handler as well.

Signed-off-by Fabian Knittel <[email protected]...>

---
src/openvpn/multi.c | 32 ++++++++++++++++++++------------

1 file changed, 20 insertions(+), 12 deletions(-)

src/openvpn/multi.c +20 ~~-12~~

index 68a7248..ab73034 100644
@@ -1814,6 +1814,7 @@ multi_client_connect_call_plugin_v1 (struct multi_context *m,
 {
  enum client_connect_return ret = CC_RET_SKIPPED;
 #ifdef ENABLE_PLUGIN
+  struct client_connect_state *ccs = mi->client_connect_state;
  ASSERT (m);
  ASSERT (mi);
  ASSERT (option_types_found);

@@ -1821,34 +1822,41 @@ multi_client_connect_call_plugin_v1 (struct multi_context *m,
  if (plugin_defined (mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT))
    {
      int plug_ret;
-      struct gc_arena gc = gc_new ();
      struct argv argv = argv_new ();
-      const char *dc_file = create_temp_file (mi->context.options.tmp_dir, "cc", &gc);
 
-      if (!dc_file)
+      if (!ccs_gen_config_file (mi) ||
+	  !ccs_gen_deferred_ret_file (mi))
 	{
 	  ret = CC_RET_FAILED;
-	  goto script_depr_failed;
+	  goto cleanup;
 	}
 
-      argv_printf (&argv, "%s", dc_file);
+      argv_printf (&argv, "%s", ccs->config_file);
 
      plug_ret = plugin_call (mi->context.plugins,
 			      OPENVPN_PLUGIN_CLIENT_CONNECT,
 			      &argv, NULL, mi->context.c2.es);
      argv_reset (&argv);
-      if (plug_ret != OPENVPN_PLUGIN_FUNC_SUCCESS)
+      if (plug_ret == OPENVPN_PLUGIN_FUNC_SUCCESS)
+	{
+	  multi_client_connect_post (m, mi, ccs->config_file,
+				     option_types_found);
+	  ret = CC_RET_SUCCEEDED;
+	}
+      else if (plug_ret == OPENVPN_PLUGIN_FUNC_DEFERRED)
+	ret = CC_RET_DEFERRED;
+      else
 	{
 	  msg (M_WARN, "WARNING: client-connect plugin call failed");
 	  ret = CC_RET_FAILED;
 	}
-      else
+
+cleanup:
+      if (ret != CC_RET_DEFERRED)
 	{
-	  multi_client_connect_post (m, mi, dc_file, option_types_found);
-	  ret = CC_RET_SUCCEEDED;
+	  ccs_delete_config_file (mi);
+	  ccs_delete_deferred_ret_file (mi);
 	}
-script_depr_failed:
-      gc_free (&gc);
    }
 #endif
  return ret;

@@ -2141,7 +2149,7 @@ static const struct client_connect_handlers client_connect_handlers[] = {
  },
  {
    main: multi_client_connect_call_plugin_v1,
-    deferred: multi_client_connect_fail
+    deferred: multi_client_handle_deferred
  },
  {
    main: multi_client_connect_call_plugin_v2,

Signature · 13 lines

2.1.4


------------------------------------------------------------------------------
New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
GigeNET is offering a free month of service with a new server in Ashburn.
Choose from 2 high performing configs, both with 100TB of bandwidth.
Higher redundancy.Lower latency.Increased capacity.Completely compliant.
http://p.sf.net/sfu/gigenet
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

- Hessu