Re: [Openvpn-devel] [PATCH] push: Provide a warning if --ifconfig-push have argument mismatch with --topology

[Gert Doering]

Hi,

On Fri, Dec 09, 2016 at 09:13:19AM +0100, Gert Doering wrote:
> ... ifconfig_sanity_check() does *nothing* for TOP_SUBNET 

Overlooked the second patch (since it wasn't threaded).  So with the other
patch, that argument is no longer valid, of course.  Apologies.

[..]
> Also we might to re-think the warning message printed - if called from
> an ifconfig-push context, the text "the second argument to --ifconfig must
> be an IP address" is less than clear ("my --ifconfig settings are just 
> fine, what is openvpn warning about?").

That one still holds...  technically it's more "for the other patch",
but since *this* patch is calling ifconfig_sanity_check() from a new
context, it should tackle the now-misleading warning text, I think.

(So maybe apply the other one first, after verifying endian correctness 
on sparc or mips/be architecture, and this one then removes the static,
adds a flag for "ifconfig or ifconfig-push context?" and adapts the
messages accordingly)

gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: PGP signature
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] push: Provide a warning if --ifconfig-push have argument mismatch with --topology

[Gert Doering]

Hi,

On Fri, Dec 09, 2016 at 03:50:48AM +0100, David Sommerseth wrote:
> This adds a warning to the log file if --topology is configured to use
> subnet or net30 and the 'subnet mask' argument of an --ifconfig-push option
> is not an subnet mask.
> 
> v2 - Make use of ifconfig_sanity_check() in tun.c instead of doing the exact
>  same check and warning in prepare_push_reply().  Also improve 
> documentation
>  of ifconfig_sanity_check() while at it.

Not being the one to complain about your code all the time... but...
since trac #755 is about "topology subnet"...

