[Openvpn-devel] [PATCH v6] ipv6-pool: get rid of size constraint

2020-06-08 Thread Antonio Quartulli
Signed-off-by: Antonio Quartulli 

---
Changes from v5:
- restyle base addr computation to avoid odd line wrapping

Changes from v4:
- make the base computation depending on the size of the pool:
- large pools will still start at +0x1000 (backward compatible)
- smaller pools will start at +2

 src/openvpn/helper.c  | 13 +
 src/openvpn/options.c | 13 +
 src/openvpn/pool.c| 12 
 3 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/src/openvpn/helper.c b/src/openvpn/helper.c
index 277e6972..fbfc287f 100644
--- a/src/openvpn/helper.c
+++ b/src/openvpn/helper.c
@@ -198,12 +198,17 @@ helper_client_server(struct options *o)
 print_in6_addr( add_in6_addr( o->server_network_ipv6, 2), 0, 
>gc );
 o->ifconfig_ipv6_netbits = o->server_netbits_ipv6;
 
-/* pool starts at "base address + 0x1000" - leave enough room */
-ASSERT( o->server_netbits_ipv6 <= 112 );/* want 16 bits */
+/* basic sanity check */
+ASSERT(o->server_netbits_ipv6 >= 64 && o->server_netbits_ipv6 <= 124);
 
 o->ifconfig_ipv6_pool_defined = true;
-o->ifconfig_ipv6_pool_base =
-add_in6_addr( o->server_network_ipv6, 0x1000 );
+/* For large enough pools we keep the original behaviour of adding
+ * 0x1000 when computing the base.
+ *
+ * Smaller pools can't get that far, therefore we just increase by 2
+ */
+o->ifconfig_ipv6_pool_base = add_in6_addr(o->server_network_ipv6,
+  o->server_netbits_ipv6 < 112 
? 0x1000 : 2);
 o->ifconfig_ipv6_pool_netbits = o->server_netbits_ipv6;
 
 push_option( o, "tun-ipv6", M_USAGE );
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 018f6f18..9d3a8dfe 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -6718,9 +6718,12 @@ add_option(struct options *options,
 msg(msglevel, "error parsing --server-ipv6 parameter");
 goto err;
 }
-if (netbits < 64 || netbits > 112)
+if (netbits < 64 || netbits > 124)
 {
-msg( msglevel, "--server-ipv6 settings: only /64../112 supported 
right now (not /%d)", netbits );
+msg(msglevel,
+"--server-ipv6 settings: network must be between /64 and /124 
(not /%d)",
+netbits);
+
 goto err;
 }
 options->server_ipv6_defined = true;
@@ -6840,9 +6843,11 @@ add_option(struct options *options,
 msg(msglevel, "error parsing --ifconfig-ipv6-pool parameters");
 goto err;
 }
-if (netbits < 64 || netbits > 112)
+if (netbits < 64 || netbits > 124)
 {
-msg( msglevel, "--ifconfig-ipv6-pool settings: only /64../112 
supported right now (not /%d)", netbits );
+msg(msglevel,
+"--ifconfig-ipv6-pool settings: network must be between /64 
and /124 (not /%d)",
+netbits);
 goto err;
 }
 
diff --git a/src/openvpn/pool.c b/src/openvpn/pool.c
index 4e303712..29667623 100644
--- a/src/openvpn/pool.c
+++ b/src/openvpn/pool.c
@@ -207,6 +207,12 @@ ifconfig_pool_init(const bool ipv4_pool, enum pool_type 
type, in_addr_t start,
 ASSERT(0);
 }
 
+if (pool->ipv4.size < 2)
+{
+msg(M_FATAL, "IPv4 pool size is too small (%d), must be at least 
2",
+pool->ipv4.size);
+}
+
 msg(D_IFCONFIG_POOL, "IFCONFIG POOL IPv4: base=%s size=%d",
 print_in_addr_t(pool->ipv4.base, 0, ), pool->ipv4.size);
 }
@@ -245,6 +251,12 @@ ifconfig_pool_init(const bool ipv4_pool, enum pool_type 
type, in_addr_t start,
   ? (1 << (128 - ipv6_netbits)) - base
   : IFCONFIG_POOL_MAX;
 
+if (pool->ipv6.size < 2)
+{
+msg(M_FATAL, "IPv6 pool size is too small (%d), must be at least 
2",
+pool->ipv6.size);
+}
+
 msg(D_IFCONFIG_POOL, "IFCONFIG POOL IPv6: base=%s size=%d netbits=%d",
 print_in6_addr(pool->ipv6.base, 0, ), pool->ipv6.size,
 ipv6_netbits);
-- 
2.27.0



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


[Openvpn-devel] [PATCH applied] Re: options: enable IPv4 redirection logic only if really required

2020-06-08 Thread Gert Doering
Acked-by: Gert Doering 

Thanks :-)

Tested with "redirect-private", "redirect-gateway", "redirect-gateay !ipv4",
and it seems to do what we want - not fumble any hostroutes if !ipv4 is
set, but *do* fumble them if needed.

If you do "--redirect-private --redirect-gateway !ipv4 ipv6", it will
unset the effect of "--redirect-private" (which is expected).  If you
do it the other way round, it will actually do what you told it to -
redirect IPv6 default, and add a /32 host route for IPv4.  So even 
in this somewhat very special case, we're not losing functionality, 
but if it might need reordering of config options.

There is changed behaviour for anything that has "!ipv4" in it - if
you do "redirect-gateway !ipv4 block-local", before this patch, we'd
"route the local lan into the tunnel, but not the default gateway" -
now, we will just ignore the "block-local" bit.  Anything with IPv4,
actually, as RG_ENABLE gets dropped.  This was not intentional, but
it is actually making sense - you said "!ipv4", after all.


Your patch has been applied to the master branch.

commit 070319c13524125d8325a0df15fe795cc2a4bcf2
Author: Antonio Quartulli
Date:   Mon Jun 8 17:32:39 2020 +0200

 options: enable IPv4 redirection logic only if really required

 Signed-off-by: Antonio Quartulli 
 Acked-by: Gert Doering 
 Message-Id: <20200608153239.2260-...@unstable.cc>
 URL: 
https://www.mail-archive.com/search?l=mid=20200608153239.2260-...@unstable.cc
 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 v5] ipv6-pool: get rid of size constraint

