| Message ID | 20211019183127.614175-9-arne@rfc2549.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | OpenSSL 3.0 improvements for OpenVPN | expand |
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 <<a href="mailto:arne@rfc2549.org" target="_blank">arne@rfc2549.org</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">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 <<a href="mailto:arne@rfc2549.org" target="_blank">arne@rfc2549.org</a>><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 < 0x10100000L && !defined(ENABLE_CRYPTO_WOLFSSL) */<br> +<br> +/* Functionality missing in 1.1.1 */<br> +#if OPENSSL_VERSION_NUMBER < 0x30000000L && !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 < 3 */<br> +#if OPENSSL_VERSION_NUMBER < 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 --- "git commit -p" malfunction? </div><div><br></div><div>Unless the "unused functions police" 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 < 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 && 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), &len))<br> + {<br> + curve = groupname;<br> + }<br> + else<br> {<br> curve = "(error getting curve name)";<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?
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); + + strncpynt(gname, curve, gname_sz); + *gname_len = min_int(strlen(curve), gname_sz); + 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) +{ + (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)"; }
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(-)