Re: [Openvpn-devel] Coding style clean-up ... phase 1

2016-12-14 Thread Selva Nair
On Wed, Dec 14, 2016 at 4:18 PM, David Sommerseth  wrote:

> On 13/12/16 22:42, David Sommerseth wrote:
> >
> > Hi all,
> >
> > So the first phase of the great reformatting is on its way.  I have
> > just pushed out a reformatting branch to the following
> > repositories:
> >
> > https://github.com:OpenVPN/openvpn.git
> > https://gitlab.com:openvpn/openvpn.git
> > git://git.code.sf.net/p/openvpn/openvpn
> >
>
> An updated reformatting branch have been pushed out.  If you have
> already fetched the it, git will most likely complain as to make
> these tests flow reasonably well without cluttering the git tree
> too much we have used forced pushes on this branch.
>
> And again, the The Great Reformatting patch is PGP signed as well,
> which can be verified with 'git log --show-signature'.  The
> signing key is 0x86CF944C9671FDF2, which is the same as this mail
> is signed with.


The reformatting looks pretty good for an automated process. The result
agrees with independently running the script on master.

The only issue I noticed is that some functions, for loops and switch
statements have their opening braces on the same line
ssl.c line 272, tun.c line 697 and many such in the several files.
(try grep ") {$"  *.c).

Probably needed nl_switch_brace, nl_fcall_brace etc in the config -- urgh..
Too many options is a blessing and bane at the same time..

This is not a show stopper for me. Could be handled manually in future
edits when code in the vicinity is touched..

Selva
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] Coding style clean-up ... phase 1

2016-12-14 Thread Steffan Karger
Hi,

On 14-12-16 22:18, David Sommerseth wrote:
> On 13/12/16 22:42, David Sommerseth wrote:
>> So the first phase of the great reformatting is on its way.  I have
>> just pushed out a reformatting branch to the following 
>> repositories:
> 
>> https://github.com:OpenVPN/openvpn.git 
>> https://gitlab.com:openvpn/openvpn.git 
>> git://git.code.sf.net/p/openvpn/openvpn

> An updated reformatting branch have been pushed out.  If you have
> already fetched the it, git will most likely complain as to make
> these tests flow reasonably well without cluttering the git tree
> too much we have used forced pushes on this branch.

Great, getting really close now!  I just verified that commit
81d882d5302b8b647202a6893b57dfdc61fd6df2 is the result of running the
reformat-all.sh script from commit
2417d55c4945d491e528dd0e4cf24047da5ceae9 on that commit.

Scrolling through the changes almost everything looks good to me.  Some
details will need to be fixed when we touch the code, but in general
this looks like a *huge* improvement.

I also ran my usual tests: 'make check' including t_client for both
openssl and mbed TLS.

All this looks good, so ACK on merging
81d882d5302b8b647202a6893b57dfdc61fd6df2 into master.

-Steffan



signature.asc
Description: OpenPGP digital signature
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] Summary of today's (Wed, 14th Dec 2016) community meeting

2016-12-14 Thread Samuli Seppänen

Hi,

Here's the summary of today's IRC meeting.

---

COMMUNITY MEETING

Place: #openvpn-meeting on irc.freenode.net
Date: Wednesday 14th Dec 2016
Time: 20:00 CET (19:00 UTC)

Planned meeting topics for this meeting were here:



The next meeting has been scheduled to a week from now (Wed 21st 
December), at the same time as today.


Your local meeting time is easy to check from services such as



SUMMARY

dazo, krzee, mattock, plaisthos, selvanair and syzzer participated in 
this meeting.


--

Agreed to merge, and merged, the following pull requests:




--

Discussed the OpenVPN 2.4_rc2 release:



Made some changes to the Great Reformatting script. The modified 
versions were sent to the list for review.


Decided to run the reformatting patch (produced by the above script) 
through buildbot before merging it to the main Git repositories. This 
helps ensure the quality of the reformatted codebase.


Decided to aim for a release on Friday 14th December.

--

Discussed broken MacOS X Travis builds:



Plaisthos did not have a neat solution to this problem. Either the build 
needs to be fixed or unit tests skipped on OS X.


---

Full chatlog has been attached to this email.

--
Samuli Seppänen
Community Manager
OpenVPN Technologies, Inc

irc freenode net: mattock

(21:06:38) L'argomento di #openvpn-meeting è: Meeting 2016-12-14 1900 UTC: 
Agenda at https://community.openvpn.net/openvpn/wiki/Topics-2016-12-14
(21:06:38) L'argomento per #openvpn-meeting è stato impostato da 
dazo!~dazo@openvpn/corp/developer/dazo a 21:02:40 su 14/12/2016
(21:06:53) mattock: hi!
(21:06:55) krzee: hey mattock, time for a 2.4 changelog link here i think 
https://openvpn.net/index.php/open-source/documentation/change-log.html
(21:06:56) vpnHelper: Title: Change Log (at openvpn.net)
(21:07:26) mattock: krzee: I will do my best :)
(21:07:45) mattock: the Joomla installation is quite "entangled" as James would 
say
(21:07:55) dazo: :)
(21:08:09) mattock: so I'm not entirely sure where to find that particular 
page, and whether it's dynamically created or not
(21:08:28) krzee: oh i see
(21:09:03) mattock: anyways, I'll put that to my TODO
(21:09:09) krzee: no biggie anyways, i figured it was just overlooked
(21:09:14) dazo: I think openvpn tech should just shutdown their joomla and 
move over to a wordpress.com hosted site ... it's not that expensive for a 
company as they are
(21:09:15) mattock: yeah, it was
(21:09:49) mattock: dazo: the problem is that lots of stuff has been integrated 
to Joomla, so getting rid of it easily is not possible
(21:09:57) dazo: ahh :/
(21:10:06) dazo: mattock: cron2 can't come today ... and syzzer will be a bit 
late
(21:10:34) mattock: we already started the migration to Wordpress, and most of 
stuff _is_ available on a new site, but the project has stalled
(21:10:38) dazo: (not sure how far syzzer lives from his work ... but he did 
use the bike when we were in Delft, so it can't be that long :))
(21:10:56) mattock: dazo: ok, let's wait a bit for syzzer
(21:11:03) krzee: plus hes super fast on the bike
(21:11:05) dazo: mattock: hehe ... I was just glaring into the crystal ball it 
seems :-P
(21:11:06) krzee: speed racer
(21:11:12) dazo: hehe
(21:12:26) mattock: dazo: are we on schedule with 2.4_rc2?
(21:13:24) dazo: mattock: I would say we're in fairly good shape ... just a few 
minor things with the reformatting ... we need to discuss that with syzzer and 
selvanair today
(21:13:39) mattock: ok
(21:13:52) mattock: both thursday and friday would work for me as far as the 
release goes
(21:14:55) dazo: great ... I think we'll need Friday, to be honest
(21:15:05) selvanair: hi! Sorry to be late ... I just stepped out
(21:15:10) mattock: hi selvanair!
(21:15:17) dazo: the great reformatting patch is massive 
(21:15:18) dazo:  204 files changed, 66170 insertions(+), 55210 deletions(-)
(21:15:44) dazo: selvanair: no worries, we're not really started yet ... just 
chit-chatting until syzzer appears :)
(21:15:49) selvanair: mattock: did the manifest for tapinstall.exe work out?
(21:15:53) mattock: oh yes, no point in using "git show" to figure out if 
something has changed :D
(21:15:57) selvanair: dazo: good..
(21:16:17) mattock: selvanair: I have not tested the manifest yet, will need to 
look into it
(21:17:26) mattock: which reminds me that I will merge this one: 
https://github.com/OpenVPN/openvpn-build/pull/62
(21:17:27) vpnHelper: Title: Fix issue #59 by mattock · Pull Request #62 · 
OpenVPN/openvpn-build · GitHub (at github.com)
(21:17:38) mattock: I wanted to run it through Travis first
(21:18:09) selvanair: mattock: that one builds fine now..
(21:18:41) mattock: yes, and it 

