| Message ID | 20240209155109.110542-1-juliusz@wolfssl.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [Openvpn-devel] wolfssl: include "ssl.h" by "src/openvpn/ssl.h" | expand |
Hi,
On Fri, Feb 09, 2024 at 04:51:09PM +0100, Juliusz Sosinowicz wrote:
> Including "ssl.h" conflicts with the wolfSSL ssl.h header file. The include/wolfssl directory is included before openvpn/src. include/wolfssl needs to be included so that openvpn can pick up wolfSSL compatibility headers instead of OpenSSL headers without changing the paths.
NAK.
All our local includes are included without path, so we're not starting
"full path" for a single include now.
If this conflicts with WolfSSL, I'd be willing to consider a Makefile
patch that puts WolfSSL includes behind openvpn/src (so our ssl.h is
picked first).
gert
On Fri, Feb 09, 2024 at 04:51:09PM +0100, Juliusz Sosinowicz wrote:
> Including "ssl.h" conflicts with the wolfSSL ssl.h header file. The include/wolfssl directory is included before openvpn/src. include/wolfssl needs to be included so that openvpn can pick up wolfSSL compatibility headers instead of OpenSSL headers without changing the paths.
This breaks out-of-tree builds. Currently we only add top_buildir to CFLAGS
(for config.h) but NOT top_srcdir. For in-tree builds these are identical
so the patch might appear to work. But it breaks otherwise. Further changes
to buildsystem configuration would be required.
Regards,
Am 09.02.24 um 16:51 schrieb Juliusz Sosinowicz:
> Including "ssl.h" conflicts with the wolfSSL ssl.h header file. The include/wolfssl directory is included before openvpn/src. include/wolfssl needs to be included so that openvpn can pick up wolfSSL compatibility headers instead of OpenSSL headers without changing the paths.
Neither the ssl.h (2005, so forever) in OpenVPN nor the ssl.h in wolfssl
(2014 according to git blame) are particulary new. Why is this now a
problem when it was never before?
Arne
Hi Frank, thank you for the explanation. I did not test out-of-tree builds before submitting the patch. I'll try to implement Gert's solution and write a Makefile patch instead. Sincerely Juliusz Sosinowicz On 09/02/2024 17:39, Frank Lichtenheld wrote: > On Fri, Feb 09, 2024 at 04:51:09PM +0100, Juliusz Sosinowicz wrote: >> Including "ssl.h" conflicts with the wolfSSL ssl.h header file. The include/wolfssl directory is included before openvpn/src. include/wolfssl needs to be included so that openvpn can pick up wolfSSL compatibility headers instead of OpenSSL headers without changing the paths. > This breaks out-of-tree builds. Currently we only add top_buildir to CFLAGS > (for config.h) but NOT top_srcdir. For in-tree builds these are identical > so the patch might appear to work. But it breaks otherwise. Further changes > to buildsystem configuration would be required. > > Regards,
Hi Arne, commit 70b39f2bea9fd6e57f31e32b2041246731140cb2 has added the use of ACK_SIZE and RELIABLE_ACK_SIZE in test_ssl.c. These are defined in reliable.h which should be included through your ssl.h. Since our ssl.h is being picked up, these never get defined and make check results in the following error: test_ssl.c: In function ‘init_frame_parameters’: test_ssl.c:160:17: warning: implicit declaration of function ‘ACK_SIZE’ [-Wimplicit-function-declaration] 160 | overhead += ACK_SIZE(RELIABLE_ACK_SIZE); | ^~~~~~~~ test_ssl.c:160:26: error: ‘RELIABLE_ACK_SIZE’ undeclared (first use in this function) 160 | overhead += ACK_SIZE(RELIABLE_ACK_SIZE); | ^~~~~~~~~~~~~~~~~ Somehow, this has never come up as an issue. Sincerely Juliusz Sosinowicz On 09/02/2024 21:50, Arne Schwabe wrote: > Am 09.02.24 um 16:51 schrieb Juliusz Sosinowicz: >> Including "ssl.h" conflicts with the wolfSSL ssl.h header file. The >> include/wolfssl directory is included before openvpn/src. >> include/wolfssl needs to be included so that openvpn can pick up >> wolfSSL compatibility headers instead of OpenSSL headers without >> changing the paths. > > Neither the ssl.h (2005, so forever) in OpenVPN nor the ssl.h in > wolfssl (2014 according to git blame) are particulary new. Why is this > now a problem when it was never before? > > Arne >
Hi, On Mon, Feb 12, 2024 at 10:57:41AM +0100, Juliusz Sosinowicz wrote: > commit 70b39f2bea9fd6e57f31e32b2041246731140cb2 has added the use of > ACK_SIZE and RELIABLE_ACK_SIZE in test_ssl.c. These are defined in > reliable.h which should be included through your ssl.h. Since our ssl.h is > being picked up, these never get defined and make check results in the > following error: Seems the unit test compile flags could use a bit of shuffling to have our local include first... and that should solve it (if nothing else breaks, just test_ssl.c) gert
Hi Gert, that is the direction I went with the latest patch I sent over. Sincerely Juliusz Sosinowicz On 12/02/2024 13:53, Gert Doering wrote: > Hi, > > On Mon, Feb 12, 2024 at 10:57:41AM +0100, Juliusz Sosinowicz wrote: >> commit 70b39f2bea9fd6e57f31e32b2041246731140cb2 has added the use of >> ACK_SIZE and RELIABLE_ACK_SIZE in test_ssl.c. These are defined in >> reliable.h which should be included through your ssl.h. Since our ssl.h is >> being picked up, these never get defined and make check results in the >> following error: > Seems the unit test compile flags could use a bit of shuffling to > have our local include first... and that should solve it (if nothing > else breaks, just test_ssl.c) > > gert
diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c index 6787ea7d..e020bdcb 100644 --- a/src/openvpn/auth_token.c +++ b/src/openvpn/auth_token.c @@ -12,7 +12,7 @@ #include "auth_token.h" #include "push.h" #include "integer.h" -#include "ssl.h" +#include "src/openvpn/ssl.h" #include "ssl_verify.h" #include <inttypes.h> diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c index 3c91606b..eac81924 100644 --- a/src/openvpn/dco_linux.c +++ b/src/openvpn/dco_linux.c @@ -37,7 +37,7 @@ #include "socket.h" #include "tun.h" -#include "ssl.h" +#include "src/openvpn/ssl.h" #include "fdmisc.h" #include "multi.h" #include "ssl_verify.h" diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c index 27b6f90e..11c922c5 100644 --- a/src/openvpn/manage.c +++ b/src/openvpn/manage.c @@ -37,7 +37,7 @@ #include "otime.h" #include "integer.h" #include "misc.h" -#include "ssl.h" +#include "src/openvpn/ssl.h" #include "common.h" #include "manage.h" #include "openvpn.h" diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h index dabc5be4..3a3d1733 100644 --- a/src/openvpn/openvpn.h +++ b/src/openvpn/openvpn.h @@ -28,7 +28,7 @@ #include "options.h" #include "socket.h" #include "crypto.h" -#include "ssl.h" +#include "src/openvpn/ssl.h" #include "packet_id.h" #include "comp.h" #include "tun.h" diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 2c79a1ec..cb06063e 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -42,7 +42,7 @@ #include "run_command.h" #include "shaper.h" #include "crypto.h" -#include "ssl.h" +#include "src/openvpn/ssl.h" #include "ssl_ncp.h" #include "options.h" #include "misc.h" diff --git a/src/openvpn/ps.h b/src/openvpn/ps.h index 2fe0c4c5..21427480 100644 --- a/src/openvpn/ps.h +++ b/src/openvpn/ps.h @@ -28,7 +28,7 @@ #include "basic.h" #include "buffer.h" -#include "ssl.h" +#include "src/openvpn/ssl.h" typedef void (*post_fork_cleanup_func_t)(void *arg); diff --git a/src/openvpn/push.c b/src/openvpn/push.c index 22494340..854bf471 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -29,7 +29,7 @@ #include "push.h" #include "options.h" -#include "ssl.h" +#include "src/openvpn/ssl.h" #include "ssl_verify.h" #include "ssl_ncp.h" #include "manage.h" diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 33c86704..ac077a1c 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -54,7 +54,7 @@ #include "route.h" #include "tls_crypt.h" -#include "ssl.h" +#include "src/openvpn/ssl.h" #include "ssl_verify.h" #include "ssl_backend.h" #include "ssl_ncp.h" diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c index 975d31fa..3df2bc61 100644 --- a/src/openvpn/tls_crypt.c +++ b/src/openvpn/tls_crypt.c @@ -33,7 +33,7 @@ #include "platform.h" #include "run_command.h" #include "session_id.h" -#include "ssl.h" +#include "src/openvpn/ssl.h" #include "tls_crypt.h" diff --git a/tests/unit_tests/openvpn/mock_ssl_dependencies.c b/tests/unit_tests/openvpn/mock_ssl_dependencies.c index 9231d655..da22bbfe 100644 --- a/tests/unit_tests/openvpn/mock_ssl_dependencies.c +++ b/tests/unit_tests/openvpn/mock_ssl_dependencies.c @@ -34,7 +34,7 @@ #include <cmocka.h> -#include "ssl.h" +#include "src/openvpn/ssl.h" #include "ssl_verify.h" int diff --git a/tests/unit_tests/openvpn/test_pkcs11.c b/tests/unit_tests/openvpn/test_pkcs11.c index 81cdf882..76cb80e5 100644 --- a/tests/unit_tests/openvpn/test_pkcs11.c +++ b/tests/unit_tests/openvpn/test_pkcs11.c @@ -33,7 +33,7 @@ #include "xkey_common.h" #include "cert_data.h" #include "pkcs11.h" -#include "ssl.h" +#include "src/openvpn/ssl.h" #include <setjmp.h> #include <cmocka.h> diff --git a/tests/unit_tests/openvpn/test_ssl.c b/tests/unit_tests/openvpn/test_ssl.c index 8c1fb5b2..893bf8ec 100644 --- a/tests/unit_tests/openvpn/test_ssl.c +++ b/tests/unit_tests/openvpn/test_ssl.c @@ -44,7 +44,7 @@ #include "ssl_verify_backend.h" #include "win32.h" #include "test_common.h" -#include "ssl.h" +#include "src/openvpn/ssl.h" #include "buffer.h" #include "packet_id.h"
Including "ssl.h" conflicts with the wolfSSL ssl.h header file. The include/wolfssl directory is included before openvpn/src. include/wolfssl needs to be included so that openvpn can pick up wolfSSL compatibility headers instead of OpenSSL headers without changing the paths. Signed-off-by: Juliusz Sosinowicz <juliusz@wolfssl.com> --- src/openvpn/auth_token.c | 2 +- src/openvpn/dco_linux.c | 2 +- src/openvpn/manage.c | 2 +- src/openvpn/openvpn.h | 2 +- src/openvpn/options.c | 2 +- src/openvpn/ps.h | 2 +- src/openvpn/push.c | 2 +- src/openvpn/ssl.c | 2 +- src/openvpn/tls_crypt.c | 2 +- tests/unit_tests/openvpn/mock_ssl_dependencies.c | 2 +- tests/unit_tests/openvpn/test_pkcs11.c | 2 +- tests/unit_tests/openvpn/test_ssl.c | 2 +- 12 files changed, 12 insertions(+), 12 deletions(-)