Re: [Openvpn-devel] [PATCH] Fix ACL_CHECK_ADD_COMPILE_FLAGS to work with clang

2019-11-14 Thread Selva Nair 
Hi,

On Thu, Nov 14, 2019 at 3:16 PM Илья Шипицин  wrote:

> Thank you for your efforts.
> As you are touching this, can you try "dist: bionic" ? It might bring
> newer compilers
>

I don't expect newer compilers on bionic break this patch. But fwiw, I've
started a travis build with dist: bionic. For results see
https://travis-ci.org/selvanair/openvpn/jobs/612099524

Selva
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Fix ACL_CHECK_ADD_COMPILE_FLAGS to work with clang

2019-11-14 Thread Selva Nair 
Hi David

Thanks for the comments

My idea was just to add -Werror right in the line above, and not extend the
> ACL_CHECK_ADD_COMPILE_FLAGS macro with another argument.
>

I'm fine with that approach as well. Let me know if you want a v2.


> I think you said it pretty well in your mail:
>
> > Darn compilers and darn -Werror
>
>
> So your change does improve Clang ... I dunno what we will do with GCC.
>

It doesn't affect gcc as gcc does error (or ignore -- see below)
unsupported options with or without -Werror. So I think we are okay. travis
build on my fork confirms that:
https://travis-ci.org/selvanair/openvpn/builds/612019849

However, gcc does have quirky behaviour when it comes to options like
-Wno-xx-yy. It just accepts them silently[*] with or without -Werror. That
works fine for us, but it does generate an error about that unsupported
option if any other error is encountered during the build.  As other errors
are anyway a show-stopper, I think we can live with that.

Selva

[*] I think their reasoning is that -Wno-xx-yy can be thus used to suppress
warnings added to newer versions without breaking builds with older ones.
But clang doesn't share that view.
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Fix ACL_CHECK_ADD_COMPILE_FLAGS to work with clang

2019-11-14 Thread David Sommerseth 
On 14/11/2019 20:40, selva.n...@gmail.com wrote:
> From: Selva Nair 
> 
> Some compilers (e.g., clang) only issue a warning for
> unsupported options unless additional flags such
> as -Werror are used to convert the warning to an error.
> 
> Add support for extra flags in ACL_CHECK_ADD_COMPILE_FLAGS.
> 
> Note: a similar approach is used in AX_CHECK_COMPILE_FLAG
> in autoconf archives.

I'm glad I didn't spend more time on thinking about this earlier today, as you
have almost the same ideas as I had :)

