| Message ID | 20200711093655.23686-13-arne@rfc2549.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [Openvpn-devel,v5,01/14] Allow changing fallback cipher from ccd files/client-connect | expand |
Hi, On Sat, Jul 11, 2020 at 11:36:54AM +0200, Arne Schwabe wrote: > 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. I'm not sure I understand a word of this. If we want async cc, we should do it in the V2 API - and since v2 plugin async auth works just fine (with file), I wonder why it is hard for async cc? Because we cannot pass back the returned config data? gert
Am 13.07.20 um 17:40 schrieb Gert Doering: > Hi, > > On Sat, Jul 11, 2020 at 11:36:54AM +0200, Arne Schwabe wrote: >> 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. > > I'm not sure I understand a word of this. > > If we want async cc, we should do it in the V2 API - and since v2 plugin > async auth works just fine (with file), I wonder why it is hard for > async cc? Because we cannot pass back the returned config data? Not async cc. That works fine. But async notification via file notify does not work without a file being involved. v1 API returns config and status via files. v2 returns status as return value (OPENVPN_PLUGIN_FUNC_SUCCESS, OPENVPN_PLUGIN_FUNC_DEFERRED...) and the config file via memory in plugin_return. So no file involved and no way to use inotify Arne
Hi, On Mon, Jul 13, 2020 at 06:08:04PM +0200, Arne Schwabe wrote: > >> 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. > > > > I'm not sure I understand a word of this. > > > > If we want async cc, we should do it in the V2 API - and since v2 plugin > > async auth works just fine (with file), I wonder why it is hard for > > async cc? Because we cannot pass back the returned config data? > > Not async cc. That works fine. But async notification via file notify > does not work without a file being involved. > > v1 API returns config and status via files. > > v2 returns status as return value (OPENVPN_PLUGIN_FUNC_SUCCESS, > OPENVPN_PLUGIN_FUNC_DEFERRED...) and the config file via memory in > plugin_return. So no file involved and no way to use inotify For the record, I'm all confused between lowercase and uppercase v1, v2, v3 - there is "openvpn_plugin_func_v1()"... "openvpn_plugin_func_v3", and for CLIENT_CONNECT - *only!* - there is also CLIENT_CONNECT_V2, but not for, like, async auth. So, I withdraw what I said here, and need to read this stuff up in more detail. Look at existing client-connect plugins... (if there are any). Etc. gert
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 08eb44ba..65169719 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -2114,7 +2114,8 @@ multi_client_connect_call_plugin_v1_deferred(struct multi_context *m, static enum client_connect_return multi_client_connect_call_plugin_v2(struct multi_context *m, struct multi_instance *mi, - unsigned int *option_types_found) + unsigned int *option_types_found, + bool deferred) { enum client_connect_return ret = CC_RET_SKIPPED; #ifdef ENABLE_PLUGIN @@ -2122,32 +2123,67 @@ multi_client_connect_call_plugin_v2(struct multi_context *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 */ return ret; } + +static enum client_connect_return +multi_client_connect_call_plugin_v2_initial(struct multi_context *m, + struct multi_instance *mi, + unsigned int *option_types_found) +{ + return multi_client_connect_call_plugin_v2(m, mi, option_types_found, + false); +} + +static enum client_connect_return +multi_client_connect_call_plugin_v2_deferred(struct multi_context *m, + struct multi_instance *mi, + unsigned int *option_types_found) +{ + return multi_client_connect_call_plugin_v2(m, mi, option_types_found, + true); +} + /** * Runs the --client-connect script if one is defined. */ @@ -2499,8 +2535,8 @@ static const struct client_connect_handlers client_connect_handlers[] = { .deferred = multi_client_connect_call_plugin_v1_deferred, }, { - .main = multi_client_connect_call_plugin_v2, - .deferred = multi_client_connect_fail + .main = multi_client_connect_call_plugin_v2_initial, + .deferred = multi_client_connect_call_plugin_v2_deferred, }, { .main = multi_client_connect_call_script, diff --git a/src/openvpn/plugin.c b/src/openvpn/plugin.c index ea18592f..80abb730 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"; + case OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER_V2: + return "PLUGIN_CLIENT_CONNECT"; + case OPENVPN_PLUGIN_CLIENT_DISCONNECT: return "PLUGIN_CLIENT_DISCONNECT";
The V2 API is simpler than the V1 API since there is no passing of data via files. This also means that with the current API the V2 API cannot support async notify via files. Adding a file just for async notify seems very hacky and when needed we should implement a better option when async is needed for the plugin V2 API. Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- include/openvpn-plugin.h.in | 3 +- src/openvpn/multi.c | 58 ++++++++++++++++++++++++++++++------- src/openvpn/plugin.c | 3 ++ 3 files changed, 52 insertions(+), 12 deletions(-)