static void
ifconfig_sanity_check (bool tun, in_addr_t addr, int topology)
{
  struct gc_arena gc = gc_new ();
  const bool looks_like_netmask = ((addr & 0xFF00) == 0xFF00);
  if (tun)
{
  if (looks_like_netmask && (topology == TOP_NET30 || topology == TOP_P2P))
msg (M_WARN, "WARNING: Since you are using --dev tun with a point-to-poi
nt topology, the second argument to --ifconfig must be an IP address.  You are u
sing something (%s) that looks more like a netmask. %s",
 print_in_addr_t (addr, 0, ),
 ifconfig_warn_how_to_silence);
}
  else /* tap */

... ifconfig_sanity_check() does *nothing* for TOP_SUBNET (and the code
disagrees with your commit message for "net30").

So the v3 version of that patch would need to add the (missing today,
but actually important diagnostic aid - and the point of #755) clause

if (!looks_like_netmask && topology == TOP_SUBNET)
{
msg (M_WARN, "topology subnet needs a netmask as second argument bla 
bla");
}


Also we might to re-think the warning message printed - if called from
an ifconfig-push context, the text "the second argument to --ifconfig must
be an IP address" is less than clear ("my --ifconfig settings are just 
fine, what is openvpn warning about?").


Plus, it needs to be actually tested on a non-intel architecture - the
code has been like that forever, but I would not trust it to be endian-
safe (though our use of "in_addr_t" is not actually consistent with
what *should* be in one - parts of the code treat it as "network byte
order", other parts as "host byte order", and the point of using
"in_addr_t" as well defined *type* and not "a bag of 32bits, unsigned"
should have been "as defined by the socket API", which wants network
byte order).


But since this is an enhancement and not a bug, it should not be rushed -
it does not have to be in 2.4.0.  Let's do it properly, test it, then
see if we want to have it in 2.4.1  (and while at it, we should teach
check_addr_clash() that networks do have a netmask for a reason, and
"local/remote and --ifconfig are in the same /24" could be perfectly fine 
if the pushed netmask is *not* a /24).

gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: PGP signature
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] push: Provide a warning if --ifconfig-push have argument mismatch with --topology

[David Sommerseth]

This adds a warning to the log file if --topology is configured to use
subnet or net30 and the 'subnet mask' argument of an --ifconfig-push option
is not an subnet mask.

v2 - Make use of ifconfig_sanity_check() in tun.c instead of doing the exact
 same check and warning in prepare_push_reply().  Also improve documentation
 of ifconfig_sanity_check() while at it.

Trac: #755
Signed-off-by: David Sommerseth 
---
 src/openvpn/push.c |  8 
 src/openvpn/tun.c  | 20 ++--
 src/openvpn/tun.h  |  2 ++
 3 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 9953079..5292b06 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -333,6 +333,14 @@ prepare_push_reply (struct context *c, struct gc_arena *gc,
   print_in_addr_t (ifconfig_local, 0, gc),
   print_in_addr_t (c->c2.push_ifconfig_remote_netmask,
0, gc));
+
+  /* Warn if ifconfig_remote_netmask contains an unexpected value
+   * when checking configuration up against TUN/TAP device and
+   * network topology
+   */
+  ifconfig_sanity_check(c->c1.tuntap->type == DEV_TYPE_TUN,
+c->c2.push_ifconfig_remote_netmask,
+c->options.topology);
 }
 
   /* Send peer-id if client supports it */
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 572e168..8df3489 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -285,14 +285,22 @@ guess_tuntap_dev (const char *dev,
 /* --ifconfig-nowarn disables some options sanity checking */
 static const char ifconfig_warn_how_to_silence[] = "(silence this warning with 
--ifconfig-nowarn)";
 
-/*
- * If !tun, make sure ifconfig_remote_netmask looks
- *  like a netmask.
+/**
+ * If not a tun device, make sure ifconfig_remote_netmask looks
+ * like a netmask.
+ *
+ * If a tun device, make sure ifconfig_remote_netmask looks
+ * like an IPv4 address if topology is also TOP_NET30 or TOP_P2P.
+ *
+ * The result of this check is only reported to the log file as a warning
+ * when issues are found.
+ *
+ * @param tun   Boolean; if true device is a tun device, otherwise tap
+ * @param addr  Address to do sanity check on
+ * @param topology  Expected to be TOP_NET30, TOP_P2P, TOP_SUBNET
  *
- * If tun, make sure ifconfig_remote_netmask looks
- *  like an IPv4 address.
  */
-static void
+void
 ifconfig_sanity_check (bool tun, in_addr_t addr, int topology)
 {
   struct gc_arena gc = gc_new ();
diff --git a/src/openvpn/tun.h b/src/openvpn/tun.h
index 9b5a1b7..1a1f0b2 100644
--- a/src/openvpn/tun.h
+++ b/src/openvpn/tun.h
@@ -229,6 +229,8 @@ const char *guess_tuntap_dev (const char *dev,
  const char *dev_node,
  struct gc_arena *gc);
 
+void ifconfig_sanity_check (bool tun, in_addr_t addr, int topology);
+
 struct tuntap *init_tun (const char *dev,   /* --dev option */
 const char *dev_type,  /* --dev-type option */
 int topology,  /* one of the TOP_x values */
-- 
1.8.3.1


--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] push: Provide a warning if --ifconfig-push have argument mismatch with --topology

[Arne Schwabe]

Am 01.12.16 um 13:37 schrieb Gert Doering:
> Hi,
>
> On Thu, Dec 01, 2016 at 01:31:31PM +0100, Arne Schwabe wrote:
>> Am 30.11.16 um 23:41 schrieb David Sommerseth:
>>> This adds a warning to the log file if --topology is configured to use
>>> subnet or net30 and the 'subnet mask' argument of an --ifconfig-push option
>>> is not an subnet mask.  The check done is to ensure the first octet is 0xff 
>>> (255)
>> But way you actually want to test is
>>
>> if topology == subnet or net30:
>>if gateway not in net(ip, mask):
>>   print ("Your gw and ip and netmask disagree!)
>>
>> right? And isn't there code that gets executed for the client side that
>> can be disabled by ifconfig-nowarn. Can we reuse that code?
> That is certainly something we should test as well, but the particular
> case I had in mind was
>
>  - someone uses --topology p2p
>  - they have ccd/ files with "ifconfig-push 10.4.0.14 10.4.0.13"
>(for some clients that need to get a static address)
>  - they change their server.conf to --topology subnet
>  - everything works for "dynamic clients", but the static client now
>receives "10.4.0.13" and interprets it as a "netmask", which will
>cause the most interesting things, depending on platform code
>
> so the idea was to help this case by looking at only the "netmask" thing,
> and see if it makes sense (simplified sense: starts with 255.) - and if
> not, log this on the server side right when reading the ccd/ file so
> the admin in question can see "oh, I overlooked *that* special client".
>
> (Since that happened to a friend of mine based on my advice, I know that
> this is not a purely theoretical possibility :-) )
>

You are still very likely to catch this case If you check if in
net30/subnet the netmask is indead valid (e.g. CIDR)

Arne

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

Re: [Openvpn-devel] [PATCH] push: Provide a warning if --ifconfig-push have argument mismatch with --topology

[Steffan Karger]

On 01-12-16 13:38, Gert Doering wrote:
> On Thu, Dec 01, 2016 at 01:35:49PM +0100, Steffan Karger wrote:
>> On 1 December 2016 at 13:33, Gert Doering  wrote:
>>>((uchar *)>c2.push_ifconfig_remote_netmask)[0]
>>
>> Looks like dereferencing a type-punned pointer to me ;)
> 
> I was waiting for this :-)
> 
> (...but I thought "char *" is allowed by the rules?)

