date:20170809

Re: [Openvpn-devel] [PATCH] Add coverity static analysis to Travis CI config

2017-08-09 Thread Илья Шипицин

2017-08-10 1:09 GMT+05:00 Steffan Karger :

> Hi,
>
> On 09-08-17 08:12, Илья Шипицин wrote:
> > 2017-08-09 10:47 GMT+05:00 Илья Шипицин  > >:
> > 2017-08-09 10:41 GMT+05:00 Илья Шипицин  > >:
> > 2017-08-08 20:55 GMT+05:00 Steffan Karger
> > >:
> >
> > Enable coverity analysis for the release/2.4 branch.
> >
> > We can only do a limited number of coverity scans per week
> > with our FOSS
> > account, but since we only occasionally push commits, that
> > should work out
> > fine.  But this limit is the reason we don't use the
> > standard travis addon,
> > because that would cause the coverity script to run on all
> > of our matrix
> > builds.  That would cause us to reach our limit faster, and
> > waste travis'
> > resources.
> >
> > Since our FOSS coverity account doesn't handle multiple
> > branches very well,
> > we have to pick one branch to run coverity on.  I think it's
> > best to use
> > the most recent stable branch for that (i.e. for now,
> > release/2.4).
> > Though for ease of maintenance, it's probably best to apply
> > the patch to
> > both master and release/2.4.
> >
> > I would refactor it like that
> > https://gist.github.com/chipitsine/
> 8dcae4ff1d59eb43df39f6015c6106fd
> >  8dcae4ff1d59eb43df39f6015c6106fd>
> > however, your is ok as well
> >
> > maybe, "script" would be better here
> > https://gist.github.com/chipitsine/8dcae4ff1d59eb43df39f6015c6106
> fd#file-gistfile1-txt-L74
> >  fd#file-gistfile1-txt-L74>
> > than "before_script"
> >
> > oops, travis does not support braches within matrix
> >
> > https://gist.github.com/chipitsine/8dcae4ff1d59eb43df39f6015c6106
> fd#file-gistfile1-txt-L70-L72
> >
> > sorry for that
>
> I like your idea, and toyed with it a bit.  But in the end, it needs too
> much fiddling to get right.  So I prefer my own suggested approach.
>

nevermind :)


>
> -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
>
--
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] OVPN Interactive Service for non-admin users

2017-08-09 Thread Selva Nair

Hi,


>  But that would open the OpenVPN Interactive Service to any user and
> application. This is why we would like your opinion first.
>
> Yes the service will then launch openvpn with arbitrary configs as any
> user, but that is what you want isn't it?
>
>
>
> True, I want that indeed. I was just trying to find the official way of
> doing it only to learn it's against OpenVPN team's principles. :(
>

The official way is to add the user to the designated group which by
default is expected to be named "OpenVPN Administrators". Recursive group
membership will work, so you could create a group named, say, "eduVPN
Users" or just use "Users" and add that to "OpenVPN Administrators" group
at install time (and remove it on uninstall). Personally I would avoid
tweaking permissions of a folder inside "Program Files\OpenVPN\config\"

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] Add coverity static analysis to Travis CI config

2017-08-09 Thread Steffan Karger

Hi,

On 09-08-17 08:12, Илья Шипицин wrote:
> 2017-08-09 10:47 GMT+05:00 Илья Шипицин  >:
> 2017-08-09 10:41 GMT+05:00 Илья Шипицин  >:
> 2017-08-08 20:55 GMT+05:00 Steffan Karger
> >:
> 
> Enable coverity analysis for the release/2.4 branch.
> 
> We can only do a limited number of coverity scans per week
> with our FOSS
> account, but since we only occasionally push commits, that
> should work out
> fine.  But this limit is the reason we don't use the
> standard travis addon,
> because that would cause the coverity script to run on all
> of our matrix
> builds.  That would cause us to reach our limit faster, and
> waste travis'
> resources.
> 
> Since our FOSS coverity account doesn't handle multiple
> branches very well,
> we have to pick one branch to run coverity on.  I think it's
> best to use
> the most recent stable branch for that (i.e. for now,
> release/2.4).
> Though for ease of maintenance, it's probably best to apply
> the patch to
> both master and release/2.4.
> 
> I would refactor it like that
> https://gist.github.com/chipitsine/8dcae4ff1d59eb43df39f6015c6106fd
> 
> however, your is ok as well
> 
> maybe, "script" would be better here
> 
> https://gist.github.com/chipitsine/8dcae4ff1d59eb43df39f6015c6106fd#file-gistfile1-txt-L74
> 
> 
> than "before_script"
> 
> oops, travis does not support braches within matrix
> 
> https://gist.github.com/chipitsine/8dcae4ff1d59eb43df39f6015c6106fd#file-gistfile1-txt-L70-L72
> 
> sorry for that

I like your idea, and toyed with it a bit.  But in the end, it needs too
much fiddling to get right.  So I prefer my own suggested approach.

-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] OVPN Interactive Service for non-admin users

2017-08-09 Thread Simon Rozman via Openvpn-devel

Hi Selva,

