[Openvpn-devel,v3,08/21,OSSL,3.0] Use EVP_PKEY_get_group_name to query group name

  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.

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

Hi,

I had looked at v1 of this so easy:

On Tue, Oct 19, 2021 at 2:31 PM 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.
>
> Signed-off-by: Arne Schwabe <arne@rfc2549.org>
> ---
>  src/openvpn/openssl_compat.h | 42 ++++++++++++++++++++++++++++++++++++
>  src/openvpn/ssl_openssl.c    | 14 ++++++------
>  2 files changed, 50 insertions(+), 6 deletions(-)
>
> diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
> index ce8e2b360..dda47d76c 100644
> --- a/src/openvpn/openssl_compat.h
> +++ b/src/openvpn/openssl_compat.h
> @@ -718,4 +718,46 @@ 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);
>

I would have preferred a  curve !=NULL check here. Though very unlikely to
happen, we do not want a segfault in strncpy.

+
> +    strncpynt(gname, curve, gname_sz);
> +    *gname_len = min_int(strlen(curve), gname_sz);
>

gname_sz - 1 ?

That said, our strncpynt ensures that strlen(curve) will be less than
gname_sz, so this could be just strlen(curve) or left as is.


> +    return 1;
> +}
> +#endif
> +
> +/** Mimics SSL_CTX_new_ex for OpenSSL < 3 */
> +#if OPENSSL_VERSION_NUMBER < 0x30000000L
> +static inline SSL_CTX *
> +SSL_CTX_new_ex(void *libctx, const char *propq, const SSL_METHOD *method)
>

This looks like a spill-over from one of my xkey patches --- "git commit
-p" malfunction?

Unless the "unused functions police" objects, we can keep it.

+{
> +    (void) libctx;
> +    (void) propq;
> +    return SSL_CTX_new(method);
> +}
> +#endif /* OPENSSL_VERSION_NUMBER < 0x30000000L */
> +
>  #endif /* OPENSSL_COMPAT_H_ */
> diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
> index 92d8d0eeb..8ec96e66c 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)";
>          }
>
>
Looks good otherwise and works as expected.

Selva
<div dir="ltr"><div dir="ltr">Hi,<div><br></div><div>I had looked at v1 of this so easy:</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Oct 19, 2021 at 2:31 PM Arne Schwabe &lt;<a href="mailto:arne@rfc2549.org" target="_blank">arne@rfc2549.org</a>&gt; 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">EC_Key methods are deprecated in OpenSSL 3.0. Use<br>
EVP_PKEY_get_group_name instead to query the EC group name from an<br>
EVP_PKEY and add a compatibility function for older OpenSSL versions.<br>
<br>
Signed-off-by: Arne Schwabe &lt;<a href="mailto:arne@rfc2549.org" target="_blank">arne@rfc2549.org</a>&gt;<br>
---<br>
 src/openvpn/openssl_compat.h | 42 ++++++++++++++++++++++++++++++++++++<br>
 src/openvpn/ssl_openssl.c    | 14 ++++++------<br>
 2 files changed, 50 insertions(+), 6 deletions(-)<br>
<br>
diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h<br>
index ce8e2b360..dda47d76c 100644<br>
--- a/src/openvpn/openssl_compat.h<br>
+++ b/src/openvpn/openssl_compat.h<br>
@@ -718,4 +718,46 @@ SSL_CTX_set_max_proto_version(SSL_CTX *ctx, long tls_ver_max)<br>
     return 1;<br>
 }<br>
 #endif /* if OPENSSL_VERSION_NUMBER &lt; 0x10100000L &amp;&amp; !defined(ENABLE_CRYPTO_WOLFSSL) */<br>
