diff mbox series

[Openvpn-devel] using OpenSSL3 API for EVP PKEY type name reporting

Message ID 20230319075441.13021-1-info@baentsch.ch
State Accepted
Headers
Series [Openvpn-devel] using OpenSSL3 API for EVP PKEY type name reporting |

Commit Message

Michael Baentsch March 19, 2023, 7:54 a.m. UTC 
  Signed-off-by: Michael Baentsch <info@baentsch.ch>
---
 src/openvpn/ssl_openssl.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

Comments

Arne Schwabe March 20, 2023, 12:24 p.m. UTC | #1 
Am 19.03.23 um 08:54 schrieb Michael Baentsch:
> Signed-off-by: Michael Baentsch <info@baentsch.ch>

Acked-By: Arne Schwabe <arne@rfc2549.org>

Thanks. We had a discussion/review round on gihtub before this. 
Basically the problem is that trying to print the algorithm
for algorithms that are not part of the old OpenSSL 1.x API (like a pq 
algorithm from a provider) results in putting an error on the OpenSSL 
stack and that has than bad consequences.

Arne
Gert Doering March 20, 2023, 1:01 p.m. UTC | #2 
I have not tested this extensively, just subjected to GH to compile and
run basic checks with OpenSSL 1.1.x and 3.0.x, and ran a few local tests
(Linux + OpenSSL 1.1.1).  This all passed.  Have not investigated how
to actually trigger these code lines.

Your patch has been applied to the master and release/2.6 branch.

commit 6c111be9b109a6dbcd39cac7821ea3dd78ff6adf (master)
commit a05ec70edd5178aac7b7432c57878c32aa838013 (release/2.6)
Author: Michael Baentsch
Date:   Sun Mar 19 08:54:41 2023 +0100

     using OpenSSL3 API for EVP PKEY type name reporting

     Signed-off-by: Michael Baentsch <info@baentsch.ch>
     Acked-by: Arne Schwabe <arne@rfc2549.org>
     Message-Id: <20230319075441.13021-1-info@baentsch.ch>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26439.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
Michael Baentsch March 21, 2023, 9:21 a.m. UTC | #3 
Hi Gert,

    thanks very much!

 > Have not investigated how to actually trigger these code lines.

If you're curious (TL;DR), below's a test FWIW:

The fix can be seen "in action" when using OpenVPN with a quantum-safe 
signature algorithm via oqs-provider:

Everything built into docker images:

1) New code in openquantumsafe/openvpn:23903fd579353c98:

# openvpn --version
OpenVPN 2.7_git [git:master/23903fd579353c98] x86_64-pc-linux-gnu [SSL 
(OpenSSL)] [LZO] [EPOLL] [MH/PKTINFO] [AEAD] [DCO] built on Mar 21 2023
library versions: OpenSSL 3.2.0-dev , LZO 2.10

2023-03-21 09:08:43 us=455158 10.0.5.3:37633 TLS: tls_multi_process: 
initial untrusted session promoted to trusted
WWRR2023-03-21 09:08:43 us=455383 10.0.5.3:37633 Control Channel: 
TLSv1.3, cipher TLSv1.3 TLS_AES_256_GCM_SHA384, peer certificate: 192 
bit dilithium3, signature: dilithium3
2023-03-21 09:08:43 us=455406 10.0.5.3:37633 [oqsopenvpnclient] Peer 
Connection Initiated with [AF_INET]10.0.5.3:37633

--> Connection establishment OK


2) Old code in openquantumsafe/openvpn:838474145933199a

# openvpn --version
OpenVPN 2.7_git [git:master/838474145933199a] x86_64-pc-linux-gnu [SSL 
(OpenSSL)] [LZO] [EPOLL] [MH/PKTINFO] [AEAD] [DCO] built on Mar 14 2023
library versions: OpenSSL 3.2.0-dev , LZO 2.10

2023-03-21 09:10:59 us=432368 10.0.5.3:40978 TLS: tls_multi_process: 
initial untrusted session promoted to trusted
WWRR2023-03-21 09:10:59 us=432601 10.0.5.3:40978 Control Channel: 
TLSv1.3, cipher TLSv1.3 TLS_AES_256_GCM_SHA384, peer certificate: 192 
bit unknown type, signature: dilithium3
2023-03-21 09:10:59 us=432619 10.0.5.3:40978 [oqsopenvpnclient] Peer 
Connection Initiated with [AF_INET]10.0.5.3:40978
2023-03-21 09:10:59 us=432634 10.0.5.3:40978 OpenSSL: 
error:04000065:object identifier routines::unknown nid
2023-03-21 09:10:59 us=432640 10.0.5.3:40978 TLS_ERROR: BIO read 
tls_read_plaintext error
2023-03-21 09:10:59 us=432648 10.0.5.3:40978 TLS Error: TLS object -> 
incoming plaintext read error
2023-03-21 09:10:59 us=432653 10.0.5.3:40978 TLS Error: TLS handshake failed