Is there any specific reason, why Interactive Service is so paranoid, knowing 
that it launches openvpn.exe and all external scripts as the interactive user 
anyway?

The service does privileged operations so some admin has to bless a user to 
allow certain options when launching openvpn.exe. In other words, options 
allowed in user editable configs are restricted unless the user is in a 
designated group.

  

I don't quite agree. OpenVPN needs elevation to set up connection because it 
runs in user space. IPsec VPN doesn't require elevation for the very same task 
since it runs in kernel space.

 

Therefore, elevation for OpenVPN is required for technical reasons, not 
security. Thus, an explicit blessing from the admin is an exaggeration.

 I have a work-around for this paradox in my sleeve: the eduVPN setup shall 
create an "eduVPN" subfolder in the "C:\Program Files\OpenVPN\config" folder, 
and grant all users desirable permissions*: a sort of public spool folder.

Setting up such a folder requires admin rights. If your installer has admin 
rights, just add all users to "OpenVPN Administrators" group or set the 
registry key ovpn_admin_group to "Users"

  

The installer will require admin rights of course. Here we agree installing 
software (VPN especially) needs an admin approval.

 

Thank you for your excellent advice. I haven't thought of that before. However, 
I will not follow it for the following reason…

 

eduVPN will not claim OpenVPN for all by itself. It will install it when 
missing, but will leave everything to its defaults. We would still like to 
leave the user an option to make use of OpenVPN for other purposes. Tweaking 
registry is not a step in this direction.

 But that would open the OpenVPN Interactive Service to any user and 
application. This is why we would like your opinion first.

Yes the service will then launch openvpn with arbitrary configs as any user, 
but that is what you want isn't it?

 

True, I want that indeed. I was just trying to find the official way of doing 
it only to learn it's against OpenVPN team's principles. :(

 

Well, I'll do it anyway. And I suggest you take it as a compliment: the OpenVPN 
is great for its flexibility so people can and will use it in a million of 
bizarre ways. :)

 

Best regards,

Simon



smime.p7s
Description: S/MIME cryptographic 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] OpenSSL: remove unreachable call to SSL_CTX_get0_privatekey()

2017-08-09 Thread Steffan Karger

Hi,

On 09-08-17 09:42, Antonio Quartulli wrote:
> From: Antonio Quartulli 
> 
> In tls_ctx_load_ecdh_params() the SSL_CTX_get0_privatekey() function
> is invoked only when "OPENSSL_VERSION_NUMBER >= 0x10002000L" and
> curve_name is NULL.
> 
> However, under the very same conditions the code flow will
> lead to an earlier return, thus never reaching the invocation of
> SSL_CTX_get0_privatekey().
> 
> Restructure the surrounding code in order to make the if/else
> block a bit easier to read and get rid of the unreachable
> invocation.
> 
> Signed-off-by: Antonio Quartulli 
> ---
> 
> Note: I believe that code sections like this, trying to deal with
> different library versions, should be cleaned up by moving the
> various #ifs to header files, instead of having them in the code.
> However, this should be done in a later (and bigger) cleanup.

I'm not so sure that would improve things, but I'm happy to review a
patch to be convinced otherwise ;-)