+<br>
+/* Functionality missing in 1.1.1 */<br>
+#if OPENSSL_VERSION_NUMBER &lt; 0x30000000L &amp;&amp; !defined(OPENSSL_NO_EC)<br>
+<br>
+/* Note that this is not a perfect emulation of the new function but<br>
+ * is good enough for our case of printing certificate details during<br>
+ * handshake */<br>
+static inline<br>
+int EVP_PKEY_get_group_name(EVP_PKEY *pkey, char *gname, size_t gname_sz,<br>
+                            size_t *gname_len)<br>
+{<br>
+    const EC_KEY* ec = EVP_PKEY_get0_EC_KEY(pkey);<br>
+    if (ec == NULL)<br>
+    {<br>
+        return 0;<br>
+    }<br>
+    const EC_GROUP* group = EC_KEY_get0_group(ec);<br>
+    int nid = EC_GROUP_get_curve_name(group);<br>
+<br>
+    if (nid == 0)<br>
+    {<br>
+        return 0;<br>
+    }<br>
+    const char *curve = OBJ_nid2sn(nid);<br></blockquote><div><br></div><div>I would have preferred a  curve !=NULL check here. Though very unlikely to happen, we do not want a segfault in strncpy.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+    strncpynt(gname, curve, gname_sz);<br>
+    *gname_len = min_int(strlen(curve), gname_sz);<br></blockquote><div><br></div><div>gname_sz - 1 ?</div><div><br></div><div>That said, our strncpynt ensures that strlen(curve) will be less than gname_sz, so this could be just strlen(curve) or left as is.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+    return 1;<br>
+}<br>
+#endif<br>
+<br>
+/** Mimics SSL_CTX_new_ex for OpenSSL &lt; 3 */<br>
+#if OPENSSL_VERSION_NUMBER &lt; 0x30000000L<br>
+static inline SSL_CTX *<br>
+SSL_CTX_new_ex(void *libctx, const char *propq, const SSL_METHOD *method)<br></blockquote><div><br></div><div>This looks like a spill-over from one of my xkey patches --- &quot;git commit -p&quot; malfunction? </div><div><br></div><div>Unless the &quot;unused functions police&quot; objects, we can keep it.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+{<br>
+    (void) libctx;<br>
+    (void) propq;<br>
+    return SSL_CTX_new(method);<br>
+}<br>
+#endif /* OPENSSL_VERSION_NUMBER &lt; 0x30000000L */<br>
+<br>
 #endif /* OPENSSL_COMPAT_H_ */<br>
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c<br>
index 92d8d0eeb..8ec96e66c 100644<br>
--- a/src/openvpn/ssl_openssl.c<br>
+++ b/src/openvpn/ssl_openssl.c<br>
@@ -2053,13 +2053,15 @@ print_cert_details(X509 *cert, char *buf, size_t buflen)<br>
     int typeid = EVP_PKEY_id(pkey);<br>
<br>
 #ifndef OPENSSL_NO_EC<br>
-    if (typeid == EVP_PKEY_EC &amp;&amp; EVP_PKEY_get0_EC_KEY(pkey) != NULL)<br>
+    char groupname[256];<br>
+    if (typeid == EVP_PKEY_EC)<br>
     {<br>
-        const EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey);<br>
-        const EC_GROUP *group = EC_KEY_get0_group(ec);<br>
-<br>
-        int nid = EC_GROUP_get_curve_name(group);<br>
-        if (nid == 0 || (curve = OBJ_nid2sn(nid)) == NULL)<br>
+        size_t len;<br>
+        if(EVP_PKEY_get_group_name(pkey, groupname, sizeof(groupname), &amp;len))<br>
+        {<br>
+           curve = groupname;<br>
+        }<br>
+        else<br>
         {<br>
             curve = &quot;(error getting curve name)&quot;;<br>
         }<br><br></blockquote><div><br></div><div>Looks good otherwise and works as expected.</div><div><br></div><div>Selva</div></div></div>
  

On 19/10/2021 20:31, Arne Schwabe 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.
> 
> Signed-off-by: Arne Schwabe <arne@rfc2549.org>
> ---
>   src/openvpn/openssl_compat.h | 42 ++++++++++++++++++++++++++++++++++++
>   src/openvpn/ssl_openssl.c    | 14 ++++++------
>   2 files changed, 50 insertions(+), 6 deletions(-)
> 
> diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
> index ce8e2b360..dda47d76c 100644
> --- a/src/openvpn/openssl_compat.h
> +++ b/src/openvpn/openssl_compat.h
> @@ -718,4 +718,46 @@ 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);

The old code also has a curve == NULL check. Is that not necessary here?