--> Connection setup failure

Regards,

--Michael

Am 20.03.23 um 14:01 schrieb Gert Doering:
> I have not tested this extensively, just subjected to GH to compile and
> run basic checks with OpenSSL 1.1.x and 3.0.x, and ran a few local tests
> (Linux + OpenSSL 1.1.1).  This all passed.  Have not investigated how
> to actually trigger these code lines.
>
> Your patch has been applied to the master and release/2.6 branch.
>
> commit 6c111be9b109a6dbcd39cac7821ea3dd78ff6adf (master)
> commit a05ec70edd5178aac7b7432c57878c32aa838013 (release/2.6)
> Author: Michael Baentsch
> Date:   Sun Mar 19 08:54:41 2023 +0100
>
>       using OpenSSL3 API for EVP PKEY type name reporting
>
>       Signed-off-by: Michael Baentsch<info@baentsch.ch>
>       Acked-by: Arne Schwabe<arne@rfc2549.org>
>       Message-Id:<20230319075441.13021-1-info@baentsch.ch>
>       URL:https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26439.html
>       Signed-off-by: Gert Doering<gert@greenie.muc.de>
>
>
> --
> kind regards,
>
> Gert Doering
>
diff mbox series

Patch

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 2b932af9..65b36d1c 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -1501,7 +1501,11 @@  tls_ctx_use_management_external_key(struct tls_root_ctx *ctx)
     }
     EVP_PKEY_free(privkey);
 #else  /* ifdef HAVE_XKEY_PROVIDER */
+#if OPENSSL_VERSION_NUMBER < 0x30000000L
     if (EVP_PKEY_id(pkey) == EVP_PKEY_RSA)
+#else /* OPENSSL_VERSION_NUMBER < 0x30000000L */
+    if (EVP_PKEY_is_a(pkey, "RSA"))
+#endif /* OPENSSL_VERSION_NUMBER < 0x30000000L */
     {
         if (!tls_ctx_use_external_rsa_key(ctx, pkey))
         {
@@ -1509,7 +1513,11 @@  tls_ctx_use_management_external_key(struct tls_root_ctx *ctx)
         }
     }
 #if (OPENSSL_VERSION_NUMBER > 0x10100000L) && !defined(OPENSSL_NO_EC)
+#if OPENSSL_VERSION_NUMBER < 0x30000000L
     else if (EVP_PKEY_id(pkey) == EVP_PKEY_EC)
+#else /* OPENSSL_VERSION_NUMBER < 0x30000000L */
+    else if (EVP_PKEY_is_a(pkey, "EC"))
+#endif /* OPENSSL_VERSION_NUMBER < 0x30000000L */
     {
         if (!tls_ctx_use_external_ec_key(ctx, pkey))
         {
@@ -2064,10 +2072,15 @@  print_cert_details(X509 *cert, char *buf, size_t buflen)
     }
 
     int typeid = EVP_PKEY_id(pkey);
+#if OPENSSL_VERSION_NUMBER < 0x30000000L
+    bool is_ec = typeid == EVP_PKEY_EC;
+#else
+    bool is_ec = EVP_PKEY_is_a(pkey, "EC");
+#endif
 
 #ifndef OPENSSL_NO_EC
     char groupname[256];
-    if (typeid == EVP_PKEY_EC)
+    if (is_ec)
     {
         size_t len;
         if (EVP_PKEY_get_group_name(pkey, groupname, sizeof(groupname), &len))
@@ -2080,9 +2093,9 @@  print_cert_details(X509 *cert, char *buf, size_t buflen)
         }
     }
 #endif
-    if (EVP_PKEY_id(pkey) != 0)
+    if (typeid != 0)
     {
-        int typeid = EVP_PKEY_id(pkey);
+#if OPENSSL_VERSION_NUMBER < 0x30000000L
         type = OBJ_nid2sn(typeid);
 
         /* OpenSSL reports rsaEncryption, dsaEncryption and
@@ -2104,6 +2117,13 @@  print_cert_details(X509 *cert, char *buf, size_t buflen)
         {
             type = "unknown type";
         }
+#else /* OpenSSL >= 3 */
+        type = EVP_PKEY_get0_type_name(pkey);
+        if (type == NULL)
+        {
+            type = "(error getting public key type)";
+        }
+#endif /* if OPENSSL_VERSION_NUMBER < 0x30000000L */
     }
 
     char sig[128] = { 0 };