Re: [Openvpn-devel] [PATCH 1/7] put argv_* functions into own file, add unit tests

Hi, On 08-11-16 00:28, David Sommerseth wrote: > I will wait with applying this patch to the git tree until all the other > patches in this series have been reviewed and ACKed. Actually, this patch does not need the follow-up patches, right? In that case, I would strongly prefer if we didn't wai

[Openvpn-devel] [PATCH] check c->c2.link_socket before calling do_init_route_ipv6_list()

There was an asymmetry in checks before calling do_init_route*_list(), checking c2.link_socket for IPv4 but not for IPv6 - mainly an oversight from the time when do_init_route_ipv6_list() did not yet look at the remote address to determine v6-over-v6 overlaps (2.3 code). c2.link_socket should neve

Re: [Openvpn-devel] [PATCH 1/7] put argv_* functions into own file, add unit tests

Hi, I didn't look at the code itself (David already did), but two very minor nitpicks I ran into while rebasing my src/openvpn/* unit tests on this patch: On 28-10-16 18:42, Heiko Hund wrote: > --- a/configure.ac > +++ b/configure.ac > @@ -1273,6 +1273,7 @@ AC_CONFIG_FILES([ > tests/unit

Re: [Openvpn-devel] [PATCH] clean up *sig_info handling in link_socket_init_phase2()

On 07-11-16 22:44, Gert Doering wrote: > The code was a mix of "assume that it is not NULL" and "check that > it is not NULL before using" - it cannot be NULL (due to the single > call graph, referencing c->sig with the global context), but for > good measure, add an ASSERT() upon function entry an

[Openvpn-devel] [PATCH applied] Re: clean up *sig_info handling in link_socket_init_phase2()

