Re: [Openvpn-devel] [PATCH 1/5] Refactor static/tls-auth key loading

2016-11-13 Thread Arne Schwabe
Am 08.11.16 um 21:18 schrieb Steffan Karger: > Remove duplicate code, in preparation for adding --tls-crypt, which > otherwise would have to duplicate this code again. > > This should be equivalent to the old code, except for two things: > * The log lines for static key initialization change

[Openvpn-devel] [PATCH applied] Re: Support --block-outside-dns on multiple tunnels

2016-11-13 Thread Gert Doering
ACK. The patch is basically the same as the previously-acked master patch (plus code cleanups). I have only reviewed this one, and not built a test installer and tested it - I'll go test the snapshot as soon as buildbot makes one for me, but given that the code changes are quite similar, I

[Openvpn-devel] [PATCH applied] Re: Fix builds on compilers without anonymous union support

2016-11-13 Thread Gert Doering
ACK, tested on FreeBSD 7.4 ("builds, works, happiness"), FreeBSD 10, and recent Linux ("were happy already", HAVE_ANONYMOUS... properly detected). Let's see if buildbot thinks otherwise for the remaining platforms... Your patch has been applied to the master branch. commit

Re: [Openvpn-devel] [PATCH] Add in_port_t check to configure.ac

2016-11-13 Thread Steffan Karger
On 13-11-16 16:55, Gert Doering wrote: > commit 8cac9b98d58b97 introduced using in_port_t which is not > available on (all?) mingw build environments. > > Add configure check, falling back to uint16_t. > > Signed-off-by: Gert Doering > --- > configure.ac | 6 ++ > 1

Re: [Openvpn-devel] [PATCH applied] Re: Don't deference type-punned pointers

2016-11-13 Thread Gert Doering
Hi, On Sun, Nov 13, 2016 at 02:50:43PM +0100, Gert Doering wrote: > ACK. Brilliant approach to "make the compiler happy" that is *increasing* > code readability, not making it worse. > > Reviewed ("STARE AT CODE!"), client tests on linux and freebsd, server > tests on linux. > > Your patch has

[Openvpn-devel] [PATCH] Add in_port_t check to configure.ac

2016-11-13 Thread Gert Doering
commit 8cac9b98d58b97 introduced using in_port_t which is not available on (all?) mingw build environments. Add configure check, falling back to uint16_t. Signed-off-by: Gert Doering --- configure.ac | 6 ++ 1 file changed, 6 insertions(+) diff --git a/configure.ac

[Openvpn-devel] [PATCH applied] Re: Add in_port_t check to configure.ac

2016-11-13 Thread Gert Doering
Patch has been applied to the master branch. commit dd6714ae0a47a9401898ccc0dd7079f58ba29901 Author: Gert Doering Date: Sun Nov 13 16:55:35 2016 +0100 Add in_port_t check to configure.ac Signed-off-by: Gert Doering Acked-by: Steffan Karger

Re: [Openvpn-devel] [PATCH 2/5] Add control channel encryption (--tls-crypt)