>  src/openvpn/ssl_openssl.c | 23 +--
>  1 file changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
> index e18fffc1..7ad6414e 100644
> --- a/src/openvpn/ssl_openssl.c
> +++ b/src/openvpn/ssl_openssl.c
> @@ -484,15 +484,7 @@ tls_ctx_load_ecdh_params(struct tls_root_ctx *ctx, const 
> char *curve_name
>  
>  /* Generate a new ECDH key for each SSL session (for non-ephemeral ECDH) 
> */
>  SSL_CTX_set_options(ctx->ctx, SSL_OP_SINGLE_ECDH_USE);
> -#if OPENSSL_VERSION_NUMBER >= 0x10002000L
> -/* OpenSSL 1.0.2 and newer can automatically handle ECDH parameter 
> loading */
> -if (NULL == curve_name)
> -{
> -SSL_CTX_set_ecdh_auto(ctx->ctx, 1);
> -return;
> -}
> -#endif
> -/* For older OpenSSL, we'll have to do the parameter loading on our own 
> */
> +
>  if (curve_name != NULL)
>  {
>  /* Use user supplied curve if given */
> @@ -501,14 +493,17 @@ tls_ctx_load_ecdh_params(struct tls_root_ctx *ctx, 
> const char *curve_name
>  }
>  else
>  {
> -/* Extract curve from key */
> +#if OPENSSL_VERSION_NUMBER >= 0x10002000L
> +/* OpenSSL 1.0.2 and newer can automatically handle ECDH parameter
> + * loading */
> +SSL_CTX_set_ecdh_auto(ctx->ctx, 1);
> +return;
> +#else
> +/* For older OpenSSL we have to extract the curve from key on our 
> own */
>  EC_KEY *eckey = NULL;
>  const EC_GROUP *ecgrp = NULL;
>  EVP_PKEY *pkey = NULL;
>  
> -#if OPENSSL_VERSION_NUMBER >= 0x10002000L && 
> !defined(LIBRESSL_VERSION_NUMBER)
> -pkey = SSL_CTX_get0_privatekey(ctx->ctx);
> -#else
>  /* Little hack to get private key ref from SSL_CTX, yay OpenSSL... */
>  SSL *ssl = SSL_new(ctx->ctx);
>  if (!ssl)
> @@ -517,7 +512,6 @@ tls_ctx_load_ecdh_params(struct tls_root_ctx *ctx, const 
> char *curve_name
>  }
>  pkey = SSL_get_privatekey(ssl);
>  SSL_free(ssl);
> -#endif
>  
>  msg(D_TLS_DEBUG, "Extracting ECDH curve from private key");
>  
> @@ -526,6 +520,7 @@ tls_ctx_load_ecdh_params(struct tls_root_ctx *ctx, const 
> char *curve_name
>  {
>  nid = EC_GROUP_get_curve_name(ecgrp);
>  }
> +#endif
>  }
>  
>  /* Translate NID back to name , just for kicks */
> 

ACK.  Patch looks good, and this still works as expected with both
OpenSSL 1.0.1 and 1.0.2 (both #if branches).

-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] v2 travis-ci: update pkcs11-helper to 1.22

2017-08-09 Thread Steffan Karger

Hi,

Very minor comment for next time: please put the 'v2' in the subject
inside the [PATCH] bracket, i.e. [PATCH v2].

On 09-08-17 10:12, Ilya Shipitsin wrote:
> use pkcs11-helper from https://github.com/OpenSC/pkcs11-helper/
> to match build process used in windows installer build
> 
> Signed-off-by: Ilya Shipitsin 
> ---
> v2: break lines up into 80-char, thanks Steffan Karger
> 
>  .travis.yml   | 2 +-
>  .travis/build-deps.sh | 8 ++--
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/.travis.yml b/.travis.yml
> index db90e03a..fc98a4d0 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -14,7 +14,7 @@ env:
>  - PREFIX="${HOME}/opt"
>  - TAP_WINDOWS_VERSION=9.21.2
>  - LZO_VERSION=2.10
> -- PKCS11_HELPER_VERSION=1.11
> +- PKCS11_HELPER_VERSION=1.22
>  - MBEDTLS_VERSION="2.4.0"
>  - MBEDTLS_CFLAGS="-I${PREFIX}/include"
>  - MBEDTLS_LIBS="-L${PREFIX}/lib -lmbedtls -lmbedx509 -lmbedcrypto"
> diff --git a/.travis/build-deps.sh b/.travis/build-deps.sh
> index 9cc18584..e787abab 100755
> --- a/.travis/build-deps.sh
> +++ b/.travis/build-deps.sh
> @@ -35,7 +35,7 @@ build_lzo () {
>  download_pkcs11_helper () {
>  if [ ! -f "pkcs11-helper-${PKCS11_HELPER_VERSION}.tar.bz2" ]; then
>  wget -P download-cache/ \
> -
> "http://downloads.sourceforge.net/project/opensc/pkcs11-helper/pkcs11-helper-${PKCS11_HELPER_VERSION}.tar.bz2;
> +
> "https://github.com/OpenSC/pkcs11-helper/releases/download/pkcs11-helper-${PKCS11_HELPER_VERSION}/pkcs11-helper-${PKCS11_HELPER_VERSION}.tar.bz2;
>  fi
>  }
>  
> @@ -46,7 +46,11 @@ build_pkcs11_helper () {
>  cd "pkcs11-helper-${PKCS11_HELPER_VERSION}"
>  
>  ./configure --host=${CHOST} --program-prefix='' 
> --libdir=${PREFIX}/lib \
> - --prefix=${PREFIX} --build=x86_64-pc-linux-gnu 
> --disable-crypto-engine-gnutls --disable-crypto-engine-nss
> + --prefix=${PREFIX} --build=x86_64-pc-linux-gnu \
> + --disable-crypto-engine-gnutls \
> + --disable-crypto-engine-nss \
> + --disable-crypto-engine-polarssl \
> + --disable-crypto-engine-mbedtls
>  make all install
>   )
>   echo "${PKCS11_HELPER_VERSION}" > "${PREFIX}/.pkcs11_helper-version"
> 

Patch itself looks good, 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] OVPN Interactive Service for non-admin users

2017-08-09 Thread Selva Nair

Hi Simon,

Adding to what I wrote in my reply to your private email:


> I am developing an eduVPN client for Windows. Imagine the eduVPN client as
> a custom OpenVPN GUI. The client uses openvpn.exe for connecting, the
> configuration file is provided by eduVPN server once user authenticates
> using OAuth. User running the eduVPN client is not an administrator.
> Elevation is out of the question.
>
>
>
> I would like to use the Interactive Service to start openvpn.exe, but I
> have some problems:
>
>
>
> 1.   The configuration file is dynamically downloaded by the eduVPN
> client and stored somewhere user can write (user's temporary folder for
> example). But the Interactive Service was specifically programmed to allow
> configurations from "C:\Program Files\OpenVPN\config" folder only. But user
> running eduVPN client can't write to this folder.
>
> 2.   Interactive Service can launch openvpn.exe using any
> configuration file if user is a member of the "OpenVPN Administrators"
> group. Then, I would need to add all users of the computer to that group,
> again requiring elevation.
>
>
>
> Is there any specific reason, why Interactive Service is so paranoid,
> knowing that it launches openvpn.exe and all external scripts as the
> interactive user anyway?
>

The service does privileged operations so some admin has to bless a user to
allow certain options when launching openvpn.exe. In other words, options
allowed in user editable configs are restricted unless the user is in a
designated group.

An admin installing openvpn can change this behaviour by customizing the
ovpn_admin_group and/or by adding users to that group.


>
>
> I have a work-around for this paradox in my sleeve: the eduVPN setup shall
> create an "eduVPN" subfolder in the "C:\Program Files\OpenVPN\config"
> folder, and grant all users desirable permissions*: a sort of public spool
> folder.
>

Setting up such a folder requires admin rights. If your installer has admin
rights, just add all users to "OpenVPN Administrators" group or set the
registry key ovpn_admin_group to "Users"


>
>
> But that would open the OpenVPN Interactive Service to any user and
> application. This is why we would like your opinion first.
>

Yes the service will then launch openvpn with arbitrary configs as any
user, but that is what you want isn't it?

Regards,

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

[Openvpn-devel] OVPN Interactive Service for non-admin users

2017-08-09 Thread Simon Rozman via Openvpn-devel

Hi!

 

I am developing an eduVPN client for Windows. Imagine the eduVPN client as a
custom OpenVPN GUI. The client uses openvpn.exe for connecting, the
configuration file is provided by eduVPN server once user authenticates
using OAuth. User running the eduVPN client is not an administrator.
Elevation is out of the question.

 

I would like to use the Interactive Service to start openvpn.exe, but I have
some problems:

 

1.   The configuration file is dynamically downloaded by the eduVPN
client and stored somewhere user can write (user's temporary folder for
example). But the Interactive Service was specifically programmed to allow
configurations from "C:\Program Files\OpenVPN\config" folder only. But user
running eduVPN client can't write to this folder.

2.   Interactive Service can launch openvpn.exe using any configuration
file if user is a member of the "OpenVPN Administrators" group. Then, I
would need to add all users of the computer to that group, again requiring
elevation.

 

Is there any specific reason, why Interactive Service is so paranoid,
knowing that it launches openvpn.exe and all external scripts as the
interactive user anyway?

 

I have a work-around for this paradox in my sleeve: the eduVPN setup shall
create an "eduVPN" subfolder in the "C:\Program Files\OpenVPN\config"
folder, and grant all users desirable permissions*: a sort of public spool
folder.

 

But that would open the OpenVPN Interactive Service to any user and
application. This is why we would like your opinion first.

 

Best regards,

Simon Rozman

Amebis, d. o. o., Kamnik

 

* By desirable permissions I mean: SYSTEM/Administrators = full access,
Users = create new files, CREATOR OWNER = R/W)

 



smime.p7s
Description: S/MIME cryptographic 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] tests: Add a simple build sanity check

2017-08-09 Thread David Sommerseth

This runs openvpn --help to check if the output is somewhat
sensible and sane.  It will catch if the binary segfaults,
if it is a normal build or an --enable-small build and
does some simple checks when a list of options is produced.

This is based on the discussions in this [1] mailing thread.

[1] 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15172.html
Message-Id: <20170807132301.22759-3-chipits...@gmail.com>

Signed-off-by: David Sommerseth 
---
 tests/Makefile.am   |   2 +-
 tests/t_sanity_check.sh | 118 
 2 files changed, 119 insertions(+), 1 deletion(-)
 create mode 100755 tests/t_sanity_check.sh

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 0795680c..7af5101e 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -14,7 +14,7 @@ MAINTAINERCLEANFILES = \
 
 SUBDIRS = unit_tests
 
-test_scripts = t_client.sh
+test_scripts = t_sanity_check.sh t_client.sh
 if ENABLE_CRYPTO
 test_scripts += t_lpback.sh t_cltsrv.sh
 endif
diff --git a/tests/t_sanity_check.sh b/tests/t_sanity_check.sh
new file mode 100755
index ..e6c228c8
--- /dev/null
+++ b/tests/t_sanity_check.sh
@@ -0,0 +1,118 @@
+#! /bin/sh
+#
+# t_sanity_check.sh  --  Check that openvpn --help makes somewhat sense
+#
+# Copyright (C) 2017  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, or (at your option) any later version.
+#
+# 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.
+
+set -u
+top_builddir="${top_builddir:-..}"
+
+failed=0
+count_failure()
+{
+failed=$(($failed + 1))
+}
+
+
+check_option_count()
+{
+num_min="$1"
+num_max="$2"
+
+echo -n "Checking if number of options are between $num_min and $num_max 
... "
+optcount="$(cat sanity_check_options.$$ | wc -l )"
+if [ $optcount -le $num_min ]; then
+echo "FAIL  (too few, found $optcount options)"
+count_failure
+return
+fi
+if [ $optcount -gt $num_max ]; then
+echo "FAIL  (too many, found $optcount options)"
+count_failure
+return
+fi
+echo "PASS (found $optcount options)"
+}
+
+
+check_options_present()
+{
+for opt in $*;
+do
+echo -n "Checking for option --${opt} ..."
+grep -E "^--${opt} " sanity_check_options.$$ 1>/dev/null 2>&1
+if [ $? -ne 0 ]; then
+echo "FAIL (missing option)"
+count_failure
+else
+echo "PASS"
+fi
+done
+}
+
+echo "*** OpenVPN sanity check: openvpn --help"
+echo -n "Running 'openvpn --help' ... "
+"${top_builddir}/src/openvpn/openvpn" --help > sanity_check_log.$$ 2>&1
+res=$?
+if [ $res -ne 1 ]; then
+echo "FAIL   (Something bad happened)"
+cat sanity_check_log.$$
+count_failure
+else
+echo "PASS"
+echo -n "Check build type ... "
+linecount="$(cat sanity_check_log.$$ | wc -l)"
+if [ $linecount -eq 1 ]; then
+# Is this an --enable-small build?
+grep "Usage message not available" sanity_check_log.$$ \
+1> /dev/null 2> /dev/null
+if [ $? -ne 0 ]; then
+echo "Unknown build type"
+cat sanity_check_log.$$
+count_failure
+else
+echo "PASS  (--enable-small build, no further checks)"
+fi
+else
+echo "PASS  (normal build)"
+
+# Extract only the options
+echo -n "Extracting options ... "
+grep -E -- ^-- sanity_check_log.$$ > sanity_check_options.$$
+if [ $? -ne 0 ]; then
+echo "FAIL"
+count_failure
+else
+echo "PASS"
+
+# Check that the number of option counts are between 220 and 245
+check_option_count 225 245
+
+# Check for a selected subset of options we always expect to see
+options_check="dev dev-type remote local port proto topology route 
ifconfig"
+check_options_present $options_check
+fi
+fi
+fi
+echo "*** OpenVPN sanity check result - Failed tasks: $failed"
+
+rm -f sanity_check_log.$$ sanity_check_options.$$
+if [ $failed -gt 0 ]; then
+exit 1
+fi
+exit 0
-- 
2.11.0


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [Openvpn-devel] [PATCH 3/3] add a test to "openvpn --help"

2017-08-09 Thread David Sommerseth

On 09/08/17 12:28, Илья Шипицин wrote:
> 
> 
> 2017-08-09 14:31 GMT+05:00 David Sommerseth
>  >:
> 
> On 09/08/17 07:55, Илья Шипицин wrote:
> [...]
> > > For example:
> > >
> > > $ ./openvpn --help | grep -- ^-- | wc -l
> > > 238
> >
> > But to do the spoon feeding:
> >
> > optcount="$(${top_builddir}/src/openvpn/openvpn --help | grep -E --
> > ^-- | wc -l)"
> > if [ $outcount -lt 220 ];
> > then
> > exit_code=1
> > fi
> >
> >
> > if you suggest "that's a better check", please describe your idea.
> > it is not clear for me why your approach is better
> 
> 
> The approach I suggest above covers:
> 
> a)  The program is able to execute and usage() works
> 
> b)  There is no unexpected bigger changes in usage(), the
> number of options are within a reasonable threshold.
> Granted, only minimum options is checked in the example above;
> extending with an upper limit is easy and quick (for example
> add '-o -gt 245')
> 
> c) If the program segfaults, optcount => 0 which ensures this test
>fails.
> 
> With your check only testing if the exit code is not 1, you only have an
> indication if the program segfaults or not.  You don't know if usage()
> provides nothing but garbage and then exiting with 1.  Checking that a
> certain amount of outputted lines starting with '--' gives an indication
> that usage() most likely have a reasonable output.
> 
> It would also be possible to build further on this check I suggest, to
> also check for mandatory options (--dev, --dev-type, --remote, --listen,
> --port, --proto, etc, etc).  It is also possible to have a copy of the
> expected "openvpn --help | grep -E -- ^--" output and do a diff -
> probably filter out some less important/deprecated options).   While
> these are a nice checks too, it is not as crucial as ensuring we have at
> least an reasonable expected amount of options.
> 
> 
> 
> I'm afraid that that approach introduce implicit things (while my is
> pretty explicit).
> Value seems questionable for me.