2020-06-08 Thread Antonio Quartulli
Signed-off-by: Antonio Quartulli 

---
Changes from v4:
- make the base computation depending on the size of the pool:
- large pools will still start at +0x1000 (backward compatible)
- smaller pools will start at +2

 src/openvpn/helper.c  | 21 +
 src/openvpn/options.c | 13 +
 src/openvpn/pool.c| 12 
 3 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/src/openvpn/helper.c b/src/openvpn/helper.c
index 277e6972..e353d5b7 100644
--- a/src/openvpn/helper.c
+++ b/src/openvpn/helper.c
@@ -198,12 +198,25 @@ helper_client_server(struct options *o)
 print_in6_addr( add_in6_addr( o->server_network_ipv6, 2), 0, 
>gc );
 o->ifconfig_ipv6_netbits = o->server_netbits_ipv6;
 
-/* pool starts at "base address + 0x1000" - leave enough room */
-ASSERT( o->server_netbits_ipv6 <= 112 );/* want 16 bits */
+/* basic sanity check */
+ASSERT(o->server_netbits_ipv6 >= 64 && o->server_netbits_ipv6 <= 124);
 
 o->ifconfig_ipv6_pool_defined = true;
-o->ifconfig_ipv6_pool_base =
-add_in6_addr( o->server_network_ipv6, 0x1000 );
+/* For large enough pools we keep the original behaviour of adding
+ * 0x1000 when computing the base.
+ *
+ * Smaller pools can't get that far, therefore we just increase by 2
+ */
+if (o->server_netbits_ipv6 < 112)
+{
+o->ifconfig_ipv6_pool_base = add_in6_addr(o->server_network_ipv6,
+  0x1000);
+}
+else
+{
+o->ifconfig_ipv6_pool_base = add_in6_addr(o->server_network_ipv6,
+  2);
+}
 o->ifconfig_ipv6_pool_netbits = o->server_netbits_ipv6;
 
 push_option( o, "tun-ipv6", M_USAGE );
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 018f6f18..9d3a8dfe 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -6718,9 +6718,12 @@ add_option(struct options *options,
 msg(msglevel, "error parsing --server-ipv6 parameter");
 goto err;
 }
