Re: [Openvpn-devel] [PATCH] systemd: Change the default cipher to AES-256-GCM for server configs

2020-07-11 Thread Arne Schwabe
Am 23.06.20 um 11:12 schrieb Dmitry Melekhov:
> 23.06.2020 13:02, Gert Doering пишет:
>>
>>
>> That patch is from Steffan, and review has been sitting in my lap for
>> way too long.  Need to see if it still applies.
>>
> 
> Unfortunately it is not compatible with 2.4.9, because of introduced
> change...

Can you test with current openvpn master if that works for you? That has
now allows you set the --cipher in ccd/connect-client scripts.

Arne



signature.asc
Description: OpenPGP digital signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH applied] Re: Allow changing fallback cipher from ccd files/client-connect

2020-07-11 Thread Arne Schwabe
Am 11.07.2020 um 18:48 schrieb Gert Doering:
> Acked-by: Gert Doering 
>
> The patch is trivial enough (it just allows "cipher" in ccd/ files, with
> no logic changes) - it's built on the changes in the previous patches, which
> makes it "just work".
>
> Without the patch, trying to set & push a cipher from ccd/:
>
> Jul 11 18:27:53 gentoo tap-udp-p2mp[12620]: Options error: option 'cipher' 
> cannot be used in this context (ccd/freebsd-74-amd64)
> Jul 11 18:27:55 gentoo tap-udp-p2mp[12620]: ... SENT CONTROL 
> [freebsd-74-amd64]: 'PUSH_REPLY,...,cipher CAMELLIA-128-CBC,...,cipher 
> AES-256-GCM' (status=1)
>
> With the patch *and* forcing NCP on the server side by only allowing 
> CAMELLIA-128-CBC:
>
>   $ cat ccd/freebsd-74-amd64
>   ncp-ciphers CAMELLIA-128-CBC
>   cipher CAMELLIA-128-CBC
>
> it will actually do that:
>
> Jul 11 18:42:37 gentoo tap-udp-p2mp[13661]: Outgoing Data Channel: Cipher 
> 'CAMELLIA-128-CBC' initialized with 128 bit key
> Jul 11 18:42:37 gentoo tap-udp-p2mp[13661]: Incoming Data Channel: Cipher 
> 'CAMELLIA-128-CBC' initialized with 128 bit key
> Jul 11 18:42:38 gentoo tap-udp-p2mp[13661]: SENT CONTROL [freebsd-74-amd64]: 
> 'PUSH_REPLY,...,peer-id 2,cipher CAMELLIA-128-CBC' (status=1)
>
> (if I put "CAMELLIA and some of the AES-GCM variants" in there, I get the 
> standard AES-256-GCM or AES-128-GCM variants - with no indication in the 
> logs on why it doesn't want to take the cipher --> documenting this here,
> so it can be found by googling: if you want "cipher" to work in CCD/ files,

cipher only sets the fallback cipher if we find no common cipher. All
ciphers in ncp-ciphers are still preferred to cipher. So to have the
server pick the --cipher from the either general config or ccd config,
none of the cipher in ncp-ciphers may be supported by the peer (so not
in ncp-ciphers/ncp-ciphers and not as --cipher)

Arne



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH applied] Re: Allow changing fallback cipher from ccd files/client-connect

2020-07-11 Thread Gert Doering
Acked-by: Gert Doering 

The patch is trivial enough (it just allows "cipher" in ccd/ files, with
no logic changes) - it's built on the changes in the previous patches, which
makes it "just work".

Without the patch, trying to set & push a cipher from ccd/:

Jul 11 18:27:53 gentoo tap-udp-p2mp[12620]: Options error: option 'cipher' 
cannot be used in this context (ccd/freebsd-74-amd64)
Jul 11 18:27:55 gentoo tap-udp-p2mp[12620]: ... SENT CONTROL 
[freebsd-74-amd64]: 'PUSH_REPLY,...,cipher CAMELLIA-128-CBC,...,cipher 
AES-256-GCM' (status=1)

With the patch *and* forcing NCP on the server side by only allowing 
CAMELLIA-128-CBC:

  $ cat ccd/freebsd-74-amd64
  ncp-ciphers CAMELLIA-128-CBC
  cipher CAMELLIA-128-CBC

it will actually do that:

Jul 11 18:42:37 gentoo tap-udp-p2mp[13661]: Outgoing Data Channel: Cipher 
'CAMELLIA-128-CBC' initialized with 128 bit key
Jul 11 18:42:37 gentoo tap-udp-p2mp[13661]: Incoming Data Channel: Cipher 
'CAMELLIA-128-CBC' initialized with 128 bit key
Jul 11 18:42:38 gentoo tap-udp-p2mp[13661]: SENT CONTROL [freebsd-74-amd64]: 
'PUSH_REPLY,...,peer-id 2,cipher CAMELLIA-128-CBC' (status=1)

(if I put "CAMELLIA and some of the AES-GCM variants" in there, I get the 
standard AES-256-GCM or AES-128-GCM variants - with no indication in the 
logs on why it doesn't want to take the cipher --> documenting this here,
so it can be found by googling: if you want "cipher" to work in CCD/ files,
you must also set "ncp-ciphers" accordingly).

Your patch has been applied to the master branch.

