Re: [Openvpn-devel] Async OPENVPN_PLUGIN_CLIENT_CONNECT plugin support

[Lev Stipakov]

Hi Fabian,

> You say that you've added support for the client-connect plugin call. May I
> ask what was missing?

Nothing dramatic, I just added deferred support for client-connect v2.

> So this is basically about replacing a 5s poll-interval with something that
> should proceed near instantaneously, correct?

Well, 5 seconds interval is still there, it is client-side thing. I
made server send
push_reply immediately after result is ready (written to ccr file)
without waiting
for incoming push_request.

I noticed that if authentication / authorization happens fast enough, server may
send push_reply before push_request is arrived. Sometimes client anyway sends
push_request and server sends second push_reply. To avoid that and to follow
the protocol (I haven't noticed any issues, though), in latest version
I made server wait
for the first push_request. So, if authorization takes less than
second, client will get response
in a second (with first push_request), otherwise immediately when auth is done.

I can probably modify client part and make it not send push_request if
push_reply
has already arrived. Would it be the correct way?

> Thanks for sharing! I'll have a look at it, when time permits. Though my
> focus is currently on getting some or all of the patches from the basic
> patch-set upstream.

Indeed it would be great to have your patch in upstream master.

-Lev

Re: [Openvpn-devel] Async OPENVPN_PLUGIN_CLIENT_CONNECT plugin support

[Fabian Knittel]

Hi Lev,

2014-10-07 20:50 GMT+02:00 Lev Stipakov :

> Patch works great, thanks! I have rebased it a bit and added support
> for client-connect plugin call.
>

Great to hear that my patch works for you!

I've finally rebased the patches too, due to my current users complaining
about the old OpenVPN version and missing security updates. (See
https://github.com/fknittel/openvpn/tree/feat_deferred_client-connect -
there's a branch for master and a branch for the OpenVPN 2.3 stable branch.)

You say that you've added support for the client-connect plugin call. May I
ask what was missing? I'd definitely be interested in any changes you
needed to make it work. I would have expected the patch titled
"client-connect: Add deferred support to the client-connect plugin v1
handler" to provide everything, but as I only ever used the script handler,
it's not unlikely that I missed something.

I would like to offer a related feature (and implementation) I call
> async-push.
>
> Use case: authentication / authorization takes time. I have auth/az
> code in auth-user-pass-verify and client-connect calls, and sometimes
> it takes more that second to execute those. The problem is that after
> auth-user-pass-verify is done, OpenVPN server won’t proceed with
> client-connect unless some timeout/io event happens for that client.
> Also, server will not notify client that client-connect returned
> success unless client sends PULL_REQUEST. Client, in turn, sends
> PULL_REQUEST one second after connection initiation and after that
> once per 5 seconds. So, for example, if at the moment when first pull
> request has arrived, client-connect has not finished yet, we will have
> to wait another 5 seconds for the next PULL_REQUEST.
>

So this is basically about replacing a 5s poll-interval with something that
should proceed near instantaneously, correct?


> Solution: Inotify. Since OpenVPN creates itself files (auth-contro and
> client-connect-deferred) which names it passes to the plugin, we
> create one inotify descriptor for event loop and right after creating
> those files, we add inotify watch on those. Before calling epoll (or
> whatever we use) we add inotify descriptor to the list of watched
> descriptors. We also keep watch descriptor and multi_instance in a
> hashtable.
>
> When epoll informs us that an event has happened on inotify
> descriptor, we get multi_instance by watch descriptor (fetched from
> poll event) from our new hashtable and call multi_process_post for
> given multi_instance. This will check result from the file and
> eventually call multi_connection_established, from where we call
> send_push_reply.
>
> Since implementation uses Inotify, it will work on Linux only. Code is
> under #define, which is set at compile-time (--enable-async-push=yes).
>
> I have attached an implementation. So far has been working nicely in
> my test environment. I would love to hear a feedback from the
> community. Is the whole thing done more or less right? Any bugs got
> introduced that someone could spot?
>

Thanks for sharing! I'll have a look at it, when time permits. Though my
focus is currently on getting some or all of the patches from the basic
patch-set upstream.

Cheers
Fabian

Re: [Openvpn-devel] Async OPENVPN_PLUGIN_CLIENT_CONNECT plugin support

[Lev Stipakov]

Hi Fabian & all,

Patch works great, thanks! I have rebased it a bit and added support
for client-connect plugin call.

I would like to offer a related feature (and implementation) I call async-push.

Use case: authentication / authorization takes time. I have auth/az
code in auth-user-pass-verify and client-connect calls, and sometimes
it takes more that second to execute those. The problem is that after
auth-user-pass-verify is done, OpenVPN server won’t proceed with
client-connect unless some timeout/io event happens for that client.
Also, server will not notify client that client-connect returned
success unless client sends PULL_REQUEST. Client, in turn, sends
PULL_REQUEST one second after connection initiation and after that
once per 5 seconds. So, for example, if at the moment when first pull
request has arrived, client-connect has not finished yet, we will have
to wait another 5 seconds for the next PULL_REQUEST.

Solution: Inotify. Since OpenVPN creates itself files (auth-contro and
client-connect-deferred) which names it passes to the plugin, we
create one inotify descriptor for event loop and right after creating
those files, we add inotify watch on those. Before calling epoll (or
whatever we use) we add inotify descriptor to the list of watched
descriptors. We also keep watch descriptor and multi_instance in a
hashtable.

When epoll informs us that an event has happened on inotify
descriptor, we get multi_instance by watch descriptor (fetched from
poll event) from our new hashtable and call multi_process_post for
given multi_instance. This will check result from the file and
eventually call multi_connection_established, from where we call
send_push_reply.

Since implementation uses Inotify, it will work on Linux only. Code is
under #define, which is set at compile-time (--enable-async-push=yes).

I have attached an implementation. So far has been working nicely in
my test environment. I would love to hear a feedback from the
community. Is the whole thing done more or less right? Any bugs got
introduced that someone could spot?

-Lev

2014-08-01 0:21 GMT+03:00 Fabian Knittel :
> Hi Lev,
>
> 2014-07-29 12:56 GMT+02:00 Lev Stipakov :
>>
>> I am pondering about asynchronous OPENVPN_PLUGIN_CLIENT_CONNECT
>> callback. Basically, I want _not_ to establish connection until
>> response is received and ofcI  don't want to block rest of traffic.
>
>
> [ Details of approach snipped. ]
>
>> What do you think about that? Does that approach sound reasonable?
>
>
> Some time ago I implemented something quite similar, but never quite managed
> to officially submit it. You can find my old git branch here [0].
> Unfortunately, to be of any use it would need to be ported to a current
> OpenVPN release / master first.
>
> The code has been in use for several years now [1], so the approach and the
> code basically work quite well. (I think my use case involved calling a
> Python script, but I might have implemented the plugin part too.)
>
> If the OpenVPN commiters see a certain chance, that such a change could be
> included upstream, I might even try to rebase the branch to master myself...
>
> Cheers
> Fabian
>
> 0:
> http://opensource.fsmi.uni-karlsruhe.de/gitweb/?p=openvpn.git;a=shortlog;h=refs/heads/feat_deferred_client-connect
> 1: ... in a production environment with several hundred users (together with
> the equally unofficial VLAN-tagging feature [2]). The feature is needed by a
> daemon that does asynchronous IP-configuration via a central DHCP server
> [3].
> 2:
> http://opensource.fsmi.uni-karlsruhe.de/gitweb/?p=openvpn.git;a=shortlog;h=refs/heads/feat_vlan
> 3: https://gitorious.org/odr



-- 
-Lev
diff --git a/configure.ac b/configure.ac
index ffba374..2c5c65d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -264,6 +264,13 @@ AC_ARG_ENABLE(
 	[enable_systemd="no"]
 )
 
+AC_ARG_ENABLE(
+	[async-push],
+	[AS_HELP_STRING([--enable-async-push], [enable async-push support @<:@default=no@:>@])],
+	[enable_async_push="yes"],
+	[enable_async_push="no"]
+)
+
 AC_ARG_WITH(
 	[special-build],
 	[AS_HELP_STRING([--with-special-build=STRING], [specify special build string])],
@@ -1144,6 +1151,14 @@ if test "${enable_plugin_auth_pam}" = "yes"; then
 	fi
 fi
 
+if test "${enable_async_push}" = "yes"; then
+	AC_CHECK_HEADERS(
+		[sys/inotify.h],
+		AC_DEFINE([ENABLE_ASYNC_PUSH], [1], [Enable async push]),
+		AC_MSG_ERROR([inotify.h not found.])
+	)
+fi
+
 CONFIGURE_DEFINES="`set | grep '^enable_.*=' ; set | grep '^with_.*='`"
 AC_DEFINE_UNQUOTED([CONFIGURE_DEFINES], ["`echo ${CONFIGURE_DEFINES}`"], [Configuration settings])
 
diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index 39f66e3..940d426 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -1353,6 +1353,9 @@ io_wait_dowork (struct context *c, const unsigned int flags)
 #ifdef ENABLE_MANAGEMENT
   static int management_shift = 6; /* depends on MANAGEMENT_READ and MANAGEMENT_WRITE */
 #endif

Re: [Openvpn-devel] Async OPENVPN_PLUGIN_CLIENT_CONNECT plugin support

[Fabian Knittel]

Hi Lev,

2014-07-29 12:56 GMT+02:00 Lev Stipakov :

> I am pondering about asynchronous OPENVPN_PLUGIN_CLIENT_CONNECT
> callback. Basically, I want _not_ to establish connection until
> response is received and ofcI  don't want to block rest of traffic.
>

[ Details of approach snipped. ]

What do you think about that? Does that approach sound reasonable?
>

Some time ago I implemented something quite similar, but never quite
managed to officially submit it. You can find my old git branch here [0].
Unfortunately, to be of any use it would need to be ported to a current
OpenVPN release / master first.

The code has been in use for several years now [1], so the approach and the
code basically work quite well. (I think my use case involved calling a
Python script, but I might have implemented the plugin part too.)

If the OpenVPN commiters see a certain chance, that such a change could be
included upstream, I might even try to rebase the branch to master myself...

Cheers
Fabian

0:
http://opensource.fsmi.uni-karlsruhe.de/gitweb/?p=openvpn.git;a=shortlog;h=refs/heads/feat_deferred_client-connect
1: ... in a production environment with several hundred users (together
with the equally unofficial VLAN-tagging feature [2]). The feature is
needed by a daemon that does asynchronous IP-configuration via a central
DHCP server [3].
2:
http://opensource.fsmi.uni-karlsruhe.de/gitweb/?p=openvpn.git;a=shortlog;h=refs/heads/feat_vlan
3: https://gitorious.org/odr

[Openvpn-devel] Async OPENVPN_PLUGIN_CLIENT_CONNECT plugin support

[Lev Stipakov]

Hello,

I am pondering about asynchronous OPENVPN_PLUGIN_CLIENT_CONNECT
callback. Basically, I want _not_ to establish connection until
response is received and ofcI  don't want to block rest of traffic.

My idea is to have some kind of connect_control_file (similar to
auth_conrol_file) and pass its path via env to
OPENVPN_PLUGIN_CLIENT_CONNECT. In case of plugin (or maybe script
too?) has returned OPENVPN_PLUGIN_FUNC_DEFERRED, I continue executing
"multi_connection_established" as usual except I don't set
"push_reply_deferred" to False (to prevent push response from being
sent) and I set some "connect-deferred" flag.

Next, when "process_incoming_push_msg" get called and flag
"connect-deferred" is set, I check the state of connect_control_file.
If there is, say, "1" - I send push reply and connection got
established.

What do you think about that? Does that approach sound reasonable?

-- 
-Lev