Patch has been applied to the master branch. (I've added "Found by Coverity" to the commit message, code is unchanged) commit 238cd6440312c59353a40a97b3c45d272561457f Author: Gert Doering Date: Mon Nov 7 22:44:02 2016 +0100 clean up *sig_info handling in link_socket_init_phase2() Si

Re: [Openvpn-devel] [PATCH] check c->c2.link_socket before calling do_init_route_ipv6_list()

On 08-11-16 09:39, Gert Doering wrote: > There was an asymmetry in checks before calling do_init_route*_list(), > checking c2.link_socket for IPv4 but not for IPv6 - mainly an oversight > from the time when do_init_route_ipv6_list() did not yet look at the > remote address to determine v6-over-v6 o

[Openvpn-devel] [PATCH applied] Re: check c->c2.link_socket before calling do_init_route_ipv6_list()

Patch has been applied to the master branch. commit 71e7c5f25174a3046a32720d3d6eb77f87458217 Author: Gert Doering Date: Tue Nov 8 09:39:23 2016 +0100 check c->c2.link_socket before calling do_init_route_ipv6_list() Signed-off-by: Gert Doering Acked-by: Steffan Karger Mess

[Openvpn-devel] [PATCH] Check previously-unchecked buf_alloc_write() call in crypto self-test.

"It cannot be NULL", but since this is self-test infrastructure, assume the worst - add ASSERT() check to ensure assumptions are true. Found by Coverity. Signed-off-by: Gert Doering --- src/openvpn/crypto.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/openvpn/cryp

Re: [Openvpn-devel] Summary of today's (Monday, 7th Nov 2016) community meeting

2016-11-08 1:39 GMT+05:00 Samuli Seppänen : > Hi, > > Here's the summary of today's IRC meeting. > > --- > > COMMUNITY MEETING > > Place: #openvpn-meeting on irc.freenode.net > Date: Monday 7th November 2016 > Time: 20:00 CET (19:00 UTC) > > Planned meeting topics for this meeting were here: > > <

[Openvpn-devel] [PATCH] Fix potential division by zero in shaper_reset()

shaper_reset() is only ever called with "bytes_per_second" set to a non-zero value - so the whole check "is it zero? if not, use constrain_int() to make sure it is within bounds" is not needed -> reduce check to just constrain_int() so even if somebody would call shaper_reset(..., 0) it would not l

Re: [Openvpn-devel] Summary of today's (Monday, 7th Nov 2016) community meeting

Hi, On Tue, Nov 08, 2016 at 02:22:20PM +0500, ?? wrote: > > Discussed OpenVPN 2.3.14 release. The biggest thing missing is > > "block-outside-dns v2". The > > block-outside-dns is not supported on WinXP, right ? Right. But it's not needed either. gert -- USENET is *not*

Re: [Openvpn-devel] [PATCH] Check previously-unchecked buf_alloc_write() call in crypto self-test.

On 08-11-16 10:17, Gert Doering wrote: > "It cannot be NULL", but since this is self-test infrastructure, assume > the worst - add ASSERT() check to ensure assumptions are true. > > Found by Coverity. > > Signed-off-by: Gert Doering > --- > src/openvpn/crypto.c | 5 - > 1 file changed, 4 in

Re: [Openvpn-devel] [PATCH] Fix potential division by zero in shaper_reset()

On 08-11-16 10:44, Gert Doering wrote: > shaper_reset() is only ever called with "bytes_per_second" set to > a non-zero value - so the whole check "is it zero? if not, use > constrain_int() to make sure it is within bounds" is not needed -> > reduce check to just constrain_int() so even if somebody

[Openvpn-devel] [PATCH applied] Re: Check previously-unchecked buf_alloc_write() call in crypto self-test.

Patch has been applied to the master branch (with the offending space removed). commit 63bdc6ce66a970f0584bbb1d4a8cae98b36ee831 Author: Gert Doering Date: Tue Nov 8 10:17:12 2016 +0100 Check previously-unchecked buf_alloc_write() call in crypto self-test. Signed-off-by: Gert Doering

[Openvpn-devel] [PATCH applied] Re: Fix potential division by zero in shaper_reset()

Patch has been applied to the master branch. commit 9c3a9335ee77d8447bf47e464f4ab15964fb6f1b Author: Gert Doering Date: Tue Nov 8 10:44:02 2016 +0100 Fix potential division by zero in shaper_reset() Signed-off-by: Gert Doering Acked-by: Steffan Karger Message-Id: <1478598

Re: [Openvpn-devel] [PATCH 2/7] Remove unused and unecessary argv interfaces

-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 28/10/16 18:42, Heiko Hund wrote: > Signed-off-by: Heiko Hund --- > src/openvpn/argv.c| 53 > +++ src/openvpn/argv.h > | 6 - src/openvpn/console_systemd.c | 3 +-- > src/openvpn/route.c

Re: [Openvpn-devel] [PATCH 3/7] remove unused system_str from struct argv

On 28/10/16 18:42, Heiko Hund wrote: > Signed-off-by: Heiko Hund > --- > src/openvpn/argv.c | 86 > -- > src/openvpn/argv.h | 1 - > 2 files changed, 87 deletions(-) ACK. Looks good. Valgrind also reports less heap usage, which is expected

[Openvpn-devel] [PATCH] Repair topology subnet on FreeBSD 11

We used to add "route for this subnet" by using our own address as the gateway address, which used to mean "connected to the interface, no gateway". FreeBSD commit 293159 changed the kernel side of that assumption so "my address" is now always bound to "lo0" - thus, our subnet route also ended up

[Openvpn-devel] [PATCH 0/4] Several t_client fixes and enhancements

Patch 2/4 replaces this old patch of the same name: -- Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi proc

[Openvpn-devel] [PATCH 1/4] Fix update_t_client_ips.sh for out of tree builds

From: Samuli Seppänen Signed-off-by: Samuli Seppänen --- tests/t_client.rc-sample | 4 ++-- tests/t_client.sh.in | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/t_client.rc-sample b/tests/t_client.rc-sample index 59f34c7..31dfafa 100644 --- a/tests/t_client.rc-s

[Openvpn-devel] [PATCH 4/4] Allow passing extra arguments to fping/fping6 in t_client.rc

From: Samuli Seppänen This can be useful, for example, in preventing fping failures caused by external network issues. Signed-off-by: Samuli Seppänen --- tests/t_client.rc-sample | 1 + tests/t_client.sh.in | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/t_cl

[Openvpn-devel] [PATCH 2/4] Prevent generation of duplicate EXPECT_IFCONFIG entries

From: Samuli Seppänen Previously, if t_client.rc did not source t_client_ips.rc, update_t_client_ips.sh would add (the same) EXPECT_IFCONFIG entries to t_client_ips.rc on every run. This patch makes update_t_client_ips.sh check if the entry exists before trying to add it. Signed-off-by: Samuli S

[Openvpn-devel] [PATCH 3/4] Make sure that all relevant files under test go to release tarballs

From: Samuli Seppänen Signed-off-by: Samuli Seppänen --- tests/Makefile.am | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 235cd13..e55928b 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -24,4 +24,8 @@ TESTS = $(tes

Re: [Openvpn-devel] [PATCH 4/7] Factor out %sc handling from argv_printf()

On 28/10/16 18:42, Heiko Hund wrote: > Move functionality to parse command strings into argv_parse_cmd(). > That is a preparation for the upcoming refactoring of argv_printf(). > > Signed-off-by: Heiko Hund > --- > src/openvpn/argv.c | 47 > +---

Re: [Openvpn-devel] [PATCH 4/4] Allow passing extra arguments to fping/fping6 in t_client.rc

Hi, On Tue, Nov 08, 2016 at 02:19:10PM +0100, Gert Doering wrote: > > + echo "$cmd $FPING_EXTRA_ARGS -b $bytes -C 20 -p 250 -q $targetlist" > > >>$LOGDIR/$SUF:fping.out > > + $cmd $FPING_EXTRA_ARGS -b $bytes -C 20 -p 250 -q $targetlist > > >>$LOGDIR/$SUF:fping.out 2>&1 > > I would put the a

Re: [Openvpn-devel] [PATCH 4/4] Allow passing extra arguments to fping/fping6 in t_client.rc

Hi, On Tue, Nov 08, 2016 at 02:55:29PM +0200, sam...@openvpn.net wrote: > - echo "$cmd -b $bytes -C 20 -p 250 -q $targetlist" > >>$LOGDIR/$SUF:fping.out > - $cmd -b $bytes -C 20 -p 250 -q $targetlist >>$LOGDIR/$SUF:fping.out 2>&1 > + echo "$cmd $FPING_EXTRA_ARGS -b $bytes -C 20 -p 250

[Openvpn-devel] [PATCH applied] Re: Fix update_t_client_ips.sh for out of tree builds

ACK. Code change looks good, and verifiably fixes the problem :-) (Note that there needs to be 5/4 that resets up='' when not needed...) Your patch has been applied to the master branch. commit 2af1fa8b19e9723b29f4a4c198f61478ae9e688e Author: Samuli Seppänen Date: Tue Nov 8 14:55:26 2016 +020

Re: [Openvpn-devel] [PATCH 2/4] Prevent generation of duplicate EXPECT_IFCONFIG entries

Hi, On Tue, Nov 08, 2016 at 02:55:27PM +0200, sam...@openvpn.net wrote: > +grep EXPECT_IFCONFIG4_$TESTNUM $RC > /dev/null 2>&1 NAK. This will grep for stuff like "EXPECT_IFCONFIG4_1", and if the RC file has EXPECT_IFCONFIG4_1a=10.194.1.54 the grep will be successful, and "EXPECT_IFCONFIG4_1" w

Re: [Openvpn-devel] [PATCH 1/7] put argv_* functions into own file, add unit tests

Hi, On 08-11-16 09:44, Steffan Karger wrote: > I didn't look at the code itself (David already did), but two very minor > nitpicks I ran into while rebasing my src/openvpn/* unit tests on this > patch: > > [..] Apart from this, I noticed that builds that depend on OPENSSL_CFLAGS/MBEDTLS_CFLAGS (

[Openvpn-devel] [PATCHv2 4/4] Allow passing extra arguments to fping/fping6 in t_client.rc

From: Samuli Seppänen This can be useful, for example, in preventing fping failures caused by external network issues. v2: - Allow override of the default parameters Signed-off-by: Samuli Seppänen --- tests/t_client.rc-sample | 1 + tests/t_client.sh.in | 4 ++-- 2 files changed, 3 inse

[Openvpn-devel] [PATCH applied] Re: Make sure that all relevant files under test go to release tarballs

ACK, thanks for catching this :-) Your patch has been applied to the master branch. commit e81023f00bbc503c41cea0a988a0ac00edae754a Author: Samuli Seppänen Date: Tue Nov 8 14:55:28 2016 +0200 Make sure that all relevant files under test go to release tarballs Signed-off-by: Samuli S