Heh, and I was worried that you had already though of this, so I must
have been wrong ;)

Indeed, "character types" (including unsigned char) are exempt of the
strict aliasing rules.  C99 6.5:

An object shall have its stored value accessed only by an lvalue
expression that has one of the following types:
— [...], or
— a character type.

Apologies for the noise.

-Steffan



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] push: Provide a warning if --ifconfig-push have argument mismatch with --topology

[Gert Doering]

Hi,

On Thu, Dec 01, 2016 at 01:35:49PM +0100, Steffan Karger wrote:
> On 1 December 2016 at 13:33, Gert Doering  wrote:
> >((uchar *)>c2.push_ifconfig_remote_netmask)[0]
> 
> Looks like dereferencing a type-punned pointer to me ;)

I was waiting for this :-)

(...but I thought "char *" is allowed by the rules?)

gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.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] push: Provide a warning if --ifconfig-push have argument mismatch with --topology

[Gert Doering]

Hi,

On Thu, Dec 01, 2016 at 01:31:31PM +0100, Arne Schwabe wrote:
> Am 30.11.16 um 23:41 schrieb David Sommerseth:
> > This adds a warning to the log file if --topology is configured to use
> > subnet or net30 and the 'subnet mask' argument of an --ifconfig-push option
> > is not an subnet mask.  The check done is to ensure the first octet is 0xff 
> > (255)
>
> But way you actually want to test is
> 
> if topology == subnet or net30:
>if gateway not in net(ip, mask):
>   print ("Your gw and ip and netmask disagree!)
> 
> right? And isn't there code that gets executed for the client side that
> can be disabled by ifconfig-nowarn. Can we reuse that code?

That is certainly something we should test as well, but the particular
case I had in mind was

 - someone uses --topology p2p
 - they have ccd/ files with "ifconfig-push 10.4.0.14 10.4.0.13"
   (for some clients that need to get a static address)
 - they change their server.conf to --topology subnet
 - everything works for "dynamic clients", but the static client now
   receives "10.4.0.13" and interprets it as a "netmask", which will
   cause the most interesting things, depending on platform code

so the idea was to help this case by looking at only the "netmask" thing,
and see if it makes sense (simplified sense: starts with 255.) - and if
not, log this on the server side right when reading the ccd/ file so
the admin in question can see "oh, I overlooked *that* special client".

(Since that happened to a friend of mine based on my advice, I know that
this is not a purely theoretical possibility :-) )

gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.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] push: Provide a warning if --ifconfig-push have argument mismatch with --topology

[Gert Doering]

Hi,

On Thu, Dec 01, 2016 at 01:23:52PM +0100, David Sommerseth wrote:
> > (What you can do is "peek at byte 0", which will always be the same
> > part of the netmask [network byte order!], and which might actually
> > make this easier to read .-) )
> 
> You mean like this?
> 
> in_addr_t nmask = htonl(c->c2.push_ifconfig_remote_netmask);
> if ( nmask[0] == 0xff )
>   {
>  ...
> 

No :-) - if you htonl() it, you need to work with full words and 
0xff00 etc. - but if you just peek at 

   ((uchar *)>c2.push_ifconfig_remote_netmask)[0]

you should get the first byte of the netmask, no matter what the 
host byte order is.  (I'm not absolutely sure it's [0], could be [3],
but in any case, since this is "network byte order", location *in 
memory* will not be host byte order dependent)

But right now I'm a bit too busy to properly think about this, and
would just postpone to 2.4.1 - it's really minor.

gert

-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.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] push: Provide a warning if --ifconfig-push have argument mismatch with --topology

[Arne Schwabe]