Re: [Openvpn-devel] [PATCH applied] dev-tools: Add reformat-all.sh for code style unification

2016-12-14 Thread David Sommerseth
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1


Did a quick change at commit time, changing from bash to sh

Your patch has been applied to the master branch.

commit 2417d55c4945d491e528dd0e4cf24047da5ceae9
Author: David Sommerseth
Date:   Wed Dec 14 22:05:00 2016 +0100

 dev-tools: Add reformat-all.sh for code style unification

 Signed-off-by: David Sommerseth 
 Acked-by: Steffan Karger 
 Message-Id: <1481749500-8795-1-git-send-email-dav...@openvpn.net>
 URL: 
http://www.mail-archive.com/search?l=mid=1481749500-8795-1-git-send-email-dav...@openvpn.net


- --
kind regards,

David Sommerseth

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

iQIcBAEBAgAGBQJYUbmjAAoJEIbPlEyWcf3yKokP/2ODlMt5zj+8BD4nNPup/SNN
5h236L8Rm9VE13lYZxPECYGmTQNML94qAnxcew0T/KCAeRKtprUbR4FlfEwWaC+E
eH38inXz6Gcho8BqUuq4dRHspR+A15NQkRS5Vpa7RTGJKQRUFLbFAWb1Jqacl+Gk
7qGE9CypkncYMT5OsdZ13q4o5bgy2yO6b3wKVt2Ah7tkUzcqmswIglk4Fb44Zo48
ZraTaXNtxtImLkrcqrwI/7YFlPuTMbBtg/wAQ3fUNMfnLorGMdD6I9L1vlVwWv56
RPJIBcK0lDKJk0+m2YDWDvP7bYTZ+kOw5Arkv2sp2/0Rp5Cr8O4XtVFMz1ZTntmW
s99/ZG9RO5Ln4DWeoz6cpHfTXcZrI3ltkG8q+gDbTFfocQsnW4LPLmUYhkB0lHlv
UgdNhBqzvuYWUjoh7Jr423qjNaUAkcpNrFt7kW7VI4gpsYO1bzt87zUKqMrqfE/s
cJIgHkJT6INjvOJiAxqMPbJLpUejoRCzZifNqwNg/X9d1czJF1z/K7NZ4bmNQuPN
Z4fZbEMW88GrljcQfm4qLFwrsOKo/RWL+2AJS3JcXN3YFc8pCBs73i7iSWYQKt05
Xl1CSWV93aUo+rEyJnP436iVPb4GGfjgLkfJU4QYFvDzT1TdvigDmPOG+u011cyd
LVG0zfLepzQmHSoEnCsF
=dwj/
-END PGP SIGNATURE-

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] Coding style clean-up ... phase 1

2016-12-14 Thread David Sommerseth
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 13/12/16 22:42, David Sommerseth wrote:
> 
> Hi all,
> 
> So the first phase of the great reformatting is on its way.  I have
> just pushed out a reformatting branch to the following 
> repositories:
> 
> https://github.com:OpenVPN/openvpn.git 
> https://gitlab.com:openvpn/openvpn.git 
> git://git.code.sf.net/p/openvpn/openvpn
> 

An updated reformatting branch have been pushed out.  If you have
already fetched the it, git will most likely complain as to make
these tests flow reasonably well without cluttering the git tree
too much we have used forced pushes on this branch.

And again, the The Great Reformatting patch is PGP signed as well,
which can be verified with 'git log --show-signature'.  The
signing key is 0x86CF944C9671FDF2, which is the same as this mail
is signed with.


- -- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc

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

iQIcBAEBAgAGBQJYUbceAAoJEIbPlEyWcf3yOnIQAKRMvwhlde6/X/oi4Tu94dRk
Czoqf9nVTeS/ejf/dwWuztVWR8hIfd0rf+gog6eTHBQrRSjvDHPyi62XTH2lkt9z
h+xOHZTOgL7oKyHWrfjiyGPxov2RUgz5HvMyH2nl2s6Pj3o9J/AkrfxFJbtMu/CR
YYk9w5QOn8/XfoulOgZgWh1JXwICGKUVQXFQRBoQpi7m3aVdFpRN/3uHmK+epWe/
ewEQd2kNYJQfPWCRuzDBTPCJfYmr9IDncB/mkvDGw7D08BdcFknOWeVmP1HHn4uM
XWsIqOqfilOf09dPzQnSfLxlgg0zGZW/z8wYBqKAZTM1aVO5z5Jtq6nqevWzB1UX
Nrlws0OaPlIsjGUzeL3BInGQevpYYKSHS3Acyz+cW+X42t3AIGTKUQSCgRQpFJnb
v4cRf+7c7h77t7mMIIsS7ghMJRGgog6ib2NJp07in4o75aHMFeuWHte4FmM3Osrk
vJ9MT1jUkxs1OxkFsxpnrhhzfY1UazCDmosMytoE+CaJzMIkB9HzlH77yR+jxnQK
lT3hineWNGgmEtTjmczZQJHHF68IHivfd7g07mS/xdvXWmRbkgUQNqcKOZujiaQV
OIfjvOkREtkbW1lJLinMeb3c+SW5DggKnkSwdo3yHOHvRfniPraQxVDbpDAd2dVu
0WPlpmIuHmBypIxNBpYc
=agzz
-END PGP SIGNATURE-

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v3] dev-tools: Add reformat-all.sh for code style unification

2016-12-14 Thread Steffan Karger
Hi,