[Openvpn-devel] [PATCH applied] Re: Allow passing extra arguments to fping/fping6 in t_client.rc

ACK. Tested with FPING_EXTRA_ARGS="-C 5" as "proof of concept". Your patch has been applied to the master branch. commit 6f17e18be0d6ab801704400abcc6b17d4fed9650 Author: Samuli Seppänen Date: Tue Nov 8 15:50:43 2016 +0200 Allow passing extra arguments to fping/fping6 in t_client.rc

[Openvpn-devel] [PATCHv2 2/4] Prevent generation of duplicate EXPECT_IFCONFIG entries

From: Samuli Seppänen Previously, if t_client.rc did not source t_client_ips.rc, update_t_client_ips.sh would add (the same) EXPECT_IFCONFIG entries to t_client_ips.rc on every run. This patch makes update_t_client_ips.sh check if the entry exists before trying to add it. v2: prevent partial mat

[Openvpn-devel] [PATCH applied] Re: Prevent generation of duplicate EXPECT_IFCONFIG entries

ACK. Does what it says on the lid (and no more) :-) Your patch has been applied to the master branch. commit 82a601f1e23d1c37bc2120dca12cadc4dbf2b4fc Author: Samuli Seppänen Date: Tue Nov 8 16:06:03 2016 +0200 Prevent generation of duplicate EXPECT_IFCONFIG entries Signed-off-by: S