commit 6168f53d6b7274026d4f392a22e64524a9b264d6
Author: Arne Schwabe
Date:   Sat Jul 11 11:36:42 2020 +0200

 Allow changing fallback cipher from ccd files/client-connect

 Signed-off-by: Arne Schwabe 
 Acked-by: Gert Doering 
 Message-Id: <20200711093655.23686-1-a...@rfc2549.org>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20281.html
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] [V4] Added support for DHCP option 119 (dns search suffix, list) for Windows. As of Windows 10 1809 Windows finally supports this so it, makes sense to add support to OpenV

2020-07-11 Thread Gert Doering
Hi,

On Fri, Jul 10, 2020 at 06:42:18PM +0200, Jan Just Keijser wrote:
> On 08/07/20 10:24, Gert Doering wrote:
> > Can I have a v4, please? :-)
> V4:

Okay, here we go...


Generally speaking, it works now :-)

In the "ipconfig /all" output, I can now see a long list of DNS suffixes.

Together with a DNS search list on the LAN *and* block-outside-dns, this
seems to have interesting effects on Win10 - namely, it tries all the
search domains it has (on all interfaces), but it will try the "LAN"
search domains only via the "LAN" nameservers - and these are blocked,
so there might be interesting side effects if both are used togehter -
*or* it might be a way to get Win10 DNS to behave better even without
block-outside-DNS (by configuring a search domain on the TAP side).


Codewise, a few remarks...


> @@ -1145,6 +1146,19 @@ parse_hash_fingerprint(const char *str, int nbytes, 
> int msglevel, struct gc_aren
>  #ifndef ENABLE_SMALL
>  
>  static void
> +show_dhcp_option_list(const char *name, const char * const*array, int len)
> +{
> +int i;
> +for (i = 0; i < len; ++i)
> +{
> +msg(D_SHOW_PARMS, "  %s[%d] = %s",
> +name,
> +i,
> +array[i] );
> +}
> +}

This is not "forbidden" by the style guide, but I think a more compact
form would increase readability - the old openvpn style of having 
individual function calls spread over 10+ lines because each parameter
is getting its own line is not really something we do anymore.  So:

> +msg(D_SHOW_PARMS, "  %s[%d] = %s", name, i, array[i] );

... which is well below the 80 character limit and which I find more
readable, tbh.


> +/*
> + * RFC3397 states that multiple searchdomains are encoded as follows:
> + *  - at start the length of the entire option is given
> + *  - each subdomain is preceded by its length
> + *  - each searchdomain is separated by a NUL character
> + * e.g. if you want "openvpn.net" and "duckduckgo.com" then you end up with
> + *  0x13  0x7 openvpn 0x3 net 0x00 0x0A duckduckgo 0x3 com 0x00

While at it, 0x1D :-)

> + */
> +static void
> +write_dhcp_search_str(struct buffer *buf, const int type, const char * const 
> *str_array,
> +  int array_len, bool *error)
> +{
> +char tmp_buf[256];
> +int  i;
> +int  len = 0;
> +
> +for (i=0; i < array_len; i++)
> +{
> +const char  *ptr = str_array[i], *dotptr = str_array[i];
> +int  j, k;
> +
> +msg(M_INFO, "Processing '%s'", ptr);

This line should not stay as it is - if you see it in the log, it's
mostly unclear "it's processing 'domain' - what for?", and M_INFO is
too high for unspecific info.

Most other DHCP activities do not log anything (unless error), so for
consistency I would just remove it.  But if we keep it, it needs to
be more clear, like

> +msg(D_DHCP_OPT, "dhcp search domain '%s'", ptr);

or something like that.


> +if (strlen(ptr) + len + 1 > sizeof(tmp_buf))
> +{
> +*error = true;
> +msg(M_WARN, "write_dhcp_search_str: temp buffer overflow 
> building DHCP options");
> +return;
> +}

> +/* Loop over all subdomains separated by a dot and replace the dot
> +   with the length of the subdomain */
> +while ((dotptr = strchr(ptr, '.')) != NULL)
> +{   
> +j = dotptr - ptr;
> +tmp_buf[len++] = j;
> +for (k=0; k < j; k++) tmp_buf[len++] = ptr[k];

Side note: this violate coding style - for(), as if(), mandates brackets.

> +for (k=0; k < j; k++) 
   {
   tmp_buf[len++] = ptr[k];
   }

> +ptr = dotptr + 1;
> +}   
> +
> +/* Now do the remainder after the last dot */
> +j = strlen(ptr);
> +tmp_buf[len++] = j;
> +for (k=0; k < j; k++) tmp_buf[len++] = ptr[k];

same here.

> +  
> +/* And close off with an extra NUL char */
> +tmp_buf[len++] = 0;
> +}


With these required brackets, the nested loops get long - and I do not
find them overly elegant anyway (one could do a memcpy(), which would
make more clear what is done, but it's still double-looping).

For this, I had the idea to code it as follows (mentioned yesterday, but
now actually written down) - single pass, no nested loops:

/* Loop over all subdomains separated by a dot and replace the dot
   with the length of the subdomain */

/* label_length_pos points to the byte to be replaced by the length 
 * of the following domain label */
int label_length_pos = len++;

while(true)
{
if (*ptr == '.' || *ptr == '\0' )
{
tmp_buf[label_length_pos] = (len-label_length_pos)-1;
label_length_pos = len;
if (*ptr == '\0')
{
break;
}
}
tmp_buf[len++] = *ptr++;
}
/* And close off with an extra NUL 

[Openvpn-devel] [PATCH v5 08/14] client-connect: Add CC_RET_DEFERRED and cope with deferred client-connect

2020-07-11 Thread Arne Schwabe
From: Fabian Knittel 

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

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

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

Signed-off-by: Fabian Knittel 

Patch V3: Use a static struct in multi_instance instead of using
  malloc/free and use two states (deffered with and without
  result) instead of one to eliminate the counter that was
  only tested for > 0.

Patch V5: Use new states in context_auth instead of the extra state
  that the patch series previously used.

Signed-off-by: Arne Schwabe 
---
 src/openvpn/multi.c   | 171 +++---
 src/openvpn/multi.h   |  15 +++-
 src/openvpn/openvpn.h |   9 +++
 3 files changed, 150 insertions(+), 45 deletions(-)

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index f9b8af80..ce73f8a1 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2218,32 +2218,51 @@ multi_client_connect_source_ccd(struct multi_context *m,
 return ret;
 }
 
-static inline bool
-cc_check_return(int *cc_succeeded_count,
-enum client_connect_return ret)
+typedef enum client_connect_return (*multi_client_connect_handler)
+(struct multi_context *m, struct multi_instance *mi,
+unsigned int *option_types_found);
+
+struct client_connect_handlers
+{
+multi_client_connect_handler main;
+multi_client_connect_handler deferred;
+};
+
+static enum client_connect_return
+multi_client_connect_fail(struct multi_context *m, struct multi_instance *mi,
+  unsigned int *option_types_found)
 {
-if (ret == CC_RET_SUCCEEDED)
+/* Called null call-back.  This should never happen. */
+return CC_RET_FAILED;
+}
+
+static const struct client_connect_handlers client_connect_handlers[] = {
 {
-(*cc_succeeded_count)++;
-return true;
-}
-else if (ret == CC_RET_FAILED)
+.main = multi_client_connect_source_ccd,
+.deferred = multi_client_connect_fail
+},
 {
-return false;
-}
-else if (ret == CC_RET_SKIPPED)
+.main = multi_client_connect_call_plugin_v1,
+.deferred = multi_client_connect_fail
+},
 {
-return true;
-}
-else
+.main = multi_client_connect_call_plugin_v2,
+.deferred = multi_client_connect_fail
+},
+{
+.main = multi_client_connect_call_script,
+.deferred = multi_client_connect_fail
+},
 {
-ASSERT(0);
+.main = multi_client_connect_mda,
+.deferred = multi_client_connect_fail
+},
+{
+.main = NULL,
+.deferred = NULL
+/* End of list sentinel.  */
 }
-}
-
-typedef enum client_connect_return (*multi_client_connect_handler)
-(struct multi_context *m, struct multi_instance *mi,
- unsigned int *option_types_found);
+};
 
 /*
  * Called as soon as the SSL/TLS connection is authenticated.
@@ -2273,27 +2292,83 @@ multi_connection_established(struct multi_context *m, 
struct multi_instance *mi)
 return;
 }
 
-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
-};
-
-unsigned int option_types_found = 0;
+/* We are only called for the CAS_PENDING_x states, so we
+ * can ignore other states here */
+bool from_deferred = (mi->context.c2.context_auth != CAS_PENDING);
 