Well, then I'm just giving this patch a NAK, to be explicit.


-- 
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 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 3/3] add a test to "openvpn --help"

2017-08-09 Thread David Sommerseth

On 09/08/17 07:55, Илья Шипицин wrote:
[...]
> > For example:
> >
> > $ ./openvpn --help | grep -- ^-- | wc -l
> > 238
> 
> But to do the spoon feeding:
> 
> optcount="$(${top_builddir}/src/openvpn/openvpn --help | grep -E --
> ^-- | wc -l)"
> if [ $outcount -lt 220 ];
> then
> exit_code=1
> fi
> 
> 
> if you suggest "that's a better check", please describe your idea.
> it is not clear for me why your approach is better

The approach I suggest above covers:

a)  The program is able to execute and usage() works

b)  There is no unexpected bigger changes in usage(), the
number of options are within a reasonable threshold.
Granted, only minimum options is checked in the example above;
extending with an upper limit is easy and quick (for example
add '-o -gt 245')

c) If the program segfaults, optcount => 0 which ensures this test
   fails.

With your check only testing if the exit code is not 1, you only have an
indication if the program segfaults or not.  You don't know if usage()
provides nothing but garbage and then exiting with 1.  Checking that a
certain amount of outputted lines starting with '--' gives an indication
that usage() most likely have a reasonable output.

