[Openvpn-devel,v4,OSSL,3.0] Use EVP_PKEY_get_group_name to query group name

Message ID 20211029111109.2003101-2-arne@rfc2549.org
State Accepted
Headers show
Series [Openvpn-devel,v4,OSSL,3.0] Use EVP_PKEY_get_group_name to query group name | expand

Commit Message

Arne Schwabe Oct. 29, 2021, 12:11 a.m. UTC
EC_Key methods are deprecated in OpenSSL 3.0. Use
EVP_PKEY_get_group_name instead to query the EC group name from an
EVP_PKEY and add a compatibility function for older OpenSSL versions.

Patch v4: adjust compatibility function and remove accidently included fragment
          of unrelated patch.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 src/openvpn/openssl_compat.h | 36 ++++++++++++++++++++++++++++++++++++
 src/openvpn/ssl_openssl.c    | 14 ++++++++------
 2 files changed, 44 insertions(+), 6 deletions(-)

Comments

Selva Nair Oct. 30, 2021, 8:02 a.m. UTC | #1
Somehow I managed to mess-up the reply address with this one. No wonder it
failed to show up in patchwork.
Sending again to the list

---------- Forwarded message ---------
From: Selva Nair <selva.nair@gmail.com>
Date: Fri, Oct 29, 2021 at 12:06 PM
Subject: Re: [Openvpn-devel] [PATCH v4] [OSSL 3.0] Use
EVP_PKEY_get_group_name to query group name


Hi,

On Fri, Oct 29, 2021 at 8:20 AM Arne Schwabe <arne@rfc2549.org> wrote:

> EC_Key methods are deprecated in OpenSSL 3.0. Use
> EVP_PKEY_get_group_name instead to query the EC group name from an
> EVP_PKEY and add a compatibility function for older OpenSSL versions.
>
> Patch v4: adjust compatibility function and remove accidently included
> fragment
>           of unrelated patch.
>
> Signed-off-by: Arne Schwabe <arne@rfc2549.org>
> ---
>  src/openvpn/openssl_compat.h | 36 ++++++++++++++++++++++++++++++++++++
>  src/openvpn/ssl_openssl.c    | 14 ++++++++------
>  2 files changed, 44 insertions(+), 6 deletions(-)
>
> diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
> index ce8e2b360..3951d9aca 100644
> --- a/src/openvpn/openssl_compat.h
> +++ b/src/openvpn/openssl_compat.h
> @@ -718,4 +718,40 @@ SSL_CTX_set_max_proto_version(SSL_CTX *ctx, long
> tls_ver_max)
>      return 1;
>  }
>  #endif /* if OPENSSL_VERSION_NUMBER < 0x10100000L &&
> !defined(ENABLE_CRYPTO_WOLFSSL) */
> +
> +/* Functionality missing in 1.1.1 */
> +#if OPENSSL_VERSION_NUMBER < 0x30000000L && !defined(OPENSSL_NO_EC)
> +
> +/* Note that this is not a perfect emulation of the new function but
> + * is good enough for our case of printing certificate details during
> + * handshake */
> +static inline
> +int EVP_PKEY_get_group_name(EVP_PKEY *pkey, char *gname, size_t gname_sz,
> +                            size_t *gname_len)
> +{
> +    const EC_KEY* ec = EVP_PKEY_get0_EC_KEY(pkey);
> +    if (ec == NULL)
> +    {
> +        return 0;
> +    }
> +    const EC_GROUP* group = EC_KEY_get0_group(ec);
> +    int nid = EC_GROUP_get_curve_name(group);
> +
> +    if (nid == 0)
> +    {
> +        return 0;
> +    }
> +    const char *curve = OBJ_nid2sn(nid);
> +    if (!curve)
> +    {
> +        curve = "(error fetching curve name)";
> +    }
> +
> +    strncpynt(gname, curve, gname_sz);
> +
> +    /* strncpynt ensures null termination so just strlen is fine here */
> +    *gname_len = strlen(curve);
> +    return 1;
> +}
> +#endif
>  #endif /* OPENSSL_COMPAT_H_ */
> diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
> index 6f2d6d57a..25ff50375 100644
> --- a/src/openvpn/ssl_openssl.c
> +++ b/src/openvpn/ssl_openssl.c
> @@ -2053,13 +2053,15 @@ print_cert_details(X509 *cert, char *buf, size_t
> buflen)
>      int typeid = EVP_PKEY_id(pkey);
>
>  #ifndef OPENSSL_NO_EC
> -    if (typeid == EVP_PKEY_EC && EVP_PKEY_get0_EC_KEY(pkey) != NULL)
> +    char groupname[256];
> +    if (typeid == EVP_PKEY_EC)
>      {
> -        const EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey);
> -        const EC_GROUP *group = EC_KEY_get0_group(ec);
> -
> -        int nid = EC_GROUP_get_curve_name(group);
> -        if (nid == 0 || (curve = OBJ_nid2sn(nid)) == NULL)
> +        size_t len;
> +        if(EVP_PKEY_get_group_name(pkey, groupname, sizeof(groupname),
> &len))
> +        {
> +           curve = groupname;
> +        }
> +        else
>          {
>              curve = "(error getting curve name)";
>          }
>