Am 30.11.16 um 23:41 schrieb David Sommerseth:
> This adds a warning to the log file if --topology is configured to use
> subnet or net30 and the 'subnet mask' argument of an --ifconfig-push option
> is not an subnet mask.  The check done is to ensure the first octet is 0xff 
> (255)
>
>
But way you actually want to test is

if topology == subnet or net30:
   if gateway not in net(ip, mask):
  print ("Your gw and ip and netmask disagree!)

right? And isn't there code that gets executed for the client side that
can be disabled by ifconfig-nowarn. Can we reuse that code?

Arne

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

Re: [Openvpn-devel] [PATCH] push: Provide a warning if --ifconfig-push have argument mismatch with --topology

[David Sommerseth]

On 01/12/16 09:01, Gert Doering wrote:
> Hi,
> 
> On Wed, Nov 30, 2016 at 11:41:27PM +0100, David Sommerseth wrote:
>> +  if ((c->options.topology == TOP_SUBNET || c->options.topology == 
>> TOP_NET30)
>> +  && (c->c2.push_ifconfig_remote_netmask & 0xff00) != 
>> 0xff00)
> 
> Are you sure of that?  I would assume that this is stored in network
> byte order (as it's an in_addr_t) so you can't directly compare it to
> a number but need to run through ntohl() first.
> 
> On *intel* it might end up being the same, but code dealing with IPv4
> addresses and ports always needs to be checked on both endiannesses.

Good point!  I didn't consider endianess too careful, I just looked at
what we do in print_in_addr_t(), realised that we always call it with 0
in the flags argument in same prepare_push_reply() function.

  ia.s_addr = (flags & IA_NET_ORDER) ? addr : htonl (addr);

And now (when I have slightly more brainpower) I see I misread the
logic, so 

> Some random code I just found in a BSD header file does it that way:
> 
> #define INADDR_ALLHOSTS_GROUP   (u_int32_t)0xe001   /* 224.0.0.1 */
> #define in_allhosts(x)  ((x).s_addr == htonl(INADDR_ALLHOSTS_GROUP))   
> 
> ... so it certainly needs the htonl()

... Yes!

> (What you can do is "peek at byte 0", which will always be the same
> part of the netmask [network byte order!], and which might actually
> make this easier to read .-) )

You mean like this?

in_addr_t nmask = htonl(c->c2.push_ifconfig_remote_netmask);
if ( nmask[0] == 0xff )
  {
 ...


-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc




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] push: Provide a warning if --ifconfig-push have argument mismatch with --topology

[Gert Doering]

Hi,

On Thu, Dec 01, 2016 at 05:15:11AM +0300, SviMik wrote:
> While I admit that it is *extremely* unlikely to have a network larger than 
> /8, such logic still looks a little clumsy. It does not cover all the valid 
> netmasks neither it detects all possible invalid ones.

This is true, but not really relevant.  Right now, it will just do funky
things, and there is no indication in the logs where to look.

Nobody uses non-contiguous netmasks these days (like "240.0.255.0"),
so everything *normal* starts with a string of 1-bits, and a valid IPv4
address never starts with , so checking for "255." at the start
of something that could be "a netmask or a remote IPv4 address" will
get it right in about all cases we care about.

If someone insists on doing a /7 on their tun interface, they better
know really well what they are doing.

gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.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] push: Provide a warning if --ifconfig-push have argument mismatch with --topology

[Gert Doering]

Hi,

On Wed, Nov 30, 2016 at 11:41:27PM +0100, David Sommerseth wrote:
> +  if ((c->options.topology == TOP_SUBNET || c->options.topology == 
> TOP_NET30)
> +  && (c->c2.push_ifconfig_remote_netmask & 0xff00) != 0xff00)

Are you sure of that?  I would assume that this is stored in network
byte order (as it's an in_addr_t) so you can't directly compare it to
a number but need to run through ntohl() first.

On *intel* it might end up being the same, but code dealing with IPv4
addresses and ports always needs to be checked on both endiannesses.

Some random code I just found in a BSD header file does it that way:

#define INADDR_ALLHOSTS_GROUP   (u_int32_t)0xe001   /* 224.0.0.1 */
#define in_allhosts(x)  ((x).s_addr == htonl(INADDR_ALLHOSTS_GROUP))   

... so it certainly needs the htonl()

(What you can do is "peek at byte 0", which will always be the same
part of the netmask [network byte order!], and which might actually
make this easier to read .-) )

gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.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] push: Provide a warning if --ifconfig-push have argument mismatch with --topology

[Selva Nair]