On 14-12-16 22:05, David Sommerseth wrote:
> This script will run all files related to the currently checked out
> git branch through uncrustify using a standardized style configuration.
> 
> Due to a bug in uncrustify 0.64, it is needed to add a special treatment
> to one of the files at the moment.  So this both pre- and post-patched
> before/after uncrustify is run.  This is to simply to assure that all
> file processing will happen consistently each time.
> 
> Also added doc/doxygen/doc_key_generation.h to an ignore list, as
> it carries some specific Doxygen formatting we should be careful with.
> This file is anyhow not so critical and can be managed manually.
> 
> The src/compat/compat-lz4.[ch] files are also not touched, as they
> are based on upstream formatting.  This makes it easier to update
> to a newer LZ4 version later on and even see what the differences
> are.
> 
> v2 - Include updated config from CodeStyle wiki page
>  Remove line lenght restriction for The Great Reformatting
>  Update the script with improvements by krzee
> 
> v3 - Update with a fixed config from the CodeStyle wiki page
>  Corrected a typo in the commit message (0.63->0.64)
>  Minor changes to the reformat script (no pushd/popd,
>  some new lines moved around)
> 
> Signed-off-by: David Sommerseth 
> ---
>  dev-tools/reformat-all.sh  | 136 
> +
>  .../after_include_openvpn-plugin.h.in.patch|  13 ++
>  .../before_include_openvpn-plugin.h.in.patch   |  13 ++
>  dev-tools/special-files.lst|   4 +
>  dev-tools/uncrustify.conf  |  65 ++
>  5 files changed, 231 insertions(+)
>  create mode 100755 dev-tools/reformat-all.sh
>  create mode 100644 
> dev-tools/reformat-patches/after_include_openvpn-plugin.h.in.patch
>  create mode 100644 
> dev-tools/reformat-patches/before_include_openvpn-plugin.h.in.patch
>  create mode 100644 dev-tools/special-files.lst
>  create mode 100644 dev-tools/uncrustify.conf
> 
> diff --git a/dev-tools/reformat-all.sh b/dev-tools/reformat-all.sh
> new file mode 100755
> index 000..ac5cf6f
> --- /dev/null
> +++ b/dev-tools/reformat-all.sh
> @@ -0,0 +1,136 @@
> +#!/bin/bash

Just tested, works fine when using /bin/sh too (on Ubuntu, so dash).

> +# reformat-all.sh - Reformat all git files in the checked out
> +#   git branch using uncrustify.
> +#
> +# Copyright (C) 2016 - David Sommerseth 
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License
> +# as published by the Free Software Foundation; either version 2
> +# of the License.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, 
> USA.
> +#
> +
> +tstamp="$(date +%Y%m%d-%H%M%S)"
> +files="$(pwd)/reformat-all_files-$tstamp.lst"
> +log="$(pwd)/reformat-all_log-$tstamp.txt"
> +
> +srcroot="$(git rev-parse --show-toplevel)"
> +cfg="$srcroot/dev-tools/uncrustify.conf"
> +specialfiles="$srcroot/dev-tools/special-files.lst"
> +
> +export gitfiles=0
> +export procfiles=0
> +
> +# Go to the root of the source tree
> +cd "$srcroot"
> +
> +{
> +echo -n "** Starting $0: "
> +date
> +
> +# Find all C source/header files
> +git ls-files | grep -E ".*\.[ch](\.in$|$)" > "${files}.git"
> +
> +# Manage files which needs special treatment
> +awk -F\# '{gsub("\n| ", "", $1); print $1}' "$specialfiles" > 
> "${files}.sp"
> +while read srcfile
> +do
> +res=$(grep "$srcfile" "${files}.sp" 2>/dev/null)
> +if [ $? -ne 0 ]; then
> +# If grep didn't find the file among special files,
> +# process it normally
> +echo "$srcfile" >> "$files"
> +else
> +mode=$(echo "$res" | cut -d:  -f1)
> +case "$mode" in
> +E)
> +echo "** INFO **  Excluding '$srcfile'"
> +;;
> +P)
> +echo "** INFO **  Pre-patching '$srcfile'"
> +
> patchfile="${srcroot}"/dev-tools/reformat-patches/before_$(echo "$srcfile" | 
> tr "/" "_").patch
> +if [ -r "$patchfile" ]; then
> +git apply "$patchfile"
> +if [ $? -ne 0 ]; then
> +echo "** ERROR **  Failed to apply pre-patch 
> file: $patchfile"
> +exit 2
> +fi
> +else
> +  

Re: [Openvpn-devel] [PATCH] auth-gen-token: Hardening memory cleanup on auth-token failuers

2016-12-14 Thread Steffan Karger
Hi,

