From patchwork Sun Jul 19 17:34:32 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,v8,1/5] Remove CAS_PARTIAL state X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1307 Message-Id: <20200719173436.16431-1-arne@rfc2549.org> To: openvpn-devel@lists.sourceforge.net Date: Sun, 19 Jul 2020 19:34:32 +0200 From: Arne Schwabe List-Id: This state is used to handle a corner case when multiple connect handlers are active and one of them fails. Unfortunately, this state complicates the state machine a bit without a good benefit. Current behaviour: First/all connect handler(s) fail: - client disconnect handler is not called at all At least one connect handler succeeds but a subsequent handler fails: - client disconect is called when we actually disconnect the client (a few seconds later, max tls timeout) All connect handlers suceed: - client disconect is called when we actually disconnect the client This patches changes the behaviour in the second to immediately call disconnect_handler in this case. This simplifies the logic that already caused a bug and the behaviour change is very little and affects only a pretty exotic corner case. Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- Changes.rst | 7 +++++ src/openvpn/multi.c | 59 ++++++++++++++++++++----------------------- src/openvpn/openvpn.h | 5 ---- 3 files changed, 34 insertions(+), 37 deletions(-) diff --git a/Changes.rst b/Changes.rst index e5228696..de831889 100644 --- a/Changes.rst +++ b/Changes.rst @@ -38,6 +38,13 @@ https://community.openvpn.net/openvpn/wiki/DeprecatedOptions This option was made into a NOOP option with OpenVPN 2.4. This has now been completely removed. +User-visible Changes +-------------------- +- If multiple connect handlers are used (client-connect, ccd, connect + plugin) and one of the handler succeeds but a subsequent fails, the + client-disconnect-script is now called immediately. Previously it + was called, when the VPN session was terminated. + Overview of changes in 2.4 ========================== diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 0095c871..aab15cf7 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -574,35 +574,30 @@ multi_client_disconnect_setenv(struct multi_instance *mi) static void multi_client_disconnect_script(struct multi_instance *mi) { - if (mi->context.c2.context_auth == CAS_SUCCEEDED - || mi->context.c2.context_auth == CAS_PARTIAL) - { - multi_client_disconnect_setenv(mi); + multi_client_disconnect_setenv(mi); - if (plugin_defined(mi->context.plugins, OPENVPN_PLUGIN_CLIENT_DISCONNECT)) + if (plugin_defined(mi->context.plugins, OPENVPN_PLUGIN_CLIENT_DISCONNECT)) + { + if (plugin_call(mi->context.plugins, OPENVPN_PLUGIN_CLIENT_DISCONNECT, NULL, NULL, mi->context.c2.es) != OPENVPN_PLUGIN_FUNC_SUCCESS) { - if (plugin_call(mi->context.plugins, OPENVPN_PLUGIN_CLIENT_DISCONNECT, NULL, NULL, mi->context.c2.es) != OPENVPN_PLUGIN_FUNC_SUCCESS) - { - msg(M_WARN, "WARNING: client-disconnect plugin call failed"); - } + msg(M_WARN, "WARNING: client-disconnect plugin call failed"); } + } - if (mi->context.options.client_disconnect_script) - { - struct argv argv = argv_new(); - setenv_str(mi->context.c2.es, "script_type", "client-disconnect"); - argv_parse_cmd(&argv, mi->context.options.client_disconnect_script); - openvpn_run_script(&argv, mi->context.c2.es, 0, "--client-disconnect"); - argv_free(&argv); - } + if (mi->context.options.client_disconnect_script) + { + struct argv argv = argv_new(); + setenv_str(mi->context.c2.es, "script_type", "client-disconnect"); + argv_parse_cmd(&argv, mi->context.options.client_disconnect_script); + openvpn_run_script(&argv, mi->context.c2.es, 0, "--client-disconnect"); + argv_free(&argv); + } #ifdef MANAGEMENT_DEF_AUTH - if (management) - { - management_notify_client_close(management, &mi->context.c2.mda_context, mi->context.c2.es); - } -#endif - + if (management) + { + management_notify_client_close(management, &mi->context.c2.mda_context, mi->context.c2.es); } +#endif } void @@ -683,8 +678,10 @@ multi_close_instance(struct multi_context *m, #ifdef MANAGEMENT_DEF_AUTH set_cc_config(mi, NULL); #endif - - multi_client_disconnect_script(mi); + if (mi->context.c2.context_auth == CAS_SUCCEEDED) + { + multi_client_disconnect_script(mi); + } close_context(&mi->context, SIGTERM, CC_GC_FREE); @@ -2375,16 +2372,14 @@ multi_connection_established(struct multi_context *m, struct multi_instance *mi) } else { - /* set context-level authentication flag to failed but remember - * if had a handler succeed (for cleanup) */ + /* run the disconnect script if we had a connect script that + * did not fail */ if (mi->context.c2.context_auth == CAS_PENDING_DEFERRED_PARTIAL) { - mi->context.c2.context_auth = CAS_PARTIAL; - } - else - { - mi->context.c2.context_auth = CAS_FAILED; + multi_client_disconnect_script(mi); } + + mi->context.c2.context_auth = CAS_FAILED; } /* increment number of current authenticated clients */ diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h index ccc7f118..470d67dc 100644 --- a/src/openvpn/openvpn.h +++ b/src/openvpn/openvpn.h @@ -220,11 +220,6 @@ enum client_connect_status { CAS_PENDING_DEFERRED, CAS_PENDING_DEFERRED_PARTIAL, /**< at least handler succeeded, no result yet*/ CAS_FAILED, - CAS_PARTIAL, /**< Variant of CAS_FAILED: at least one - * client-connect script/plugin succeeded - * while a later one in the chain failed - * (we still need cleanup compared to FAILED) - */ }; static inline bool From patchwork Sun Jul 19 17:34:33 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,v8,2/5] client-connect: Add deferred support to the client-connect script handler X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1305 X-Patchwork-Delegate: gert@greenie.muc.de Message-Id: <20200719173436.16431-2-arne@rfc2549.org> To: openvpn-devel@lists.sourceforge.net Date: Sun, 19 Jul 2020 19:34:33 +0200 From: Arne Schwabe List-Id: From: Fabian Knittel This patch introduces the concept of a return value file for the client-connect handlers. (This is very similar to the auth value file used during deferred authentication.) The file name is stored in the client_connect_state struct. In addition, the patch also allows the storage of the client config file name in struct client_connect_state. Both changes are used by the client-connect script handler to support deferred client-connection handling. The deferred return value file (deferred_ret_file) is passed to the actual script via the environment. If the script succeeds and writes the value for deferral into the deferred_ret_file, the handler knows to indicate deferral. Later on, the deferred handler checks whether the value of the deferred_ret_file has been updated to success or failure. Signed-off-by: Fabian Knittel Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- src/openvpn/multi.c | 229 +++++++++++++++++++++++++++++++++++++++++--- src/openvpn/multi.h | 12 +++ 2 files changed, 228 insertions(+), 13 deletions(-) diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index aab15cf7..165a3209 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -1851,6 +1851,171 @@ multi_client_set_protocol_options(struct context *c) } } +/** + * Delete the temporary file for the return value of client connect + * It also removes it from it from client_connect_defer_state and + * environment + */ +static void +ccs_delete_deferred_ret_file(struct multi_instance *mi) +{ + struct client_connect_defer_state *ccs = &(mi->client_connect_defer_state); + if (!ccs->deferred_ret_file) + { + return; + } + + 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; +} + +/** + * Create a temporary file for the return value of client connect + * and puts it into the client_connect_defer_state and environment + * as "client_connect_deferred_file" + * + * @return boolean value if creation was successfull + */ +static bool +ccs_gen_deferred_ret_file(struct multi_instance *mi) +{ + struct client_connect_defer_state *ccs = &(mi->client_connect_defer_state); + struct gc_arena gc = gc_new(); + const char *fn; + + /* Delete file if it already exists */ + ccs_delete_deferred_ret_file(mi); + + fn = platform_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. + * + * @return CC_RET_SKIPPED if the file does not exist or is empty. + * 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_defer_state *ccs = &(mi->client_connect_defer_state); + FILE *fp = fopen(ccs->deferred_ret_file, "r"); + if (!fp) + { + return CC_RET_SKIPPED; + } + + enum client_connect_return ret = CC_RET_SKIPPED; + 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: + if (feof(fp)) + { + ret = CC_RET_SKIPPED; + break; + } + + /* Not EOF, but other error fall through to error state */ + default: + /* We received an unknown/unexpected value. Assume failure. */ + msg(M_WARN, "WARNING: Unknown/unexcepted value in deferred" + "client-connect resultfile"); + ret = CC_RET_FAILED; + } + fclose(fp); + + return ret; +} + +/** + * Deletes the temporary file for the config directives of the client connect + * script and removes it into the client_connect_defer_state and environment + * + */ +static void +ccs_delete_config_file(struct multi_instance *mi) +{ + struct client_connect_defer_state *ccs = &(mi->client_connect_defer_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; + } +} + +/** + * Create a temporary file for the config directives of the client connect + * script and puts it into the client_connect_defer_state and environment + * as "client_connect_config_file" + * + * @return boolean value if creation was successfull + */ +static bool +ccs_gen_config_file(struct multi_instance *mi) +{ + struct client_connect_defer_state *ccs = &(mi->client_connect_defer_state); + struct gc_arena gc = gc_new(); + const char *fn; + + if (ccs->config_file) + { + ccs_delete_config_file(mi); + } + + fn = platform_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; +} + static enum client_connect_return multi_client_connect_call_plugin_v1(struct multi_context *m, struct multi_instance *mi, @@ -1951,7 +2116,39 @@ multi_client_connect_call_plugin_v2(struct multi_context *m, return ret; } +static enum client_connect_return +multi_client_connect_script_deferred(struct multi_context *m, + struct multi_instance *mi, + unsigned int *option_types_found) +{ + ASSERT(mi); + ASSERT(option_types_found); + struct client_connect_defer_state *ccs = &(mi->client_connect_defer_state); + enum client_connect_return ret = CC_RET_SKIPPED; + + ret = ccs_test_deferred_ret_file(mi); + if (ret == CC_RET_SKIPPED) + { + /* + * Skipped and deferred are equivalent in this context. + * skipped means that the called program has not yet + * written a return status implicitly needing more time + * while deferred is the explicit notification that it + * needs more time + */ + 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; +} /** * Runs the --client-connect script if one is defined. @@ -1964,48 +2161,54 @@ multi_client_connect_call_script(struct multi_context *m, { if (deferred) { - return CC_RET_FAILED; + return multi_client_connect_script_deferred(m, mi, option_types_found); } ASSERT(m); ASSERT(mi); enum client_connect_return ret = CC_RET_SKIPPED; + struct client_connect_defer_state *ccs = &(mi->client_connect_defer_state); if (mi->context.options.client_connect_script) { struct argv argv = argv_new(); struct gc_arena gc = gc_new(); - const char *dc_file = NULL; setenv_str(mi->context.c2.es, "script_type", "client-connect"); - dc_file = platform_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 cleanup; } argv_parse_cmd(&argv, mi->context.options.client_connect_script); - argv_printf_cat(&argv, "%s", dc_file); + argv_printf_cat(&argv, "%s", 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; } - - if (!platform_unlink(dc_file)) +cleanup: + if (ret != CC_RET_DEFERRED) { - msg(D_MULTI_ERRORS, "MULTI: problem deleting temporary file: %s", - dc_file); + ccs_delete_config_file(mi); + ccs_delete_deferred_ret_file(mi); } -cleanup: argv_free(&argv); gc_free(&gc); } diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h index 11da0209..3ebf6b9f 100644 --- a/src/openvpn/multi.h +++ b/src/openvpn/multi.h @@ -73,6 +73,18 @@ struct client_connect_defer_state /* Remember which option classes where processed for delayed option * handling. */ unsigned int option_types_found; + + /** + * The temporrary file name that contains the return status of the + * client-connect script if it exits with defer as status + */ + char *deferred_ret_file; + + /** + * The temporary file name that contains the config directives + * returned by the client-connect script + */ + char *config_file; }; /** From patchwork Sun Jul 19 17:34:34 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,v8,3/5] client-connect: Use inotify for the deferred client-connect status file X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1304 X-Patchwork-Delegate: gert@greenie.muc.de Message-Id: <20200719173436.16431-3-arne@rfc2549.org> To: openvpn-devel@lists.sourceforge.net Date: Sun, 19 Jul 2020 19:34:34 +0200 From: Arne Schwabe List-Id: As we never do client-connect and authentication at the same time it is safe to reuse the existing fields for client-connect return status file Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- src/openvpn/multi.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 165a3209..3b73ffde 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -2601,8 +2601,10 @@ multi_connection_established(struct multi_context *m, struct multi_instance *mi) #ifdef ENABLE_ASYNC_PUSH /* - * Called when inotify event is fired, which happens when acf file is closed or deleted. - * Continues authentication and sends push_reply. + * Called when inotify event is fired, which happens when acf + * or connect-status file is closed or deleted. + * Continues authentication and sends push_reply + * (or be deferred again by client-connect) */ void multi_process_file_closed(struct multi_context *m, const unsigned int mpp_flags) @@ -2888,7 +2890,15 @@ multi_process_post(struct multi_context *m, struct multi_instance *mi, const uns { multi_connection_established(m, mi); } - +#if defined(ENABLE_ASYNC_PUSH) && defined(ENABLE_DEF_AUTH) + if (is_cas_pending(mi->context.c2.context_auth) + && mi->client_connect_defer_state.deferred_ret_file) + { + add_inotify_file_watch(m, mi, m->top.c2.inotify_fd, + mi->client_connect_defer_state. + deferred_ret_file); + } +#endif /* tell scheduler to wake us up at some point in the future */ multi_schedule_context_wakeup(m, mi); } From patchwork Sun Jul 19 17:34:35 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,v8,4/5] client-connect: Add deferred support to the client-connect plugin v1 handler X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1306 X-Patchwork-Delegate: gert@greenie.muc.de Message-Id: <20200719173436.16431-4-arne@rfc2549.org> To: openvpn-devel@lists.sourceforge.net Date: Sun, 19 Jul 2020 19:34:35 +0200 From: Arne Schwabe List-Id: From: 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 PATCH V3: Modify the API to also (optionally) call the plugin on a deferred call. This allows the plugin authors to be more flexible and make the V1 API more similar to the V2 API. Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- include/openvpn-plugin.h.in | 29 +++++++------- src/openvpn/multi.c | 78 ++++++++++++++++++++++++++----------- src/openvpn/plugin.c | 3 ++ 3 files changed, 73 insertions(+), 37 deletions(-) diff --git a/include/openvpn-plugin.h.in b/include/openvpn-plugin.h.in index 103844f7..99aa1678 100644 --- a/include/openvpn-plugin.h.in +++ b/include/openvpn-plugin.h.in @@ -116,20 +116,21 @@ extern "C" { * FUNC: openvpn_plugin_client_destructor_v1 (top-level "generic" client) * FUNC: openvpn_plugin_close_v1 */ -#define OPENVPN_PLUGIN_UP 0 -#define OPENVPN_PLUGIN_DOWN 1 -#define OPENVPN_PLUGIN_ROUTE_UP 2 -#define OPENVPN_PLUGIN_IPCHANGE 3 -#define OPENVPN_PLUGIN_TLS_VERIFY 4 -#define OPENVPN_PLUGIN_AUTH_USER_PASS_VERIFY 5 -#define OPENVPN_PLUGIN_CLIENT_CONNECT 6 -#define OPENVPN_PLUGIN_CLIENT_DISCONNECT 7 -#define OPENVPN_PLUGIN_LEARN_ADDRESS 8 -#define OPENVPN_PLUGIN_CLIENT_CONNECT_V2 9 -#define OPENVPN_PLUGIN_TLS_FINAL 10 -#define OPENVPN_PLUGIN_ENABLE_PF 11 -#define OPENVPN_PLUGIN_ROUTE_PREDOWN 12 -#define OPENVPN_PLUGIN_N 13 +#define OPENVPN_PLUGIN_UP 0 +#define OPENVPN_PLUGIN_DOWN 1 +#define OPENVPN_PLUGIN_ROUTE_UP 2 +#define OPENVPN_PLUGIN_IPCHANGE 3 +#define OPENVPN_PLUGIN_TLS_VERIFY 4 +#define OPENVPN_PLUGIN_AUTH_USER_PASS_VERIFY 5 +#define OPENVPN_PLUGIN_CLIENT_CONNECT 6 +#define OPENVPN_PLUGIN_CLIENT_DISCONNECT 7 +#define OPENVPN_PLUGIN_LEARN_ADDRESS 8 +#define OPENVPN_PLUGIN_CLIENT_CONNECT_V2 9 +#define OPENVPN_PLUGIN_TLS_FINAL 10 +#define OPENVPN_PLUGIN_ENABLE_PF 11 +#define OPENVPN_PLUGIN_ROUTE_PREDOWN 12 +#define OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER 13 +#define OPENVPN_PLUGIN_N 14 /* * Build a mask out of a set of plug-in types. diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 3b73ffde..c66a4cea 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -2022,53 +2022,85 @@ multi_client_connect_call_plugin_v1(struct multi_context *m, bool deferred, unsigned int *option_types_found) { - if (deferred) - { - return CC_RET_FAILED; - } enum client_connect_return ret = CC_RET_SKIPPED; #ifdef ENABLE_PLUGIN ASSERT(m); ASSERT(mi); ASSERT(option_types_found); + struct client_connect_defer_state *ccs = &(mi->client_connect_defer_state); /* deprecated callback, use a file for passing back return info */ if (plugin_defined(mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT)) { struct argv argv = argv_new(); - struct gc_arena gc = gc_new(); - const char *dc_file = - platform_create_temp_file(mi->context.options.tmp_dir, "cc", &gc); + int call; - if (!dc_file) + if (!deferred) { - ret = CC_RET_FAILED; - goto cleanup; + call = OPENVPN_PLUGIN_CLIENT_CONNECT; + if (!ccs_gen_config_file(mi) + || !ccs_gen_deferred_ret_file(mi)) + { + ret = CC_RET_FAILED; + goto cleanup; + } + } + else + { + call = OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER; + /* the initial call should have created these files */ + ASSERT(ccs->config_file); + ASSERT(ccs->deferred_ret_file); } - 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) + argv_printf(&argv, "%s", ccs->config_file); + int plug_ret = plugin_call(mi->context.plugins, call, + &argv, NULL, mi->context.c2.es); + 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; + /** + * Contrary to the plugin v2 API, we do not demand a working + * deferred plugin as all return can be handled by the files + * and plugin_call return success if a plugin is not defined + */ } 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; } - if (!platform_unlink(dc_file)) + + /** + * plugin api v1 client connect async feature has both plugin and + * file return status, so in case that the file has a code that + * demands override, we override our return code + */ + int file_ret = ccs_test_deferred_ret_file(mi); + + if (file_ret == CC_RET_FAILED) { - msg(D_MULTI_ERRORS, "MULTI: problem deleting temporary file: %s", - dc_file); + ret = CC_RET_FAILED; + } + else if (ret == CC_RET_SUCCEEDED && file_ret == CC_RET_DEFERRED) + { + ret = CC_RET_DEFERRED; } - cleanup: argv_free(&argv); - gc_free(&gc); + + if (ret != CC_RET_DEFERRED) + { + ccs_delete_config_file(mi); + ccs_delete_deferred_ret_file(mi); + } } #endif /* ifdef ENABLE_PLUGIN */ return ret; diff --git a/src/openvpn/plugin.c b/src/openvpn/plugin.c index 4de1d6b7..9a6fa3cb 100644 --- a/src/openvpn/plugin.c +++ b/src/openvpn/plugin.c @@ -104,6 +104,9 @@ plugin_type_name(const int type) case OPENVPN_PLUGIN_CLIENT_CONNECT_V2: return "PLUGIN_CLIENT_CONNECT"; + case OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER: + return "PLUGIN_CLIENT_CONNECT_DEFER"; + case OPENVPN_PLUGIN_CLIENT_DISCONNECT: return "PLUGIN_CLIENT_DISCONNECT"; From patchwork Sun Jul 19 17:34:36 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,v8,5/5] client-connect: Implement deferred connect support for plugin API v2 X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1308 Message-Id: <20200719173436.16431-5-arne@rfc2549.org> To: openvpn-devel@lists.sourceforge.net Date: Sun, 19 Jul 2020 19:34:36 +0200 From: Arne Schwabe List-Id: The V2 API is simpler than the V1 API since there is no passing of data via files. This also means that with the current API the V2 API cannot support async notify via files. Adding a file just for async notify seems very hacky and when needed we should implement a better option when async is needed for the plugin V2 API. Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- include/openvpn-plugin.h.in | 3 ++- src/openvpn/multi.c | 36 ++++++++++++++++++++++++------------ src/openvpn/plugin.c | 3 +++ 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/include/openvpn-plugin.h.in b/include/openvpn-plugin.h.in index 99aa1678..38fbe097 100644 --- a/include/openvpn-plugin.h.in +++ b/include/openvpn-plugin.h.in @@ -130,7 +130,8 @@ extern "C" { #define OPENVPN_PLUGIN_ENABLE_PF 11 #define OPENVPN_PLUGIN_ROUTE_PREDOWN 12 #define OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER 13 -#define OPENVPN_PLUGIN_N 14 +#define OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER_V2 14 +#define OPENVPN_PLUGIN_N 15 /* * Build a mask out of a set of plug-in types. diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index c66a4cea..06f8e6c1 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -2112,36 +2112,48 @@ multi_client_connect_call_plugin_v2(struct multi_context *m, bool deferred, unsigned int *option_types_found) { - if (deferred) - { - return CC_RET_FAILED; - } enum client_connect_return ret = CC_RET_SKIPPED; #ifdef ENABLE_PLUGIN ASSERT(m); ASSERT(mi); ASSERT(option_types_found); + int call = deferred ? OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER_V2 : + OPENVPN_PLUGIN_CLIENT_CONNECT_V2; /* V2 callback, use a plugin_return struct for passing back return info */ - if (plugin_defined(mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT_V2)) + if (plugin_defined(mi->context.plugins, call)) { struct plugin_return pr; plugin_return_init(&pr); - if (plugin_call(mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT_V2, - NULL, &pr, mi->context.c2.es) - != OPENVPN_PLUGIN_FUNC_SUCCESS) + int plug_ret = plugin_call(mi->context.plugins, call, + NULL, &pr, mi->context.c2.es); + if (plug_ret == OPENVPN_PLUGIN_FUNC_SUCCESS) { - msg(M_WARN, "WARNING: client-connect-v2 plugin call failed"); - ret = CC_RET_FAILED; + multi_client_connect_post_plugin(m, mi, &pr, option_types_found); + ret = CC_RET_SUCCEEDED; + } + else if (plug_ret == OPENVPN_PLUGIN_FUNC_DEFERRED) + { + ret = CC_RET_DEFERRED; + if (!(plugin_defined(mi->context.plugins, + OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER_V2))) + { + msg(M_WARN, "A plugin that defers from the " + "OPENVPN_PLUGIN_CLIENT_CONNECT_V2 call must also " + "declare support for " + "OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER_V2"); + ret = CC_RET_FAILED; + } } else { - multi_client_connect_post_plugin(m, mi, &pr, option_types_found); - ret = CC_RET_SUCCEEDED; + msg(M_WARN, "WARNING: client-connect-v2 plugin call failed"); + ret = CC_RET_FAILED; } + plugin_return_free(&pr); } #endif /* ifdef ENABLE_PLUGIN */ diff --git a/src/openvpn/plugin.c b/src/openvpn/plugin.c index 9a6fa3cb..8b351c45 100644 --- a/src/openvpn/plugin.c +++ b/src/openvpn/plugin.c @@ -107,6 +107,9 @@ plugin_type_name(const int type) case OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER: return "PLUGIN_CLIENT_CONNECT_DEFER"; + case OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER_V2: + return "PLUGIN_CLIENT_CONNECT_DEFER_V2"; + case OPENVPN_PLUGIN_CLIENT_DISCONNECT: return "PLUGIN_CLIENT_DISCONNECT";