Hi,

Some nitpicking on the eleventh hour...

The comment does not agree with the code:


> +++ b/src/openvpn/push.c
> @@ -333,6 +333,16 @@ prepare_push_reply (struct context *c, struct
> gc_arena *gc,
>print_in_addr_t (ifconfig_local, 0, gc),
>print_in_addr_t (c->c2.push_ifconfig_remote_
> netmask,
> 0, gc));
> +
> +  /* Warn if ifconfig_remote_netmask does not contain
> +   * subnet mask in subnet topology */
>

If we go by the code this should be subnet and net30 topology.
By the way, style alert -- we want the ending */ on a line by itself, don't
we?


> +  if ((c->options.topology == TOP_SUBNET || c->options.topology ==
> TOP_NET30)
> +  && (c->c2.push_ifconfig_remote_netmask & 0xff00) !=
> 0xff00)
> +{
> +  msg(M_WARN, "WARNING: When --topology is subnet or net30, the
> second argument "
> +  "to --ifconfig-push MUST be a subnet mask.  This client
> will not be able "
> +  "to connect.");

+}
>  }
>

I don't know the code path well enough, so cant say anything technical...

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

Re: [Openvpn-devel] [PATCH] push: Provide a warning if --ifconfig-push have argument mismatch with --topology

[SviMik]

While I admit that it is *extremely* unlikely to have a network larger than /8, 
such logic still looks a little clumsy. It does not cover all the valid 
netmasks neither it detects all possible invalid ones.

If you wish to test if the netmask is valid, this solution could be better:
http://stackoverflow.com/questions/17401067/c-code-for-valid-netmask


> This adds a warning to the log file if --topology is configured to use
> subnet or net30 and the 'subnet mask' argument of an --ifconfig-push option
> is not an subnet mask.  The check done is to ensure the first octet is 0xff
> (255)
> 
> Trac: #755
> Signed-off-by: David Sommerseth 
> ---
>  src/openvpn/push.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/src/openvpn/push.c b/src/openvpn/push.c
> index 9953079..0819bf8 100644
> --- a/src/openvpn/push.c
> +++ b/src/openvpn/push.c
> @@ -333,6 +333,16 @@ prepare_push_reply (struct context *c, struct gc_arena
> *gc,
>  print_in_addr_t (ifconfig_local, 0, gc),
>  print_in_addr_t (c->c2.push_ifconfig_remote_netmask,
>   0, gc));
> +
> +  /* Warn if ifconfig_remote_netmask does not contain
> +   * subnet mask in subnet topology */
> +  if ((c->options.topology == TOP_SUBNET || c->options.topology ==
> TOP_NET30)
> +  && (c->c2.push_ifconfig_remote_netmask & 0xff00) != 0xff00)
> +{
> +  msg(M_WARN, "WARNING: When --topology is subnet or net30, the
> second argument "
> +  "to --ifconfig-push MUST be a subnet mask.  This client will
> not be able "
> +  "to connect.");
> +}
>  }
>  
>/* Send peer-id if client supports it */
> -- 
> 1.8.3.1
> 
> 
> --
> ___
> 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

[Openvpn-devel] [PATCH] push: Provide a warning if --ifconfig-push have argument mismatch with --topology

[David Sommerseth]

This adds a warning to the log file if --topology is configured to use
subnet or net30 and the 'subnet mask' argument of an --ifconfig-push option
is not an subnet mask.  The check done is to ensure the first octet is 0xff 
(255)

Trac: #755
Signed-off-by: David Sommerseth 
---
 src/openvpn/push.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index 9953079..0819bf8 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -333,6 +333,16 @@ prepare_push_reply (struct context *c, struct gc_arena *gc,
   print_in_addr_t (ifconfig_local, 0, gc),
   print_in_addr_t (c->c2.push_ifconfig_remote_netmask,
0, gc));
+
+  /* Warn if ifconfig_remote_netmask does not contain
+   * subnet mask in subnet topology */
+  if ((c->options.topology == TOP_SUBNET || c->options.topology == 
TOP_NET30)
+  && (c->c2.push_ifconfig_remote_netmask & 0xff00) != 0xff00)
+{
+  msg(M_WARN, "WARNING: When --topology is subnet or net30, the second 
argument "
+  "to --ifconfig-push MUST be a subnet mask.  This client will not 
be able "
+  "to connect.");
+}
 }
 
   /* Send peer-id if client supports it */
-- 
1.8.3.1


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