Re: [Openvpn-devel] [PATCH] systemd: Improve the systemd unit files

Hi, I now have these unit files working on all my test VMS. The only problem before was that on debian 8 these services failed to start after reboot. (systemctl enable openvpn-{client/server}@config was enabled) The error message was: main process exited, code=exited, status=233/RUNTIME_DIRECTORY

[Openvpn-devel] [PATCH] Fix missing return value checks in multi_process_float()

Fix the missing return value checks on hash_remove() and hash_add() by replacing the calls with an single hash_add() call with the replace parameters set to true so that is can't fail. Then just ASSERT() that this is indeed the case. This also replaces the other add/remove combinations with a sin

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

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 slightly, from "Static Encrypt/Decrypt" to "Incoming/Outgo

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

This adds a --tls-crypt option, which uses a pre-shared static key (like the --tls-auth key) to encrypt control channel packets. Encrypting control channel packets has three main advantages: * It provides more privacy by hiding the certificate used for the TLS connection. * It is harder to iden

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

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 belongs in options_post

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

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. Signed-off-by: Steffan Karger --- src/openvpn/error.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/openvpn/error.h b/src/openvpn

[Openvpn-devel] [PATCH 5/5] Add --tls-crypt unit tests

These help verify the tls-crypt functionality - they already caught a bug during development. We should however probably also add some t_client tests once this feature is in. To test --tls-crypt with as few dependencies as possible, this adds a mock implementation of msg() (or actually x_msg()).

[Openvpn-devel] [PATCH] Add control channel encryption (--tls-crypt)

Hi, This patch set implements the control channel encryption feature proposed on this list in msg (https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg12357.html). This patch set requires patch 1/7 of Heiko's struct argv overhaul (and my suggested fixup) to be applied first. See

Re: [Openvpn-devel] [PATCH] systemd: Improve the systemd unit files

On 08/11/16 16:40, debbie10t wrote: > Hi, > > I now have these unit files working on all my test VMS. > > The only problem before was that on debian 8 these services failed to > start after reboot. (systemctl enable openvpn-{client/server}@config > was enabled) The error message was: > main proce

Re: [Openvpn-devel] [PATCH] systemd: Improve the systemd unit files

On Tue, Nov 08, 2016 at 09:27:20PM +0100, David Sommerseth wrote: > On 08/11/16 16:40, debbie10t wrote: > > Hi, > > > > I now have these unit files working on all my test VMS. > > > > The only problem before was that on debian 8 these services failed to > > start after reboot. (systemctl enable o

Re: [Openvpn-devel] [PATCH] systemd: Improve the systemd unit files

-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 08/11/16 22:02, Alberto Gonzalez Iniesta wrote: > On Tue, Nov 08, 2016 at 09:27:20PM +0100, David Sommerseth wrote: >> On 08/11/16 16:40, debbie10t wrote: >>> Hi, >>> >>> I now have these unit files working on all my test VMS. >>> >>> The only pro

[Openvpn-devel] [PATCH] Remove unneeded check for extra_certs_file_inline

As with all the file/file_inline variable, the _inline variable is only relevant if the file variable is equal to INLINE_FILE_TAG. The tls_ctx_load_extra_certs() function nicely follows this mantra. Removing this unneeded check silences a coverity 'dereference after null check' warning (tls_ctx_l