-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);
+struct client_connect_defer_state *defer_state =
+&(mi->client_connect_defer_state);
 
-for (int i = 0; cc_succeeded && handlers[i]; i++)
+/* We are called for the first time */
+if (!from_deferred)
 {
-ret = handlers[i](m, mi, _types_found);
-cc_succeeded = cc_check_return(_succeeded_count, ret);
+defer_state->cur_handler_index = 0;
+defer_state->option_types_found = 0;
+/* Initially we have no handler that has returned a result */
+mi->context.c2.context_auth = 

[Openvpn-devel] [PATCH v5 06/14] client-connect: Refactor client-connect handling to calling a bunch of hooks in a loop

2020-07-11 Thread Arne Schwabe
From: Fabian Knittel 

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

Signed-off-by: Fabian Knittel 
Signed-off-by: Arne Schwabe 

Patch V5: Rebase on master.

Signed-off-by: Arne Schwabe 
---
 src/openvpn/multi.c | 43 +--
 1 file changed, 17 insertions(+), 26 deletions(-)

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 9bb52ef7..83848fdc 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2241,6 +2241,10 @@ cc_check_return(int *cc_succeeded_count,
 }
 }
 
+typedef enum client_connect_return (*multi_client_connect_handler)
+(struct multi_context *m, struct multi_instance *mi,
+ unsigned int *option_types_found);
+
 /*
  * Called as soon as the SSL/TLS connection is authenticated.
  *
@@ -2268,7 +2272,17 @@ multi_connection_established(struct multi_context *m, 
struct multi_instance *mi)
 {
 return;
 }
-unsigned int option_types_found = 0;
+
+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
+};
+
+unsigned int option_types_found = 0;
 
 int cc_succeeded = true; /* client connect script status */
 int cc_succeeded_count = 0;
@@ -2276,32 +2290,9 @@ multi_connection_established(struct multi_context *m, 
struct multi_instance *mi)
 
 multi_client_connect_early_setup(m, mi);
 