It would also be possible to build further on this check I suggest, to
also check for mandatory options (--dev, --dev-type, --remote, --listen,
--port, --proto, etc, etc).  It is also possible to have a copy of the
expected "openvpn --help | grep -E -- ^--" output and do a diff -
probably filter out some less important/deprecated options).   While
these are a nice checks too, it is not as crucial as ensuring we have at
least an reasonable expected amount of options.

-- 
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 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 travis-ci: update pkcs11-helper to 1.22

2017-08-09 Thread Ilya Shipitsin

use pkcs11-helper from https://github.com/OpenSC/pkcs11-helper/
to match build process used in windows installer build

Signed-off-by: Ilya Shipitsin 
---
v2: break lines up into 80-char, thanks Steffan Karger

 .travis.yml   | 2 +-
 .travis/build-deps.sh | 8 ++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index db90e03a..fc98a4d0 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -14,7 +14,7 @@ env:
 - PREFIX="${HOME}/opt"
 - TAP_WINDOWS_VERSION=9.21.2
 - LZO_VERSION=2.10
-- PKCS11_HELPER_VERSION=1.11
+- PKCS11_HELPER_VERSION=1.22
 - MBEDTLS_VERSION="2.4.0"
 - MBEDTLS_CFLAGS="-I${PREFIX}/include"
 - MBEDTLS_LIBS="-L${PREFIX}/lib -lmbedtls -lmbedx509 -lmbedcrypto"
