diff mbox series

[Openvpn-devel,1/2] xkey: fix msvc build

Message ID 20220121052259.508-2-lstipakov@gmail.com
State Accepted
Headers show
Series *** msvc: switch to openssl3 *** | expand

Commit Message

Lev Stipakov Jan. 20, 2022, 6:22 p.m. UTC 
From: Lev Stipakov <lev@openvpn.net>

 - use sizeof(void *) since msvc doesn't support sizeof of function ptr

 - use XKEY_PROV_PROPS macro instead of props since msvc
  requires constant expression in aggregate initializers

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(-)

Comments

Selva Nair Jan. 24, 2022, 10:41 a.m. UTC | #1 
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 &lt;<a href="mailto:lstipakov@gmail.com" target="_blank">lstipakov@gmail.com</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">From: Lev Stipakov &lt;<a href="mailto:lev@openvpn.net" target="_blank">lev@openvpn.net</a>&gt;<br>
<br>
 - use sizeof(void *) since msvc doesn&#39;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&#39;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 &lt;<a href="mailto:lev@openvpn.net" target="_blank">lev@openvpn.net</a>&gt;<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>
         {&quot;xkey-origin&quot;, OSSL_PARAM_UTF8_STRING, (char *) origin, 0, 0},<br>
         {&quot;pubkey&quot;, OSSL_PARAM_OCTET_STRING, &amp;pubkey, sizeof(pubkey), 0},<br>
         {&quot;handle&quot;, OSSL_PARAM_OCTET_PTR, &amp;handle, sizeof(handle), 0},<br>
-        {&quot;sign_op&quot;, OSSL_PARAM_OCTET_PTR, (void **) &amp;sign_op, sizeof(sign_op), 0},<br>
-        {&quot;free_op&quot;, OSSL_PARAM_OCTET_PTR, (void **) &amp;free_op, sizeof(free_op), 0},<br>
+        {&quot;sign_op&quot;, OSSL_PARAM_OCTET_PTR, (void **) &amp;sign_op, sizeof(void *), 0},<br>
+        {&quot;free_op&quot;, OSSL_PARAM_OCTET_PTR, (void **) &amp;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 &lt;openssl/evp.h&gt;<br>
 #include &lt;openssl/err.h&gt;<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 = &quot;OpenVPN External Key Provider&quot;;<br>
<br>
@@ -592,9 +589,9 @@ static const OSSL_DISPATCH ec_keymgmt_functions[] = {<br>
 };<br>
<br>
 const OSSL_ALGORITHM keymgmts[] = {<br>
-    {&quot;RSA:rsaEncryption&quot;, props, rsa_keymgmt_functions, &quot;OpenVPN xkey RSA Key Manager&quot;},<br>
-    {&quot;RSA-PSS:RSASSA-PSS&quot;, props, rsa_keymgmt_functions, &quot;OpenVPN xkey RSA-PSS Key Manager&quot;},<br>
-    {&quot;EC:id-ecPublicKey&quot;, props, ec_keymgmt_functions, &quot;OpenVPN xkey EC Key Manager&quot;},<br>
+    {&quot;RSA:rsaEncryption&quot;, XKEY_PROV_PROPS, rsa_keymgmt_functions, &quot;OpenVPN xkey RSA Key Manager&quot;},<br>
+    {&quot;RSA-PSS:RSASSA-PSS&quot;, XKEY_PROV_PROPS, rsa_keymgmt_functions, &quot;OpenVPN xkey RSA-PSS Key Manager&quot;},<br>
+    {&quot;EC:id-ecPublicKey&quot;, XKEY_PROV_PROPS, ec_keymgmt_functions, &quot;OpenVPN xkey EC Key Manager&quot;},<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>
-    {&quot;RSA:rsaEncryption&quot;, props, signature_functions, &quot;OpenVPN xkey RSA Signature&quot;},<br>
-    {&quot;ECDSA&quot;, props, signature_functions, &quot;OpenVPN xkey ECDSA Signature&quot;},<br>
+    {&quot;RSA:rsaEncryption&quot;, XKEY_PROV_PROPS, signature_functions, &quot;OpenVPN xkey RSA Signature&quot;},<br>
+    {&quot;ECDSA&quot;, XKEY_PROV_PROPS, signature_functions, &quot;OpenVPN xkey ECDSA Signature&quot;},<br>
     {NULL, NULL, NULL, NULL}<br>
 }; </blockquote><div> </div><div>Acked-by: Selva Nair &lt;<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a>&gt;</div></div></div>
Gert Doering Jan. 26, 2022, 2:33 a.m. UTC | #2 
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 mbox series

Patch

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}
 };