date:20160918

Re: [Openvpn-devel] [PATCH] Fix win32 building with C99 mode

2016-09-18 Thread Gert Doering

Hi,

On Sun, Sep 18, 2016 at 10:20:16AM -0400, Selva Nair wrote:
> Yeah, to me that only confirms my feeling that #ifdef WIN32 is not
> reliable. Much better to use _WIN32 which appears to be defined by all (?)
> C compilers that support windows and doesn't require any particular header
> to be included.
> http://nadeausoftware.com/articles/2012/01/c_c_tip_how_
> use_compiler_predefined_macros_detect_operating_system#
> WindowsCygwinnonPOSIXandMinGW
> 
> Anyway, right now the priority being un-break building on windows, your v2
> looks good. I'll test it and report back.

Thanks for testing and ACKing.  Merged.

I think I'll open a trac ticket for the next step, so it isn't lost - we 
intend to do a major round of formatting cleanup before 2.4, and changing
WIN32 to _WIN32 in the process would nicely fit :-)

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

[Openvpn-devel] [PATCH applied] Re: Fix win32 building with C99 mode

2016-09-18 Thread Gert Doering

Patch has been applied to the master branch.

commit 6cd7e08d89cc9c39d00989866fb4782d5e7041dc
Author: Gert Doering
Date:   Sun Sep 18 14:14:23 2016 +0200

 Fix win32 building with C99 mode

 Signed-off-by: Gert Doering 
 Acked-by: Selva Nair 
 Message-Id: <20160918121423.52139-1-g...@greenie.muc.de>
 URL: 
http://www.mail-archive.com/search?l=mid=20160918121423.52139-1-g...@greenie.muc.de
 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] Feature proposal: tls-crypt

2016-09-18 Thread Selva Nair

Hi,

On Sun, Sep 18, 2016 at 8:25 AM, Steffan Karger  wrote:

> Hi,
>
> On 27 July 2016 at 16:42, Steffan Karger 
> wrote:
> > Our customers, as well as community users, have asked for encryption of
> > control channel packets to hide their certificate (containing perhaps
> > the users' name or organisation), or to provide some basic form of
> > post-quantum security (see e.g. trac #633).
> >
> > We've been thinking about this for a while, and would like to implement
> > such a feature.  I've attached a proposal for an extension of tls-auth
> > to achieve this in OpenVPN.  Comments and/or questions are very welcome.
> >  I hope to be able to start implementing this soon.
>
> I just pushed an experimental branch with --tls-crypt support:
> https://github.com/syzzer/openvpn/tree/tls-crypt-preview
>
> Any comments and test results or very much welcome.
>

Not qualified to comment on the implementation details, but the feature
looks very useful to have. Arguably its too early to plan for a
post-quantum world, but encrypting control channel packets is nice..
.
Does this mean that --tls-crypt will imply --tls-auth with the same
key-file (or make the latter redudnant?). The man-page description in the
patch appears to imply so, but not very clear..

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

Re: [Openvpn-devel] [PATCHv2] Fix win32 building with C99 mode

2016-09-18 Thread Selva Nair

Hi,

On Sun, Sep 18, 2016 at 8:14 AM, Gert Doering  wrote:

> In -std=c99 mode, WIN32 is not defined to be "1" anymore, but just
> "#define WIN32" - so the "#if WIN32" breaks, needs to be "#ifdef WIN32"
>
> v2: also fix block_dns.c (include config.h + compat.h)  (Selva Nair)
>
> Signed-off-by: Gert Doering 
> ---
>  src/openvpn/block_dns.c | 11 +++
>  src/openvpn/misc.c  |  2 +-
>  src/openvpnserv/Makefile.am |  2 +-
>  3 files changed, 13 insertions(+), 2 deletions(-)
>

The patch looks good, fixes windows cross-compile and appears to work fine
(as per a quick run on Win 7). Thanks.

ACK from me.

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

Re: [Openvpn-devel] [PATCH] cppcheck finding: add "ASSERT( maxoutput > 0 || separator != NULL )" to prevent possible null pointer derefence

2016-09-18 Thread Jens Neuhalfen

> 
> Hello,
> 
> this defect was found by cppcheck, however cppcheck still complains.
> so, we did not make it happy yet.
> 
> I think, the best would be split this function into 2 separate functions
> (with either null argument) or leave it like that.

Unrelated to an actual fix, IMHO we should at least break up

out = alloc_buf_gc (maxoutput ? maxoutput :
((size * 2) + (size / (space_break_flags & 
FHE_SPACE_BREAK_MASK)) * (int) strlen (separator) + 2),
gc);

and create a variable with a semantic variable for the result of   "((size * 2) 
+ (size / (space_break_flags & FHE_SPACE_BREAK_MASK)) * (int) strlen 
(separator) + 2)”

Honestly the calculation looks very arcane and I would better understand it, if 
i knew what it means :-)

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