On 14-12-16 16:12, David Sommerseth wrote:
> Further improve the memory management when a clients --auth-token
> fails the server side token authentication enabled via --auth-gen-token.
> 
> Signed-off-by: David Sommerseth 
> ---
>  src/openvpn/ssl_verify.c | 22 ++
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
> index 4328828..1effdf5 100644
> --- a/src/openvpn/ssl_verify.c
> +++ b/src/openvpn/ssl_verify.c
> @@ -1108,6 +1108,21 @@ verify_user_pass_management (struct tls_session 
> *session, const struct user_pass
>  }
>  #endif
>  
> +/**
> + *  Wipes the authentication token out of the memory, frees and cleans up 
> related buffers and flags
> + *
> + *  @param multi  Pointer to a multi object holding the auth_token variables
> + */
> +static void
> +wipe_auth_token(struct tls_multi *multi)
> +{
> +  secure_memzero (multi->auth_token, AUTH_TOKEN_SIZE);
> +  free (multi->auth_token);
> +  multi->auth_token = NULL;
> +  multi->auth_token_sent = false;
> +}
> +
> +
>  /*
>   * Main username/password verification entry point
>   */
> @@ -1157,6 +1172,7 @@ verify_user_pass(struct user_pass *up, struct tls_multi 
> *multi,
>/* Ensure that the username has not changed */
>if (!tls_lock_username(multi, up->username))
>  {
> +  wipe_auth_token(multi);
>ks->authenticated = false;
>goto done;
>  }
> @@ -1168,6 +1184,7 @@ verify_user_pass(struct user_pass *up, struct tls_multi 
> *multi,
>&& (multi->auth_token_tstamp + session->opt->auth_token_lifetime) 
> < now)
>  {
>msg (D_HANDSHAKE, "Auth-token for client expired\n");
> +  wipe_auth_token(multi);
>ks->authenticated = false;
>goto done;
>  }
> @@ -1176,10 +1193,7 @@ verify_user_pass(struct user_pass *up, struct 
> tls_multi *multi,
>if (memcmp_constant_time(multi->auth_token, up->password,
>   strlen(multi->auth_token)) != 0)
>  {
> -  secure_memzero (multi->auth_token, AUTH_TOKEN_SIZE);
> -  free (multi->auth_token);
> -  multi->auth_token = NULL;
> -  multi->auth_token_sent = false;
> +  wipe_auth_token(multi);
>ks->authenticated = false;
>tls_deauthenticate (multi);
>  

Looks good, but I think there's one more occurance you should
incorporate in the patch:

  if (openvpn_base64_encode(tok, AUTH_TOKEN_SIZE,
>auth_token) < AUTH_TOKEN_SIZE)
{
  msg(D_TLS_ERRORS, "BASE64 encoding of token failed. "
  "No auth-token will be activated now");
  if (multi->auth_token)
{
  secure_memzero(multi->auth_token, AUTH_TOKEN_SIZE);
  free(multi->auth_token);
  multi->auth_token = NULL;
}

-Steffan

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH v3] dev-tools: Add reformat-all.sh for code style unification

2016-12-14 Thread David Sommerseth
This script will run all files related to the currently checked out
git branch through uncrustify using a standardized style configuration.

Due to a bug in uncrustify 0.64, it is needed to add a special treatment
to one of the files at the moment.  So this both pre- and post-patched
before/after uncrustify is run.  This is to simply to assure that all
file processing will happen consistently each time.

Also added doc/doxygen/doc_key_generation.h to an ignore list, as
it carries some specific Doxygen formatting we should be careful with.
This file is anyhow not so critical and can be managed manually.

The src/compat/compat-lz4.[ch] files are also not touched, as they
are based on upstream formatting.  This makes it easier to update
to a newer LZ4 version later on and even see what the differences
are.

v2 - Include updated config from CodeStyle wiki page
 Remove line lenght restriction for The Great Reformatting
 Update the script with improvements by krzee

v3 - Update with a fixed config from the CodeStyle wiki page
 Corrected a typo in the commit message (0.63->0.64)
 Minor changes to the reformat script (no pushd/popd,
 some new lines moved around)

Signed-off-by: David Sommerseth 
---
 dev-tools/reformat-all.sh  | 136 +
 .../after_include_openvpn-plugin.h.in.patch|  13 ++
 .../before_include_openvpn-plugin.h.in.patch   |  13 ++
 dev-tools/special-files.lst|   4 +
 dev-tools/uncrustify.conf  |  65 ++
 5 files changed, 231 insertions(+)
 create mode 100755 dev-tools/reformat-all.sh
 create mode 100644 
dev-tools/reformat-patches/after_include_openvpn-plugin.h.in.patch
 create mode 100644 
dev-tools/reformat-patches/before_include_openvpn-plugin.h.in.patch
 create mode 100644 dev-tools/special-files.lst
 create mode 100644 dev-tools/uncrustify.conf

diff --git a/dev-tools/reformat-all.sh b/dev-tools/reformat-all.sh
new file mode 100755
index 000..ac5cf6f
--- /dev/null
+++ b/dev-tools/reformat-all.sh
@@ -0,0 +1,136 @@
+#!/bin/bash
+# reformat-all.sh - Reformat all git files in the checked out
+#   git branch using uncrustify.
+#
+# Copyright (C) 2016 - David Sommerseth 
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, 
USA.
+#
+
+tstamp="$(date +%Y%m%d-%H%M%S)"
+files="$(pwd)/reformat-all_files-$tstamp.lst"
+log="$(pwd)/reformat-all_log-$tstamp.txt"
+
+srcroot="$(git rev-parse --show-toplevel)"
+cfg="$srcroot/dev-tools/uncrustify.conf"
+specialfiles="$srcroot/dev-tools/special-files.lst"
+
+export gitfiles=0
+export procfiles=0
+
+# Go to the root of the source tree
+cd "$srcroot"
+
+{
+echo -n "** Starting $0: "
+date
+
+# Find all C source/header files
+git ls-files | grep -E ".*\.[ch](\.in$|$)" > "${files}.git"
+
+# Manage files which needs special treatment
+awk -F\# '{gsub("\n| ", "", $1); print $1}' "$specialfiles" > "${files}.sp"
+while read srcfile
+do
+res=$(grep "$srcfile" "${files}.sp" 2>/dev/null)
+if [ $? -ne 0 ]; then
+# If grep didn't find the file among special files,
+# process it normally
+echo "$srcfile" >> "$files"
+else
+mode=$(echo "$res" | cut -d:  -f1)
+case "$mode" in
+E)
+echo "** INFO **  Excluding '$srcfile'"
+;;
+P)
+echo "** INFO **  Pre-patching '$srcfile'"
+
patchfile="${srcroot}"/dev-tools/reformat-patches/before_$(echo "$srcfile" | tr 
"/" "_").patch
+if [ -r "$patchfile" ]; then
+git apply "$patchfile"
+if [ $? -ne 0 ]; then
+echo "** ERROR **  Failed to apply pre-patch file: 
$patchfile"
+exit 2
+fi
+else
+echo "** WARN ** Pre-patch file for $srcfile is 
missing: $patchfile"
+fi
+echo "$srcfile" >> "${files}.postpatch"
+echo "$srcfile" >> "$files"
+;;
+*)
+echo "** WARN ** Unknown mode '$mode' for file '$srcfile'"
+

Re: [Openvpn-devel] [PATCH] dev-tools: Add reformat-all.sh for code style unification

2016-12-14 Thread Steffan Karger
Hi,

On 14-12-16 21:07, David Sommerseth wrote:
> Due to a bug in uncrustify 0.63

Should be 0.64.

> diff --git a/dev-tools/reformat-all.sh b/dev-tools/reformat-all.sh
> new file mode 100755
> index 000..11a6eca
> --- /dev/null
> +++ b/dev-tools/reformat-all.sh
> @@ -0,0 +1,136 @@
> +#!/bin/bash

If you replace the pushd/popd with a subshell, /bin.sh suffices (sligtly
more portable).

And, it's a good habit to start shell scripts with set -eu, to make sure
you exit on failures, and don't use uninitialised variables.

Finally, git complains about whitespace errors:

Applying: dev-tools: Add reformat-all.sh for code style unification
.git/rebase-apply/patch:173: trailing whitespace.

.git/rebase-apply/patch:192: trailing whitespace.

warning: 2 lines add whitespace errors.


Other than these details (and what Selva mailed), the patch looks good.

-Steffan

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] dev-tools: Add reformat-all.sh for code style unification

2016-12-14 Thread Selva Nair
On Wed, Dec 14, 2016 at 3:07 PM, David Sommerseth 
wrote:

> +++ b/dev-tools/uncrustify.conf
> @@ -0,0 +1,64 @@
> +# Use Allman-style
> +indent_columns=4
> +indent_braces=false
> +indent_else_if=false
> +indent_switch_case=4
> +indent_label=1nl_if_brace=add
>

What happened there? Is it my mailer or a newline missing?

Selva
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v2] Disable async_push if plugins are disabled

2016-12-14 Thread David Sommerseth
On 14/12/16 17:23, Steffan Karger wrote:
> On 14-12-16 16:39, David Sommerseth wrote:
>> On 14/12/16 10:09, Gert Doering wrote:
>>> Hi,
>>
>>> On Wed, Dec 14, 2016 at 10:51:18AM +0200, Lev Stipakov wrote:
 +/* + * Disable async-push if plugins are disabled + */ +#if
 !defined(ENABLE_PLUGIN) && defined(ENABLE_ASYNC_PUSH) +#undef
 ENABLE_ASYNC_PUSH +#endif +
>>
>>> That one gets a NAK from me, for the reasons given.
>>
>>> As long as we *have* --enable/--disable options in configure, they
>>> should be validated *there* and not silently ignored.
>>
>>> David's argument about "configure is for finding system dependent
>>> things and compiler settings" has some merits, but then we should
>>> get rid of all compile-time --enable/--disable settings.
>>
>> By this logic should we enforce users to add --disable-def-auth if
>> --disable-plugin is set?  If so, then we should probably also do
>> something with --disable-plugin-auth-pam and
>> --disable-plugin-down-root.  And with --disable-plugin-auth-pam, we
>> need to look at --enable-pam-dlopen as well.
> 
> No, unless the user specified --enable-def-auth.  And yes, I think it
> would be nice if we would throw an error if the user did.  "Fail-fast"
> is a really good habit.

