diff mbox series

[Openvpn-devel] crypto: Fix mbedtls builds

Message ID 20220121201038.49684-1-openvpn@sf.lists.topphemmelig.net
State Rejected
Headers
Series [Openvpn-devel] crypto: Fix mbedtls builds |

Commit Message

David Sommerseth Jan. 21, 2022, 9:10 a.m. UTC 
  From: David Sommerseth <davids@openvpn.net>

With commit 544330fefedc87, the openssl_compat.h got included in
crypto.c.  This caused issues when building against mbed TLS, which this
compat layer is not targeting.

This issue is resolved by only including this header when the OpenSSL
library is in use.  The OPENSSL_FIPS macro should never be set when
compiling against the mbed TLS library.  But we check against the main
ENABLE_CRYPTO_OPENSSL macro here, in case future updates adds more
OpenSSL specific fragments.

Signed-off-by: David Sommerseth <davids@openvpn.net>
---
 src/openvpn/crypto.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Antonio Quartulli Jan. 25, 2022, 5:24 a.m. UTC | #1 
Hi,

On 21/01/2022 21:10, David Sommerseth wrote:
> From: David Sommerseth <davids@openvpn.net>
> 
> With commit 544330fefedc87, the openssl_compat.h got included in
> crypto.c.  This caused issues when building against mbed TLS, which this
> compat layer is not targeting.
> 
> This issue is resolved by only including this header when the OpenSSL
> library is in use.  The OPENSSL_FIPS macro should never be set when
> compiling against the mbed TLS library.  But we check against the main
> ENABLE_CRYPTO_OPENSSL macro here, in case future updates adds more
> OpenSSL specific fragments.
> 
> Signed-off-by: David Sommerseth <davids@openvpn.net>

I am personally NAK'ing this patch, however, after further discussion on 
IRC I can say that this was the consensus reached with David and Arne as 
well.

The idea is that having OpenSSL specific code in crypto.c is a no-go.

Like all other functionalities, we need to hide the FIPS check behind 
our SSL abstraction, so that each backend can decie what to do (mbedtls 
will probably just say "sure, go ahead").

So this patch should be dropped.


Best Regards,
diff mbox series

Patch

diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 5f6ad675..d55ca099 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -34,7 +34,9 @@ 
 #include "error.h"
 #include "integer.h"
 #include "platform.h"
+#ifdef ENABLE_CRYPTO_OPENSSL
 #include "openssl_compat.h"
+#endif
 
 #include "memdbg.h"