diff mbox series

[Openvpn-devel,3/3] Require EC key support in Windows builds

Message ID 20211019034118.28987-3-selva.nair@gmail.com
State Accepted
Headers
Series [Openvpn-devel,1/3] Require Windows CNG keys for cryptoapicert |

Commit Message

Selva Nair Oct. 18, 2021, 4:41 p.m. UTC 
  From: Selva Nair <selva.nair@gmail.com>

Do not support the use of OPENSSL_NO_EC on Windows.

We build Windows releases with EC key support enabled in
OpenSSL and there is no reason to disable it in OpenVPN.

TODO: If there are no platforms of interest where EC support
cannot be enabled in OpenSSL, we should make !defined(OPENSSL_NO_EC)
a general requirement.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
---
 src/openvpn/crypto_openssl.c | 4 ++++
 src/openvpn/cryptoapi.c      | 6 ------
 2 files changed, 4 insertions(+), 6 deletions(-)

Comments

Gert Doering Oct. 19, 2021, 4:47 a.m. UTC | #1 
Acked-by: Gert Doering <gert@greenie.muc.de>

Now this one is straightforward, even I can ACK it :-) - this change is
good as it will avoid compile mishaps, and since we control what SSL 
build we want to use on Windows, we can enforce "always EC!".  Not sure
about other platforms (maybe the Software Museum has OpenSSL versions
compiled without EC...).

I'd love to have that in 2.5 as well, given that this bit us with the
2.5.4 release - but of course it needs 1/3 as a prerequisite, which I
find a bit intrusive for "right in the middle of the 2.5 series".

Can you resend just this one for 2.5?

Your patch has been applied to the master branch.

commit ec9f698d3bac29b50094b23a8ff63e523e6a3787
Author: Selva Nair
Date:   Mon Oct 18 23:41:18 2021 -0400

     Require EC key support in Windows builds

     Signed-off-by: Selva Nair <selva.nair@gmail.com>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20211019034118.28987-3-selva.nair@gmail.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22952.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
Arne Schwabe Oct. 19, 2021, 4:54 a.m. UTC | #2 
Am 19.10.21 um 17:47 schrieb Gert Doering:
> Acked-by: Gert Doering <gert@greenie.muc.de>
> 
> Now this one is straightforward, even I can ACK it :-) - this change is
> good as it will avoid compile mishaps, and since we control what SSL 
> build we want to use on Windows, we can enforce "always EC!".  Not sure
> about other platforms (maybe the Software Museum has OpenSSL versions
> compiled without EC...).

The software museum wants to support FIPS and FIPS mandates Suite B
curves. I would more think that embedded system that stripped their
OpenSSL to the mimimum like that one that removed DH/DHCE support a few
years ago are problematic.

Arne
diff mbox series

Patch

diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index 419265a5..60fbec12 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -55,6 +55,10 @@ 
 #include <openssl/kdf.h>
 #endif
 
+#if defined(_WIN32) && defined(OPENSSL_NO_EC)
+#error Windows build with OPENSSL_NO_EC: disabling EC key is not supported.
+#endif
+
 /*
  * Check for key size creepage.
  */
diff --git a/src/openvpn/cryptoapi.c b/src/openvpn/cryptoapi.c
index c97dbfbf..7fe3c57c 100644
--- a/src/openvpn/cryptoapi.c
+++ b/src/openvpn/cryptoapi.c
@@ -236,8 +236,6 @@  rsa_finish(RSA *rsa)
     return 1;
 }
 
-#if !defined(OPENSSL_NO_EC)
-
 static EC_KEY_METHOD *ec_method = NULL;
 
 /** EC_KEY_METHOD callback: called when the key is freed */
@@ -423,8 +421,6 @@  err:
     return 0;
 }
 
-#endif /* !defined(OPENSSL_NO_EC) */
-
 static const CERT_CONTEXT *
 find_certificate_in_store(const char *cert_prop, HCERTSTORE cert_store)
 {
@@ -853,7 +849,6 @@  SSL_CTX_use_CryptoAPI_certificate(SSL_CTX *ssl_ctx, const char *cert_prop)
             goto err;
         }
     }
-#if !defined(OPENSSL_NO_EC)
     else if (EVP_PKEY_id(pkey) == EVP_PKEY_EC)
     {
         if (!ssl_ctx_set_eckey(ssl_ctx, cd, pkey))
@@ -861,7 +856,6 @@  SSL_CTX_use_CryptoAPI_certificate(SSL_CTX *ssl_ctx, const char *cert_prop)
             goto err;
         }
     }
-#endif /* !defined(OPENSSL_NO_EC) */
     else
     {
         msg(M_WARN|M_INFO, "WARNING: cryptoapicert: key type <%d> not supported",