Message ID | 20220121052259.508-2-lstipakov@gmail.com |
---|---|
State | Accepted |
Headers | show |
Series | *** msvc: switch to openssl3 *** | expand |
On Mon, Jan 24, 2022 at 2:22 PM Lev Stipakov <lstipakov@gmail.com> wrote: > From: Lev Stipakov <lev@openvpn.net> > > - use sizeof(void *) since msvc doesn't support sizeof of function ptr > This is not just an msvc problem, but signals a bigger issue it seems. In retrospect, passing function pointers pickled this was probably a bad design decision on my part though we are forced by OpenSSL 3's design of using OSSL_PARAMs to pass data to providers. For now, The proposed fix (i.e., to use void*) looks okay to me especially since its handled like a normal pointer during key import. But we may have to find a better way for passing these function pointers if this comes back to bite us. > > - use XKEY_PROV_PROPS macro instead of props since msvc > requires constant expression in aggregate initializers > Makes sense. > Signed-off-by: Lev Stipakov <lev@openvpn.net> > --- > src/openvpn/xkey_helper.c | 4 ++-- > src/openvpn/xkey_provider.c | 13 +++++-------- > 2 files changed, 7 insertions(+), 10 deletions(-) > > diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c > index c667f7be..50231335 100644 > --- a/src/openvpn/xkey_helper.c > +++ b/src/openvpn/xkey_helper.c > @@ -125,8 +125,8 @@ xkey_load_generic_key(OSSL_LIB_CTX *libctx, void > *handle, EVP_PKEY *pubkey, > {"xkey-origin", OSSL_PARAM_UTF8_STRING, (char *) origin, 0, 0}, > {"pubkey", OSSL_PARAM_OCTET_STRING, &pubkey, sizeof(pubkey), 0}, > {"handle", OSSL_PARAM_OCTET_PTR, &handle, sizeof(handle), 0}, > - {"sign_op", OSSL_PARAM_OCTET_PTR, (void **) &sign_op, > sizeof(sign_op), 0}, > - {"free_op", OSSL_PARAM_OCTET_PTR, (void **) &free_op, > sizeof(free_op), 0}, > + {"sign_op", OSSL_PARAM_OCTET_PTR, (void **) &sign_op, sizeof(void > *), 0}, > + {"free_op", OSSL_PARAM_OCTET_PTR, (void **) &free_op, sizeof(void > *), 0}, > {NULL, 0, NULL, 0, 0}}; > > /* Do not use EVP_PKEY_new_from_pkey as that will take keymgmt from > pubkey */ > diff --git a/src/openvpn/xkey_provider.c b/src/openvpn/xkey_provider.c > index c2d560c5..115b9931 100644 > --- a/src/openvpn/xkey_provider.c > +++ b/src/openvpn/xkey_provider.c > @@ -44,9 +44,6 @@ > #include <openssl/evp.h> > #include <openssl/err.h> > > -/* propq set all on all ops we implement */ > -static const char *const props = XKEY_PROV_PROPS; > - > /* A descriptive name */ > static const char *provname = "OpenVPN External Key Provider"; > > @@ -592,9 +589,9 @@ static const OSSL_DISPATCH ec_keymgmt_functions[] = { > }; > > const OSSL_ALGORITHM keymgmts[] = { > - {"RSA:rsaEncryption", props, rsa_keymgmt_functions, "OpenVPN xkey RSA > Key Manager"}, > - {"RSA-PSS:RSASSA-PSS", props, rsa_keymgmt_functions, "OpenVPN xkey > RSA-PSS Key Manager"}, > - {"EC:id-ecPublicKey", props, ec_keymgmt_functions, "OpenVPN xkey EC > Key Manager"}, > + {"RSA:rsaEncryption", XKEY_PROV_PROPS, rsa_keymgmt_functions, > "OpenVPN xkey RSA Key Manager"}, > + {"RSA-PSS:RSASSA-PSS", XKEY_PROV_PROPS, rsa_keymgmt_functions, > "OpenVPN xkey RSA-PSS Key Manager"}, > + {"EC:id-ecPublicKey", XKEY_PROV_PROPS, ec_keymgmt_functions, "OpenVPN > xkey EC Key Manager"}, > {NULL, NULL, NULL, NULL} > }; > > @@ -1074,8 +1071,8 @@ static const OSSL_DISPATCH signature_functions[] = { > }; > > const OSSL_ALGORITHM signatures[] = { > - {"RSA:rsaEncryption", props, signature_functions, "OpenVPN xkey RSA > Signature"}, > - {"ECDSA", props, signature_functions, "OpenVPN xkey ECDSA Signature"}, > + {"RSA:rsaEncryption", XKEY_PROV_PROPS, signature_functions, "OpenVPN > xkey RSA Signature"}, > + {"ECDSA", XKEY_PROV_PROPS, signature_functions, "OpenVPN xkey ECDSA > Signature"}, > {NULL, NULL, NULL, NULL} > }; Acked-by: Selva Nair <selva.nair@gmail.com> <div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jan 24, 2022 at 2:22 PM Lev Stipakov <<a href="mailto:lstipakov@gmail.com" target="_blank">lstipakov@gmail.com</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">From: Lev Stipakov <<a href="mailto:lev@openvpn.net" target="_blank">lev@openvpn.net</a>><br> <br> - use sizeof(void *) since msvc doesn't support sizeof of function ptr<br></blockquote><div><br></div><div>This is not just an msvc problem, but signals a bigger issue it seems. In retrospect, passing function pointers pickled this was probably a bad design decision on my part though we are forced by OpenSSL 3's design of using OSSL_PARAMs to pass data to providers.</div><div><br></div><div>For now, The proposed fix (i.e., to use void*) looks okay to me especially since its handled like a normal pointer during key import. But we may have to find a better way for passing these function pointers if this comes back to bite us.<br></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"> <br> - use XKEY_PROV_PROPS macro instead of props since msvc<br> requires constant expression in aggregate initializers<br></blockquote><div><br></div><div>Makes sense.</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> Signed-off-by: Lev Stipakov <<a href="mailto:lev@openvpn.net" target="_blank">lev@openvpn.net</a>><br> ---<br> src/openvpn/xkey_helper.c | 4 ++--<br> src/openvpn/xkey_provider.c | 13 +++++--------<br> 2 files changed, 7 insertions(+), 10 deletions(-)<br> <br> diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c<br> index c667f7be..50231335 100644<br> --- a/src/openvpn/xkey_helper.c<br> +++ b/src/openvpn/xkey_helper.c<br> @@ -125,8 +125,8 @@ xkey_load_generic_key(OSSL_LIB_CTX *libctx, void *handle, EVP_PKEY *pubkey,<br> {"xkey-origin", OSSL_PARAM_UTF8_STRING, (char *) origin, 0, 0},<br> {"pubkey", OSSL_PARAM_OCTET_STRING, &pubkey, sizeof(pubkey), 0},<br> {"handle", OSSL_PARAM_OCTET_PTR, &handle, sizeof(handle), 0},<br> - {"sign_op", OSSL_PARAM_OCTET_PTR, (void **) &sign_op, sizeof(sign_op), 0},<br> - {"free_op", OSSL_PARAM_OCTET_PTR, (void **) &free_op, sizeof(free_op), 0},<br> + {"sign_op", OSSL_PARAM_OCTET_PTR, (void **) &sign_op, sizeof(void *), 0},<br> + {"free_op", OSSL_PARAM_OCTET_PTR, (void **) &free_op, sizeof(void *), 0},<br> {NULL, 0, NULL, 0, 0}};<br> <br> /* Do not use EVP_PKEY_new_from_pkey as that will take keymgmt from pubkey */<br> diff --git a/src/openvpn/xkey_provider.c b/src/openvpn/xkey_provider.c<br> index c2d560c5..115b9931 100644<br> --- a/src/openvpn/xkey_provider.c<br> +++ b/src/openvpn/xkey_provider.c<br> @@ -44,9 +44,6 @@<br> #include <openssl/evp.h><br> #include <openssl/err.h><br> <br> -/* propq set all on all ops we implement */<br> -static const char *const props = XKEY_PROV_PROPS;<br> -<br> /* A descriptive name */<br> static const char *provname = "OpenVPN External Key Provider";<br> <br> @@ -592,9 +589,9 @@ static const OSSL_DISPATCH ec_keymgmt_functions[] = {<br> };<br> <br> const OSSL_ALGORITHM keymgmts[] = {<br> - {"RSA:rsaEncryption", props, rsa_keymgmt_functions, "OpenVPN xkey RSA Key Manager"},<br> - {"RSA-PSS:RSASSA-PSS", props, rsa_keymgmt_functions, "OpenVPN xkey RSA-PSS Key Manager"},<br> - {"EC:id-ecPublicKey", props, ec_keymgmt_functions, "OpenVPN xkey EC Key Manager"},<br> + {"RSA:rsaEncryption", XKEY_PROV_PROPS, rsa_keymgmt_functions, "OpenVPN xkey RSA Key Manager"},<br> + {"RSA-PSS:RSASSA-PSS", XKEY_PROV_PROPS, rsa_keymgmt_functions, "OpenVPN xkey RSA-PSS Key Manager"},<br> + {"EC:id-ecPublicKey", XKEY_PROV_PROPS, ec_keymgmt_functions, "OpenVPN xkey EC Key Manager"},<br> {NULL, NULL, NULL, NULL}<br> };<br> <br> @@ -1074,8 +1071,8 @@ static const OSSL_DISPATCH signature_functions[] = {<br> };<br> <br> const OSSL_ALGORITHM signatures[] = {<br> - {"RSA:rsaEncryption", props, signature_functions, "OpenVPN xkey RSA Signature"},<br> - {"ECDSA", props, signature_functions, "OpenVPN xkey ECDSA Signature"},<br> + {"RSA:rsaEncryption", XKEY_PROV_PROPS, signature_functions, "OpenVPN xkey RSA Signature"},<br> + {"ECDSA", XKEY_PROV_PROPS, signature_functions, "OpenVPN xkey ECDSA Signature"},<br> {NULL, NULL, NULL, NULL}<br> }; </blockquote><div> </div><div>Acked-by: Selva Nair <<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a>></div></div></div>
This time I'm sure I have an ACK :-) - and the changes look reasonable. Compile tested on linux/3.0.1, just to avoid typos that went unnoticed (all fine). Your patch has been applied to the master branch. commit 627d1a3d286386067a93b755def308ea70060310 Author: Lev Stipakov Date: Fri Jan 21 07:22:58 2022 +0200 xkey: fix msvc build Signed-off-by: Lev Stipakov <lev@openvpn.net> Acked-by: Selva Nair <selva.nair@gmail.com> Message-Id: <20220121052259.508-2-lstipakov@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23643.html Signed-off-by: Gert Doering <gert@greenie.muc.de> -- kind regards, Gert Doering
diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c index c667f7be..50231335 100644 --- a/src/openvpn/xkey_helper.c +++ b/src/openvpn/xkey_helper.c @@ -125,8 +125,8 @@ xkey_load_generic_key(OSSL_LIB_CTX *libctx, void *handle, EVP_PKEY *pubkey, {"xkey-origin", OSSL_PARAM_UTF8_STRING, (char *) origin, 0, 0}, {"pubkey", OSSL_PARAM_OCTET_STRING, &pubkey, sizeof(pubkey), 0}, {"handle", OSSL_PARAM_OCTET_PTR, &handle, sizeof(handle), 0}, - {"sign_op", OSSL_PARAM_OCTET_PTR, (void **) &sign_op, sizeof(sign_op), 0}, - {"free_op", OSSL_PARAM_OCTET_PTR, (void **) &free_op, sizeof(free_op), 0}, + {"sign_op", OSSL_PARAM_OCTET_PTR, (void **) &sign_op, sizeof(void *), 0}, + {"free_op", OSSL_PARAM_OCTET_PTR, (void **) &free_op, sizeof(void *), 0}, {NULL, 0, NULL, 0, 0}}; /* Do not use EVP_PKEY_new_from_pkey as that will take keymgmt from pubkey */ diff --git a/src/openvpn/xkey_provider.c b/src/openvpn/xkey_provider.c index c2d560c5..115b9931 100644 --- a/src/openvpn/xkey_provider.c +++ b/src/openvpn/xkey_provider.c @@ -44,9 +44,6 @@ #include <openssl/evp.h> #include <openssl/err.h> -/* propq set all on all ops we implement */ -static const char *const props = XKEY_PROV_PROPS; - /* A descriptive name */ static const char *provname = "OpenVPN External Key Provider"; @@ -592,9 +589,9 @@ static const OSSL_DISPATCH ec_keymgmt_functions[] = { }; const OSSL_ALGORITHM keymgmts[] = { - {"RSA:rsaEncryption", props, rsa_keymgmt_functions, "OpenVPN xkey RSA Key Manager"}, - {"RSA-PSS:RSASSA-PSS", props, rsa_keymgmt_functions, "OpenVPN xkey RSA-PSS Key Manager"}, - {"EC:id-ecPublicKey", props, ec_keymgmt_functions, "OpenVPN xkey EC Key Manager"}, + {"RSA:rsaEncryption", XKEY_PROV_PROPS, rsa_keymgmt_functions, "OpenVPN xkey RSA Key Manager"}, + {"RSA-PSS:RSASSA-PSS", XKEY_PROV_PROPS, rsa_keymgmt_functions, "OpenVPN xkey RSA-PSS Key Manager"}, + {"EC:id-ecPublicKey", XKEY_PROV_PROPS, ec_keymgmt_functions, "OpenVPN xkey EC Key Manager"}, {NULL, NULL, NULL, NULL} }; @@ -1074,8 +1071,8 @@ static const OSSL_DISPATCH signature_functions[] = { }; const OSSL_ALGORITHM signatures[] = { - {"RSA:rsaEncryption", props, signature_functions, "OpenVPN xkey RSA Signature"}, - {"ECDSA", props, signature_functions, "OpenVPN xkey ECDSA Signature"}, + {"RSA:rsaEncryption", XKEY_PROV_PROPS, signature_functions, "OpenVPN xkey RSA Signature"}, + {"ECDSA", XKEY_PROV_PROPS, signature_functions, "OpenVPN xkey ECDSA Signature"}, {NULL, NULL, NULL, NULL} };