diff --git a/.travis/build-deps.sh b/.travis/build-deps.sh
index 9cc18584..e787abab 100755
--- a/.travis/build-deps.sh
+++ b/.travis/build-deps.sh
@@ -35,7 +35,7 @@ build_lzo () {
 download_pkcs11_helper () {
 if [ ! -f "pkcs11-helper-${PKCS11_HELPER_VERSION}.tar.bz2" ]; then
 wget -P download-cache/ \
-
"http://downloads.sourceforge.net/project/opensc/pkcs11-helper/pkcs11-helper-${PKCS11_HELPER_VERSION}.tar.bz2;
+
"https://github.com/OpenSC/pkcs11-helper/releases/download/pkcs11-helper-${PKCS11_HELPER_VERSION}/pkcs11-helper-${PKCS11_HELPER_VERSION}.tar.bz2;
 fi
 }
 
@@ -46,7 +46,11 @@ build_pkcs11_helper () {
 cd "pkcs11-helper-${PKCS11_HELPER_VERSION}"
 
 ./configure --host=${CHOST} --program-prefix='' 
--libdir=${PREFIX}/lib \
- --prefix=${PREFIX} --build=x86_64-pc-linux-gnu 
--disable-crypto-engine-gnutls --disable-crypto-engine-nss
+ --prefix=${PREFIX} --build=x86_64-pc-linux-gnu \
+ --disable-crypto-engine-gnutls \
+ --disable-crypto-engine-nss \
+ --disable-crypto-engine-polarssl \
+ --disable-crypto-engine-mbedtls
 make all install
  )
  echo "${PKCS11_HELPER_VERSION}" > "${PREFIX}/.pkcs11_helper-version"
-- 
2.13.3


--
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] OpenSSL: remove unreachable call to SSL_CTX_get0_privatekey()

2017-08-09 Thread Antonio Quartulli

From: Antonio Quartulli 

In tls_ctx_load_ecdh_params() the SSL_CTX_get0_privatekey() function
is invoked only when "OPENSSL_VERSION_NUMBER >= 0x10002000L" and
curve_name is NULL.

However, under the very same conditions the code flow will
lead to an earlier return, thus never reaching the invocation of
SSL_CTX_get0_privatekey().

Restructure the surrounding code in order to make the if/else
block a bit easier to read and get rid of the unreachable
invocation.

Signed-off-by: Antonio Quartulli 
---