2016-11-13 Thread Arne Schwabe
> This boils down to the following on-the-wire packet format: > >-opcode- || -session_id- || -packet_id- || auth_tag || * payload * I am pretty that opcode is *not* authenticated looking the code. Which is probably not a problem but should not be documented as authenticated. (There is a

Re: [Openvpn-devel] [PATCH 3/5] Add missing includes in error.h

2016-11-13 Thread Arne Schwabe
Am 08.11.16 um 15:18 schrieb Steffan Karger: > error.h depends on these, but is apparently never used by files that do > not include them. When implementing the --tls-crypt unit tests, I ran > into this. > ACK --

[Openvpn-devel] [PATCH applied] Re: Support --block-outside-dns on multiple tunnels

2016-11-13 Thread Gert Doering
ACK. I won't claim I have deep insights in the WFP stuff (truly not), but the code looks reasonable otherwise - much of it is just re-shuffling and changing to a well-known GUID -, the explanation makes sense, and it works for me :-) - tested on Win7, with one or two active tunnels both using

[Openvpn-devel] [PATCH] Fix builds on compilers without anonymous union support

2016-11-13 Thread Steffan Karger
The "Don't dereference type-punned pointers" patch introduced an anonymous union, which older compilers do not support (or refuse to support when -std=c99 is defined). Add a configure check, and some wrapper defines to repair builds on those compilers. Signed-off-by: Steffan Karger

[Openvpn-devel] [PATCH v3] Don't deference type-punned pointers

2016-11-13 Thread Steffan Karger
Dereferencing type-punned pointers is undefined behaviour according to the C standard. We should either obey the standard, or ensure that all supported compilers deal with dereferencing type-punned pointers as we want them to. I think just obeying the standard is the easiest solution. See e.g.

[Openvpn-devel] [PATCH] Don't deference type-punned pointers

2016-11-13 Thread Steffan Karger
Dereferencing type-punned pointers is undefined behaviour according to the C standard. We should either obey the standard, or ensure that all supported compilers deal with dereferencing type-punned pointers as we want them to. I think just obeying the standard is the easiest solution. See e.g.

[Openvpn-devel] [PATCH applied] Re: Don't deference type-punned pointers

2016-11-13 Thread Gert Doering
ACK. Brilliant approach to "make the compiler happy" that is *increasing* code readability, not making it worse. Reviewed ("STARE AT CODE!"), client tests on linux and freebsd, server tests on linux. Your patch has been applied to the master branch. commit

[Openvpn-devel] [PATCH 4/5 v2] Move private file access checks to options_postprocess_filechecks()

2016-11-13 Thread Steffan Karger
From: Steffan Karger This removes the dependency of crypto.c on misc.c, which makes testing (stuff that needs) crypto.c functionality easier. In particular, this simplifies the --tls-crypt tests in one of the follow-up patches. Apart from that, testing file access really

[Openvpn-devel] [PATCH] Fix compilation on MinGW with -std=c99

2016-11-13 Thread Gert Doering
commit 9223336a88bc moved the CFLAGS="-std=c99" bit in configure.ac before the "socklen_t" test, which relies on #ifdef WIN32 to decide whether to include or - which is no longer defined then, and things explode in interesting ways. Change to _WIN32, which is the "always defined on all

[Openvpn-devel] [PATCH applied] Re: Fix compilation on MinGW with -std=c99

2016-11-13 Thread Gert Doering
Patch has been applied to the master branch. Hope buildbot likes me again :) commit 11eedcd0071e7185fc3011cda4703f5cc75fe979 Author: Gert Doering Date: Sun Nov 13 20:36:45 2016 +0100 Fix compilation on MinGW with -std=c99 Signed-off-by: Gert Doering

[Openvpn-devel] [PATCH v2] Replace WIN32 by _WIN32

2016-11-13 Thread Gert Doering
With c99, "WIN32" is no longer automatically defined when (cross-)building for Windows, and proper compilation relies on including , before checking the macro. "_WIN32" is the official define that is guaranteed to be defined by the compiler itself, no includes are needed. So, mechanically change

Re: [Openvpn-devel] [PATCH 2/5] Add control channel encryption (--tls-crypt)

2016-11-13 Thread Steffan Karger
Hi, Thanks for reviewing! Replies inline. On 13-11-16 17:41, Arne Schwabe wrote: > >> This boils down to the following on-the-wire packet format: >> >>-opcode- || -session_id- || -packet_id- || auth_tag || * payload * > > I am pretty that opcode is *not* authenticated looking the code.

Re: [Openvpn-devel] [PATCH] Fix compilation on MinGW with -std=c99

2016-11-13 Thread Steffan Karger
On 13-11-16 20:36, Gert Doering wrote: > commit 9223336a88bc moved the CFLAGS="-std=c99" bit in configure.ac > before the "socklen_t" test, which relies on #ifdef WIN32 to decide > whether to include or - which is no longer > defined then, and things explode in interesting ways. > > Change to