Well, there is still --enable-pam-dlopen --disable-plugin-auth-pam ...
and what else can we stubmble accross if we try --disable-server?  Or
you don't want to bother about that kind of consistency?

>> And to put things in context of the async-push stuff.  In the C code
>> we already check if --disable-def-auth is enabled or not.
>>
>> If we truly start to look into this for the other options, we will
>> have quite a good job ahead of us.  Otherwise we end up with even more
>> mess where we are not even consistent.
> 
> Sure, but "The other stuff is broken" is no argument to not add
> something that is not broken.

But it is broken.  The v1 patch compiles just fine with
--enable-async-push --disable-def-auth

$ grep DEF_AUTH ./config.h
/* #undef ENABLE_DEF_AUTH */

And then have a look at multi.c:2214.  This is a key code path for this
feature.  This is behaving exactly how you do not want.  If so, the
current v1 patch need a NAK.

And if you have --enable-async-push --disable-server, that should also
fail.  I want to have some consistency.

Which is why I am saying we do not do such sanity checking in
configure.ac today.

>> I say it again, configure.ac is *NOT* the place to do sanity checks on
>> which openvpn features depends on each-other.  We have not done that
>> before, and we should not do it now.  It just provides far more
>> non-production code to maintain, and more places to look for issues
>> when code we expect to to be compiled in isn't being compiled in.
>> This way leads is a rats nest and will cause much more maintenance
>> burden in the future, which I will not accept lightly.
> 
> Yes we do.  Just check the mbed TLS pkcs11 checking.  That check has
> saved me some hours of re-running builds at the least.  I wish we had
> more such checks.

And *that* is a correct check, not comparable to --enable-async-push at all.

This check detects that your mbedtls library does not support PKCS#11,
thus it fails to build as it needs that support by default.  The mbedtls
library isn't something you normally build yourself, as that is most
commonly shipped by the distro - so fixing that either requires
disabling PKCS#11 support in OpenVPN or building mbedtls yourself.
Failing to do so will _not_ result in a complete build.


But fair enough, I can fully understand the argument that it shouldn't
turn off things behind your back, and I can understand the "fail fast"
argument too (even though I don't care too much about it in such a small
project as OpenVPN).  But then lets fix it *properly* in a more
consistent way.  And we have #error and #warning which may be used in
such scenarios.

If the general attitude is that we should do all sanity checking via
configure.ac so that ./configure fails on inconsistency, then we need to
do something far better than just fixing --enable-async-push
--disable-{plugins,def-auth,server} variants.  Otherwise we will end up
needing to clean-up configure.ac once again [1].

Perhaps it then is better to have some C code which ./configure builds
at the very end - where we do all kind of #if defined(...) checks based
on ENABLE_* macros defined and bails out with #error or something
similar when something is bad.  Just so we do this sanity check *one
single place* and in a simple manner.  Currently we have some 20
ENABLE_* macros handled by config.h alone.


[1] Last massive clean-up:
$ git log --stat 8a4eaf5aa6debaca434..0e4b6c455e0236a4eb4 \
  --follow -- configure.ac

-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc




signature.asc
Description: OpenPGP digital signature
--
Check out the vibrant tech community on one of the world's most 
engaging 

Re: [Openvpn-devel] [PATCH v2] Disable async_push if plugins are disabled

2016-12-14 Thread Steffan Karger
On 14-12-16 16:39, David Sommerseth wrote:
> On 14/12/16 10:09, Gert Doering wrote:
>> Hi,
> 
>> On Wed, Dec 14, 2016 at 10:51:18AM +0200, Lev Stipakov wrote:
>>> +/* + * Disable async-push if plugins are disabled + */ +#if
>>> !defined(ENABLE_PLUGIN) && defined(ENABLE_ASYNC_PUSH) +#undef
>>> ENABLE_ASYNC_PUSH +#endif +
> 
>> That one gets a NAK from me, for the reasons given.
> 
>> As long as we *have* --enable/--disable options in configure, they
>> should be validated *there* and not silently ignored.
> 
>> David's argument about "configure is for finding system dependent
>> things and compiler settings" has some merits, but then we should
>> get rid of all compile-time --enable/--disable settings.
> 
> By this logic should we enforce users to add --disable-def-auth if
> --disable-plugin is set?  If so, then we should probably also do
> something with --disable-plugin-auth-pam and
> --disable-plugin-down-root.  And with --disable-plugin-auth-pam, we
> need to look at --enable-pam-dlopen as well.

No, unless the user specified --enable-def-auth.  And yes, I think it
would be nice if we would throw an error if the user did.  "Fail-fast"
is a really good habit.

> And to put things in context of the async-push stuff.  In the C code
> we already check if --disable-def-auth is enabled or not.
> 
> If we truly start to look into this for the other options, we will
> have quite a good job ahead of us.  Otherwise we end up with even more
> mess where we are not even consistent.

Sure, but "The other stuff is broken" is no argument to not add
something that is not broken.

> I say it again, configure.ac is *NOT* the place to do sanity checks on
> which openvpn features depends on each-other.  We have not done that
> before, and we should not do it now.  It just provides far more
> non-production code to maintain, and more places to look for issues
> when code we expect to to be compiled in isn't being compiled in.
> This way leads is a rats nest and will cause much more maintenance
> burden in the future, which I will not accept lightly.

Yes we do.  Just check the mbed TLS pkcs11 checking.  That check has
saved me some hours of re-running builds at the least.  I wish we had
more such checks.

> Yes, I see the short-term convenience to solve that specific Trac
> ticket.  But it really isn't the right long-term solution.

I respectfully disagree.  I believe this *is* the right long-term fix,
and we should aim at doing it this way.

-Steffan



signature.asc
Description: OpenPGP digital signature
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v2] Disable async_push if plugins are disabled

2016-12-14 Thread Selva Nair
On Wed, Dec 14, 2016 at 10:39 AM, David Sommerseth  wrote:

> > That one gets a NAK from me, for the reasons given.
> >
> > As long as we *have* --enable/--disable options in configure, they
> > should be validated *there* and not silently ignored.
> >
> > David's argument about "configure is for finding system dependent
> > things and compiler settings" has some merits, but then we should
> > get rid of all compile-time --enable/--disable settings.
>
> By this logic should we enforce users to add --disable-def-auth if
> - --disable-plugin is set?  If so, then we should probably also do
> something with --disable-plugin-auth-pam and
> - --disable-plugin-down-root.  And with --disable-plugin-auth-pam, we
> need to look at --enable-pam-dlopen as well.
>
> And to put things in context of the async-push stuff.  In the C code
> we already check if --disable-def-auth is enabled or not.
>
> If we truly start to look into this for the other options, we will
> have quite a good job ahead of us.  Otherwise we end up with even more
> mess where we are not even consistent.
>
> I say it again, configure.ac is *NOT* the place to do sanity checks on
> which openvpn features depends on each-other.  We have not done that
> before, and we should not do it now.  It just provides far more
> non-production code to maintain, and more places to look for issues
> when code we expect to to be compiled in isn't being compiled in.
> This way leads is a rats nest and will cause much more maintenance
> burden in the future, which I will not accept lightly.
>
> Yes, I see the short-term convenience to solve that specific Trac
> ticket.  But it really isn't the right long-term solution.