-ret = multi_client_connect_source_ccd(m, mi, _types_found);
-cc_succeeded = cc_check_return(_succeeded_count, ret);
-
-if (cc_succeeded)
-{
-ret = multi_client_connect_call_plugin_v1(m, mi, _types_found);
-cc_succeeded = cc_check_return(_succeeded_count, ret);
-}
-
-if (cc_succeeded)
-{
-ret = multi_client_connect_call_plugin_v2(m, mi, _types_found);
-cc_succeeded = cc_check_return(_succeeded_count, ret);
-}
-
-
-if (cc_succeeded)
+for (int i = 0; cc_succeeded && handlers[i]; i++)
 {
-ret = multi_client_connect_call_script(m, mi, _types_found);
-cc_succeeded = cc_check_return(_succeeded_count, ret);
-}
-
-if (cc_succeeded)
-{
-
-ret = multi_client_connect_mda(m, mi, _types_found);
+ret = handlers[i](m, mi, _types_found);
 cc_succeeded = cc_check_return(_succeeded_count, ret);
 }
 
-- 
2.26.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH v5 04/14] client-connect: Move multi_client_connect_setenv into early_setup

2020-07-11 Thread Arne Schwabe
From: Fabian Knittel 

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

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

Signed-off-by: Fabian Knittel 
Signed-off-by: Arne Schwabe 

Patch V5: Rebase on master

Signed-off-by: Arne Schwabe 
---
 src/openvpn/multi.c | 22 +-
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 35e0bd10..539ebfc0 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2150,6 +2150,12 @@ 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);
+
 }
 
 /**
@@ -2195,6 +2201,13 @@ multi_client_connect_source_ccd(struct multi_context *m,
   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);
+
+multi_client_connect_setenv(m, mi);
 }
 gc_free();
 }
@@ -2236,15 +2249,6 @@ multi_connection_established(struct multi_context *m, 
struct multi_instance *mi)
 
 multi_client_connect_source_ccd(m, mi, _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, _types_found,
 _succeeded,
 _succeeded_count);
-- 
2.26.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH v5 02/14] client-connect: Split multi_connection_established into separate functions

2020-07-11 Thread Arne Schwabe
From: Fabian Knittel 

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 

PATCH v3: Since the code has changed enough from the time the original patch
to the current master, the splitting has been redone from the current code.
Also some style and minor code changes have been added doing this patch.
This elimininates and the big reformatting done before eliminates the follow
up patch with indentation changes.

The original patch already replaces some instances of option_permission_mask
with CLIENT_CONNECT_OPT_MASK. The V3 version does this more consistenly.

Patch v4: Move config -> mi->cc_config into its own commit

Patch v5: Clean up some minor issues, add one missing check on
temporary file deletion, rebase on latest master.

Signed-off-by: Arne Schwabe 
---
 src/openvpn/multi.c | 588 ++--
 src/openvpn/multi.h |   4 +-
 2 files changed, 350 insertions(+), 242 deletions(-)

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index a2af071a..3c4ceeb5 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -1638,7 +1638,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? */
@@ -1647,7 +1646,7 @@ multi_client_connect_post(struct multi_context *m,
 options_server_import(>context.options,
   dc_file,
   D_IMPORT_ERRORS|M_OPTERR,
-  option_permissions_mask,
+  CLIENT_CONNECT_OPT_MASK,
   option_types_found,
   mi->context.c2.es);
 
@@ -1671,7 +1670,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;
@@ -1689,7 +1687,7 @@ multi_client_connect_post_plugin(struct multi_context *m,
 options_string_import(>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);
 }
@@ -1716,7 +1714,6 @@ multi_client_connect_post_plugin(struct multi_context *m,
 static void
 multi_client_connect_mda(struct multi_context *m,
  struct multi_instance *mi,
- unsigned int option_permissions_mask,
  unsigned int *option_types_found)
 {
 if (mi->cc_config)
@@ -1729,7 +1726,7 @@ multi_client_connect_mda(struct multi_context *m,
 options_string_import(>context.options,
   opt,
   D_IMPORT_ERRORS|M_OPTERR,
-  option_permissions_mask,
+  CLIENT_CONNECT_OPT_MASK,
   option_types_found,
   mi->context.c2.es);
 }
@@ -1843,160 +1840,46 @@ multi_client_set_protocol_options(struct context *c)
 }
 }
 
-/**
- * Generates the data channel keys
- */
-static bool
-multi_client_generate_tls_keys(struct context *c)
-{
-struct frame *frame_fragment = NULL;
-#ifdef ENABLE_FRAGMENT
-if (c->options.ce.fragment)
-{
-frame_fragment = >c2.frame_fragment;
-}
-#endif
-struct tls_session *session = >c2.tls_multi->session[TM_ACTIVE];
-if (!tls_session_update_crypto_params(session, >options,
-  >c2.frame, frame_fragment))
-{
-msg(D_TLS_ERRORS, "TLS Error: initializing data channel failed");
-register_signal(c, SIGUSR1, "process-push-msg-failed");
-return false;
-}
-
-return true;
-}
-
-/*
- * 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 

[Openvpn-devel] [PATCH v5 09/14] client-connect: Add deferred support to the client-connect script handler

2020-07-11 Thread Arne Schwabe
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 
---
 src/openvpn/multi.c | 230 +---
 src/openvpn/multi.h |  12 +++
 2 files changed, 227 insertions(+), 15 deletions(-)

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index ce73f8a1..271d09d8 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -1843,6 +1843,168 @@ 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)
+{
+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;
+
+if (ccs->deferred_ret_file)
+{
+ccs_delete_deferred_ret_file(mi);
+}
+
+fn = platform_create_temp_file(mi->context.options.tmp_dir, "ccr", );
+if (!fn)
+{
+gc_free();
+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();
+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);
+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:
+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);
+}
+  

[Openvpn-devel] [PATCH v5 07/14] client-connect: Change cas_context from int to enum

2020-07-11 Thread Arne Schwabe
This deviates from Fabian's original patch that relied on the now
removed connection_established bool as pointer being NULL or non NULL as
implicit third state and makeing connection_established as a substate of
(cas_context == CAS_PENDING)

Signed-off-by: Arne Schwabe 

Patch V5: extend cas_context with two new states instead adding an
  extra mini state machine.

Signed-off-by: Arne Schwabe 
---
 src/openvpn/multi.c   |  2 +-
 src/openvpn/multi.h   |  1 +
 src/openvpn/openvpn.h | 24 +---
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 83848fdc..f9b8af80 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2163,7 +2163,7 @@ multi_client_connect_early_setup(struct multi_context *m,
  * Try to source a dynamic config file from the
  * --client-config-dir directory.
  */
-enum client_connect_return
+static enum client_connect_return
 multi_client_connect_source_ccd(struct multi_context *m,
 struct multi_instance *mi,
 unsigned int *option_types_found)
diff --git a/src/openvpn/multi.h b/src/openvpn/multi.h
index 4fb4d0b6..1d30dcc6 100644
--- a/src/openvpn/multi.h
+++ b/src/openvpn/multi.h
@@ -62,6 +62,7 @@ struct deferred_signal_schedule_entry
 struct timeval wakeup;
 };
 
