[Openvpn-devel,v2] xkey_provider: fix building with --disable-management

Message ID 20220727221830.31861-1-selva.nair@gmail.com
State Accepted
Headers show
Series [Openvpn-devel,v2] xkey_provider: fix building with --disable-management | expand

Commit Message

Selva Nair July 27, 2022, 12:18 p.m. UTC
From: Selva Nair <selva.nair@gmail.com>

v2: also fix building test_provider
 - ifdefs in test_provider.c
 - include integer.h for min_int as manage.h
   may not always pull it in

Too many ifdefs, unfortunately..

Signed-off-by: Selva Nair <selva.nair@gmail.com>
---
 src/openvpn/xkey_helper.c                | 4 ++++
 tests/unit_tests/openvpn/test_provider.c | 7 +++++++
 2 files changed, 11 insertions(+)

Comments

Frank Lichtenheld July 29, 2022, 12:33 a.m. UTC | #1
On Wed, Jul 27, 2022 at 06:18:30PM -0400, selva.nair@gmail.com wrote:
> From: Selva Nair <selva.nair@gmail.com>
> 
> v2: also fix building test_provider
>  - ifdefs in test_provider.c
>  - include integer.h for min_int as manage.h
>    may not always pull it in

FTR, buildbot is happy now with these changes.
uncrustify is not, see below, but that can probably
be fixed on apply.

Acked-By: Frank Lichtenheld <frank@lichtenheld.com>

> diff --git a/tests/unit_tests/openvpn/test_provider.c b/tests/unit_tests/openvpn/test_provider.c
> index 47e7e395..d146af62 100644
> --- a/tests/unit_tests/openvpn/test_provider.c
> +++ b/tests/unit_tests/openvpn/test_provider.c
> @@ -30,6 +30,7 @@
>  
>  #include "syshead.h"
>  #include "manage.h"
> +#include "integer.h"
>  #include "xkey_common.h"
>  
>  #ifdef HAVE_XKEY_PROVIDER
> @@ -127,7 +128,9 @@ init_test()
>      /* set default propq matching what we use in ssl_openssl.c */
>      EVP_set_default_properties(NULL, "?provider!=ovpn.xkey");
>  
> +#ifdef ENABLE_MANAGEMENT
>      management = test_calloc(sizeof(*management), 1);
> +#endif
>  }
>  
>  static void
> @@ -272,6 +275,7 @@ done:
>      return sig;
>  }
>  
> +#ifdef ENABLE_MANAGEMENT
>  /* Check loading of management external key and have sign callback exercised
>   * for RSA and EC keys with and without digest support in management client.
>   * Sha256 digest used for both cases with pss padding for RSA.
> @@ -310,6 +314,7 @@ again:
>          EVP_PKEY_free(privkey);
>      }
>  }
> +#endif

uncrustify wants this
--- "b/tests/unit_tests/openvpn/test_provider.c"        2022-07-29 11:34:20.067492200 +0200
+++ "a/tests/unit_tests/openvpn/test_provider.c"        2022-07-29 11:34:20.054239000 +0200
@@ -314,7 +314,7 @@
         EVP_PKEY_free(privkey);
     }
 }
-#endif
+#endif /* ifdef ENABLE_MANAGEMENT */

 /* helpers for testing generic key load and sign */
 static int xkey_free_called;

>  /* helpers for testing generic key load and sign */
>  static int xkey_free_called;
> @@ -409,7 +414,9 @@ main(void)
>  
>      const struct CMUnitTest tests[] = {
>          cmocka_unit_test(xkey_provider_test_fetch),
> +#ifdef ENABLE_MANAGEMENT
>          cmocka_unit_test(xkey_provider_test_mgmt_sign_cb),
> +#endif
>          cmocka_unit_test(xkey_provider_test_generic_sign_cb),
>      };
Gert Doering Aug. 1, 2022, 5:10 a.m. UTC | #2
Thanks for v2, and "too many #ifdef" indeed.  Uncrustify complained that
one of the #endif wants a comment, but with the new commit hooks this 
is easily fixed.  (Maybe we want to have a deep look into "do we really
want to keep ENABLE_MANAGEMENT?" one day... how much extra code size
does that bring to the binaries?  It's not *that* much code, is it?)

I have fed the code to buildbot, and it pacified all these modern
instances (fedora-36 etc.) ;-)