I agree with the spirit of this argument, though I do not know the details
of all those options.

But:

When I do
./configure --enable-xyz
I expect the feature to be either enabled or spew out an error saying
'inconsistent options, check configure --help' at the very least.

The build process silently changing the option behind my back and producing
a package with no xyz is not good.

Selva
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v2] Disable async_push if plugins are disabled

2016-12-14 Thread David Sommerseth
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 14/12/16 10:09, Gert Doering wrote:
> Hi,
> 
> On Wed, Dec 14, 2016 at 10:51:18AM +0200, Lev Stipakov wrote:
>> +/* + * Disable async-push if plugins are disabled + */ +#if
>> !defined(ENABLE_PLUGIN) && defined(ENABLE_ASYNC_PUSH) +#undef
>> ENABLE_ASYNC_PUSH +#endif +
> 
> That one gets a NAK from me, for the reasons given.
> 
> As long as we *have* --enable/--disable options in configure, they
> should be validated *there* and not silently ignored.
> 
> David's argument about "configure is for finding system dependent
> things and compiler settings" has some merits, but then we should
> get rid of all compile-time --enable/--disable settings.

By this logic should we enforce users to add --disable-def-auth if
- --disable-plugin is set?  If so, then we should probably also do
something with --disable-plugin-auth-pam and
- --disable-plugin-down-root.  And with --disable-plugin-auth-pam, we
need to look at --enable-pam-dlopen as well.

And to put things in context of the async-push stuff.  In the C code
we already check if --disable-def-auth is enabled or not.

If we truly start to look into this for the other options, we will
have quite a good job ahead of us.  Otherwise we end up with even more
mess where we are not even consistent.

I say it again, configure.ac is *NOT* the place to do sanity checks on
which openvpn features depends on each-other.  We have not done that
before, and we should not do it now.  It just provides far more
non-production code to maintain, and more places to look for issues
when code we expect to to be compiled in isn't being compiled in.
This way leads is a rats nest and will cause much more maintenance
burden in the future, which I will not accept lightly.

Yes, I see the short-term convenience to solve that specific Trac
ticket.  But it really isn't the right long-term solution.


- -- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc

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

iQIcBAEBAgAGBQJYUWe4AAoJEIbPlEyWcf3yX8QQAMDl+us2tVSCCABm0RZ77srQ
kRNIrxitehRgzlJi6/Hsod4lDtUxiJ9Q70EjFswHeRkAw9jAD1C69h2Ht4z7etb7
zdqPO+qH44YEA+lDeOu8dcP73q9Ff3izEJdF35ripHCbNymrKkoT+4+OkYDj3VIl
xRMLnem3MWfpmdqxzhGdkU/NkSMUMiGF8S7xAJDakvZ7CGvNOfUByWlG7dmUXU9U
oRxb0oyJdnSuj8SWDS5sQb6VsXyqAGnKKnE82YL6KZxTYOmri/SkO3HcfBT61dlq
bR/CypjuDCppP6bTRegV0xP0V7qNyjm3F6VzTu7Wt5TyFw60AN9xsKV6B71vwUzp
h2dhe3QEYQHYJ2m+vcwxRI0CfzO1zQM7UiJ23OjMgYeyMu7Y01fpm/Y1MixTVMct
rSeXOpJQkNFCs3v/NPrA8a85IIptDa8cLiOzz3Hca3hIESRmPhZgPxSeAGz+cqNQ
Ql8pZ2YMroFUQZIGcsJblXO/s/G8ud4urruXPGnrCj26mGUNBjmht0iLIZs7EJK1
bhviQSkPXeoUSWuonw9TN+hx3w8YRJocHaUkVz26+F9HnLJbma8Sew7BK+pqs1g6
2yNpxYUQtlChbItP+q7dYCYm23m5uRs28sP52u1AKMfeIbA5FPGpqBB/Kjz0ueQn
sh5T6CFecL9NAGIxJf9h
=26tU
-END PGP SIGNATURE-

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH] auth-gen-token: Hardening memory cleanup on auth-token failuers

2016-12-14 Thread David Sommerseth
Further improve the memory management when a clients --auth-token
fails the server side token authentication enabled via --auth-gen-token.

Signed-off-by: David Sommerseth 
---
 src/openvpn/ssl_verify.c | 22 ++
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 4328828..1effdf5 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -1108,6 +1108,21 @@ verify_user_pass_management (struct tls_session 
*session, const struct user_pass
 }
 #endif
 
+/**
+ *  Wipes the authentication token out of the memory, frees and cleans up 
related buffers and flags
+ *
+ *  @param multi  Pointer to a multi object holding the auth_token variables
+ */
+static void
+wipe_auth_token(struct tls_multi *multi)
+{
+  secure_memzero (multi->auth_token, AUTH_TOKEN_SIZE);
+  free (multi->auth_token);
+  multi->auth_token = NULL;
+  multi->auth_token_sent = false;
+}
+
+
 /*
  * Main username/password verification entry point
  */
