| Message ID | 20200207174818.21050-1-juliusz@wolfssl.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [Openvpn-devel] Support for wolfSSL with OpenVPN master branch | expand |
Hi Juliusz, please send patches out of a git tree, coming from a git commit with "git commit -s", and having a somewhat relevant commit message. Besides this, please do not > --- a/sample/sample-config-files/loopback-client > +++ b/sample/sample-config-files/loopback-client > @@ -25,3 +25,4 @@ tls-auth sample-keys/ta.key 1 > cipher AES-256-GCM > ping 1 > inactive 120 10000000 > +cipher AES-256-CBC ... modify the sample config files (and *if* you do, do not just add a second cipher line, which will confuse users quite a bit). If WolfSSL does not support GCM, this needs to be documented, but our sample config files contain the recommended cipher for the existing crypto systems, and this is (and will continue to be for the time) GCM - faster, and lower overhead. gert
Hi Gert, thank you for your comments. My intention was not to add a second cipher line in the sample config file. I added "cipher AES-256-CBC" to an earlier version of OpenVPN when there was no cipher specified in the loopback-client and loopback-server files. After rebasing my commit onto master I didn't notice the double cipher lines in the config files. I will remove this in my next patch as wolfSSL does support GCM mode but not yet in the compatibility layer. I will add GCM support to our compatibility layer and send an updated signed-off patch with a better commit message explaining what is happening in the patch. Thanks Juliusz On 08/02/2020 09:45, Gert Doering wrote: > Hi Juliusz, > > please send patches out of a git tree, coming from a git commit with > "git commit -s", and having a somewhat relevant commit message. > > Besides this, please do not > >> --- a/sample/sample-config-files/loopback-client >> +++ b/sample/sample-config-files/loopback-client >> @@ -25,3 +25,4 @@ tls-auth sample-keys/ta.key 1 >> cipher AES-256-GCM >> ping 1 >> inactive 120 10000000 >> +cipher AES-256-CBC > ... modify the sample config files (and *if* you do, do not just add > a second cipher line, which will confuse users quite a bit). > > If WolfSSL does not support GCM, this needs to be documented, but our > sample config files contain the recommended cipher for the existing > crypto systems, and this is (and will continue to be for the time) > GCM - faster, and lower overhead. > > gert
Hi Juliusz, if wolfssl support is being introduced by means of the openssl compatibility layer, why do people need to configure OpenVPN with "./configure --with-crypto-library=wolfssl", rather than just using openssl and specifying a different path for headers/libraries? Isn't the compat layer in wolfssl operating as a drop-in replacement for openssl? Regards, On 09/02/2020 10:18, Juliusz Sosinowicz wrote: > Hi Gert, > > thank you for your comments. My intention was not to add a second cipher > line in the sample config file. I added "cipher AES-256-CBC" to an > earlier version of OpenVPN when there was no cipher specified in the > loopback-client and loopback-server files. After rebasing my commit onto > master I didn't notice the double cipher lines in the config files. I > will remove this in my next patch as wolfSSL does support GCM mode but > not yet in the compatibility layer. > > I will add GCM support to our compatibility layer and send an updated > signed-off patch with a better commit message explaining what is > happening in the patch. > > Thanks > Juliusz > > On 08/02/2020 09:45, Gert Doering wrote: >> Hi Juliusz, >> >> please send patches out of a git tree, coming from a git commit with >> "git commit -s", and having a somewhat relevant commit message. >> >> Besides this, please do not >> >>> --- a/sample/sample-config-files/loopback-client >>> +++ b/sample/sample-config-files/loopback-client >>> @@ -25,3 +25,4 @@ tls-auth sample-keys/ta.key 1 >>> cipher AES-256-GCM >>> ping 1 >>> inactive 120 10000000 >>> +cipher AES-256-CBC >> ... modify the sample config files (and *if* you do, do not just add >> a second cipher line, which will confuse users quite a bit). >> >> If WolfSSL does not support GCM, this needs to be documented, but our >> sample config files contain the recommended cipher for the existing >> crypto systems, and this is (and will continue to be for the time) >> GCM - faster, and lower overhead. >> >> gert > > > _______________________________________________ > Openvpn-devel mailing list > Openvpn-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/openvpn-devel >
Hi, On Sun, Feb 09, 2020 at 10:44:48AM +0100, Antonio Quartulli wrote: > if wolfssl support is being introduced by means of the openssl > compatibility layer, why do people need to configure OpenVPN with > "./configure --with-crypto-library=wolfssl", rather than just using > openssl and specifying a different path for headers/libraries? > > Isn't the compat layer in wolfssl operating as a drop-in replacement for > openssl? This question has been asked before and answered :-) - most of the compat functions seem to be implemented as macros, which our configure will not find. So, configure needs to explicitely define what is there and what not. I do not like the extra include very much, but that seems to be hard to avoid with the current WolfSSL header file setup. gert
Hi, On 09/02/2020 10:50, Gert Doering wrote: > Hi, > > On Sun, Feb 09, 2020 at 10:44:48AM +0100, Antonio Quartulli wrote: >> if wolfssl support is being introduced by means of the openssl >> compatibility layer, why do people need to configure OpenVPN with >> "./configure --with-crypto-library=wolfssl", rather than just using >> openssl and specifying a different path for headers/libraries? >> >> Isn't the compat layer in wolfssl operating as a drop-in replacement for >> openssl? > > This question has been asked before and answered :-) - most of the > compat functions seem to be implemented as macros, which our configure > will not find. So, configure needs to explicitely define what is there > and what not. > > I do not like the extra include very much, but that seems to be hard > to avoid with the current WolfSSL header file setup. > ouch, thanks for the reminder :-)
Hi Antonio,Gert is correct, our compatibility layer is a set of functions in wolfSSL which emulate the OpenSSL API. These functions are then macro defined to have the same names as the OpenSSL functions. The configure script needs to know where the wolfSSL headers are and that it should link against the wolfSSL binary, not OpenSSL. This is the reason for the configure script changes.Sincerely Juliusz -------- Original message --------From: Antonio Quartulli <a@unstable.cc> Date: 09/02/2020 10:52 (GMT+01:00) To: Gert Doering <gert@greenie.muc.de> Cc: Juliusz Sosinowicz <juliusz@wolfssl.com>, openvpn-devel@lists.sourceforge.net Subject: Re: [Openvpn-devel] [PATCH] Support for wolfSSL with OpenVPN master branch Hi,On 09/02/2020 10:50, Gert Doering wrote:> Hi,> > On Sun, Feb 09, 2020 at 10:44:48AM +0100, Antonio Quartulli wrote:>> if wolfssl support is being introduced by means of the openssl>> compatibility layer, why do people need to configure OpenVPN with>> "./configure --with-crypto-library=wolfssl", rather than just using>> openssl and specifying a different path for headers/libraries?>>>> Isn't the compat layer in wolfssl operating as a drop-in replacement for>> openssl?> > This question has been asked before and answered :-) - most of the> compat functions seem to be implemented as macros, which our configure> will not find. So, configure needs to explicitely define what is there> and what not.> > I do not like the extra include very much, but that seems to be hard > to avoid with the current WolfSSL header file setup.> ouch, thanks for the reminder :-)-- Antonio Quartulli <html><head><meta http-equiv="Content-Type" content="text/html; charset=UTF-8"></head><body dir="auto">Hi Antonio,<div dir="auto"><br></div><div dir="auto">Gert is correct, our compatibility layer is a set of functions in wolfSSL which emulate the OpenSSL API. These functions are then macro defined to have the same names as the OpenSSL functions. The configure script needs to know where the wolfSSL headers are and that it should link against the wolfSSL binary, not OpenSSL. This is the reason for the configure script changes.</div><div dir="auto"><br></div><div dir="auto">Sincerely </div><div dir="auto">Juliusz </div><div><br></div><div style="font-size:100%;color:#000000" dir="auto"><!-- originalMessage --><div>-------- Original message --------</div><div>From: Antonio Quartulli <a@unstable.cc> </div><div>Date: 09/02/2020 10:52 (GMT+01:00) </div><div>To: Gert Doering <gert@greenie.muc.de> </div><div>Cc: Juliusz Sosinowicz <juliusz@wolfssl.com>, openvpn-devel@lists.sourceforge.net </div><div>Subject: Re: [Openvpn-devel] [PATCH] Support for wolfSSL with OpenVPN master branch </div><div><br></div></div>Hi,<br><br>On 09/02/2020 10:50, Gert Doering wrote:<br>> Hi,<br>> <br>> On Sun, Feb 09, 2020 at 10:44:48AM +0100, Antonio Quartulli wrote:<br>>> if wolfssl support is being introduced by means of the openssl<br>>> compatibility layer, why do people need to configure OpenVPN with<br>>> "./configure --with-crypto-library=wolfssl", rather than just using<br>>> openssl and specifying a different path for headers/libraries?<br>>><br>>> Isn't the compat layer in wolfssl operating as a drop-in replacement for<br>>> openssl?<br>> <br>> This question has been asked before and answered :-) - most of the<br>> compat functions seem to be implemented as macros, which our configure<br>> will not find. So, configure needs to explicitely define what is there<br>> and what not.<br>> <br>> I do not like the extra include very much, but that seems to be hard <br>> to avoid with the current WolfSSL header file setup.<br>> <br><br>ouch, thanks for the reminder :-)<br><br><br><br>-- <br>Antonio Quartulli<br></body></html>
Am 09.02.20 um 12:08 schrieb Juliusz Sosinowicz: > Hi Antonio, > > Gert is correct, our compatibility layer is a set of functions in > wolfSSL which emulate the OpenSSL API. These functions are then macro > defined to have the same names as the OpenSSL functions. The configure > script needs to know where the wolfSSL headers are and that it should > link against the wolfSSL binary, not OpenSSL. This is the reason for the > configure script changes. But OpenSSL also its opensslconf.h header and does not force you to important into each file it uses. I find that argument very weak. If it is something that is essential for your compat layer then you should probably include it in the top of each compatibility header. Arne
diff --git a/configure.ac b/configure.ac index 98fd39ce..564f21a6 100644 --- a/configure.ac +++ b/configure.ac @@ -276,10 +276,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 ], @@ -1029,6 +1029,79 @@ elif test "${with_crypto_library}" = "mbedtls"; then AC_DEFINE([ENABLE_CRYPTO_MBEDTLS], [1], [Use mbed TLS library]) CRYPTO_CFLAGS="${MBEDTLS_CFLAGS}" CRYPTO_LIBS="${MBEDTLS_LIBS}" + +elif test "${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 AC_CHECK_FUNCS since these are defined as macros]) + AC_DEFINE([HAVE_EVP_PKEY_GET0_RSA], [1], [Emulate AC_CHECK_FUNCS since these are defined as macros]) + AC_DEFINE([HAVE_EVP_PKEY_GET0_DSA], [1], [Emulate AC_CHECK_FUNCS since these are defined as macros]) + AC_DEFINE([HAVE_EVP_PKEY_GET0_EC_KEY], [1], [Emulate AC_CHECK_FUNCS since these are defined as macros]) + AC_DEFINE([HAVE_RSA_SET_FLAGS], [1], [Emulate AC_CHECK_FUNCS since these are defined as macros]) + AC_DEFINE([HAVE_RSA_BITS], [1], [Emulate AC_CHECK_FUNCS since these are defined as macros]) + AC_DEFINE([HAVE_RSA_GET0_KEY], [1], [Emulate AC_CHECK_FUNCS since these are defined as macros]) + AC_DEFINE([HAVE_RSA_SET0_KEY], [1], [Emulate AC_CHECK_FUNCS since these are defined as macros]) + AC_DEFINE([HAVE_DSA_GET0_PQG], [1], [Emulate AC_CHECK_FUNCS since these are defined as macros]) + AC_DEFINE([HAVE_DSA_BITS], [1], [Emulate AC_CHECK_FUNCS since these are defined as macros]) + AC_DEFINE([HAVE_RSA_METH_NEW], [1], [Emulate AC_CHECK_FUNCS since these are defined as macros]) + AC_DEFINE([HAVE_RSA_METH_FREE], [1], [Emulate AC_CHECK_FUNCS since these are defined as macros]) + AC_DEFINE([HAVE_RSA_METH_SET_PUB_ENC], [1], [Emulate AC_CHECK_FUNCS since these are defined as macros]) + AC_DEFINE([HAVE_RSA_METH_SET_PUB_DEC], [1], [Emulate AC_CHECK_FUNCS since these are defined as macros]) + AC_DEFINE([HAVE_RSA_METH_SET_PRIV_ENC], [1], [Emulate AC_CHECK_FUNCS since these are defined as macros]) + AC_DEFINE([HAVE_RSA_METH_SET_PRIV_DEC], [1], [Emulate AC_CHECK_FUNCS since these are defined as macros]) + AC_DEFINE([HAVE_RSA_METH_SET_INIT], [1], [Emulate AC_CHECK_FUNCS since these are defined as macros]) + AC_DEFINE([HAVE_RSA_METH_SET_SIGN], [1], [Emulate AC_CHECK_FUNCS since these are defined as macros]) + AC_DEFINE([HAVE_RSA_METH_SET_FINISH], [1], [Emulate AC_CHECK_FUNCS since these are defined as macros]) + AC_DEFINE([HAVE_RSA_METH_SET0_APP_DATA], [1], [Emulate AC_CHECK_FUNCS since these are defined as macros]) + AC_DEFINE([HAVE_RSA_METH_GET0_APP_DATA], [1], [Emulate AC_CHECK_FUNCS since these are defined as macros]) + AC_DEFINE([HAVE_EC_GROUP_ORDER_BITS], [1], [Emulate AC_CHECK_FUNCS since these are defined as macros]) + + have_crypto_aead_modes="no" + have_crypto="yes" + + WOLFSSL_CFLAGS="${WOLFSSL_CFLAGS} -I${wolfssldir}" + CFLAGS="${WOLFSSL_CFLAGS} ${CFLAGS}" + LIBS="${WOLFSSL_LIBS} ${LIBS}" + AC_DEFINE([ENABLE_CRYPTO_WOLFSSL], [1], [Use wolfSSL crypto library]) + AC_DEFINE([ENABLE_CRYPTO_OPENSSL], [1], [Use wolfSSL openssl compatibility layer]) + CRYPTO_CFLAGS="${WOLFSSL_CFLAGS}" + CRYPTO_LIBS="${WOLFSSL_LIBS}" else AC_MSG_ERROR([Invalid crypto library: ${with_crypto_library}]) fi diff --git a/include/openvpn-plugin.h.in b/include/openvpn-plugin.h.in index 103844f7..f6d116da 100644 --- a/include/openvpn-plugin.h.in +++ b/include/openvpn-plugin.h.in @@ -33,6 +33,9 @@ typedef mbedtls_x509_crt openvpn_x509_cert_t; #endif #else /* ifdef ENABLE_CRYPTO_MBEDTLS */ +#ifdef ENABLE_CRYPTO_WOLFSSL +#include <wolfssl/options.h> +#endif #include <openssl/x509.h> #ifndef __OPENVPN_X509_CERT_T_DECLARED #define __OPENVPN_X509_CERT_T_DECLARED diff --git a/sample/sample-config-files/loopback-client b/sample/sample-config-files/loopback-client index 1734aa8b..e0f5b23e 100644 --- a/sample/sample-config-files/loopback-client +++ b/sample/sample-config-files/loopback-client @@ -25,3 +25,4 @@ tls-auth sample-keys/ta.key 1 cipher AES-256-GCM ping 1 inactive 120 10000000 +cipher AES-256-CBC diff --git a/sample/sample-config-files/loopback-server b/sample/sample-config-files/loopback-server index 58daeb56..7abc0213 100644 --- a/sample/sample-config-files/loopback-server +++ b/sample/sample-config-files/loopback-server @@ -25,3 +25,4 @@ tls-auth sample-keys/ta.key 0 cipher AES-256-GCM ping 1 inactive 120 10000000 +cipher AES-256-CBC diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index 65e789ed..8b8f97ea 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -428,7 +428,7 @@ openvpn_decrypt_aead(struct buffer *buf, struct buffer work, tag_ptr = BPTR(buf); ASSERT(buf_advance(buf, tag_size)); dmsg(D_PACKET_CONTENT, "DECRYPT MAC: %s", format_hex(tag_ptr, tag_size, 0, &gc)); -#if defined(ENABLE_CRYPTO_OPENSSL) && OPENSSL_VERSION_NUMBER < 0x10001040L +#if defined(ENABLE_CRYPTO_OPENSSL) && OPENSSL_VERSION_NUMBER < 0x10001040L && !defined(ENABLE_CRYPTO_WOLFSSL) /* OpenSSL <= 1.0.1c bug requires set tag before processing ciphertext */ if (!EVP_CIPHER_CTX_ctrl(ctx->cipher, EVP_CTRL_GCM_SET_TAG, tag_size, tag_ptr)) { diff --git a/src/openvpn/crypto_openssl.h b/src/openvpn/crypto_openssl.h index 64754480..db0c4ccd 100644 --- a/src/openvpn/crypto_openssl.h +++ b/src/openvpn/crypto_openssl.h @@ -29,6 +29,9 @@ #ifndef CRYPTO_OPENSSL_H_ #define CRYPTO_OPENSSL_H_ +#ifdef ENABLE_CRYPTO_WOLFSSL +#include <wolfssl/options.h> +#endif #include <openssl/evp.h> #include <openssl/hmac.h> #include <openssl/md5.h> diff --git a/src/openvpn/cryptoapi.c b/src/openvpn/cryptoapi.c index 2f2eee77..5618ad1a 100644 --- a/src/openvpn/cryptoapi.c +++ b/src/openvpn/cryptoapi.c @@ -39,6 +39,10 @@ #ifdef ENABLE_CRYPTOAPI +#ifdef ENABLE_CRYPTO_WOLFSSL +#error wolfSSL does not support CryptoAPI +#endif + #include <openssl/ssl.h> #include <openssl/evp.h> #include <openssl/err.h> diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h index 4ac8f24d..c8ce9933 100644 --- a/src/openvpn/openssl_compat.h +++ b/src/openvpn/openssl_compat.h @@ -42,6 +42,10 @@ #include "buffer.h" +#ifdef ENABLE_CRYPTO_WOLFSSL +#include <wolfssl/options.h> +#include <openssl/err.h> +#endif #include <openssl/rsa.h> #include <openssl/ssl.h> #include <openssl/x509.h> diff --git a/src/openvpn/ssl_openssl.h b/src/openvpn/ssl_openssl.h index 835878c3..1ec6944d 100644 --- a/src/openvpn/ssl_openssl.h +++ b/src/openvpn/ssl_openssl.h @@ -29,6 +29,9 @@ #ifndef SSL_OPENSSL_H_ #define SSL_OPENSSL_H_ +#ifdef ENABLE_CRYPTO_WOLFSSL +#include <wolfssl/options.h> +#endif #include <openssl/ssl.h> /** diff --git a/src/openvpn/ssl_verify_openssl.h b/src/openvpn/ssl_verify_openssl.h index 118e16fc..1707a76e 100644 --- a/src/openvpn/ssl_verify_openssl.h +++ b/src/openvpn/ssl_verify_openssl.h @@ -30,6 +30,9 @@ #ifndef SSL_VERIFY_OPENSSL_H_ #define SSL_VERIFY_OPENSSL_H_ +#ifdef ENABLE_CRYPTO_WOLFSSL +#include <wolfssl/options.h> +#endif #include <openssl/x509.h> #ifndef __OPENVPN_X509_CERT_T_DECLARED