Your patch has been applied to the master branch.

commit fc75ab33401718fab3a79b19698eefb70a617a83
Author: Selva Nair
Date:   Wed Jul 27 18:18:30 2022 -0400

     xkey_provider: fix building with --disable-management

     Signed-off-by: Selva Nair <selva.nair@gmail.com>
     Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
     Message-Id: <20220727221830.31861-1-selva.nair@gmail.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24749.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/xkey_helper.c b/src/openvpn/xkey_helper.c
index 81dd71dc..27e87d79 100644
--- a/src/openvpn/xkey_helper.c
+++ b/src/openvpn/xkey_helper.c
@@ -85,6 +85,7 @@  xkey_digest(const unsigned char *src, size_t srclen, unsigned char *buf,
     return 1;
 }
 
+#ifdef ENABLE_MANAGEMENT
 /**
  * Load external key for signing via management interface.
  * The public key must be passed in by the caller as we may not
@@ -107,6 +108,7 @@  xkey_load_management_key(OSSL_LIB_CTX *libctx, EVP_PKEY *pubkey)
 
     return xkey_load_generic_key(libctx, dummy, pubkey, sign_op, NULL);
 }
+#endif
 
 /**
  * Load a generic key into the xkey provider.
@@ -147,6 +149,7 @@  xkey_load_generic_key(OSSL_LIB_CTX *libctx, void *handle, EVP_PKEY *pubkey,
     return pkey;
 }
 
+#ifdef ENABLE_MANAGEMENT
 /**
  * Signature callback for xkey_provider with management-external-key
  *
@@ -277,6 +280,7 @@  xkey_management_sign(void *unused, unsigned char *sig, size_t *siglen,
 
     return (*siglen > 0);
 }
+#endif /* ENABLE MANAGEMENT */
 
 /**
  * Add PKCS1 DigestInfo to tbs and return the result in *enc.
diff --git a/tests/unit_tests/openvpn/test_provider.c b/tests/unit_tests/openvpn/test_provider.c
index 47e7e395..d146af62 100644
--- a/tests/unit_tests/openvpn/test_provider.c
+++ b/tests/unit_tests/openvpn/test_provider.c
@@ -30,6 +30,7 @@ 
 
 #include "syshead.h"
 #include "manage.h"
+#include "integer.h"
 #include "xkey_common.h"
 
 #ifdef HAVE_XKEY_PROVIDER
@@ -127,7 +128,9 @@  init_test()
     /* set default propq matching what we use in ssl_openssl.c */
     EVP_set_default_properties(NULL, "?provider!=ovpn.xkey");
 
+#ifdef ENABLE_MANAGEMENT
     management = test_calloc(sizeof(*management), 1);
+#endif
 }
 
 static void
@@ -272,6 +275,7 @@  done:
     return sig;
 }
 
+#ifdef ENABLE_MANAGEMENT
 /* Check loading of management external key and have sign callback exercised
  * for RSA and EC keys with and without digest support in management client.
  * Sha256 digest used for both cases with pss padding for RSA.
@@ -310,6 +314,7 @@  again:
         EVP_PKEY_free(privkey);
     }
 }
+#endif
 
 /* helpers for testing generic key load and sign */
 static int xkey_free_called;
@@ -409,7 +414,9 @@  main(void)
 
     const struct CMUnitTest tests[] = {
         cmocka_unit_test(xkey_provider_test_fetch),
+#ifdef ENABLE_MANAGEMENT
         cmocka_unit_test(xkey_provider_test_mgmt_sign_cb),
+#endif
         cmocka_unit_test(xkey_provider_test_generic_sign_cb),
     };