@@ -1157,6 +1172,7 @@ verify_user_pass(struct user_pass *up, struct tls_multi 
*multi,
   /* Ensure that the username has not changed */
   if (!tls_lock_username(multi, up->username))
 {
+  wipe_auth_token(multi);
   ks->authenticated = false;
   goto done;
 }
@@ -1168,6 +1184,7 @@ verify_user_pass(struct user_pass *up, struct tls_multi 
*multi,
   && (multi->auth_token_tstamp + session->opt->auth_token_lifetime) < 
now)
 {
   msg (D_HANDSHAKE, "Auth-token for client expired\n");
+  wipe_auth_token(multi);
   ks->authenticated = false;
   goto done;
 }
@@ -1176,10 +1193,7 @@ verify_user_pass(struct user_pass *up, struct tls_multi 
*multi,
   if (memcmp_constant_time(multi->auth_token, up->password,
  strlen(multi->auth_token)) != 0)
 {
-  secure_memzero (multi->auth_token, AUTH_TOKEN_SIZE);
-  free (multi->auth_token);
-  multi->auth_token = NULL;
-  multi->auth_token_sent = false;
+  wipe_auth_token(multi);
   ks->authenticated = false;
   tls_deauthenticate (multi);
 
-- 
1.8.3.1


--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH applied] Changes.rst: Mainatiner update on C99

2016-12-14 Thread David Sommerseth
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Your patch has been applied to the master branch

commit a7acb6b48e31c5b83983f7eb9caf308adb7b76f1
Author: David Sommerseth
Date:   Tue Dec 13 13:16:56 2016 +0100

 Changes.rst: Mainatiner update on C99

 Acked-by: Gert Doering 
 Message-Id: <1481631416-15377-1-git-send-email-dav...@openvpn.net>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13518.html
 Signed-off-by: David Sommerseth 


- --
kind regards,

David Sommerseth

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

iQIcBAEBAgAGBQJYUUD5AAoJEIbPlEyWcf3yD0AP+wQmD5Ifm2HJMeRr+8vNIRVs
9i5eQpeLUtyI0crhXMKrBNvVdLDmP5Jvx+EUGePhwgIAZ8OnBSpT8UImYBOIRsFL
W1QTBvlG0lxnYdcbh2yQ0yMkModt9KCbhDV+Ft3GN2NZPZpSiCsheSotJDe2jGk3
ovHzInn+ptxh+B0FOjhFjEsbh1DU+qR3pDxjtl6Zunus2/xTtq0Et1W6kZ9Q6K3S
kITcYFY6BMKoF3Jlrf403aewE3Hg+nhKLrsKbeIvfmncJkLPJVZclEdgVKzsuKKl
xjSR9k4fXuokjRQwlqNlAiB0GzTkYQeBnzn9ebbJyUSgDbfDrF2jlIpXeGlepfYd
eg0FoGHuNWULv3VbWc03Fpag36hDa+kXHMkzeEGC9oMA4wM0lG/adDcBpLLXvLjm
lDkRv3whD3IoefyfiDzlEJaeZMZWJO9Wb24Ytuh1UORJVIdFJp36lMmQ49sXt0lJ
ujId7fePoIYAu5lhnmfchJOcTioe1i3EVn6e9hy7j0mwC2FcNXKd6FPl9i6Pa0YA
dlh8RIrF868dwlBezjB1dQAOaVtJVqN50ET/qFVyQ2GLoTrtaQ1NNUIVFSzSwhae
9vLPgztjiMag4owmhg/joYtDVsXK0/mli48Y9PTOwEmSVQ4Tnz2Uoumj74WX9gSE
vJrAF/9hKmj20xToKq1P
=vM4R
-END PGP SIGNATURE-

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH applied] Further enhance async-push feature description

2016-12-14 Thread David Sommerseth
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Your patch has been applied to the master branch

commit 1a8f6b9159708a943ebdb64404de4c5fc887303b
Author: David Sommerseth
Date:   Wed Dec 14 13:23:30 2016 +0100

 Further enhance async-push feature description

 Signed-off-by: David Sommerseth 
 Acked-by: Steffan Karger 
 Message-Id: <1481718210-15673-1-git-send-email-dav...@openvpn.net>
 URL: 
http://www.mail-archive.com/search?l=mid=1481718210-15673-1-git-send-email-dav...@openvpn.net


- --
kind regards,

David Sommerseth

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

iQIcBAEBAgAGBQJYUUDQAAoJEIbPlEyWcf3yEU8QAI45qh0vPSuWT2u+ATEFYDHJ
awATBJxfZMhn4oP1v7AYd/5CYC2g6tPxEVpDBqvMoXvdfc19nz3OdMVIcycrJroH
qTaeDzrz8eERMTHhGJoYb4EG+b18j/FDl/ZHmiBAiBje+3f174oxpDkTrL6PyZfZ
vuAyWl2OfCKT3IuIYPVLwL6ihmdhuDjfLPemRZ4SbAe6Uz521fsI4wur2v1iMtIq
t5sLV+i+P4bnR1vj7SQNSTxcE+RVIjKP4lktEGhU8n39DF5/iRe2Xpttd9hgy9gp
IWUf1ARh7yvvGo5v269tsTr8dq5Xcdda5/nDAFBoRNrQCmBLz4GRa7jiHGqLd0g9
QxdqIMQk06/mfqkN/1dkBH8oVkHXrDPA70X+f1T+B+Yi7xRYbLKXYg/jgXr6uox+
kpzhiBfDL4OwacxGfNzaj5bNIKqGeSf2jAx+ghaMsF+wrzst8PTHhPPguy2379mW
1O7EeJOxHomXwV0VN8iPOlKO1QDPl0RioRwl4olvo7OFgt4XaDktoRBWY3K3v3TV
zycBqos+7QvBowTtK4lxfBmGN12oWMgdT4anP3J7T0p0UjAxFkj0ba52zfRG7NYR
oeDVu4iaVHMU9YqeguyhCc4zu7kYMkkrr4BzbKGINXyNVCH9RgVaCu7mHf/bn0aM
iZ9bPgI+QEsMyte3pBd1
=26OE
-END PGP SIGNATURE-

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] Make --enable-async-push depend on --enable-plugins

2016-12-14 Thread Steffan Karger
On 12-12-16 12:47, Lev Stipakov wrote:
> Async push functionality makes sense only with deferred authentication,
> which requires plugins.
> 
> Trac #783
> 
> Signed-off-by: Lev Stipakov 
> ---
>  configure.ac | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/configure.ac b/configure.ac
> index 27bdcc3..946f3db 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1204,6 +1204,10 @@ if test "${enable_plugin_auth_pam}" = "yes"; then
>  fi
>  
>  if test "${enable_async_push}" = "yes"; then
> + if test "${enable_plugins}" = "no"; then
> + AC_MSG_ERROR([--enable_async_push requires --enable-plugins])
> + fi
> +
>   AC_CHECK_HEADERS(
>   [sys/inotify.h],
>   AC_DEFINE([ENABLE_ASYNC_PUSH], [1], [Enable async push]),
> 

I followed the discussion, gave the arguments some more thought, but
still believe this is The Right Thing.  So, ACK.

-Steffan

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] Further enhance async-push feature description

2016-12-14 Thread Steffan Karger
On 14-12-16 13:23, David Sommerseth wrote:
> Signed-off-by: David Sommerseth 
> ---
>  Changes.rst  | 9 +
>  configure.ac | 2 +-
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/Changes.rst b/Changes.rst
> index a5002dd..7da1119 100644
> --- a/Changes.rst
> +++ b/Changes.rst
> @@ -148,10 +148,11 @@ Control channel encryption (``--tls-crypt``)
>  post-quantum security.
>  
>  Asynchronous push reply
> -If asynchronous authentication is enabled and completed after server 
> received
> -PUSH_REQUEST message, server sends PUSH_REPLY immediately without 
> waiting for next
> -PUSH_REQUEST. Requires use of ``--enable-async-push`` as ./configure 
> parameter at
> -build time.
> +Plug-ins providing support for deferred authentication can benefit from 
> a more
> +responsive authentication where the server sends PUSH_REPLY immediately 
> once
> +the authentication result is ready instead of waiting for the the client 
> to
> +to send PUSH_REQUEST once more.  This requires OpenVPN to be built with
> +``./configure --enable-async-push``.  This is a compile-time only switch.
>  
>  
>  Deprecated features
> diff --git a/configure.ac b/configure.ac
> index 27bdcc3..4f086ea 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -266,7 +266,7 @@ AC_ARG_ENABLE(
>  
>  AC_ARG_ENABLE(
>   [async-push],
> - [AS_HELP_STRING([--enable-async-push], [enable async-push support 
> @<:@default=no@:>@])],
> + [AS_HELP_STRING([--enable-async-push], [enable async-push support for 
> plugins providing deferred authentication @<:@default=no@:>@])],
>   ,
>   [enable_async_push="no"]
>  )
> 

ACK.

Related question:  looks both useful and harmless.  Should we enable
this by default in 2.5?

-Steffan

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH applied] man: mention that --ecdh-curve does not work on mbed TLS builds