Note: I believe that code sections like this, trying to deal with
different library versions, should be cleaned up by moving the
various #ifs to header files, instead of having them in the code.
However, this should be done in a later (and bigger) cleanup.


 src/openvpn/ssl_openssl.c | 23 +--
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index e18fffc1..7ad6414e 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -484,15 +484,7 @@ tls_ctx_load_ecdh_params(struct tls_root_ctx *ctx, const 
char *curve_name
 
 /* Generate a new ECDH key for each SSL session (for non-ephemeral ECDH) */
 SSL_CTX_set_options(ctx->ctx, SSL_OP_SINGLE_ECDH_USE);
-#if OPENSSL_VERSION_NUMBER >= 0x10002000L
-/* OpenSSL 1.0.2 and newer can automatically handle ECDH parameter loading 
*/
-if (NULL == curve_name)
-{
-SSL_CTX_set_ecdh_auto(ctx->ctx, 1);
-return;
-}
-#endif
-/* For older OpenSSL, we'll have to do the parameter loading on our own */
+
 if (curve_name != NULL)
 {
 /* Use user supplied curve if given */
@@ -501,14 +493,17 @@ tls_ctx_load_ecdh_params(struct tls_root_ctx *ctx, const 
char *curve_name
 }
 else
 {
-/* Extract curve from key */
+#if OPENSSL_VERSION_NUMBER >= 0x10002000L
+/* OpenSSL 1.0.2 and newer can automatically handle ECDH parameter
+ * loading */
+SSL_CTX_set_ecdh_auto(ctx->ctx, 1);
+return;
+#else
+/* For older OpenSSL we have to extract the curve from key on our own 
*/
 EC_KEY *eckey = NULL;
 const EC_GROUP *ecgrp = NULL;
 EVP_PKEY *pkey = NULL;
 
-#if OPENSSL_VERSION_NUMBER >= 0x10002000L && !defined(LIBRESSL_VERSION_NUMBER)
-pkey = SSL_CTX_get0_privatekey(ctx->ctx);
-#else
 /* Little hack to get private key ref from SSL_CTX, yay OpenSSL... */
 SSL *ssl = SSL_new(ctx->ctx);
 if (!ssl)
@@ -517,7 +512,6 @@ tls_ctx_load_ecdh_params(struct tls_root_ctx *ctx, const 
char *curve_name
 }
 pkey = SSL_get_privatekey(ssl);
 SSL_free(ssl);
-#endif
 
 msg(D_TLS_DEBUG, "Extracting ECDH curve from private key");
 
@@ -526,6 +520,7 @@ tls_ctx_load_ecdh_params(struct tls_root_ctx *ctx, const 
char *curve_name
 {
 nid = EC_GROUP_get_curve_name(ecgrp);
 }
+#endif
 }
 
 /* Translate NID back to name , just for kicks */
-- 
2.13.3


--
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] Add coverity static analysis to Travis CI config

2017-08-09 Thread Илья Шипицин

2017-08-09 10:47 GMT+05:00 Илья Шипицин :

>
>
> 2017-08-09 10:41 GMT+05:00 Илья Шипицин :
>
>>
>>
>> 2017-08-08 20:55 GMT+05:00 Steffan Karger :
>>
>>> Enable coverity analysis for the release/2.4 branch.
>>>
>>> We can only do a limited number of coverity scans per week with our FOSS
>>> account, but since we only occasionally push commits, that should work
>>> out
>>> fine.  But this limit is the reason we don't use the standard travis
>>> addon,
>>> because that would cause the coverity script to run on all of our matrix
>>> builds.  That would cause us to reach our limit faster, and waste travis'
>>> resources.
>>>
>>> Since our FOSS coverity account doesn't handle multiple branches very
>>> well,
>>> we have to pick one branch to run coverity on.  I think it's best to use
>>> the most recent stable branch for that (i.e. for now, release/2.4).
>>> Though for ease of maintenance, it's probably best to apply the patch to
>>> both master and release/2.4.
>>>
>>
>>
>> I would refactor it like that https://gist.github.com/chipit
>> sine/8dcae4ff1d59eb43df39f6015c6106fd
>> however, your is ok as well
>>
>
>
> maybe, "script" would be better here https://gist.github.com/chipitsine/
> 8dcae4ff1d59eb43df39f6015c6106fd#file-gistfile1-txt-L74 than
> "before_script"
>


oops, travis does not support braches within matrix

https://gist.github.com/chipitsine/8dcae4ff1d59eb43df39f6015c6106fd#file-gistfile1-txt-L70-L72


sorry for that

>
>
>>
>>
>>>
>>> Signed-off-by: Steffan Karger 
>>> ---
>>>  .travis.yml |  8 +++-
>>>  .travis/coverity.sh | 17 +
>>>  2 files changed, 24 insertions(+), 1 deletion(-)
>>>  create mode 100755 .travis/coverity.sh
>>>
>>> diff --git a/.travis.yml b/.travis.yml
>>> index db90e03..131d002 100644
>>> --- a/.travis.yml
>>> +++ b/.travis.yml
>>> @@ -21,10 +21,13 @@ env:
>>>  - OPENSSL_VERSION="1.0.2k"
>>>  - OPENSSL_CFLAGS="-I${PREFIX}/include"
>>>  - OPENSSL_LIBS="-L${PREFIX}/lib -lssl -lcrypto"
>>> +# The next declaration is the encrypted COVERITY_SCAN_TOKEN, created
>>> +#   via the "travis encrypt" command using the project repo's
>>> public key
>>> +- secure: "l9mSnEW4LJqjxftH5i1NdDaYfGmQB
>>> 1mPXnSB3DXnsjzkCWZ+yJLfBemfQ0tx/wS7chBYxqUaUIMT0hw4zJVp/LANF
>>> Jo2vfh//ymTS6h0uApRY1ofg9Pp1BFcV1laG6/u8pwSZ2EBy/GhCd3DS436o
>>> E8sYBRaFM9FU62L/oeQBfJ7r4ID/0eB1b8bqlbD4paty9MHui2P8EZJwR+KA
>>> D84prtfpZOcrSMxPh9OUhJxzxUvvVoP4s4+lZ5Kgg1bBQ3yzKGDqe8VOgK2B
>>> WCEuezqhMMc8oeKmAe7CUkoz5gsGYH++k3I9XzP9Z4xeJKoQnC/82qi4xkJm
>>> laOxdionej9bHIcjfRt7D8j1J0U+wOj4p8VrDy7yHaxuN2fi0K5MGa/CaXQS
>>> rkna8dePniCng+xQ2MY/zxuRX2gA6xPNLUyQLU9LqIug7wj4z84Hk9iWib4L
>>> 20MoPjeEo+vAUNq8FtjOPxMuHNpv4iGGx6kgJm7RXl5vC5hxfK6MprrnYe2U
>>> 5Mcd8jpzagKBaKHL3zV2FxX9k0jRO9Mccz7M2WnaV0MQ6zcngzTN4+s0kCjh
>>> fGKd2F2ANT2Gkhj3Me36eNHfuE0dBbvYCMh4b3Mgd7b/OuXwQWdJ8PjJ1WHX
>>> jSOw5sHw1suaV6cEO2Meyz5j1tOkyOi0M9QF+LFenQ9vLH4sBCww8U="
>>>
>>>  matrix:
>>>include:
>>> -- env: SSLLIB="openssl"
>>> +- env: SSLLIB="openssl" RUN_COVERITY="1"
>>>os: linux
>>>compiler: gcc
>>>  - env: SSLLIB="openssl" OPENSSL_VERSION="1.1.0f"
>>> @@ -91,5 +94,8 @@ install:
>>>- if [ ! -z "${CHOST}" ]; then unset CC; fi
>>>- .travis/build-deps.sh > build-deps.log 2>&1 || (cat build-deps.log
>>> && exit 1)
>>>
>>> +before_script:
>>> +  - .travis/coverity.sh
>>> +
>>>  script:
>>>- .travis/build-check.sh
>>> diff --git a/.travis/coverity.sh b/.travis/coverity.sh
>>> new file mode 100755
>>> index 000..8bb40f4
>>> --- /dev/null
>>> +++ b/.travis/coverity.sh
>>> @@ -0,0 +1,17 @@
>>> +#!/bin/sh
>>> +set -eu
>>> +
>>> +RUN_COVERITY="${RUN_COVERITY:-0}"
>>> +
>>> +export COVERITY_SCAN_PROJECT_NAME="OpenVPN/openvpn"
>>> +export COVERITY_SCAN_BRANCH_PATTERN="release\/2.4"
>>> +export COVERITY_SCAN_NOTIFICATION_EMAIL="scan-repo...@openvpn.net"
>>> +export COVERITY_SCAN_BUILD_COMMAND_PREPEND="autoreconf -vi &&
>>> ./configure --enable-iproute2 && make clean"
>>> +export COVERITY_SCAN_BUILD_COMMAND="make"
>>> +
>>> +if [ "${RUN_COVERITY}" = "1" ]; then
>>> +# Ignore exit code, script exits with 1 if we're not on the right
>>> branch
>>> +curl -s "https://scan.coverity.com/scr
>>> ipts/travisci_build_coverity_scan.sh" | bash || true
>>> +else
>>> +echo "Skipping coverity scan because \$RUN_COVERITY != \"1\""
>>> +fi
>>> --
>>> 2.7.4
>>>
>>>
>>> 
>>> --
>>> 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
>>>
>>
>>
>
--
Check out the vibrant tech

Re: [Openvpn-devel] [PATCH] Add coverity static analysis to Travis CI config

Re: [Openvpn-devel] OVPN Interactive Service for non-admin users

Re: [Openvpn-devel] [PATCH] Add coverity static analysis to Travis CI config

Re: [Openvpn-devel] OVPN Interactive Service for non-admin users

Re: [Openvpn-devel] [PATCH] OpenSSL: remove unreachable call to SSL_CTX_get0_privatekey()

Re: [Openvpn-devel] [PATCH] v2 travis-ci: update pkcs11-helper to 1.22

Re: [Openvpn-devel] OVPN Interactive Service for non-admin users

[Openvpn-devel] OVPN Interactive Service for non-admin users

[Openvpn-devel] [PATCH] tests: Add a simple build sanity check

Re: [Openvpn-devel] [PATCH 3/3] add a test to "openvpn --help"

Re: [Openvpn-devel] [PATCH 3/3] add a test to "openvpn --help"

[Openvpn-devel] [PATCH] v2 travis-ci: update pkcs11-helper to 1.22

[Openvpn-devel] [PATCH] OpenSSL: remove unreachable call to SSL_CTX_get0_privatekey()

Re: [Openvpn-devel] [PATCH] Add coverity static analysis to Travis CI config

14 matches

Site Navigation

Mail list logo

Footer information