+
 /**
  * Server-mode state structure for one single VPN tunnel.
  *
diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h
index a1308852..7c469b01 100644
--- a/src/openvpn/openvpn.h
+++ b/src/openvpn/openvpn.h
@@ -210,6 +210,21 @@ struct context_1
 #endif
 };
 
+
+/* client authentication state, CAS_SUCCEEDED must be 0 since
+ * non multi code path still checks this variable but does not initialise it
+ * so the code depends on zero initialisation */
+enum client_connect_status {
+CAS_SUCCEEDED=0,
+CAS_PENDING,
+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)
+ */
+};
+
 /**
  * Level 2 %context containing state that is reset on both \c SIGHUP and
  * \c SIGUSR1 restarts.
@@ -444,13 +459,8 @@ struct context_2
 int push_ifconfig_ipv6_netbits;
 struct in6_addr push_ifconfig_ipv6_remote;
 
-/* client authentication state, CAS_SUCCEEDED must be 0 */
-#define CAS_SUCCEEDED 0
-#define CAS_PENDING   1
-#define CAS_FAILED2
-#define CAS_PARTIAL   3  /* at least one client-connect script/plugin
-  * succeeded while a later one in the chain failed */
-int context_auth;
+
+enum client_connect_status context_auth;
 
 struct event_timeout push_request_interval;
 int n_sent_push_requests;
-- 
2.26.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH v5 11/14] client-connect: Use inotify for the deferred client-connect status file

2020-07-11 Thread Arne Schwabe
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 
---
 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 dafc85f1..09a25a58 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2618,8 +2618,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)
@@ -2905,7 +2907,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);
 }
-- 
2.26.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH v5 05/14] client-connect: Refactor to use return values instead of modifying a passed-in flag

2020-07-11 Thread Arne Schwabe
From: Fabian Knittel 

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

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

Signed-off-by: Fabian Knittel 

Patch V5: Minor style fixes

Signed-off-by: Arne Schwabe 
---
 src/openvpn/multi.c | 135 ++--
 src/openvpn/multi.h |  10 
 2 files changed, 91 insertions(+), 54 deletions(-)

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 539ebfc0..9bb52ef7 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -1706,20 +1706,21 @@ multi_client_connect_post_plugin(struct multi_context 
*m,
 
 #endif /* ifdef ENABLE_PLUGIN */
 
-#ifdef MANAGEMENT_DEF_AUTH
+
 
 /*
  * Called to load management-derived client-connect config
  */
-static void
+enum client_connect_return
 multi_client_connect_mda(struct multi_context *m,
  struct multi_instance *mi,
  unsigned int *option_types_found)
 {
+enum client_connect_return ret = CC_RET_SKIPPED;
+#ifdef MANAGEMENT_DEF_AUTH
 if (mi->cc_config)
 {
 struct buffer_entry *be;
-
 for (be = mi->cc_config->head; be != NULL; be = be->next)
 {
 const char *opt = BSTR(>buf);
@@ -1739,10 +1740,12 @@ multi_client_connect_mda(struct multi_context *m,
  */
 multi_select_virtual_addr(m, mi);
 multi_set_virtual_addr_env(mi);
-}
-}
 
+ret = CC_RET_SUCCEEDED;
+}
 #endif /* ifdef MANAGEMENT_DEF_AUTH */
+return ret;
+}
 
 static void
 multi_client_connect_setenv(struct multi_context *m,
@@ -1840,19 +1843,16 @@ multi_client_set_protocol_options(struct context *c)
 }
 }
 
-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);
 
 /* deprecated callback, use a file for passing back return info */
 if (plugin_defined(mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT))
@@ -1864,7 +1864,7 @@ multi_client_connect_call_plugin_v1(struct multi_context 
*m,
 
 if (!dc_file)
 {
-cc_succeeded = false;
+ret = CC_RET_FAILED;
 goto cleanup;
 }
 
@@ -1874,12 +1874,12 @@ multi_client_connect_call_plugin_v1(struct 
multi_context *m,
 != 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;
 }
 
 if (!platform_unlink(dc_file))
@@ -1893,21 +1893,19 @@ cleanup:
 gc_free();
 }
 #endif /* ifdef ENABLE_PLUGIN */
