travis-ci: update pkcs11-helper to 1.22

---
 .travis.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

.travis.yml +0 -0

index fc98a4d0..0b531529 100644
@@ -15,10 +15,10 @@ env:
     - TAP_WINDOWS_VERSION=9.21.2
     - LZO_VERSION=2.10

inspired by https://www.mail-archive.com/[email protected]/msg13808.html

---
 tests/Makefile.am |  2 +-
 tests/t_usage.sh  | 29 +++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)
 create mode 100755 tests/t_usage.sh

tests/Makefile.am +1 -1

index 0795680c..7306d1d1 100644
@@ -14,7 +14,7 @@ MAINTAINERCLEANFILES = \
 
 SUBDIRS = unit_tests
 
-test_scripts = t_client.sh
+test_scripts = t_client.sh t_usage.sh
 if ENABLE_CRYPTO
 test_scripts += t_lpback.sh t_cltsrv.sh
 endif

tests/t_usage.sh new file +17 -0

new file mode 100755
index 00000000..f4845468
@@ -0,0 +1,29 @@
+#!/bin/sh
+#
+# run "openvpn --help"
+# - check that openvpn did not crash
+#
+# prerequisites:
+# - openvpn binary in current directory
+#
+# inspired by https://www.mail-archive.com/[email protected]/msg13808.html
+#
+
+top_builddir="${top_builddir:-..}"
+
+if [ ! -x "${top_builddir}/src/openvpn/openvpn" ]
+then
+    echo "no (executable) openvpn binary in current build tree. FAIL." >&2
+    exit 1

inspired by https://www.mail-archive.com/[email protected]...

---
 tests/Makefile.am |  2 +-
 tests/t_usage.sh  | 29 +++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)
 create mode 100755 tests/t_usage.sh

tests/Makefile.am +1 -1

index 0795680c..7306d1d1 100644
@@ -14,7 +14,7 @@ MAINTAINERCLEANFILES = \
 
 SUBDIRS = unit_tests
 
-test_scripts = t_client.sh
+test_scripts = t_client.sh t_usage.sh
 if ENABLE_CRYPTO
 test_scripts += t_lpback.sh t_cltsrv.sh
 endif

tests/t_usage.sh new file +0 -0

new file mode 100755
index 00000000..f4845468

tests/t_usage.sh new file +7 -0

+#
+
+top_builddir="${top_builddir:-..}"
+
+if [ ! -x "${top_builddir}/src/openvpn/openvpn" ]
+then
+    echo "no (executable) openvpn binary in current build tree. FAIL."

&2

tests/t_usage.sh new file +1 -0

+    exit 1

tests/t_usage.sh line 27

Quoted 6 lines

+"${top_builddir}/src/openvpn/openvpn" --help >/dev/null

tests/t_usage.sh new file +5 -0

+
+if [ $? -ne 1 ]
+then
+   exit_code=1
+fi

This check is odd. Where the --help option is parsed, it calls usage(), usage_small(). And those functions will always call:

Sometimes it segfaults.

openvpn_exit(OPENVPN_EXIT_STATUS_USAGE);

And OPENVPN_EXIT_STATUS_USAGE is defined as:

error.h:#define OPENVPN_EXIT_STATUS_USAGE 1

I would rather recommend you to grep for some information you expect to be listed in --help and check if that was found instead.

I tried to implement it with cmocka. However, I gave up

    For example:

    $ ./openvpn --help | grep -- ^-- | wc -l
    238

[...]

Quoted 7 lines

Quoted 6 lines

+"${top_builddir}/src/openvpn/openvpn" --help >/dev/null

tests/t_usage.sh new file +5 -0

+
+if [ $? -ne 1 ]
+then
+   exit_code=1
+fi

Quoted 1 line

This check is odd. Where the --help option is parsed, it calls

usage(),

Quoted 3 lines

usage_small(). And those functions will always call:

Sometimes it segfaults.

That's no real argument, see below.

Quoted 8 lines

openvpn_exit(OPENVPN_EXIT_STATUS_USAGE);

And OPENVPN_EXIT_STATUS_USAGE is defined as:

error.h:#define OPENVPN_EXIT_STATUS_USAGE 1

I would rather recommend you to grep for some information you expect

to

Quoted 4 lines

be listed in --help and check if that was found instead.

I tried to implement it with cmocka. However, I gave up

I didn't say this should be implemented in C and CMocka ... hence the example below. And that check below would implicitly account for SEGV scenarios as well.

Quoted 4 lines

    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

--
kind regards,

David Sommerseth
OpenVPN Technologies, Inc

Quoted 4 lines

    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

[...]

Quoted 15 lines

Quoted 4 lines

    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

Quoted 2 lines

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

2017-08-09 14:31 GMT+05:00 David Sommerseth
<[email protected]...
<mailto:[email protected]...>>:

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.

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

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

.travis.yml +1 -1

index db90e03a..fc98a4d0 100644
@@ -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"

.travis/build-deps.sh +1 -1

index 9cc18584..7d8f911b 100755
@@ -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
 }