2016-12-14 Thread David Sommerseth
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

ACK.

Your patch has been applied to the master branch

commit 07d0d73a38326c0e935ea35eb12452b69abafada
Author: Steffan Karger
Date:   Tue Dec 13 20:51:12 2016 +0100

 man: mention that --ecdh-curve does not work on mbed TLS builds

 Trac: #789
 Signed-off-by: Steffan Karger 
 Acked-by: David Sommerseth 
 Message-Id: <1481658672-5110-1-git-send-email-stef...@karger.me>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13523.html
 Signed-off-by: David Sommerseth 


- --
kind regards,

David Sommerseth

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

iQIcBAEBAgAGBQJYUTzaAAoJEIbPlEyWcf3yVhEQALfKBnjP35QmgFl1aQP2XhQf
v+Cc0q0p2KEBMXU9gxUq4cxoY9cxaEU9tCnEMRLTOdJDzhvVB59NPzKHGIzLXZ89
6hD/Oin9B6UA+ahhkLMcNzHhEjsWYSWfAoFOHElC3ARGE58+g2ZZ5iGTLRKSSqY5
jHVFdjJXVju8AfQWklJlBOx8mPRwE6DVWcZgZoSLezN7+LooawbsxtIri5uL3pHI
X+/icRaA3LSXsFhFdFtIX/j0zGiVfRiSgWBHsrRfm9aHnkg2QyKTv9UEOQbviK9l
pgsEO6T7G21j1XtY2xUt4r37Dw1l4SJBVOeQFaKqVkPA5KubgWgTVECA1Z1bPszu
+MW1di0tBcijUTt1l1hDyZ98sex+vJkmMu7yQR/EOYvJUZAoHiaPXa4Q8az8t38n
V46f2nhNaxKgO2FBmrfA+Bl1DU8D8WalgB/oxOHfNBvR2G8A6TP8+c/WJ5Eng4E3
7p1ob0TQB7TOoe16aQFy3qNeceT3g7xfxw1/psJYDM4OxfGedMHM9Nl/wWFFZhBs
Fd9t8RmOc+mU18JpCLv9bf8oNzVRkRekO+ah0TQJiyZuh0/LBHlAfKnIfnOEdI9c
P6PCwj62EOz2THxRmZs5pVbakR2E9gtO3Ocleq+Lwl5yrJDeVzAp60mtBTmuYyBj
QqR/HJ6JY5POz39Vqutr
=q11f
-END PGP SIGNATURE-

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH applied] Unhide a line in man page by fixing a typo

2016-12-14 Thread David Sommerseth
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Your patch has been applied to the master branch

commit c22428fb609a0f4ec451200a421b5d1090a962a5
Author: Selva Nair
Date:   Tue Dec 13 11:11:38 2016 -0500

 Unhide a line in man page by fixing a typo

 Signed-off-by: Selva Nair 
 Acked-by: Steffan Karger 
 Message-Id: <1481645498-22043-1-git-send-email-selva.n...@gmail.com>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13520.html
 Signed-off-by: David Sommerseth 


- --
kind regards,

David Sommerseth

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

iQIcBAEBAgAGBQJYUTyxAAoJEIbPlEyWcf3yfJEP/3fnkdjniHri6j96yHuVoeE0
GZWFrQt7ZBh/M6z6NszVMAdz8BjHLf3ns2/Xx+iHra2UHcwFzk/peEQWjjZCNG7S
zEiF1GQMIKTImBBM1ylYu719nLid4BoGohIj2sSI6bjE39nxXx+RxCtElGSaqkyc
r3gxGyEgS9YyGIXSnwFt7+CaiTN0zkCcgNmIEIqmpHXxK1pSGwnBZOLuOzztKlii
l0fp2q1jADom7sW76FxBryXuG7WuClPQ2AlSCn/dlLdKV6n8N2+Eg3JBbzUYE+Yi
VKTqIYvdSoNCiEhG6ZN2uOdKcmZrNje+gpe6dXq5NYc/586GF2rlNKlo2Iz8yQbq
J7GERaurlE3PGkDAXJ7Jvu3sLKH0vYG7jfKISlWLkdYdVrMK+Mt1sPBtJJ/CeBtg
wVxNrtLCfcKBgzARyL8T0wv/hKOcAi1tPC4Wlb1JyVdLymSv5CMelpBZx1VHCDRd
B+1jRo8CcrillA5RURiKnlnDn+Yza7i2E3QFS0zKX+CxkBCOYa+SaREQSbZuOy8A
HhQEZe++mW/MCrMsBN+nyv2903/xLFoCETIlesNWpHGMktDmIxulbTH1GeBUyoIJ
mtIO6HHivnFGL6+VCmLs8oosHQ5x+1k3yCB0F58l20DRgq25zX7SXXU+c5KMMDTT
wRtjCFuUoOB/Gm6KTNrZ
=aTh3
-END PGP SIGNATURE-

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v2] Disable async_push if plugins are disabled

2016-12-14 Thread Gert Doering
Hi,

On Wed, Dec 14, 2016 at 10:51:18AM +0200, Lev Stipakov wrote:
> +/*
> + * Disable async-push if plugins are disabled
> + */
> +#if !defined(ENABLE_PLUGIN) && defined(ENABLE_ASYNC_PUSH)
> +#undef ENABLE_ASYNC_PUSH
> +#endif
> +

That one gets a NAK from me, for the reasons given.

As long as we *have* --enable/--disable options in configure, they should
be validated *there* and not silently ignored.

David's argument about "configure is for finding system dependent things
and compiler settings" has some merits, but then we should get rid of all
compile-time --enable/--disable settings.

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
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH v2] Disable async_push if plugins are disabled

2016-12-14 Thread Lev Stipakov
Async push is a sub-feature of plugins.

Trac #783

Signed-off-by: Lev Stipakov 
---
 src/openvpn/syshead.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/openvpn/syshead.h b/src/openvpn/syshead.h
index f5008b7..c7c3259 100644
--- a/src/openvpn/syshead.h
+++ b/src/openvpn/syshead.h
@@ -702,4 +702,11 @@ socket_defined (const socket_descriptor_t sd)
 #define ENABLE_MEMSTATS
 #endif
 
+/*
+ * Disable async-push if plugins are disabled
+ */
+#if !defined(ENABLE_PLUGIN) && defined(ENABLE_ASYNC_PUSH)
+#undef ENABLE_ASYNC_PUSH
+#endif
+
 #endif
-- 
1.9.1


--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel