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

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

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

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

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

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

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 ;) >

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

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

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

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.

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

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

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

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

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

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) > >

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

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

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

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

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

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), >

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

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: