[Openvpn-devel] [PATCH applied] Re: Add demo plugin that excercises CLIENT_CONNECT and CLIENT_CONNECT_V2 paths

2020-09-17 Thread Gert Doering
Patch has been applied to the master and release/2.5 branch.

commit 94cebf8261d20a55b0260cce61ad892a98bc24d8 (master)
commit 44e9f3933e27988193e4a0346559f53e5b876bde (release/2.5)
Author: Gert Doering
Date:   Thu Sep 17 18:19:09 2020 +0200

 Add demo plugin that excercises CLIENT_CONNECT and CLIENT_CONNECT_V2 paths

 Acked-by: David Sommerseth 
 Message-Id: <20200917161909.11573-1-g...@greenie.muc.de>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21047.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] Add demo plugin that excercises "CLIENT_CONNECT" and "CLIENT_CONNECT_V2" paths

2020-09-17 Thread David Sommerseth
On 17/09/2020 18:19, Gert Doering wrote:
> This is a new "samples" plugin which does not do many useful things,
> besides
>  - show how a plugin is programmed
>  - how the various messages get dispatched
>  - how to pass back information from a client-connect/v2 plugin
>  - how to do async-cc plugins  [not yet implemented]
> 
> the operation of the plugin is controlled by UV_WANT_* environment variables
> controlled by the client ("--setenv UV_WANT_CC_FAIL 1 --push-peer-info"),
> to "fail CLIENT_CONNECT" or "use async-cc for CLIENT_CONNECT_V2" or
> "send 'disable' back from ...") - which is useful for automated testing
> of server success/defer/fail code paths for the CLIENT_CONNECT_* functions.
> 
> See samples/sample-plugins/client-connect/README for details how to do this.
> 
> v2:
>   - implement async / deferred operation both for CLIENT_CONNECT and
> CLIENT_CONNECT_V2 plugin calls
>   - implement returning openvpn-controlled (setenv) config snippets
> (so the client side can verify in automated testing that the plugin
> operated correctly, without hard-coding something in the plugin code)
> 
> v3:
>   - remove -Wno-unused-variable from Makefile
>   - remove unused "char ** argv" (commented out, but kept as reference)
> 
> v4:
>   - upgrade to use the build infra brought by commit 0b5141d8f946
>   - remove local Makefile
>   - include "config.h" to get what is needed to get rid of the strdup()
> warning
> ---
>  sample/sample-plugins/Makefile.plugins|   3 +-
>  sample/sample-plugins/README  |   6 +
>  sample/sample-plugins/client-connect/README   |  38 ++
>  .../client-connect/sample-client-connect.c| 612 ++
>  4 files changed, 658 insertions(+), 1 deletion(-)
>  create mode 100644 sample/sample-plugins/client-connect/README
>  create mode 100644 
> sample/sample-plugins/client-connect/sample-client-connect.c
> 

I've only glared at important code pieces, diffed against the v2 of this patch
and compiled it on RHEL-7 (gcc-4.8.5 and gcc-9.3.1/devtoolset-9).  Since
everything is as expected now (no compiler complaints, diff is good) and prior
review testing worked as expected ...

Acked-By: David Sommerseth 


-- 
kind regards,

David Sommerseth
OpenVPN Inc




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


[Openvpn-devel] [PATCH v4] Add demo plugin that excercises "CLIENT_CONNECT" and "CLIENT_CONNECT_V2" paths

2020-09-17 Thread Gert Doering
This is a new "samples" plugin which does not do many useful things,
besides
 - show how a plugin is programmed
 - how the various messages get dispatched
 - how to pass back information from a client-connect/v2 plugin
 - how to do async-cc plugins  [not yet implemented]

the operation of the plugin is controlled by UV_WANT_* environment variables
controlled by the client ("--setenv UV_WANT_CC_FAIL 1 --push-peer-info"),
to "fail CLIENT_CONNECT" or "use async-cc for CLIENT_CONNECT_V2" or
"send 'disable' back from ...") - which is useful for automated testing
of server success/defer/fail code paths for the CLIENT_CONNECT_* functions.

