| Message ID | 20220119182126.56880-1-openvpn@sf.lists.topphemmelig.net |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel,v3] crypto: Fix OPENSSL_FIPS enabled builds | expand |
Acked-by: Gert Doering <gert@greenie.muc.de> We'll add build tests on Fedora / CentOS, as soon as September brings the new buildbot infrastructure... so we get the "looks like FIPS but isn't" stuff tested as well. (cipher) changed as instructed on IRC. Your patch has been applied to the master branch. commit 544330fefedc87a74b4e17e105ad9151b8ad1dc9 Author: David Sommerseth Date: Wed Jan 19 19:21:26 2022 +0100 crypto: Fix OPENSSL_FIPS enabled builds Signed-off-by: David Sommerseth <davids@openvpn.net> Acked-by: Gert Doering <gert@greenie.muc.de> Message-Id: <20220119182126.56880-1-openvpn@sf.lists.topphemmelig.net> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23570.html Signed-off-by: Gert Doering <gert@greenie.muc.de> -- kind regards, Gert Doering
Hi, On Wed, Jan 19, 2022 at 07:21:26PM +0100, David Sommerseth wrote: > index 5626e2b6..eb0b1254 100644 > --- a/src/openvpn/crypto.c > +++ b/src/openvpn/crypto.c > @@ -34,6 +34,7 @@ > #include "error.h" > #include "integer.h" > #include "platform.h" > +#include "openssl_compat.h" > > #include "memdbg.h" This breaks compilation for mbedtls builds, depending on which version of OpenSSL happens to be installed on the system (if any). In this particular case, mbedtls build with a system openssl of 0.9.8, it blows up with In file included from crypto.c:37: openssl_compat.h: In function 'SSL_CTX_get_min_proto_version': openssl_compat.h:635: error: 'SSL_OP_NO_TLSv1_1' undeclared (first use in this (and more of this) which is unsurprising - it's not supposed to pull in these headers in the first place. I wondered about this header, but did not wonder enough to verify that it indeed must not be included for non-openssl-builds. gert
Hi On Fri, Jan 21, 2022 at 12:10 PM Gert Doering <gert@greenie.muc.de> wrote: > Hi, > > On Wed, Jan 19, 2022 at 07:21:26PM +0100, David Sommerseth wrote: > > index 5626e2b6..eb0b1254 100644 > > --- a/src/openvpn/crypto.c > > +++ b/src/openvpn/crypto.c > > @@ -34,6 +34,7 @@ > > #include "error.h" > > #include "integer.h" > > #include "platform.h" > > +#include "openssl_compat.h" > > > > #include "memdbg.h" > > This breaks compilation for mbedtls builds, depending on which version > of OpenSSL happens to be installed on the system (if any). > > In this particular case, mbedtls build with a system openssl of 0.9.8, > it blows up with > > In file included from crypto.c:37: > openssl_compat.h: In function 'SSL_CTX_get_min_proto_version': > openssl_compat.h:635: error: 'SSL_OP_NO_TLSv1_1' undeclared > (first use in this > > (and more of this) > > which is unsurprising - it's not supposed to pull in these headers > in the first place. > Looking back at it, the patch and the problem it's trying to solve are both misplaced in crypto.c. That file should be ssl-lib agnostic and openssl related bits should go to crypto_openssl.c... I think we need to remove that OPENSSL_FIPS clause and think of providing that extra info somewhere else if possible. Selva <div dir="ltr"><div>Hi</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jan 21, 2022 at 12:10 PM Gert Doering <<a href="mailto:gert@greenie.muc.de">gert@greenie.muc.de</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi,<br> <br> On Wed, Jan 19, 2022 at 07:21:26PM +0100, David Sommerseth wrote:<br> > index 5626e2b6..eb0b1254 100644<br> > --- a/src/openvpn/crypto.c<br> > +++ b/src/openvpn/crypto.c<br> > @@ -34,6 +34,7 @@<br> > #include "error.h"<br> > #include "integer.h"<br> > #include "platform.h"<br> > +#include "openssl_compat.h"<br> > <br> > #include "memdbg.h"<br> <br> This breaks compilation for mbedtls builds, depending on which version<br> of OpenSSL happens to be installed on the system (if any).<br> <br> In this particular case, mbedtls build with a system openssl of 0.9.8,<br> it blows up with<br> <br> In file included from crypto.c:37:<br> openssl_compat.h: In function 'SSL_CTX_get_min_proto_version':<br> openssl_compat.h:635: error: 'SSL_OP_NO_TLSv1_1' undeclared <br> (first use in this<br> <br> (and more of this)<br> <br> which is unsurprising - it's not supposed to pull in these headers<br> in the first place.<br></blockquote><div><br></div><div>Looking back at it, the patch and the problem it's trying to solve are both misplaced in crypto.c. That file should be ssl-lib agnostic and openssl related bits should go to crypto_openssl.c...</div><div><br></div><div>I think we need to remove that OPENSSL_FIPS clause and think of providing that extra info somewhere else if possible.<br></div><div><br></div><div>Selva</div></div></div>
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index 5626e2b6..eb0b1254 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -34,6 +34,7 @@ #include "error.h" #include "integer.h" #include "platform.h" +#include "openssl_compat.h" #include "memdbg.h" @@ -1704,10 +1705,15 @@ print_cipher(const char *ciphername) printf(", TLS client/server mode only"); } #ifdef OPENSSL_FIPS - if (FIPS_mode() && !(EVP_CIPHER_flags(cipher) & EVP_CIPH_FLAG_FIPS)) + evp_cipher_type *cipher = EVP_CIPHER_fetch(NULL, ciphername, NULL); + + if (FIPS_mode() + && (NULL != cipher) + && !(EVP_CIPHER_flags(cipher) & EVP_CIPH_FLAG_FIPS)) { printf(", disabled by FIPS mode"); } + EVP_CIPHER_free(cipher); #endif printf(")\n");