Acked-by: Selva Nair <selva.nair@gmail.com>
Gert Doering Nov. 1, 2021, 8:30 a.m. UTC | #2
Stared a bit at the code (verified that "curve" is never used out
of scope, since it's now a local array), die minimum test run vs.
1.1.1 and 3.0.0 - no surprises (t_lpback.sh still fails due to
the FETCH parts missing, but they can now go in).

Your patch has been applied to the master branch.

commit 4b3c1e76d747d0e7e6aec280680777aef356a940
Author: Arne Schwabe
Date:   Fri Oct 29 13:11:09 2021 +0200

     Use EVP_PKEY_get_group_name to query group name

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Selva Nair <selva.nair@gmail.com>
     Message-Id: <20211029111109.2003101-2-arne@rfc2549.org>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23077.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
index ce8e2b360..3951d9aca 100644
--- a/src/openvpn/openssl_compat.h
+++ b/src/openvpn/openssl_compat.h
@@ -718,4 +718,40 @@  SSL_CTX_set_max_proto_version(SSL_CTX *ctx, long tls_ver_max)
     return 1;
 }
 #endif /* if OPENSSL_VERSION_NUMBER < 0x10100000L && !defined(ENABLE_CRYPTO_WOLFSSL) */
+
+/* Functionality missing in 1.1.1 */
+#if OPENSSL_VERSION_NUMBER < 0x30000000L && !defined(OPENSSL_NO_EC)
+
+/* Note that this is not a perfect emulation of the new function but
+ * is good enough for our case of printing certificate details during
+ * handshake */
+static inline
+int EVP_PKEY_get_group_name(EVP_PKEY *pkey, char *gname, size_t gname_sz,
+                            size_t *gname_len)
+{
+    const EC_KEY* ec = EVP_PKEY_get0_EC_KEY(pkey);
+    if (ec == NULL)
+    {
+        return 0;
+    }
+    const EC_GROUP* group = EC_KEY_get0_group(ec);
+    int nid = EC_GROUP_get_curve_name(group);
+
+    if (nid == 0)
+    {
+        return 0;
+    }
+    const char *curve = OBJ_nid2sn(nid);
+    if (!curve)
+    {
+        curve = "(error fetching curve name)";
+    }
+
+    strncpynt(gname, curve, gname_sz);
+
+    /* strncpynt ensures null termination so just strlen is fine here */
+    *gname_len = strlen(curve);
+    return 1;
+}
+#endif
 #endif /* OPENSSL_COMPAT_H_ */
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 6f2d6d57a..25ff50375 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -2053,13 +2053,15 @@  print_cert_details(X509 *cert, char *buf, size_t buflen)
     int typeid = EVP_PKEY_id(pkey);
 
 #ifndef OPENSSL_NO_EC
-    if (typeid == EVP_PKEY_EC && EVP_PKEY_get0_EC_KEY(pkey) != NULL)
+    char groupname[256];
+    if (typeid == EVP_PKEY_EC)
     {
-        const EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey);
-        const EC_GROUP *group = EC_KEY_get0_group(ec);
-
-        int nid = EC_GROUP_get_curve_name(group);
-        if (nid == 0 || (curve = OBJ_nid2sn(nid)) == NULL)
+        size_t len;
+        if(EVP_PKEY_get_group_name(pkey, groupname, sizeof(groupname), &len))
+        {
+           curve = groupname;
+        }
+        else
         {
             curve = "(error getting curve name)";
         }