[...snip...]
>  
> +# Usage: ACL_CHECK_ADD_COMPILE_FLAGS(flag, [extra-flags])
> +# Some compilers require extra flags to trigger an error on
> +# unsupported options. E.g., clang requires -Werror.
>  AC_DEFUN([ACL_CHECK_ADD_COMPILE_FLAGS], [
>  old_cflags="$CFLAGS"
> -CFLAGS="$1 $CFLAGS"
> -AC_MSG_CHECKING([whether the compiler acceppts $1])
> -AC_COMPILE_IFELSE([AC_LANG_PROGRAM()], [AC_MSG_RESULT([yes])],
> +CFLAGS="$2 $1 $CFLAGS"
> +AC_MSG_CHECKING([whether the compiler accepts $1])
> +AC_COMPILE_IFELSE([AC_LANG_PROGRAM()], [AC_MSG_RESULT([yes])]; 
> CFLAGS="$1 $old_cflags",

My idea was just to add -Werror right in the line above, and not extend the
ACL_CHECK_ADD_COMPILE_FLAGS macro with another argument.

But I stopped when I realized that GCC has some quirks as well, as I needed to
ponder about this a bit.

Clang is fairly consistent:

* first with an unknown -Wno-* argument

$ clang-6.0 -o test test.c -Wall -Wno-non-existing-flag
warning: unknown warning option '-Wno-non-existing-flag'; did you mean
  '-Wno-non-virtual-dtor'? [-Wunknown-warning-option]

$ clang-6.0 -o test test.c -Wall -Wno-non-existing-flag  -Werror
error: unknown warning option '-Wno-non-existing-flag'; did you mean
  '-Wno-non-virtual-dtor'? [-Werror,-Wunknown-warning-option]

* then with an unknown -W* argument (without the no- prefix)

$ clang-6.0 -o test test.c -Wall -Wnon-existing-flag
warning: unknown warning option '-Wnon-existing-flag'; did you mean
  '-Wnon-virtual-dtor'? [-Wunknown-warning-option]

$ clang-6.0 -o test test.c -Wall -Wnon-existing-flag  -Werror
error: unknown warning option '-Wnon-existing-flag'; did you mean
  '-Wnon-virtual-dtor'? [-Werror,-Wunknown-warning-option]

This is consistent and I can live with this behavior, using -Werror to bail
out on warnings.

So lets running the same tests with GCC:

* first with an unknown -Wno-* argument

$ gcc -o test test.c -Wall -Wno-non-existing-flag
(no error nor warning)

$ gcc -o test test.c -Wall -Wno-non-existing-flag -Werror
(no error nor warning)

* then with an unknown -W* argument (without the no- prefix)

$ gcc -o test test.c -Wall -Wnon-existing-flag
gcc: error: unrecognized command line option ‘-Wnon-existing-flag’


I think you said it pretty well in your mail:

> Darn compilers and darn -Werror


So your change does improve Clang ... I dunno what we will do with GCC.


-- 
kind regards,

David Sommerseth
OpenVPN Inc




signature.asc
Description: OpenPGP digital signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Fix ACL_CHECK_ADD_COMPILE_FLAGS to work with clang

2019-11-14 Thread Илья Шипицин 
Thank you for your efforts.
As you are touching this, can you try "dist: bionic" ? It might bring newer
compilers

On Thu, Nov 14, 2019, 8:41 PM  wrote:

> From: Selva Nair 
>
> Some compilers (e.g., clang) only issue a warning for
> unsupported options unless additional flags such
> as -Werror are used to convert the warning to an error.
>
> Add support for extra flags in ACL_CHECK_ADD_COMPILE_FLAGS.
>
> Note: a similar approach is used in AX_CHECK_COMPILE_FLAG
> in autoconf archives.
>
> Signed-off-by: Selva Nair 
> ---
>
> For successful travis build result see
> https://travis-ci.org/selvanair/openvpn/builds/612019849
>
> But this + https://patchwork.openvpn.net/patch/908/
> alone does not fix the travis build as clang is not
> happy with struct initializers: Like this
>
> crypto.c:1860:31: error: suggest braces around initialization of subobject
>   [-Werror,-Wmissing-braces]
> struct key server_key = { 0 };
>
> I think clang wants {{0}} there.
>
> Darn compilers and darn -Werror :)
>
>  configure.ac | 17 ++---
>  1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 807804e5..e59bd91b 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1275,18 +1275,21 @@ if test "${enable_pkcs11}" = "yes"; then
> )
>  fi
>
> +# Usage: ACL_CHECK_ADD_COMPILE_FLAGS(flag, [extra-flags])
> +# Some compilers require extra flags to trigger an error on
> +# unsupported options. E.g., clang requires -Werror.
>  AC_DEFUN([ACL_CHECK_ADD_COMPILE_FLAGS], [
>  old_cflags="$CFLAGS"
> -CFLAGS="$1 $CFLAGS"
> -AC_MSG_CHECKING([whether the compiler acceppts $1])
> -AC_COMPILE_IFELSE([AC_LANG_PROGRAM()], [AC_MSG_RESULT([yes])],
> +CFLAGS="$2 $1 $CFLAGS"
> +AC_MSG_CHECKING([whether the compiler accepts $1])
> +AC_COMPILE_IFELSE([AC_LANG_PROGRAM()], [AC_MSG_RESULT([yes])];
> CFLAGS="$1 $old_cflags",
>  [AC_MSG_RESULT([no]); CFLAGS="$old_cflags"])]
>  )
>
> -ACL_CHECK_ADD_COMPILE_FLAGS([-Wno-stringop-truncation])
> -ACL_CHECK_ADD_COMPILE_FLAGS([-Wno-unused-function])
> -ACL_CHECK_ADD_COMPILE_FLAGS([-Wno-unused-parameter])
> -ACL_CHECK_ADD_COMPILE_FLAGS([-Wall])
> +ACL_CHECK_ADD_COMPILE_FLAGS([-Wno-stringop-truncation], [-Werror])
> +ACL_CHECK_ADD_COMPILE_FLAGS([-Wno-unused-function], [-Werror])
> +ACL_CHECK_ADD_COMPILE_FLAGS([-Wno-unused-parameter], [-Werror])
> +ACL_CHECK_ADD_COMPILE_FLAGS([-Wall], [-Werror])
>
>  if test "${enable_pedantic}" = "yes"; then
> enable_strict="yes"
> --
> 2.20.1
>
>
>
> ___
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Fix ACL_CHECK_ADD_COMPILE_FLAGS to work with clang

2019-11-14 Thread selva . nair 
From: Selva Nair 

Some compilers (e.g., clang) only issue a warning for
unsupported options unless additional flags such
as -Werror are used to convert the warning to an error.

Add support for extra flags in ACL_CHECK_ADD_COMPILE_FLAGS.

Note: a similar approach is used in AX_CHECK_COMPILE_FLAG
in autoconf archives.

Signed-off-by: Selva Nair 
---

For successful travis build result see
https://travis-ci.org/selvanair/openvpn/builds/612019849

But this + https://patchwork.openvpn.net/patch/908/
alone does not fix the travis build as clang is not
happy with struct initializers: Like this

crypto.c:1860:31: error: suggest braces around initialization of subobject
  [-Werror,-Wmissing-braces]
struct key server_key = { 0 };

I think clang wants {{0}} there.

Darn compilers and darn -Werror :)

 configure.ac | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/configure.ac b/configure.ac
index 807804e5..e59bd91b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1275,18 +1275,21 @@ if test "${enable_pkcs11}" = "yes"; then
)
 fi
 
+# Usage: ACL_CHECK_ADD_COMPILE_FLAGS(flag, [extra-flags])
+# Some compilers require extra flags to trigger an error on
+# unsupported options. E.g., clang requires -Werror.
 AC_DEFUN([ACL_CHECK_ADD_COMPILE_FLAGS], [
 old_cflags="$CFLAGS"
-CFLAGS="$1 $CFLAGS"
-AC_MSG_CHECKING([whether the compiler acceppts $1])
-AC_COMPILE_IFELSE([AC_LANG_PROGRAM()], [AC_MSG_RESULT([yes])],
+CFLAGS="$2 $1 $CFLAGS"
+AC_MSG_CHECKING([whether the compiler accepts $1])
+AC_COMPILE_IFELSE([AC_LANG_PROGRAM()], [AC_MSG_RESULT([yes])]; CFLAGS="$1 
$old_cflags",
 [AC_MSG_RESULT([no]); CFLAGS="$old_cflags"])]
 )
 
-ACL_CHECK_ADD_COMPILE_FLAGS([-Wno-stringop-truncation])
-ACL_CHECK_ADD_COMPILE_FLAGS([-Wno-unused-function])
-ACL_CHECK_ADD_COMPILE_FLAGS([-Wno-unused-parameter])
-ACL_CHECK_ADD_COMPILE_FLAGS([-Wall])
+ACL_CHECK_ADD_COMPILE_FLAGS([-Wno-stringop-truncation], [-Werror])
+ACL_CHECK_ADD_COMPILE_FLAGS([-Wno-unused-function], [-Werror])
+ACL_CHECK_ADD_COMPILE_FLAGS([-Wno-unused-parameter], [-Werror])
+ACL_CHECK_ADD_COMPILE_FLAGS([-Wall], [-Werror])
 
 if test "${enable_pedantic}" = "yes"; then
enable_strict="yes"
-- 
2.20.1



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Support for wolfSSL with OpenVPN v2.4.8

2019-11-14 Thread David Sommerseth 
On 14/11/2019 11:22, Juliusz Sosinowicz wrote:
> From: David Garske 
> 
> wolfSSL:
> 
> Support added in: https://github.com/wolfSSL/wolfssl/pull/2503
> 
> ```sh
> git clone https://github.com/wolfSSL/wolfssl.git
> cd wolfssl
> ./autogen.sh
> ./configure --enable-opensslall --enable-des3 --enable-crl --enable-certgen 
> --enable-certext --enable-aesctr --enable-sessioncerts 
> CFLAGS="-DWOLFSSL_DES_ECB -DHAVE_EX_DATA"
> make
> sudo make install
> ```
> 
> OpenVPN:
> 
> ```sh
> autoreconf -i -v -f
> ./configure --with-crypto-library=wolfssl
> make
> make check
> sudo make install
> ```

NAK.

This patch adds a new feature to the 2.4 branch.  We don't really want to do
that, especially if the change is intrusive (13 files changed, 108 insertions
<< that is intrusive).  WolfSSL support will at best see the light in the
coming 2.5 release (At the hackathon we aim for late 2020Q1 or 2020Q2)

In previous rounds we have asked a lot of questions; there has been no real
responses to those.  This has not even been touched in the relation to this 
patch.

One good thing I do see, is that it seems to try to use an OpenSSL support
layer in WolfSSL - which is good.  But then I wonder why we see additions like
this all over.

+#ifdef ENABLE_CRYPTO_WOLFSSL
+#include 
+#endif

In addition, the change in configure.ac with all the AC_DEFINE lines, tagged
with "Emulate X since these are defined as macros" is also making a lot of
mess.

And then comes the most critical point to all of this:  Who will maintain
WolfSSL support in OpenVPN once this has been applied?  What kind of
commitment will we see from the WolfSSL organization?

The OpenVPN developers community will have an IRC meeting next Thursday (Nov
21 @ 20:00 CET, #openvpn-meeting on FreeNode [1]).  I strongly recommend you
to attend this meeting to follow up your request.


[1] You need to have your nick registered to join



-- 
kind regards,

David Sommerseth
OpenVPN Inc




signature.asc
Description: OpenPGP digital signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Support for wolfSSL with OpenVPN v2.4.8

2019-11-14 Thread Juliusz Sosinowicz 
From: David Garske 

wolfSSL:

Support added in: https://github.com/wolfSSL/wolfssl/pull/2503

```sh
git clone https://github.com/wolfSSL/wolfssl.git
cd wolfssl
./autogen.sh
./configure --enable-opensslall --enable-des3 --enable-crl --enable-certgen 
--enable-certext --enable-aesctr --enable-sessioncerts 
CFLAGS="-DWOLFSSL_DES_ECB -DHAVE_EX_DATA"
make
sudo make install
```

OpenVPN:

```sh
autoreconf -i -v -f
./configure --with-crypto-library=wolfssl
make
make check
sudo make install
```
---
 configure.ac   | 77 +-
 include/openvpn-plugin.h.in|  3 +
 sample/sample-config-files/loopback-client |  1 +
 sample/sample-config-files/loopback-server |  1 +
 src/openvpn/crypto.c   |  2 +-
 src/openvpn/crypto_openssl.c   |  3 +
 src/openvpn/crypto_openssl.h   |  3 +
 src/openvpn/cryptoapi.c|  4 ++
 src/openvpn/openssl_compat.h   |  5 ++
 src/openvpn/ssl_openssl.c  |  3 +
 src/openvpn/ssl_openssl.h  |  3 +
 src/openvpn/ssl_verify_openssl.c   |  3 +
 src/openvpn/ssl_verify_openssl.h   |  3 +
 13 files changed, 108 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index e45ce2f3..63cf3001 100644
--- a/configure.ac
+++ b/configure.ac
@@ -283,10 +283,10 @@ AC_ARG_WITH(
 
 AC_ARG_WITH(
[crypto-library],
-   [AS_HELP_STRING([--with-crypto-library=library], [build with the given 
crypto library, TYPE=openssl|mbedtls @<:@default=openssl@:>@])],
+   [AS_HELP_STRING([--with-crypto-library=library], [build with the given 
crypto library, TYPE=openssl|mbedtls|wolfssl @<:@default=openssl@:>@])],
[
case "${withval}" in
-   openssl|mbedtls) ;;
+   openssl|mbedtls|wolfssl) ;;
*) AC_MSG_ERROR([bad value ${withval} for 
--with-crypto-library]) ;;
esac
],
@@ -1028,6 +1028,79 @@ elif test "${enable_crypto}" = "yes" -a 
"${with_crypto_library}" = "mbedtls"; th
AC_DEFINE([ENABLE_CRYPTO_MBEDTLS], [1], [Use mbed TLS library])
CRYPTO_CFLAGS="${MBEDTLS_CFLAGS}"
CRYPTO_LIBS="${MBEDTLS_LIBS}"
+
+elif test "${enable_crypto}" = "yes" -a "${with_crypto_library}" = "wolfssl"; 
then
+   AC_ARG_VAR([WOLFSSL_CFLAGS], [C compiler flags for wolfssl])
+   AC_ARG_VAR([WOLFSSL_LIBS], [linker flags for wolfssl])
+   AC_ARG_VAR([WOLFSSL_DIR], [Path to the wolfssl directory 
@<:@default=/usr/local/include/wolfssl@:>@])
+   if test -n "${WOLFSSL_DIR}"; then
+   wolfssldir="${WOLFSSL_DIR}"
+   else
+   wolfssldir="/usr/local/include/wolfssl"
+   fi
+
+   saved_CFLAGS="${CFLAGS}"
+   saved_LIBS="${LIBS}"
+
+   if test -z "${WOLFSSL_CFLAGS}" -a -z "${WOLFSSL_LIBS}"; then
+   # if the user did not explicitly specify flags, try to 
autodetect
+   LIBS="${LIBS} -lwolfssl -lm -pthread"
+   AC_CHECK_LIB(
+   [wolfssl],
+   [wolfSSL_Init],
+   [],
+   [AC_MSG_ERROR([Could not link wolfSSL library.])]
+   )
+   AC_CHECK_HEADER([wolfssl/options.h],,[AC_MSG_ERROR([wolfSSL 
header wolfssl/options.h not found!])])
+   fi
+
+   AC_DEFINE([HAVE_HMAC_CTX_NEW], [1], [Emulate AC_CHECK_FUNCS since these 
are defined as macros])
+   AC_DEFINE([HAVE_HMAC_CTX_FREE], [1], [Emulate AC_CHECK_FUNCS since 
these are defined as macros])
+   AC_DEFINE([HAVE_HMAC_CTX_RESET], [1], [Emulate AC_CHECK_FUNCS since 
these are defined as macros])
+   AC_DEFINE([HAVE_EVP_MD_CTX_NEW], [1], [Emulate AC_CHECK_FUNCS since 
these are defined as macros])
+   AC_DEFINE([HAVE_EVP_MD_CTX_FREE], [1], [Emulate AC_CHECK_FUNCS since 
these are defined as macros])
+   AC_DEFINE([HAVE_EVP_MD_CTX_RESET], [1], [Emulate AC_CHECK_FUNCS since 
these are defined as macros])
+   AC_DEFINE([HAVE_OPENSSL_VERSION], [1], [Emulate AC_CHECK_FUNCS since 
these are defined as macros])
+   AC_DEFINE([HAVE_SSL_CTX_GET_DEFAULT_PASSWD_CB], [1], [Emulate 
AC_CHECK_FUNCS since these are defined as macros])
+   AC_DEFINE([HAVE_SSL_CTX_GET_DEFAULT_PASSWD_CB_USERDATA], [1], [Emulate 
AC_CHECK_FUNCS since these are defined as macros])
+   AC_DEFINE([HAVE_SSL_CTX_SET_SECURITY_LEVEL], [1], [Emulate 
AC_CHECK_FUNCS since these are defined as macros])
+   AC_DEFINE([HAVE_X509_GET0_PUBKEY], [1], [Emulate AC_CHECK_FUNCS since 
these are defined as macros])
+   AC_DEFINE([HAVE_X509_STORE_GET0_OBJECTS], [1], [Emulate AC_CHECK_FUNCS 
since these are defined as macros])
+   AC_DEFINE([HAVE_X509_OBJECT_FREE], [1], [Emulate AC_CHECK_FUNCS since 
these are defined as macros])
+   AC_DEFINE([HAVE_X509_OBJECT_GET_TYPE], [1], [Emulate AC_CHECK_FUNCS 
since these are defined as macros])
+   AC_DEFINE([HAVE_EVP_PKEY_ID], [1], [Emulate