See samples/sample-plugins/client-connect/README for details how to do this.

v2:
  - implement async / deferred operation both for CLIENT_CONNECT and
CLIENT_CONNECT_V2 plugin calls
  - implement returning openvpn-controlled (setenv) config snippets
(so the client side can verify in automated testing that the plugin
operated correctly, without hard-coding something in the plugin code)

v3:
  - remove -Wno-unused-variable from Makefile
  - remove unused "char ** argv" (commented out, but kept as reference)

v4:
  - upgrade to use the build infra brought by commit 0b5141d8f946
  - remove local Makefile
  - include "config.h" to get what is needed to get rid of the strdup()
warning
---
 sample/sample-plugins/Makefile.plugins|   3 +-
 sample/sample-plugins/README  |   6 +
 sample/sample-plugins/client-connect/README   |  38 ++
 .../client-connect/sample-client-connect.c| 612 ++
 4 files changed, 658 insertions(+), 1 deletion(-)
 create mode 100644 sample/sample-plugins/client-connect/README
 create mode 100644 sample/sample-plugins/client-connect/sample-client-connect.c

diff --git a/sample/sample-plugins/Makefile.plugins 
b/sample/sample-plugins/Makefile.plugins
index f550fc19..37559a86 100644
--- a/sample/sample-plugins/Makefile.plugins
+++ b/sample/sample-plugins/Makefile.plugins
@@ -11,7 +11,8 @@ PLUGINS = \
keying-material-exporter-demo/keyingmaterialexporter \
log/log log/log_v3 \
simple/base64 \
-   simple/simple
+   simple/simple \
+   client-connect/sample-client-connect
 
 # All the plugins to build - rewritten with .so extension
 all : $(foreach var, $(PLUGINS), $(var).so)
diff --git a/sample/sample-plugins/README b/sample/sample-plugins/README
index 3ed60348..cf1b355e 100644
--- a/sample/sample-plugins/README
+++ b/sample/sample-plugins/README
@@ -13,6 +13,12 @@ log/log_v3.c-- A variant of log/log.c, which makes use 
of the
OpenVPN plug-in v3 API.  This will also log even more
information related to certificates in use.
 
+* client-connect (and logging)
+client-connect/sample-client-connect -- demonstrate how to use the
+   CLIENT_CONNECT and CLIENT_CONNECT_V2 hooks to achieve
+   "per client configuration / logging / ..." actions,
+   both in synchronous and async/deferred mode
+
 * cryptography related
 simple/base64.c -- Example using the OpenVPN exported base64 encode/decode