Re: [Openvpn-devel] [PATCH] cppcheck finding: add "ASSERT( maxoutput > 0 || separator != NULL )" to prevent possible null pointer derefence

2016-09-18 Thread Илья Шипицин

Hello,

this defect was found by cppcheck, however cppcheck still complains.
so, we did not make it happy yet.

I think, the best would be split this function into 2 separate functions
(with either null argument) or leave it like that.

2016-09-18 20:14 GMT+05:00 David Sommerseth <
open...@sf.lists.topphemmelig.net>:

> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
> On 18/09/16 17:19, Ilya Shipitsin wrote:
> > --- src/openvpn/buffer.c | 6 -- 1 file changed, 4
> > insertions(+), 2 deletions(-)
> >
> > diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c index
> > 52c6ab9..57bded9 100644 --- a/src/openvpn/buffer.c +++
> > b/src/openvpn/buffer.c @@ -438,10 +438,12 @@ format_hex_ex (const
> > uint8_t *data, int size, int maxoutput, unsigned int
> > space_break_flags, const char* separator, struct gc_arena *gc) { -
> > struct buffer out = alloc_buf_gc (maxoutput ? maxoutput : +  int i;
> > +  struct buffer out; +  ASSERT( maxoutput > 0 || separator != NULL
> > ); +  out = alloc_buf_gc (maxoutput ? maxoutput : ((size * 2) +
> > (size / (space_break_flags & FHE_SPACE_BREAK_MASK)) * (int) strlen
> > (separator) + 2), gc); -  int i; for (i = 0; i < size; ++i) { if
> > (separator && i && !(i % (space_break_flags &
> > FHE_SPACE_BREAK_MASK)))
> >
>
> This looks dangerous and somewhat wrong.  This can actually stop a
> server completely if the ASSERT() check fails.  I'm not sure we want
> to do that.
>
> The ASSERT() also expects maxoutput to be higher than 0, while the
> alloc_buf_gc() supports/allows maxoutput to be 0, which again confirms
> my bad feeling about this.  If maxoutput == 0, alloc_buf_gc() will
> actually allocate a reasonable buffer regardless.
>
> I do see that there might be an issue if separator == NULL
> (strlen(NULL) will segfault), so this needs to be checked far more
> carefully and compared against all the callers of format_hex_ex().
>
> The question which pops up in my head is: Is this patch purely
> targeted to silence a code analyser warning?  If so, this is most
> likely not the right fix for the OpenVPN code base.
>
>
> - --
> kind regards,
>
> David Sommerseth
> OpenVPN Technologies, Inc
>
> -BEGIN PGP SIGNATURE-
> Version: GnuPG v2.0.22 (GNU/Linux)
>
> iQIcBAEBAgAGBQJX3q9BAAoJEIbPlEyWcf3yGkwQAKKZbI2idz6a7h5ZkbQJCHMk
> JDNIpdR6XTFmG16jBrycYeew+9IRvfwpF4vh2/QPaRjDNLnKvPHP+9uEetqYmUQi
> rWzJyg3nzXSEUhFRSbdJcOK7zEWoiUUsSTiBKIYP6D5qD+QBQOZaQseCCzafrt5b
> IKrUesWMWPXAjQBYbUvcc801+JR5S/gSaSbk9TuO8Nemiry/GW/pSt0OVTggPdcm
> LiUyutQ8HXeXS9uAL32FLqFH3qq7QTHjxdVoKGGx9w/lmznqp+EaKCuANsWnt/PW
> ZTzVYM5YG5jI8XNM/wyCklu5n3yqJ37Noxd3ZRSD6ATTL/MExVgWINOOGA7f+Pj3
> P6g4ZSc//zaWUCRpUKg4O8h6jaO+0stDS5eKDpyUCHYRHRdMI4IGLuwzIzKmxWq9
> LpNGGY3Ikpf5IM+i2R04HctnNp+izEeW3vBCKnBBUHSyegRQGQ191Z5QC3SPk6VP
> HwxkHCZiOsie8WsP0Sl2dur1GjV9uyslVTkmt1GBVuoOGxpmZLJRJRayO70goZ17
> ToavpadDjt193g79j1/dpE7OCPcCitso/+egxVzDWPTtLj+7rS/gfdJBNdCHr7xK
> Pf8KKbpD7yEbEln0K7wzsjVG749qZe103KmrhZulvu3H5+SrbT5ALhroaYidhRpg
> lLcx4g4A+nZjM6kabBGy
> =QvxV
> -END PGP SIGNATURE-
>
--
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] cppcheck finding: add "ASSERT( maxoutput > 0 || separator != NULL )" to prevent possible null pointer derefence

2016-09-18 Thread Gert Doering

Hi,

On Sun, Sep 18, 2016 at 06:14:09PM +0300, David Sommerseth wrote:
> This looks dangerous and somewhat wrong.  This can actually stop a
> server completely if the ASSERT() check fails.  I'm not sure we want
> to do that.

This was discussed in a github PR, but I do not have the number right
now.

Basically, we *know* that we always call this like this, but if we should
ever call it differently, the alloc_buf_gc() call will lead to funnies
- thus, better ASSERT() in that case, so it's clear the calling contract
has been violated.

> I do see that there might be an issue if separator == NULL
> (strlen(NULL) will segfault), so this needs to be checked far more
> carefully and compared against all the callers of format_hex_ex().

See PR :)

(And yes, if things are discussed in a PR and patches get then sent
to the list months(!) later, it would be useful to actually mention the
PR in the commit message - "commit --amend" before sending)

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] cppcheck finding: add "ASSERT( maxoutput > 0 || separator != NULL )" to prevent possible null pointer derefence