-if (netbits < 64 || netbits > 112)
+if (netbits < 64 || netbits > 124)
 {
-msg( msglevel, "--server-ipv6 settings: only /64../112 supported 
right now (not /%d)", netbits );
+msg(msglevel,
+"--server-ipv6 settings: network must be between /64 and /124 
(not /%d)",
+netbits);
+
 goto err;
 }
 options->server_ipv6_defined = true;
@@ -6840,9 +6843,11 @@ add_option(struct options *options,
 msg(msglevel, "error parsing --ifconfig-ipv6-pool parameters");
 goto err;
 }
-if (netbits < 64 || netbits > 112)
+if (netbits < 64 || netbits > 124)
 {
-msg( msglevel, "--ifconfig-ipv6-pool settings: only /64../112 
supported right now (not /%d)", netbits );
+msg(msglevel,
+"--ifconfig-ipv6-pool settings: network must be between /64 
and /124 (not /%d)",
+netbits);
 goto err;
 }
 
diff --git a/src/openvpn/pool.c b/src/openvpn/pool.c
index 4e303712..29667623 100644
--- a/src/openvpn/pool.c
+++ b/src/openvpn/pool.c
@@ -207,6 +207,12 @@ ifconfig_pool_init(const bool ipv4_pool, enum pool_type 
type, in_addr_t start,
 ASSERT(0);
 }
 
+if (pool->ipv4.size < 2)
+{
+msg(M_FATAL, "IPv4 pool size is too small (%d), must be at least 
2",
+pool->ipv4.size);
+}
+
 msg(D_IFCONFIG_POOL, "IFCONFIG POOL IPv4: base=%s size=%d",
 print_in_addr_t(pool->ipv4.base, 0, ), pool->ipv4.size);
 }
@@ -245,6 +251,12 @@ ifconfig_pool_init(const bool ipv4_pool, enum pool_type 
type, in_addr_t start,
   ? (1 << (128 - ipv6_netbits)) - base
   : IFCONFIG_POOL_MAX;
 
+if (pool->ipv6.size < 2)
+{
+msg(M_FATAL, "IPv6 pool size is too small (%d), must be at least 
2",
+pool->ipv6.size);
+}
+
 msg(D_IFCONFIG_POOL, "IFCONFIG POOL IPv6: base=%s size=%d netbits=%d",
 print_in6_addr(pool->ipv6.base, 0, ), pool->ipv6.size,
 ipv6_netbits);
-- 
2.27.0



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


Re: [Openvpn-devel] [PATCH v4 7/7] ipv6-pool: get rid of size constraint

2020-06-08 Thread Antonio Quartulli
Hi,

On 07/06/2020 12:59, Gert Doering wrote:
> My idea would be to make this conditional on the pool size - if the size
> is /64.../111, make it +0x1000 ("keep existing behaviour for large-enough
> pools"), if it's /112.../124, make it +2
> 
> Thoughts?

I think it makes sense, especially to preserve the current behaviour on
already deployed setups.

New patch incoming!


-- 
Antonio Quartulli


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


[Openvpn-devel] [PATCH v5] options: enable IPv4 redirection logic only if really required

2020-06-08 Thread Antonio Quartulli
From: Antonio Quartulli 

If no IPv4 redirection flag is set, do not enable the IPv4
redirection logic at all so that it won't bother adding any
useless IPv4 route.

Trac: #208
Signed-off-by: Antonio Quartulli 

---
Changes from v4:
- add warning about undefined behaviour when specifying
  redirect-gateway/private at the same time
- fix behaviour of redirect-private

Changes from v3:
- move error message modification to previous patch

Changes from v2:
- patchset rebased on top of pre-ipv6-only patchset
---
 src/openvpn/options.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 7556e7ee..018f6f18 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -6542,6 +6542,18 @@ add_option(struct options *options,
 int j;
 VERIFY_PERMISSION(OPT_P_ROUTE);
 rol_check_alloc(options);
+
+if (options->routes->flags & RG_ENABLE)
+{
+msg(M_WARN,
+"WARNING: You have specified redirect-gateway and "
+"redirect-private at the same time (or the same option "
+"multiple times). This is not well supported and may lead to "
+"unexpected results");
+}
+
+options->routes->flags |= RG_ENABLE;
+
 if (streq(p[0], "redirect-gateway"))
 {
 options->routes->flags |= RG_REROUTE_GW;
@@ -6579,7 +6591,7 @@ add_option(struct options *options,
 }
 else if (streq(p[j], "!ipv4"))
 {
-options->routes->flags &= ~RG_REROUTE_GW;
+options->routes->flags &= ~(RG_REROUTE_GW | RG_ENABLE);
 }
 else
 {
@@ -6591,7 +6603,6 @@ add_option(struct options *options,
 /* we need this here to handle pushed --redirect-gateway */
 remap_redirect_gateway_flags(options);
 #endif
-options->routes->flags |= RG_ENABLE;
 }
 else if (streq(p[0], "block-ipv6") && !p[1])
 {
-- 
2.27.0



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


Re: [Openvpn-devel] [PATCH v4 6/7] options: enable IPv4 redirection logic only if really required

2020-06-08 Thread Gert Doering
Hi,

On Sun, Jun 07, 2020 at 01:25:01PM +0200, Gert Doering wrote:
> Can we make this conditional in a way that does not break "redirect-private"?

A very simple patch would be

if (streq(p[0], "redirect-gateway"))
{
options->routes->flags |= RG_REROUTE_GW;
}
+   if (streq(p[0], "redirect-private"))
+   {
+   options->routes->flags |= RG_ENABLE;
+   }

and then take your patch as-is ("redirect-gateway with no options" would
set RG_REROUTE_GW, which, if not cleared by !ipv4, would set RG_ENABLE
later on, while "redirect-private" sets the RG_ENABLE itself).

Alternatively, set RG_ENABLE at the top (always), and clear it for "!ipv4"

else if (streq(p[j], "!ipv4"))
{
options->routes->flags &= ~(RG_REROUTE_GW|RG_ENABLE);
}

... this should do the same thing, with less code convolutions.

Configs that have *both* "redirect-private" and "redirect-gateway !ipv4 ipv6"
would still be broken.  But I'm not sure such a config is well-defined
in the first place.

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


[Openvpn-devel] [PATCH applied] Re: crypto_openssl: add include for openssl/conf.h

2020-06-08 Thread Gert Doering
Your patch has been applied to the master branch.

commit 25266ebba97d7f4169f902b8b0d3c38eaa4c43a4
Author: James Bottomley
Date:   Sun Jun 7 15:10:58 2020 -0700

 crypto_openssl: add include for openssl/conf.h

 Signed-off-by: James Bottomley 
 Acked-by: Arne Schwabe 
 Message-Id: <1591567858.4011.15.ca...@hansenpartnership.com>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19996.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 v6 2/3] crypto_openssl: add initialization to pick up local configuration

2020-06-08 Thread Илья Шипицин
пн, 8 июн. 2020 г. в 15:06, Arne Schwabe :

>
> >
> > Sorry about that.  Best guess is it's missing an include for
> > openssl/conf.h.  You don't need that today because pretty much every
> > other openssl header includes it, but that may not always have been so.
> >
> > Does the below patch fix it?  If it does, it should probably be folded
> > into the other patch.  It should be safe because openssl/conf.h has
> > existed for every version of openssl you support.
>
>
> I am also puzzeld by this. On my local machine the OpenSSL 1.0.2 buildis
> fine without this extra include.
>
> But I am ACKing this alone on the basis that including the conf.h header
> is the corect thing to do (the man page says it should be included)
>
> It fixes the travis build error
> (https://travis-ci.org/github/schwabe/openvpn/builds/695947378) but I do
> not really understand why OpenSSL behaves different there.
>

it might happen if includes are mixed from OS openssl and custom openssl.
I'll have a look (what is include path in travis).



>
> So
>
> Acked-By: Arne Schwabe 
>
> >
> > James
> >
> > ---8>8>8><8<8<8---
> > From: James Bottomley 
> > Subject: [PATCH] crypto_openssl: add include for openssl/conf.h
> >
> > Fix build failure on older versions of openssl.
> >
> > Signed-off-by: James Bottomley 
> > ---
> >  src/openvpn/crypto_openssl.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
> > index fd57edd2..94b6d85b 100644
> > --- a/src/openvpn/crypto_openssl.c
> > +++ b/src/openvpn/crypto_openssl.c
> > @@ -43,6 +43,7 @@
> >  #include "crypto_backend.h"
> >  #include "openssl_compat.h"
> >
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> >
>
>
> ___
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v6 2/3] crypto_openssl: add initialization to pick up local configuration

2020-06-08 Thread Arne Schwabe

> 
> Sorry about that.  Best guess is it's missing an include for
> openssl/conf.h.  You don't need that today because pretty much every
> other openssl header includes it, but that may not always have been so.
> 
> Does the below patch fix it?  If it does, it should probably be folded
> into the other patch.  It should be safe because openssl/conf.h has
> existed for every version of openssl you support.


I am also puzzeld by this. On my local machine the OpenSSL 1.0.2 buildis
fine without this extra include.

But I am ACKing this alone on the basis that including the conf.h header
is the corect thing to do (the man page says it should be included)

It fixes the travis build error
(https://travis-ci.org/github/schwabe/openvpn/builds/695947378) but I do
not really understand why OpenSSL behaves different there.

So

Acked-By: Arne Schwabe 

> 
> James
> 
> ---8>8>8><8<8<8---
> From: James Bottomley 
> Subject: [PATCH] crypto_openssl: add include for openssl/conf.h
> 
> Fix build failure on older versions of openssl.
> 
> Signed-off-by: James Bottomley 
> ---
>  src/openvpn/crypto_openssl.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
> index fd57edd2..94b6d85b 100644
> --- a/src/openvpn/crypto_openssl.c
> +++ b/src/openvpn/crypto_openssl.c
> @@ -43,6 +43,7 @@
>  #include "crypto_backend.h"
>  #include "openssl_compat.h"
>  
> +#include 
>  #include 
>  #include 
>  #include 
> 




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