+return ret;
 }
 
-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);
 
 /* V2 callback, use a plugin_return struct for passing back return info */
 if (plugin_defined(mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT_V2))
@@ -1921,17 +1919,18 @@ multi_client_connect_call_plugin_v2(struct 
multi_context *m,
 != 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, , option_types_found);
-(*cc_succeeded_count)++;
+ret = CC_RET_SUCCEEDED;
 }
 
 plugin_return_free();
 }
 #endif /* ifdef ENABLE_PLUGIN */
+return ret;
 }
 
 
@@ -1939,15 

[Openvpn-devel] [PATCH v5 03/14] client-connect: Refactor multi_client_connect_source_ccd

2020-07-11 Thread Arne Schwabe
From: Fabian Knittel 

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

Signed-off-by: Fabian Knittel 

Patch V5: Simplify the logic even further to make more easy to understand.

Signed-off-by: Arne Schwabe 
---
 src/openvpn/multi.c | 43 +--
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 3c4ceeb5..35e0bd10 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2164,15 +2164,30 @@ multi_client_connect_source_ccd(struct multi_context *m,
 if (mi->context.options.client_config_dir)
 {
 struct gc_arena gc = gc_new();
-const char *ccd_file;
+const char *ccd_file = NULL;
+
+const char *ccd_client = 
platform_gen_path(mi->context.options.client_config_dir,
+   
tls_common_name(mi->context.c2.tls_multi,
+   false),
+   );
+
+const char *ccd_default = 
platform_gen_path(mi->context.options.client_config_dir,
+CCD_DEFAULT,
+);
 
-ccd_file = platform_gen_path(mi->context.options.client_config_dir,
- tls_common_name(mi->context.c2.tls_multi,
- false),
- );
 
 /* try common-name file */
-if (platform_test_file(ccd_file))
+if (platform_test_file(ccd_client))
+{
+ccd_file = ccd_client;
+}
+/* try default file */
+else if (platform_test_file(ccd_default))
+{
+ccd_file = ccd_default;
+}
+
+if (ccd_file)
 {
 options_server_import(>context.options,
   ccd_file,
@@ -2181,22 +2196,6 @@ multi_client_connect_source_ccd(struct multi_context *m,
   option_types_found,
   mi->context.c2.es);
 }
-else /* try default file */
-{
-ccd_file = platform_gen_path(mi->context.options.client_config_dir,
- CCD_DEFAULT,
- );
-
-if (platform_test_file(ccd_file))
-{
-options_server_import(>context.options,
-  ccd_file,
-  D_IMPORT_ERRORS|M_OPTERR,
-  CLIENT_CONNECT_OPT_MASK,
-  option_types_found,
-  mi->context.c2.es);
-}
-}
 gc_free();
 }
 }
-- 
2.26.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH v5 13/14] client-connect: Implement deferred connect support for plugin API v2

2020-07-11 Thread Arne Schwabe
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 
---
 include/openvpn-plugin.h.in |  3 +-
 src/openvpn/multi.c | 58 ++---
 src/openvpn/plugin.c|  3 ++
 3 files changed, 52 insertions(+), 12 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_PF11
 #define OPENVPN_PLUGIN_ROUTE_PREDOWN12
 #define OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER 13
-#define OPENVPN_PLUGIN_N14
+#define OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER_V2  14
+#define OPENVPN_PLUGIN_N15
 
 /*
  * 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();
 
-if (plugin_call(mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT_V2,
-NULL, , mi->context.c2.es)
-!= OPENVPN_PLUGIN_FUNC_SUCCESS)
+int plug_ret = plugin_call(mi->context.plugins, call,
+   NULL, , 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, , 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, , option_types_found);
-ret = CC_RET_SUCCEEDED;
+msg(M_WARN, "WARNING: client-connect-v2 plugin call failed");
+ret = CC_RET_FAILED;
 }
 
+
 plugin_return_free();
 }
 #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
+

[Openvpn-devel] [PATCH v5 01/14] Allow changing fallback cipher from ccd files/client-connect

2020-07-11 Thread Arne Schwabe
This allows to control the fallback cipher that is used when the
client/server do have any common cipher on a per client basis.

The patch is similar to Steffan's
[PATCH v4] Allow changing cipher from a ccd file.

Steffan's old patch also moves the cipher negotiation to
multi_established_connection() which I independently discovered and
implemented in

Extract process_incoming_push_reply from process_incoming_push_msg
(#FIXME add commitsh when commited to master)

Signed-off-by: Arne Schwabe 
---
 src/openvpn/options.c | 2 +-
 src/openvpn/options.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index b93fd4fe..bf2760e1 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -7892,7 +7892,7 @@ add_option(struct options *options,
 }
 else if (streq(p[0], "cipher") && p[1] && !p[2])
 {
-VERIFY_PERMISSION(OPT_P_NCP);
+VERIFY_PERMISSION(OPT_P_NCP|OPT_P_INSTANCE);
 options->ciphername = p[1];
 }
 else if (streq(p[0], "ncp-ciphers") && p[1] && !p[2])
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index c83a46aa..c37006d3 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -677,7 +677,7 @@ struct options
 #define OPT_P_MTU (1<<14) /* TODO */
 #define OPT_P_NICE(1<<15)
 #define OPT_P_PUSH(1<<16)