functions
diff --git a/sample/sample-plugins/client-connect/README 
b/sample/sample-plugins/client-connect/README
new file mode 100644
index ..cb3e0f3c
--- /dev/null
+++ b/sample/sample-plugins/client-connect/README
@@ -0,0 +1,38 @@
+OpenVPN plugin examples.
+
+Examples provided:
+
+sample-client-connect.c
+
+  - hook to all plugin hooks that openvpn offers
+  - log which hook got called
+  - on CLIENT_CONNECT or CLIENT_CONNECT_V2 set some config variables
+(controlled by "setenv plugin_cc_config ..." and "plugin_cc2_config"
+in openvpn's config)
+
+  - if the environment variable UV_WANT_CC_FAIL is set, fail
+  - if the environment variable UV_WANT_CC_DISABLE is set, reject ("disable")
+  - if the environment variable UV_WANT_CC_ASYNC is set, go to
+asynchronous/deferred mode on CLIENT_CONNECT, and sleep for
+${UV_WANT_CC_ASYNC} seconds
+
+  - if the environment variable UV_WANT_CC2_FAIL is set, fail CC2
+  - if the environment variable UV_WANT_CC2_DISABLE is set, reject ("disable")
+  - if the environment variable UV_WANT_CC2_ASYNC is set, go to
+asynchronous/deferred mode on CLIENT_CONNECT_V2, and sleep for
+${UV_WANT_CC2_ASYNC} seconds
+
+(this can be client-controlled with --setenv UV_WANT_CC_ASYNC nnn
+ etc. --> for easy testing server code paths)
+
+To build for unixy platforms (not very sophisticated right now, needs gmake):
+
+  .../sample-plugins$ gmake client-connect/sample-client-connect.so
+
+(This plugin has not been tested on Windows, and might not even work due
+to its use of fork() and wait().  Let us know if it does or needs patches)
+
+
+To use in OpenVPN, add to config file:
+
+  plugin sample-client-connect.so (Linux/BSD/etc.)
diff --git a/sample/sample-plugins/client-connect/sample-client-connect.c 

Re: [Openvpn-devel] [PATCH] Support for wolfSSL in OpenVPN

2020-09-17 Thread Juliusz Sosinowicz

Hi Arne,

thank you for your extensive review of OpenVPN with wolfSSL.

On 17/09/2020 00:05, Arne Schwabe wrote:

...
I am still seeing this warning:


2020-09-16 23:20:14 WARNING: 'auth' is used inconsistently, local='auth
SHA', remote='auth SHA1'

Are you internally calling SHA1 just SHA and are also returned that as
name when querying for the name? And do the other SHA variant also just
return SHA?
Could you describe how you generated this warning? Looking into our 
sources, we do call SHA1 just SHA in wolfSSL. Other variants have names 
in the format of SHA.

This snippet in the configure.ac looks strange:

if test -n "${WOLFSSL_DIR}"; then
wolfssldir="${WOLFSSL_DIR}"
else
wolfssldir="/usr/local/include/wolfssl"
fi

I am not sure hardcoding a /usr/local path is something we want/allow.
The people better at autoconf might have a better idea on this.
Our default installation path is /usr/local/include which is why we set 
it as the default path in projects that use wolfSSL. Additionally, 
adding /usr/local/include/wolfssl to the include path allows including 
wolfSSL without changing *.c and *.h files. I'll look into David 
Sommerseth's suggestion of using pkg-config to get the path and see if 
it would be possible to append wolfssl to the path.

...

I am surprised you are targeting OpenSSL < 1.1.0 API. We will probably
drop OpenSSL 1.0.2 support from our code base as soon as we drop RHEL7
support. The 1.1.0+ code path in that patch uses the
EVP_PKEY_derive_*/EVP_PKEY_CTX_set_tls1_prf functions and those will be
needed then.

I think that for now we will target OpenSSL < 1.1.0 API. Once other 
issues have been resolved we will start moving to OpenSSL >= 1.1.0 APIs.


I am still in the process of going through your report but I can 
reproduce most of the other issues and have found some additional ones 
as well. I have closed the PR for now until your comments are resolved 
and will re-open to include all fixes in one PR.


Sincerely
Juliusz



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


Re: [Openvpn-devel] [PATCH] Support for wolfSSL in OpenVPN

2020-09-17 Thread Arne Schwabe
Am 17.09.20 um 17:50 schrieb Juliusz Sosinowicz:
> Could you describe how you generated this warning? Looking into our
> sources, we do call SHA1 just SHA in wolfSSL. Other variants have names
> in the format of SHA.

Just connecting to a server.

Arne



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


[Openvpn-devel] [PATCH applied] Re: If IPv6 pool specification sets pool start to ::0 address, increment.

2020-09-17 Thread Gert Doering
Patch has been applied to the master and release/2.5 branch.

Release/2.4 has the same "unintended feature", but the pool code
is sufficiently different that this patch will not work - I do
not see this as significant problem ("a documented workaround 
exists"), so do not currently plan to backport it.

commit 4dff236811a1ec9c97a27ad93182ad4beb12377f (master)
commit 611382c16cbac83195cc0fd4825553dad149ff9a (release/2.5)
Author: Gert Doering
Date:   Thu Sep 17 10:59:41 2020 +0200

 If IPv6 pool specification sets pool start to ::0 address, increment.

 Signed-off-by: Gert Doering 
 Acked-by: Antonio Quartulli 
 Message-Id: <20200917085941.20972-1-g...@greenie.muc.de>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21039.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 v2] If IPv6 pool specification sets pool start to ::0 address, increment.

2020-09-17 Thread Antonio Quartulli
Hi,

On 17/09/2020 10:59, Gert Doering wrote:
> The first IPv6 address in a subnet is not usable (IPv6 anycast address),
> but our pool code ignored this.
> 
> Instead of assigning an unusable address or erroring out, just log the
> fact, and increment the pool start to ::1
> 
> NOTE: this is a bit simplistic.  A pool that is larger than /96 and
> has non-0 bits in the "uppermost bits" will still get the increment
> as we only look at the lowermost 32 bits.
> 
> NOTE2: if the pool is specified with "--server-ipv6 $base/$bits", this
> is a non-issue, as the address for the pool start will be incremented
> anyway.
> 
> v2: make comment more explicit about "we're only talking about the
> host part here" and "base sees only only 32 bit of the host part"
> 
> Reported-by: NicolaF_ in Trac
> Trac: #1282
> 
> Signed-off-by: Gert Doering 

Acked-by: Antonio Quartulli 

-- 
Antonio Quartulli


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


[Openvpn-devel] [PATCH v5] Add DNS SRV remote host discovery support

2020-09-17 Thread Vladislav Grishenko
DNS SRV remote host discovery allows to have multiple OpenVPN servers for
a single domain w/o explicit profile enumeration, to move services from
host to host with little fuss, and to designate hosts as primary servers
for a service and others as backups.
Feature has been asked several times already, should be useful in case of
substantial number of clients & servers deployed.

Patch introduces "--remote-srv domain [service] [proto]" option.
The "service" and "proto" arguments are optional. Client will try
to resolve DNS SRV record "_service._proto.domain" and use returned
DNS SRV records as remote server list ordered by server selection
mechanism defined in RFC2782 (https://tools.ietf.org/html/rfc2782):

A client MUST attempt to contact the target host with the
lowest-numbered priority field value it can reach, target hosts
with the same priority SHOULD be tried in an order defined by the
weight field.
The weight field specifies a relative weight for entries with the
same priority. Larger weights SHOULD be given a proportionately
higher probability of being selected.
Domain administrators SHOULD use Weight 0 when there isn't any
server selection to do. In the presence of records containing
weights greater than 0, records with Weight 0 SHOULD have a very
small chance of being selected.

Note: OpenVPN server selection mechanism implementation indeed will
give records with weight of zero a very small chance of being selected
first, but never skip them.

Example: instead of multiple --remote in order:
remote server1.example.net 1194 udp
remote server2.example.net 1194 udp
remote server3.example.net 1194 udp
remote server4.example.net 443 tcp

now it's possible to specify just one --remote-srv:
remote-srv example.net

and configure following DNS SRV records:
name prio weight port target
_openvpn._udp.example.net IN SRV 10   60 1194 server1.example.net
_openvpn._udp.example.net IN SRV 10   40 1194 server2.example.net
_openvpn._udp.example.net IN SRV 10   0  1194 server3.example.net
_openvpn._tcp.example.net IN SRV 20   0   443 server4.example.net

For "--remote-srv example.net" following will happen in order:
1. The client will first try to resolve "_openvpn._udp.example.net"
   and "_openvpn._tcp.example.net".
2. Records "server1.example.net:1194", "server2.example.net:1194"
   and "server3.example.net:1194" will be selected before record
   "server4.example.net:443" as their priority 10 is smaller than 20.
3. Records "server1.example.net:1194", "server2.example.net:1194"
   and "server3.example.net:1194" will be randomly selected with
   weight probability: first will be either "server1.example.net:1194"
   with 60% probability or "server2.example.net:1194" with 40% or
   "server3.example.net:1194" with almost zero probability.
4. If "server1.example.net:1194" was selected, the second record will
   be either "server2.example.net:1194" with almost 100% probability
   or "server3.example.net:1194" with almost 0%.
5. If "server2.example.net:1194" was selected, the third record will
   be the only last record of priority 10 - "server3.example.net:1194".
6. Record "server4.example.net:443" will be the last one selected as
   the only record with priority 20.
7. Each of the resulting "target:port" remote hosts will be resolved
   and accessed if its protocol has no conflict with the rest of the
   OpenVPN options.

  If DNS SRV name can't be resolved or no valid records were returned,
  client will move on to the next connection entry.

Tested on Linux/Glibc & Windows 10.

v2: use DNS SRV for feature naming
change option name to --remote-service-discovery
add --remote-service-discovery [service] optional parameter support
rewrite man, add algo/prio/weight description and examples
fix random weight selection was not implicit
remove pulled REMOTE_DISCOVERY support, not needed
split windows/unix-specific parts into extra functions
rename functions into servinfo scope, add doxygen comments when appropriate
remove addrinfo hack, use servinfo containers of addrinfo list instead
better proxy support (tcp mode not supported so far)
log discovery attempts and results, if enabled

v3: complete logic rewrite
use separate --remote-srv [service] [proto] option
remove fallback, same is achieved with additiona --remote/--remote-srv
add "auto" protocol support to allow both udp & tcp hosts be discovered
add support for tcp / http proxy (natively)
man update

v4: due RFC 2782 ambiguity, prefer to use all resolved DNS SRV records, even
ones with weight 0 after the records containing weights greater than 0
were all selected, keep related code disabled for historical reasons.
man update

v5: rebase against upstream with connection advancing fix
allow management skip/accept for exact remote service hosts as for --remote
improve 

Re: [Openvpn-devel] [PATCH applied] Re: Fix fatal error at switching remotes (#629)

2020-09-17 Thread Vladislav Grishenko
Hi, Gert

> > That "fix for real" is about persist_remote_ip option as far as I
> > understand, not directly related to this fatal assert fix.
> 
> Well, the whole preresolve / connection entry "complex" is old and has
been
> extended and updated a few times, and your SVR patch also builds on top of
> that. 

That's true, I hit this assert for SRV initially, 'coz same advancing logic
was used, v5 version is upcoming following this commit.

> At some point, refactoring is needed...
> (We have some other thing to consider which is even more intrusive - when
we
> reconnect to a different IP address, and that new IP address is currently
routed
> into the tunnel, we need to set up new /32 host routes before moving to a
new
> server can work... openvpn3, as I understand, sets up "all host routes!"
right at
> the start, but that might or might not be the best solution either)

New address is being handled in disconnected (yet) state, so tunnel routes
should not be active, since 2.x supports at most one tunnel active.
While this is preserved, /32 host route can be made anytime between resolved
state and connection attempt, sounds not so intrusive, if I got you right.

Meanwhile, "--persist-remote-ip" documented as "Preserve most recently
authenticated remote IP address and port number across SIGUSR1 and
--ping-restart".
Current implementation doesn't follow it precisely, instead it does
"Preserve most recently authenticated remote host name and port...", if that
remote name resolves into multiple addresses - they will be still iterated.
Guess, this is what was meant by "fix by real"

> 
> gert
> --
> "If was one thing all people took for granted, was conviction that if you
feed
> honest figures into a computer, honest figures come out. Never doubted  it
> myself till I met a computer with a sense of humor."
>  Robert A. Heinlein, The Moon is a Harsh
Mistress
> 
> Gert Doering - Munich, Germany
g...@greenie.muc.de



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


Re: [Openvpn-devel] [PATCH applied] Re: Fix fatal error at switching remotes (#629)

2020-09-17 Thread Lev Stipakov
Hi,

> openvpn3, as I understand, sets up "all host routes!" right at the start

It depends on how openvpn3 library is used.

OpenVPN3 Linux client adds bypass route for the specific remote
just before connection attempt. Same for our Connect Windows / Mac clients,
which are partially closed-source, but this specific logic is part of
opensource library and "agent" (windows agent is opensourced,
mac agent is still closed-source) - a component similar to "iservice".

IIRC, "test" client (cli) indeed adds all routes from the beginning.

-- 
-Lev


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


[Openvpn-devel] [PATCH v2] If IPv6 pool specification sets pool start to ::0 address, increment.

2020-09-17 Thread Gert Doering
The first IPv6 address in a subnet is not usable (IPv6 anycast address),
but our pool code ignored this.

Instead of assigning an unusable address or erroring out, just log the
fact, and increment the pool start to ::1

NOTE: this is a bit simplistic.  A pool that is larger than /96 and
has non-0 bits in the "uppermost bits" will still get the increment
as we only look at the lowermost 32 bits.

NOTE2: if the pool is specified with "--server-ipv6 $base/$bits", this
is a non-issue, as the address for the pool start will be incremented
anyway.

v2: make comment more explicit about "we're only talking about the
host part here" and "base sees only only 32 bit of the host part"

Reported-by: NicolaF_ in Trac
Trac: #1282

Signed-off-by: Gert Doering 
---
 doc/man-sections/server-options.rst |  3 ++-
 src/openvpn/pool.c  | 18 ++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/doc/man-sections/server-options.rst 
b/doc/man-sections/server-options.rst
index 2009953c..569a 100644
--- a/doc/man-sections/server-options.rst
+++ b/doc/man-sections/server-options.rst
@@ -204,7 +204,8 @@ fast hardware. SSL/TLS authentication must be used in this 
mode.
  ifconfig-ipv6-pool ipv6addr/bits
 
   The pool starts at ``ipv6addr`` and matches the offset determined from
-  the start of the IPv4 pool.
+  the start of the IPv4 pool.  If the host part of the given IPv6
+  address is ``0``, the pool starts at ``ipv6addr`` +1.
 
 --ifconfig-pool-persist args
   Persist/unpersist ifconfig-pool data to ``file``, at ``seconds``
diff --git a/src/openvpn/pool.c b/src/openvpn/pool.c
index 1f74ac57..2c5dd72d 100644
--- a/src/openvpn/pool.c
+++ b/src/openvpn/pool.c
@@ -224,6 +224,24 @@ ifconfig_pool_init(const bool ipv4_pool, enum pool_type 
type, in_addr_t start,
 }
 
 pool->ipv6.base = ipv6_base;
+
+/* if a pool starts at a base address that has all-zero in the
+ * host part, that first IPv6 address must not be assigned to
+ * clients because it is not usable (subnet anycast address).
+ * Start with 1, then.
+ *
+ * NOTE: this will also (mis-)fire for something like
+ *ifconfig-ipv6-pool 2001:db8:0:1:1234::0/64
+ * as we only check the rightmost 32 bits of the host part.  So be it.
+ */
+if (base == 0)
+{
+msg(D_IFCONFIG_POOL, "IFCONFIG POOL IPv6: incrementing pool start "
+   "to avoid ::0 assignment");
+base++;
+pool->ipv6.base.s6_addr[15]++;
+}
+
 pool_ipv6_size = ipv6_netbits >= 112
   ? (1 << (128 - ipv6_netbits)) - base
   : IFCONFIG_POOL_MAX;
-- 
2.26.2



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


Re: [Openvpn-devel] [PATCH applied] Re: Fix fatal error at switching remotes (#629)

2020-09-17 Thread Gert Doering
Hi,

On Thu, Sep 17, 2020 at 01:54:39PM +0500, Vladislav Grishenko wrote:
> Thank you a lot,

Not needed :-)

> That "fix for real" is about persist_remote_ip option as far as I
> understand, not directly related to this fatal assert fix.

Well, the whole preresolve / connection entry "complex" is old and
has been extended and updated a few times, and your SVR patch also
builds on top of that.  At some point, refactoring is needed...

(We have some other thing to consider which is even more intrusive - 
when we reconnect to a different IP address, and that new IP address
is currently routed into the tunnel, we need to set up new /32 host
routes before moving to a new server can work... openvpn3, as I
understand, sets up "all host routes!" right at the start, but that
might or might not be the best solution either)

gert
-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


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


Re: [Openvpn-devel] [PATCH applied] Re: Fix fatal error at switching remotes (#629)

2020-09-17 Thread Vladislav Grishenko
Thank you a lot,
That "fix for real" is about persist_remote_ip option as far as I
understand, not directly related to this fatal assert fix.

--
Best Regards, Vladislav Grishenko

> -Original Message-
> From: Gert Doering 
> Sent: Thursday, September 17, 2020 1:46 PM
> To: Vladislav Grishenko 
> Cc: openvpn-devel@lists.sourceforge.net
> Subject: [PATCH applied] Re: Fix fatal error at switching remotes (#629)
> 
> Your patch has been applied to the master, release/2.5 and release/2.4
branch
> (bugfix).
> 
> I have fixed a few "addinfo" occurances and re-wrapped the comment
slightly.
> Not checked the actual code, just ran a t_client test on
> 2.4 "to be sure".
> 
> As Arne wrote there is a "fix for real" dangling here... :-)
> 
> commit 3ad86c2534a92af137809b6d446d570193e6d01f (master) commit
> 6554025a422d3d7e5465bcbfad34fa3e196b53b0 (release/2.5) commit
> 7fdcd286a15fb4f64e979c4fdbf52223d4bdede0 (release/2.4)
> Author: Vladislav Grishenko
> Date:   Wed Sep 16 19:17:55 2020 +0500
> 
>  Fix fatal error at switching remotes (#629)
> 
>  Signed-off-by: Vladislav Grishenko 
>  Acked-by: Lev Stipakov 
>  Message-Id: <20200916141755.1923-1-themi...@yandex-team.ru>
>  URL: https://www.mail-archive.com/openvpn-
> de...@lists.sourceforge.net/msg21019.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


[Openvpn-devel] [PATCH applied] Re: Fix fatal error at switching remotes (#629)

2020-09-17 Thread Gert Doering
Your patch has been applied to the master, release/2.5 and release/2.4 branch
(bugfix).

I have fixed a few "addinfo" occurances and re-wrapped the comment 
slightly.  Not checked the actual code, just ran a t_client test on
2.4 "to be sure".

As Arne wrote there is a "fix for real" dangling here... :-)

commit 3ad86c2534a92af137809b6d446d570193e6d01f (master)
commit 6554025a422d3d7e5465bcbfad34fa3e196b53b0 (release/2.5)
commit 7fdcd286a15fb4f64e979c4fdbf52223d4bdede0 (release/2.4)
Author: Vladislav Grishenko
Date:   Wed Sep 16 19:17:55 2020 +0500

 Fix fatal error at switching remotes (#629)

 Signed-off-by: Vladislav Grishenko 
 Acked-by: Lev Stipakov 
 Message-Id: <20200916141755.1923-1-themi...@yandex-team.ru>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21019.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] If IPv6 pool specification sets pool start to ::0 address, increment.

2020-09-17 Thread Antonio Quartulli
Hi,

On 17/09/2020 09:01, Gert Doering wrote:
> We look at "base", which is only the host part, but "at most 32 bits of
> the host part".
> 
> (This is *your* code...!)

(self-shaming dance mode=ON)

Riiight, then drop this comment. The patch looks good, except for the
comment that needs more verbosity.


> 
> gert
> 

-- 
Antonio Quartulli


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


Re: [Openvpn-devel] [PATCH] If IPv6 pool specification sets pool start to ::0 address, increment.

2020-09-17 Thread Gert Doering
Hi,

On Thu, Sep 17, 2020 at 08:55:07AM +0200, Antonio Quartulli wrote:
> >  }
> >  
> >  pool->ipv6.base = ipv6_base;
> > +
> > +/* if a pool starts at ::0, that first IPv6 address is not usable
> 
> can we reword a bit this comment? I.e.: "if the starting address of a
> pool has the host part all zero, that first "
> 
> The "starts at ::0" confused me as if we were targeting pools starting
> at [::].

Okay.  I did not want to make this comment too long, but it seems more
verbosity is needed.  I'll send a v2

> > + * first clients (subnet anycast address).  Start with 1, then.
> > + * NOTE: this will also fire for something like
> > + *ifconfig-ipv6-pool 2001:db8:0:1:1234::0/64
> > + * as we only look at the rightmost 32 bits.  So be it...
> 
> wouldn't this test miserably fail when the host part is smaller than 32?
> like for a 2001:db8:0:1:1234::0/124?

We look at "base", which is only the host part, but "at most 32 bits of
the host part".

(This is *your* code...!)

gert
-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


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


Re: [Openvpn-devel] [PATCH] If IPv6 pool specification sets pool start to ::0 address, increment.

2020-09-17 Thread Antonio Quartulli
Hi,

On 11/09/2020 13:59, Gert Doering wrote:
> The first IPv6 address in a subnet is not usable (IPv6 anycast address),
> but our pool code ignored this.
> 
> Instead of assigning an unusable address or erroring out, just log the
> fact, and increment the pool start to ::1
> 
> NOTE: this is a bit simplistic.  A pool that is larger than /96 and
> has non-0 bits in the "uppermost bits" will still get the increment
> as we only look at the lowermost 32 bits.
> 
> NOTE2: if the pool is specified with "--server-ipv6 $base/$bits", this
> is a non-issue, as the address for the pool start will be incremented
> anyway.
> 
> Reported-by: NicolaF_ in Trac
> Trac: #1282
> 
> Signed-off-by: Gert Doering 
> ---
>  doc/man-sections/server-options.rst |  3 ++-
>  src/openvpn/pool.c  | 15 +++
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/man-sections/server-options.rst 
> b/doc/man-sections/server-options.rst
> index 2009953c..569a 100644
> --- a/doc/man-sections/server-options.rst
> +++ b/doc/man-sections/server-options.rst
> @@ -204,7 +204,8 @@ fast hardware. SSL/TLS authentication must be used in 
> this mode.
>   ifconfig-ipv6-pool ipv6addr/bits
>  
>The pool starts at ``ipv6addr`` and matches the offset determined from
> -  the start of the IPv4 pool.
> +  the start of the IPv4 pool.  If the host part of the given IPv6
> +  address is ``0``, the pool starts at ``ipv6addr`` +1.
>  
>  --ifconfig-pool-persist args
>Persist/unpersist ifconfig-pool data to ``file``, at ``seconds``
> diff --git a/src/openvpn/pool.c b/src/openvpn/pool.c
> index 1f74ac57..2814ff46 100644
> --- a/src/openvpn/pool.c
> +++ b/src/openvpn/pool.c
> @@ -224,6 +224,21 @@ ifconfig_pool_init(const bool ipv4_pool, enum pool_type 
> type, in_addr_t start,
>  }
>  
>  pool->ipv6.base = ipv6_base;
> +
> +/* if a pool starts at ::0, that first IPv6 address is not usable

can we reword a bit this comment? I.e.: "if the starting address of a
pool has the host part all zero, that first "

The "starts at ::0" confused me as if we were targeting pools starting
at [::].


> + * first clients (subnet anycast address).  Start with 1, then.
> + * NOTE: this will also fire for something like
> + *ifconfig-ipv6-pool 2001:db8:0:1:1234::0/64
> + * as we only look at the rightmost 32 bits.  So be it...

wouldn't this test miserably fail when the host part is smaller than 32?
like for a 2001:db8:0:1:1234::0/124?


Regards,

> + */
> +if (base == 0)
> +{
> +msg(D_IFCONFIG_POOL, "IFCONFIG POOL IPv6: incrementing pool 
> start "
> + "to avoid ::0 assignment");
> +base++;
> +pool->ipv6.base.s6_addr[15]++;
> +}
> +
>  pool_ipv6_size = ipv6_netbits >= 112
>? (1 << (128 - ipv6_netbits)) - base
>: IFCONFIG_POOL_MAX;
> 

-- 
Antonio Quartulli


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


Re: [Openvpn-devel] [PATCH v2] Fix fatal error at switching remotes (#629)

2020-09-17 Thread Lev Stipakov
Hi,

> If remote server has been resolved to multiple addresses, at
> least one connection attemt

typ0

> Fix this behaviour by cleaning stale addinfo objects.

Stared at code and it looks fine - same cleanup logic as in
do_close_link_socket().
Built and tested with MSVC - reproduced assert without patch and no
assert with patch.

Acked-by: Lev Stipakov 


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