2016-09-18 Thread David Sommerseth

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 18/09/16 17:19, Ilya Shipitsin wrote:
> --- src/openvpn/buffer.c | 6 -- 1 file changed, 4 
> insertions(+), 2 deletions(-)
> 
> diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c index 
> 52c6ab9..57bded9 100644 --- a/src/openvpn/buffer.c +++ 
> b/src/openvpn/buffer.c @@ -438,10 +438,12 @@ format_hex_ex (const 
> uint8_t *data, int size, int maxoutput, unsigned int 
> space_break_flags, const char* separator, struct gc_arena *gc) { - 
> struct buffer out = alloc_buf_gc (maxoutput ? maxoutput : +  int i;
> +  struct buffer out; +  ASSERT( maxoutput > 0 || separator != NULL
> ); +  out = alloc_buf_gc (maxoutput ? maxoutput : ((size * 2) +
> (size / (space_break_flags & FHE_SPACE_BREAK_MASK)) * (int) strlen
> (separator) + 2), gc); -  int i; for (i = 0; i < size; ++i) { if
> (separator && i && !(i % (space_break_flags & 
> FHE_SPACE_BREAK_MASK)))
> 

This looks dangerous and somewhat wrong.  This can actually stop a
server completely if the ASSERT() check fails.  I'm not sure we want
to do that.

The ASSERT() also expects maxoutput to be higher than 0, while the
alloc_buf_gc() supports/allows maxoutput to be 0, which again confirms
my bad feeling about this.  If maxoutput == 0, alloc_buf_gc() will
actually allocate a reasonable buffer regardless.

I do see that there might be an issue if separator == NULL
(strlen(NULL) will segfault), so this needs to be checked far more
carefully and compared against all the callers of format_hex_ex().

The question which pops up in my head is: Is this patch purely
targeted to silence a code analyser warning?  If so, this is most
likely not the right fix for the OpenVPN code base.


- -- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc

-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJX3q9BAAoJEIbPlEyWcf3yGkwQAKKZbI2idz6a7h5ZkbQJCHMk
JDNIpdR6XTFmG16jBrycYeew+9IRvfwpF4vh2/QPaRjDNLnKvPHP+9uEetqYmUQi
rWzJyg3nzXSEUhFRSbdJcOK7zEWoiUUsSTiBKIYP6D5qD+QBQOZaQseCCzafrt5b
IKrUesWMWPXAjQBYbUvcc801+JR5S/gSaSbk9TuO8Nemiry/GW/pSt0OVTggPdcm
LiUyutQ8HXeXS9uAL32FLqFH3qq7QTHjxdVoKGGx9w/lmznqp+EaKCuANsWnt/PW
ZTzVYM5YG5jI8XNM/wyCklu5n3yqJ37Noxd3ZRSD6ATTL/MExVgWINOOGA7f+Pj3
P6g4ZSc//zaWUCRpUKg4O8h6jaO+0stDS5eKDpyUCHYRHRdMI4IGLuwzIzKmxWq9
LpNGGY3Ikpf5IM+i2R04HctnNp+izEeW3vBCKnBBUHSyegRQGQ191Z5QC3SPk6VP
HwxkHCZiOsie8WsP0Sl2dur1GjV9uyslVTkmt1GBVuoOGxpmZLJRJRayO70goZ17
ToavpadDjt193g79j1/dpE7OCPcCitso/+egxVzDWPTtLj+7rS/gfdJBNdCHr7xK
Pf8KKbpD7yEbEln0K7wzsjVG749qZe103KmrhZulvu3H5+SrbT5ALhroaYidhRpg
lLcx4g4A+nZjM6kabBGy
=QvxV
-END PGP SIGNATURE-

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

Re: [Openvpn-devel] [PATCH] Fix win32 building with C99 mode

2016-09-18 Thread Selva Nair

On Sun, Sep 18, 2016 at 2:59 AM, Gert Doering  wrote:

> Ah.  Here we go... trying to redefine WIN32 at the end of syshead.h
> shows what is happening:
>
> In file included from /usr/share/mingw-w64/include/windef.h:8:0,
>  from /usr/share/mingw-w64/include/windows.h:69,
>  from /usr/share/mingw-w64/include/winsock2.h:23,
>  from ../../src/compat/compat.h:29,
>  from syshead.h:28,
>
> so compat.h is pulling it via
>
> #ifdef HAVE_WINSOCK2_H
> #include 
> #endif
>
> whee...
>

Yeah, to me that only confirms my feeling that #ifdef WIN32 is not
reliable. Much better to use _WIN32 which appears to be defined by all (?)
C compilers that support windows and doesn't require any particular header
to be included.
http://nadeausoftware.com/articles/2012/01/c_c_tip_how_
use_compiler_predefined_macros_detect_operating_system#
WindowsCygwinnonPOSIXandMinGW

Anyway, right now the priority being un-break building on windows, your v2
looks good. I'll test it and report back.

Thanks,

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

[Openvpn-devel] [PATCH] cppcheck finding: add "ASSERT( maxoutput > 0 || separator != NULL )" to prevent possible null pointer derefence

2016-09-18 Thread Ilya Shipitsin

---
 src/openvpn/buffer.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
index 52c6ab9..57bded9 100644
--- a/src/openvpn/buffer.c
+++ b/src/openvpn/buffer.c
@@ -438,10 +438,12 @@ format_hex_ex (const uint8_t *data, int size, int 
maxoutput,
   unsigned int space_break_flags, const char* separator,
   struct gc_arena *gc)
 {
-  struct buffer out = alloc_buf_gc (maxoutput ? maxoutput :
+  int i;
+  struct buffer out;
+  ASSERT( maxoutput > 0 || separator != NULL );
+  out = alloc_buf_gc (maxoutput ? maxoutput :
((size * 2) + (size / (space_break_flags & 
FHE_SPACE_BREAK_MASK)) * (int) strlen (separator) + 2),
gc);
-  int i;
   for (i = 0; i < size; ++i)
 {
   if (separator && i && !(i % (space_break_flags & FHE_SPACE_BREAK_MASK)))
-- 
2.5.5


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

Re: [Openvpn-devel] Feature proposal: tls-crypt

2016-09-18 Thread Steffan Karger

Hi,

On 27 July 2016 at 16:42, Steffan Karger  wrote:
> Our customers, as well as community users, have asked for encryption of
> control channel packets to hide their certificate (containing perhaps
> the users' name or organisation), or to provide some basic form of
> post-quantum security (see e.g. trac #633).
>
> We've been thinking about this for a while, and would like to implement
> such a feature.  I've attached a proposal for an extension of tls-auth
> to achieve this in OpenVPN.  Comments and/or questions are very welcome.
>  I hope to be able to start implementing this soon.

I just pushed an experimental branch with --tls-crypt support:
https://github.com/syzzer/openvpn/tree/tls-crypt-preview

Any comments and test results or very much welcome.

This code has not yet been reviewed, or tested by anyone else but me,
so do not use in production yet.

-Steffan

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

[Openvpn-devel] [PATCHv2] Fix win32 building with C99 mode

2016-09-18 Thread Gert Doering

In -std=c99 mode, WIN32 is not defined to be "1" anymore, but just
"#define WIN32" - so the "#if WIN32" breaks, needs to be "#ifdef WIN32"

v2: also fix block_dns.c (include config.h + compat.h)  (Selva Nair)

Signed-off-by: Gert Doering 
---
 src/openvpn/block_dns.c | 11 +++
 src/openvpn/misc.c  |  2 +-
 src/openvpnserv/Makefile.am |  2 +-
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/openvpn/block_dns.c b/src/openvpn/block_dns.c
index af2db18..ee20c68 100644
--- a/src/openvpn/block_dns.c
+++ b/src/openvpn/block_dns.c
@@ -24,6 +24,17 @@
  *  59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#elif defined(_MSC_VER)
+#include "config-msvc.h"
+#endif
+#ifdef HAVE_CONFIG_VERSION_H
+#include "config-version.h"
+#endif
+
+#include "syshead.h"
+
 #ifdef WIN32
 
 #include 
diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
index 0991d79..2982cd0 100644
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -977,7 +977,7 @@ hostname_randomize(const char *hostname, struct gc_arena 
*gc)
 const char *
 gen_path (const char *directory, const char *filename, struct gc_arena *gc)
 {
-#if WIN32
+#ifdef WIN32
   const int CC_PATH_RESERVED = CC_LESS_THAN|CC_GREATER_THAN|CC_COLON|
 CC_DOUBLE_QUOTE|CC_SLASH|CC_BACKSLASH|CC_PIPE|CC_QUESTION_MARK|CC_ASTERISK;
 #else
diff --git a/src/openvpnserv/Makefile.am b/src/openvpnserv/Makefile.am
index 3c757d6..3521a34 100644
--- a/src/openvpnserv/Makefile.am
+++ b/src/openvpnserv/Makefile.am
@@ -18,7 +18,7 @@ EXTRA_DIST = \
openvpnserv.vcxproj.filters
 
 AM_CPPFLAGS = \
-   -I$(top_srcdir)/include -I$(top_srcdir)/src/openvpn
+   -I$(top_srcdir)/include -I$(top_srcdir)/src/openvpn 
-I$(top_srcdir)/src/compat
 
 if WIN32
 sbin_PROGRAMS = openvpnserv
-- 
1.9.1


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

Re: [Openvpn-devel] [PATCH applied] Incorporate the Debian typo fixes where appropriate and make show_opt default message clearer

2016-09-18 Thread Gert Doering

ACK.

Your patch has been applied to the following branches

commit c42fcbfe708f4c97da063642cf8874f0d4d1a645  (master)
commit 1f12d8dd8a4d4efbbbf9ba9b57ee1250593268ba  (release/2.3)
Author: Arne Schwabe
Date:   Thu Jul 14 13:25:19 2016 +0200

 Incorporate the Debian typo fixes where appropriate and make show_opt 
default message clearer

 Acked-by: Gert Doering 
 Message-Id: <1468495519-25102-1-git-send-email-a...@rfc2549.org>
 URL: 
http://www.mail-archive.com/search?l=mid=1468495519-25102-1-git-send-email-a...@rfc2549.org
 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] Fix win32 building with C99 mode

2016-09-18 Thread Илья Шипицин

we already run openvpn/openvpn-build in travis.
build only is not complicated.

there are minor issues with 14.04 mingw gcc, which is too old for openvpn
master. I'll make patch after I will resolve it.

2016-09-17 18:58 GMT+05:00 Gert Doering :

> Hi,
> On Sat, Sep 17, 2016 at 06:40:00PM +0500,  ?? wrote:
> > Should we add mingw compiler to travis-ci matrix?
>
> If travis can do mingw builds, that would be good.
>
> But it is complicated.
>
> 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
>
--
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH applied] Re: Support for disabled peer-id

2016-09-18 Thread Gert Doering

Your patch has been applied to the master branch.

commit e8e1377d0fc3f11b65e415b68e6065d2e9eb836e
Author: Lev Stipakov
Date:   Sun Sep 18 09:51:36 2016 +0300

 Support for disabled peer-id

 Acked-by: Steffan Karger 
 Message-Id: <1474181496-24846-1-git-send-email-lstipa...@gmail.com>
 URL: 
http://www.mail-archive.com/search?l=mid=1474181496-24846-1-git-send-email-lstipa...@gmail.com
 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 v5] Support for disabled peer-id

2016-09-18 Thread Steffan Karger

Hi,

On 18 September 2016 at 08:51, Lev Stipakov  wrote:
> From: Lev Stipakov 
>
> v5:
> * Few more nickpicks
>
> v4:
> * replace magic number with define
> * show user a decimal value instead of hex
>
> v3:
> * move assert outside of loop
> * add max-clients value check to options
>
> v2:
> * Add round brackets for clarity
> * Rephrase comment
>
> Support for disabled peer-id
>
> When peer-id value is 0xFF, server should ignore it and treat packet
> in a same way as P_DATA_V1.
> ---
>  src/openvpn/mudp.c| 13 ++---
>  src/openvpn/multi.c   |  3 ++-
>  src/openvpn/openvpn.h |  3 +++
>  src/openvpn/options.c |  5 +
>  4 files changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
> index 21a7e97..fec5e8d 100644
> --- a/src/openvpn/mudp.c
> +++ b/src/openvpn/mudp.c
> @@ -64,12 +64,16 @@ multi_get_create_instance_udp (struct multi_context *m, 
> bool *floated)
>struct hash_bucket *bucket = hash_bucket (hash, hv);
>uint8_t* ptr = BPTR(>top.c2.buf);
>uint8_t op = ptr[0] >> P_OPCODE_SHIFT;
> +  bool v2 = (op == P_DATA_V2) && (m->top.c2.buf.len >= (1 + 3));
> +  bool peer_id_disabled = false;
>
>/* make sure buffer has enough length to read opcode (1 byte) and 
> peer-id (3 bytes) */
> -  if (op == P_DATA_V2 && m->top.c2.buf.len >= (1 + 3))
> +  if (v2)
> {
>   uint32_t peer_id = ntohl(*(uint32_t*)ptr) & 0xFF;
> - if ((peer_id < m->max_clients) && (m->instances[peer_id]))
> + peer_id_disabled = (peer_id == MAX_PEER_ID);
> +
> + if (!peer_id_disabled && (peer_id < m->max_clients) && 
> (m->instances[peer_id]))
> {
>   mi = m->instances[peer_id];
>
> @@ -84,7 +88,7 @@ multi_get_create_instance_udp (struct multi_context *m, 
> bool *floated)
>   }
> }
> }
> -  else
> +  if (!v2 || peer_id_disabled)
> {
>   he = hash_lookup_fast (hash, bucket, , hv);
>   if (he)
> @@ -107,6 +111,9 @@ multi_get_create_instance_udp (struct multi_context *m, 
> bool *floated)
>   hash_add_fast (hash, bucket, >real, hv, mi);
>   mi->did_real_hash = true;
>
> + /* max_clients must be less then max peer-id value */
> + ASSERT(m->max_clients < MAX_PEER_ID);
> +
>   for (i = 0; i < m->max_clients; ++i)
> {
>   if (!m->instances[i])
> diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
> index ba7f2c0..3bc6ee9 100644
> --- a/src/openvpn/multi.c
> +++ b/src/openvpn/multi.c
> @@ -605,7 +605,8 @@ multi_close_instance (struct multi_context *m,
> }
>  #endif
>
> -  m->instances[mi->context.c2.tls_multi->peer_id] = NULL;
> +  if (mi->context.c2.tls_multi->peer_id != MAX_PEER_ID)
> +   m->instances[mi->context.c2.tls_multi->peer_id] = NULL;
>
>schedule_remove_entry (m->schedule, (struct schedule_entry *) mi);
>
> diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h
> index 1a458f1..65a183a 100644
> --- a/src/openvpn/openvpn.h
> +++ b/src/openvpn/openvpn.h
> @@ -595,4 +595,7 @@ struct context
>  #define CIPHER_ENABLED(c) (false)
>  #endif
>
> +/* this represents "disabled peer-id" */
> +#define MAX_PEER_ID 0xFF
> +
>  #endif
> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index c9688c3..4b7203d 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -5893,6 +5893,11 @@ add_option (struct options *options,
>   msg (msglevel, "--max-clients must be at least 1");
>   goto err;
> }
> +  if (max_clients >= MAX_PEER_ID) /* max peer-id value */
> +   {
> + msg (msglevel, "--max-clients must be less than %d", MAX_PEER_ID);
> + goto err;
> +   }
>options->max_clients = max_clients;
>  }
>else if (streq (p[0], "max-routes-per-client") && p[1] && !p[2])
> --
> 1.9.1

Thanks.  No more nitpicking from me ;)  ACK.

-Steffan

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

Re: [Openvpn-devel] [PATCH] Fix win32 building with C99 mode

2016-09-18 Thread Gert Doering

Hi,

On Sat, Sep 17, 2016 at 07:11:56PM -0400, Selva Nair wrote:
> On Sat, Sep 17, 2016 at 9:20 AM, Gert Doering  wrote:
> 
> > In -std=c99 mode, WIN32 is not defined to be "1" anymore, but just
> > "#define WIN32" - so the "#if WIN32" breaks, needs to be "#ifdef WIN32"
> 
> Indeed...
> 
> To depend on the compiler or system headers to define WIN32 was not a great
> idea anyway. Using  _WIN32 or __WIN32 is probably more reliable. Or we
> should define WIN32 in config.h. With std=c99, it seems WIN32 gets defined
> by chance through some header pulled-in by syshead.h -- not something one
> can rely on.

Heiko says that "WIN32 is what everybody is testing for on Windows", so
that seems to be an official API define...

With c99, on mingw, the #define WIN32 comes from "minwindef.h", which
is included from "windef.h", which is included from "windows.h" -- but
I wonder how we ever include that in the first place, given this
code in syshead.h

#ifdef WIN32
#include 
#include 
#define sleep(x) Sleep((x)*1000)
#define random rand
#define srandom srand
#endif


... so maybe it is actually *never* set, even with my patch, and we just
do not know it's breaking because it's not linking...?


Ah.  Here we go... trying to redefine WIN32 at the end of syshead.h
shows what is happening:

In file included from /usr/share/mingw-w64/include/windef.h:8:0,
 from /usr/share/mingw-w64/include/windows.h:69,
 from /usr/share/mingw-w64/include/winsock2.h:23,
 from ../../src/compat/compat.h:29,
 from syshead.h:28,

so compat.h is pulling it via

#ifdef HAVE_WINSOCK2_H
#include 
#endif

whee...


> Compiler-defined macros with std=c99:
> $ x86_64-w64-mingw32-gcc -std=c99 -E -dM -  #define _WIN32 1
> #define __WIN32 1
> #define __WIN32__ 1
> 
> As for a quick fix, the patch takes care of misc.c, but  there is an issue
> with block_dns.c as well. It does not include the required config.h and
> syshead.h at the top before #ifdef WIN32, so now the whole file gets
> skipped -- mea culpa, mea culpa...
> 
> I think we can just remove #ifdef WIN32 from block_dns.c (its compiled only
> on Windows), and add config.h and syshead.h headers at the top. Then we
> also need -I$(top_srcdir)/src/compat in src/openvpnsev/Makefile.am as
> compat.h is included by syshead.h.

Do you have time to cook up & test such a patch?  Then I'll go and focus
a bit more on my windows testing environment in the meantime, and merge
the patch later today...

thanks,

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

[Openvpn-devel] [PATCH v5] Support for disabled peer-id

2016-09-18 Thread Lev Stipakov

From: Lev Stipakov 

v5:
* Few more nickpicks

v4:
* replace magic number with define
* show user a decimal value instead of hex

v3:
* move assert outside of loop
* add max-clients value check to options

v2:
* Add round brackets for clarity
* Rephrase comment

Support for disabled peer-id

When peer-id value is 0xFF, server should ignore it and treat packet
in a same way as P_DATA_V1.
---
 src/openvpn/mudp.c| 13 ++---
 src/openvpn/multi.c   |  3 ++-
 src/openvpn/openvpn.h |  3 +++
 src/openvpn/options.c |  5 +
 4 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index 21a7e97..fec5e8d 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -64,12 +64,16 @@ multi_get_create_instance_udp (struct multi_context *m, 
bool *floated)
   struct hash_bucket *bucket = hash_bucket (hash, hv);
   uint8_t* ptr = BPTR(>top.c2.buf);
   uint8_t op = ptr[0] >> P_OPCODE_SHIFT;
+  bool v2 = (op == P_DATA_V2) && (m->top.c2.buf.len >= (1 + 3));
+  bool peer_id_disabled = false;
 
   /* make sure buffer has enough length to read opcode (1 byte) and 
peer-id (3 bytes) */
-  if (op == P_DATA_V2 && m->top.c2.buf.len >= (1 + 3))
+  if (v2)
{
  uint32_t peer_id = ntohl(*(uint32_t*)ptr) & 0xFF;
- if ((peer_id < m->max_clients) && (m->instances[peer_id]))
+ peer_id_disabled = (peer_id == MAX_PEER_ID);
+
+ if (!peer_id_disabled && (peer_id < m->max_clients) && 
(m->instances[peer_id]))
{
  mi = m->instances[peer_id];
 
@@ -84,7 +88,7 @@ multi_get_create_instance_udp (struct multi_context *m, bool 
*floated)
  }
}
}
-  else
+  if (!v2 || peer_id_disabled)
{
  he = hash_lookup_fast (hash, bucket, , hv);
  if (he)
@@ -107,6 +111,9 @@ multi_get_create_instance_udp (struct multi_context *m, 
bool *floated)
  hash_add_fast (hash, bucket, >real, hv, mi);
  mi->did_real_hash = true;
 
+ /* max_clients must be less then max peer-id value */
+ ASSERT(m->max_clients < MAX_PEER_ID);
+
  for (i = 0; i < m->max_clients; ++i)
{
  if (!m->instances[i])
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index ba7f2c0..3bc6ee9 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -605,7 +605,8 @@ multi_close_instance (struct multi_context *m,
}
 #endif
 
-  m->instances[mi->context.c2.tls_multi->peer_id] = NULL;
+  if (mi->context.c2.tls_multi->peer_id != MAX_PEER_ID)
+   m->instances[mi->context.c2.tls_multi->peer_id] = NULL;
 
   schedule_remove_entry (m->schedule, (struct schedule_entry *) mi);
 
diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h
index 1a458f1..65a183a 100644
--- a/src/openvpn/openvpn.h
+++ b/src/openvpn/openvpn.h
@@ -595,4 +595,7 @@ struct context
 #define CIPHER_ENABLED(c) (false)
 #endif
 
+/* this represents "disabled peer-id" */
+#define MAX_PEER_ID 0xFF
+
 #endif
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index c9688c3..4b7203d 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -5893,6 +5893,11 @@ add_option (struct options *options,
  msg (msglevel, "--max-clients must be at least 1");
  goto err;
}
+  if (max_clients >= MAX_PEER_ID) /* max peer-id value */
+   {
+ msg (msglevel, "--max-clients must be less than %d", MAX_PEER_ID);
+ goto err;
+   }
   options->max_clients = max_clients;
 }
   else if (streq (p[0], "max-routes-per-client") && p[1] && !p[2])
-- 
1.9.1


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

Re: [Openvpn-devel] [PATCH] Fix win32 building with C99 mode

[Openvpn-devel] [PATCH applied] Re: Fix win32 building with C99 mode

Re: [Openvpn-devel] Feature proposal: tls-crypt

Re: [Openvpn-devel] [PATCHv2] Fix win32 building with C99 mode

Re: [Openvpn-devel] [PATCH] cppcheck finding: add "ASSERT( maxoutput > 0 || separator != NULL )" to prevent possible null pointer derefence

Re: [Openvpn-devel] [PATCH] cppcheck finding: add "ASSERT( maxoutput > 0 || separator != NULL )" to prevent possible null pointer derefence

Re: [Openvpn-devel] [PATCH] cppcheck finding: add "ASSERT( maxoutput > 0 || separator != NULL )" to prevent possible null pointer derefence

Re: [Openvpn-devel] [PATCH] cppcheck finding: add "ASSERT( maxoutput > 0 || separator != NULL )" to prevent possible null pointer derefence

Re: [Openvpn-devel] [PATCH] Fix win32 building with C99 mode

[Openvpn-devel] [PATCH] cppcheck finding: add "ASSERT( maxoutput > 0 || separator != NULL )" to prevent possible null pointer derefence

Re: [Openvpn-devel] Feature proposal: tls-crypt

[Openvpn-devel] [PATCHv2] Fix win32 building with C99 mode

Re: [Openvpn-devel] [PATCH applied] Incorporate the Debian typo fixes where appropriate and make show_opt default message clearer

Re: [Openvpn-devel] [PATCH] Fix win32 building with C99 mode

[Openvpn-devel] [PATCH applied] Re: Support for disabled peer-id

Re: [Openvpn-devel] [PATCH v5] Support for disabled peer-id

Re: [Openvpn-devel] [PATCH] Fix win32 building with C99 mode

[Openvpn-devel] [PATCH v5] Support for disabled peer-id

18 matches

Site Navigation

Mail list logo

Footer information