-#define OPT_P_INSTANCE(1<<17)
+#define OPT_P_INSTANCE(1<<17) /**< allowed in ccd, client-connect etc*/
 #define OPT_P_CONFIG  (1<<18)
 #define OPT_P_EXPLICIT_NOTIFY (1<<19)
 #define OPT_P_ECHO(1<<20)
-- 
2.26.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH v5 12/14] client-connect: Add deferred support to the client-connect plugin v1 handler

2020-07-11 Thread Arne Schwabe
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 
---
 include/openvpn-plugin.h.in | 29 +--
 src/openvpn/multi.c | 97 -
 src/openvpn/plugin.c|  3 ++
 3 files changed, 93 insertions(+), 36 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_UP0
-#define OPENVPN_PLUGIN_DOWN  1
-#define OPENVPN_PLUGIN_ROUTE_UP  2
-#define OPENVPN_PLUGIN_IPCHANGE  3
-#define OPENVPN_PLUGIN_TLS_VERIFY4
-#define OPENVPN_PLUGIN_AUTH_USER_PASS_VERIFY 5
-#define OPENVPN_PLUGIN_CLIENT_CONNECT6
-#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_UP0
+#define OPENVPN_PLUGIN_DOWN  1
+#define OPENVPN_PLUGIN_ROUTE_UP  2
+#define OPENVPN_PLUGIN_IPCHANGE  3
+#define OPENVPN_PLUGIN_TLS_VERIFY4
+#define OPENVPN_PLUGIN_AUTH_USER_PASS_VERIFY 5
+#define OPENVPN_PLUGIN_CLIENT_CONNECT6
+#define OPENVPN_PLUGIN_CLIENT_DISCONNECT 7
+#define OPENVPN_PLUGIN_LEARN_ADDRESS 8
+#define OPENVPN_PLUGIN_CLIENT_CONNECT_V2 9
+#define OPENVPN_PLUGIN_TLS_FINAL10
+#define OPENVPN_PLUGIN_ENABLE_PF11
+#define OPENVPN_PLUGIN_ROUTE_PREDOWN12
+#define OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER 13
+#define OPENVPN_PLUGIN_N14
 
 /*
  * Build a mask out of a set of plug-in types.
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 09a25a58..08eb44ba 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2008,56 +2008,109 @@ ccs_gen_config_file(struct multi_instance *mi)
 static enum client_connect_return
 multi_client_connect_call_plugin_v1(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
 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", );
+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(, "%s", dc_file);
-if (plugin_call(mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT,
-, NULL, mi->context.c2.es)
-!= OPENVPN_PLUGIN_FUNC_SUCCESS)
+argv_printf(, "%s", ccs->config_file);
+int plug_ret = plugin_call(mi->context.plugins, call,
+   , 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-devel] [PATCH v5 14/14] client-connect: Add documentation for the deferred client connect feature

2020-07-11 Thread Arne Schwabe
Patch V5: Fix typos, clarify man page section about deferred client-connect
  script. Add section to Changes.rst

Signed-off-by: Arne Schwabe 
---
 Changes.rst |  4 +++
 doc/openvpn.8   | 55 +++--
 include/openvpn-plugin.h.in | 21 ++
 3 files changed, 71 insertions(+), 9 deletions(-)

diff --git a/Changes.rst b/Changes.rst
index 42f0d190..47fa6883 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -3,6 +3,10 @@ Overview of changes in 2.5
 
 New features
 
+Deferred client-connect
+client-connect and the connect plugin API allow now asynchronous/deferred
+return of the configuration file in the same way as the auth-plugin.
+
 Client-specific tls-crypt keys (``--tls-crypt-v2``)
 ``tls-crypt-v2`` adds the ability to supply each client with a unique
 tls-crypt key.  This allows large organisations and VPN providers to profit
diff --git a/doc/openvpn.8 b/doc/openvpn.8
index 03ae5ac5..7a0080bf 100644
--- a/doc/openvpn.8
+++ b/doc/openvpn.8
@@ -3422,6 +3422,13 @@ is significant.  If
 .B script
 returns a non\-zero error status, it will cause the client
 to be disconnected.
+
+If a
+.B \-\-client\-connect cmd
+wants to defer the generating of the configuration the script, should
+use the client_connect_deferred_file and client_connect_config_file
+environment variables and write status accordingly into these files
+(See the environment section below for more details).
 .\"*
 .TP
 .B \-\-client\-disconnect cmd
@@ -3505,12 +3512,18 @@ This directory will be used by in the following cases:
 
 *
 .B \-\-client\-connect
-scripts to dynamically generate client\-specific
-configuration files.
+scripts and
+.B OPENVPN_PLUGIN_CLIENT_CONNECT
+plugin hook
+to dynamically generate client\-specific configuration files
+and return success/failure via client_connect_deferred_file
+when using deferred client connect method
 
 *
 .B OPENVPN_PLUGIN_AUTH_USER_PASS_VERIFY
-plugin hook to return success/failure via auth_control_file
+and
+
+plugin hook to return success/failure via auth_control_file/
 when using deferred auth method
 
 *
@@ -6654,6 +6667,42 @@ Set prior to execution of the
 script.
 .\"*
 .TP
+.B client_connect_config_file
+The path to the configuration file that should be written by
+the
+.B \-\-client\-connect
+script. The content of this environment variable is identical
+to the file as a argument of the called
+.B \-\-client\-connect
+script.
+.\"*
+.TP
+.B client_connect_deferred_file
+This file can be optionally written to communicate a status
+code of the
+.TP
+.B \-\-client\-connect
+script. If used for deferring, this file must be written
+before the
+.B \-\-client\-connect
+script exits. The first character in the file has to be
+'1' is to indicate normal script execution, '0' indicates an
+error (in the same way that a non zero exit status does) and
+'2' indicates that the script deferred returning the config
+file. When the script defers returning the configuration, it
+must also write '2' to to the file to indicate the deferral.
+A background process or similar must then take care of writing the
+configuration to the file indicated by the
+.B
+client_connect_config_file
+environment variable and when finished, write the a '1' to this
+file (or '0' in case of an error).
+
+The absence of any character in the file when the script finishes
+executing is interpreted the same as '1'. This allows script that
+are not written to support the defer mechanism to be used unmodified.
+.\"*
+.TP
 .B common_name
 The X509 common name of an authenticated client.
 Set prior to execution of
diff --git a/include/openvpn-plugin.h.in b/include/openvpn-plugin.h.in
index 38fbe097..64b20886 100644
--- a/include/openvpn-plugin.h.in
+++ b/include/openvpn-plugin.h.in
@@ -557,12 +557,21 @@ OPENVPN_PLUGIN_DEF openvpn_plugin_handle_t 
OPENVPN_PLUGIN_FUNC(openvpn_plugin_op
  * OPENVPN_PLUGIN_FUNC_SUCCESS on success, OPENVPN_PLUGIN_FUNC_ERROR on failure
  *
  * In addition, OPENVPN_PLUGIN_FUNC_DEFERRED may be returned by
- * OPENVPN_PLUGIN_AUTH_USER_PASS_VERIFY.  This enables asynchronous
- * authentication where the plugin (or one of its agents) may indicate
- * authentication success/failure some number of seconds after the return
- * of the OPENVPN_PLUGIN_AUTH_USER_PASS_VERIFY handler by writing a single
- * char to the file named by auth_control_file in the environmental variable
- * list (envp).
+ * OPENVPN_PLUGIN_AUTH_USER_PASS_VERIFY, OPENVPN_PLUGIN_CLIENT_CONNECT and
+ * OPENVPN_PLUGIN_CLIENT_CONNECT_V2. This enables asynchronous
+ * authentication or client connect  where the plugin (or one of its agents)
+ * may indicate authentication success/failure or client configuration some
+ * number of seconds 

[Openvpn-devel] [PATCH v5 10/14] client-connect: Move adding inotify watch into its own function

2020-07-11 Thread Arne Schwabe
This make the code a bit better readable and also prepares resuing
the function for client-connect return files

Signed-off-by: Arne Schwabe 
---
 src/openvpn/multi.c | 43 ---
 1 file changed, 28 insertions(+), 15 deletions(-)

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 271d09d8..dafc85f1 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2828,6 +2828,32 @@ multi_schedule_context_wakeup(struct multi_context *m, 
struct multi_instance *mi
compute_wakeup_sigma(>context.c2.timeval));
 }
 
+#if defined(ENABLE_ASYNC_PUSH) && defined(ENABLE_DEF_AUTH)
+static void
+add_inotify_file_watch(struct multi_context *m, struct multi_instance *mi,
+   int inotify_fd, const char *file)
+{
+/* watch acf file */
+long watch_descriptor = inotify_add_watch(inotify_fd, file,
+  IN_CLOSE_WRITE | IN_ONESHOT);
+if (watch_descriptor >= 0)
+{
+if (mi->inotify_watch != -1)
+{
+hash_remove(m->inotify_watchers,
+(void *) (unsigned long)mi->inotify_watch);
+}
+hash_add(m->inotify_watchers, (const uintptr_t *)watch_descriptor,
+ mi, true);
+mi->inotify_watch = watch_descriptor;
+}
+else
+{
+msg(M_NONFATAL | M_ERRNO, "MULTI: inotify_add_watch error");
+}
+}
+#endif /* if defined(ENABLE_ASYNC_PUSH) && defined(ENABLE_DEF_AUTH) */
+
 /*
  * Figure instance-specific timers, convert
  * earliest to absolute time in mi->wakeup,
@@ -2865,21 +2891,8 @@ multi_process_post(struct multi_context *m, struct 
multi_instance *mi, const uns
 if (ks && ks->auth_control_file && was_unauthenticated
 && (ks->authenticated == KS_AUTH_DEFERRED))
 {
-/* watch acf file */
-long watch_descriptor = inotify_add_watch(m->top.c2.inotify_fd, 
ks->auth_control_file, IN_CLOSE_WRITE | IN_ONESHOT);
-if (watch_descriptor >= 0)
-{
-if (mi->inotify_watch != -1)
-{
-hash_remove(m->inotify_watchers, (void *) (unsigned 
long)mi->inotify_watch);
-}
-hash_add(m->inotify_watchers, (const uintptr_t 
*)watch_descriptor, mi, true);
-mi->inotify_watch = watch_descriptor;
-}
-else
-{
-msg(M_NONFATAL | M_ERRNO, "MULTI: inotify_add_watch error");
-}
+add_inotify_file_watch(m, mi, m->top.c2.inotify_fd,
+   ks->auth_control_file);
 }
 #endif
 
-- 
2.26.2



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel