Fix temporary file leak
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
@@ -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); }
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.cwhich 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@@ -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
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
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
@@ -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 *mioptions_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 *mioptions_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 *miif (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
@@ -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
PATCH 2/9 client-connect: Properly indent all functions
In reply to Fabian KnittelThis 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
@@ -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
PATCH 3/9 client-connect: Refactor multi_client_connect_source_ccd
In reply to Fabian KnittelRefactor 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
@@ -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
PATCH 4/9 client-connect: Move multi_client_connect_setenv into early_setup
In reply to Fabian KnittelThis 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
@@ -1635,15 +1635,6 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mimulti_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
PATCH 5/9 client-connect: Refactor to use return values instead of modifying a passed-in flag
In reply to Fabian KnittelThis 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
@@ -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 *miunsigned 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
@@ -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
PATCH 6/9 client-connect: Refactor client-connect handling to calling a bunch of hooks in a loop
In reply to Fabian KnittelThis 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
@@ -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 *mimulti_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
PATCH 7/9 client-connect: Add CC_RET_DEFERRED and cope with deferred client-connect
In reply to Fabian KnittelThis 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
@@ -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
@@ -54,6 +54,22 @@ struct multi_reaptime_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
PATCH 8/9 client-connect: Add deferred support to the client-connect script handler
In reply to Fabian KnittelThis 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
@@ -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
@@ -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
PATCH 9/9 client-connect: Add deferred support to the client-connect plugin v1 handler
In reply to Fabian KnittelUses 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
@@ -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
v2 cover Refactor client-connect / add support for deferred handling
In reply to Fabian KnittelHi,
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 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 | 839 +++++++++++++++++++++++++++++++++++--------------- src/openvpn/multi.h | 32 +- src/openvpn/options.h | 6 + 3 files changed, 636 insertions(+), 241 deletions(-)
Signature · 1 line
2.1.4
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
@@ -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 *mioptions_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 *mioptions_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 *miif (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
@@ -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
PATCH v2 2/9 client-connect: Properly indent all functions
In reply to Fabian KnittelThis 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
@@ -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
PATCH v2 3/9 client-connect: Refactor multi_client_connect_source_ccd
In reply to Fabian KnittelRefactor 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
@@ -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
PATCH v2 4/9 client-connect: Move multi_client_connect_setenv into early_setup
In reply to Fabian KnittelThis 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
@@ -1646,15 +1646,6 @@ multi_connection_established (struct multi_context *m, struct multi_instance *mimulti_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
PATCH v2 5/9 client-connect: Refactor to use return values instead of modifying a passed-in flag
In reply to Fabian KnittelThis 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
@@ -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 *miunsigned 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
@@ -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
PATCH v2 6/9 client-connect: Refactor client-connect handling to calling a bunch of hooks in a loop
In reply to Fabian KnittelThis 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
@@ -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 *mimulti_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
PATCH v2 7/9 client-connect: Add CC_RET_DEFERRED and cope with deferred client-connect
In reply to Fabian KnittelThis 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
@@ -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
@@ -56,6 +56,22 @@ struct multi_reaptime_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
PATCH v2 8/9 client-connect: Add deferred support to the client-connect script handler
In reply to Fabian KnittelThis 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 | 185 ++++++++++++++++++++++++++++++++++++++++++++++++---- src/openvpn/multi.h | 3 + 2 files changed, 177 insertions(+), 11 deletions(-)
src/openvpn/multi.c +174 -11
@@ -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
@@ -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
PATCH v2 9/9 client-connect: Add deferred support to the client-connect plugin v1 handler
In reply to Fabian KnittelUses 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
@@ -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
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@@ -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
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 frommulti_client_connect_post()intomulti_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
@@ -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 *mimulti_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); }
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-----
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 frommulti_client_connect_post()intomulti_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
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 frommulti_client_connect_post()intomulti_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
7da9d40243e0743e2d050ceb6ae34e467dd58973Author: 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