Re: [PATCH ima-evm-utils v5 10/17] Disable use of OpenSSL "engine" support

[Stefan Berger <stefanb@xxxxxxxxxxxxx>]

 <stefanb@xxxxxxxxxxxxx>

On 11/3/22 14:38, Mimi Zohar wrote:
> OpenSSL v3 "engine" support is deprecated and replaced with "providers".
> Engine support will continue to work for a while, but results in
> deprecated declaration and other messages.  One option is simply to hide
> them ("-Wno-deprecated-declarations").  The other alternative is to
> conditionally build ima-evm-utils without OpenSSL engine support and
> without disabling deprecated declarations.
>
> Based on "--disable-engine" or "--enable-engine=no" configuration
> option, disable OpenSSL "engine" support.
>
> As suggested by Vitaly,
> - verify ENGINE_init symbol is defined in libcrypto
> - disable engine support if either OPENSSL_NO_DYNAMIC_ENGINE or
> OPENSSL_NO_ENGINE variables are defined
>
> Signed-off-by: Mimi Zohar <zohar@xxxxxxxxxxxxx>
Reviewed-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>

> ---
>   configure.ac    |  6 ++++++
>   src/Makefile.am |  8 ++++++++
>   src/evmctl.c    | 17 ++++++++++++++++-
>   src/imaevm.h    |  6 ++++++
>   src/libimaevm.c |  7 ++++++-
>   5 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index dc666f2bb1fa..90646da22061 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -54,6 +54,11 @@ AC_ARG_ENABLE(sigv1,
>   	AM_CONDITIONAL([CONFIG_SIGV1], [test "x$enable_sigv1" = "xyes"])
>   	AS_IF([test "$enable_sigv1"  != "yes"], [enable_sigv1="no"])
>   
> +AC_ARG_ENABLE(engine,
> +	      [AS_HELP_STRING([--disable-engine], [build ima-evm-utils without OpenSSL engine support])],,[enable_engine=yes])
> +	AC_CHECK_LIB([crypto], [ENGINE_init],, [enable_engine=no])
> +	AM_CONDITIONAL([CONFIG_IMA_EVM_ENGINE], [test "x$enable_engine" = "xyes"])
> +
>   #debug support - yes for a while
>   PKG_ARG_ENABLE(debug, "yes", DEBUG, [Enable Debug support])
>   if test $pkg_cv_enable_debug = yes; then
> @@ -89,5 +94,6 @@ echo	"      tss2-esys: $ac_cv_lib_tss2_esys_Esys_Free"
>   echo	" tss2-rc-decode: $ac_cv_lib_tss2_rc_Tss2_RC_Decode"
>   echo    "         ibmtss: $ac_cv_header_ibmtss_tss_h"
>   echo    "         sigv1:  $enable_sigv1"
> +echo    "         engine: $enable_engine"
>   echo	"            doc: $have_doc"
>   echo
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 90c7249020cf..0527a7b9df5c 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -11,6 +11,10 @@ if CONFIG_SIGV1
>   libimaevm_la_CFLAGS = -DCONFIG_SIGV1
>   endif
>   
> +if CONFIG_IMA_EVM_ENGINE
> +libimaevm_la_CFLAGS = -DCONFIG_IMA_EVM_ENGINE
> +endif
> +
>   include_HEADERS = imaevm.h
>   
>   nodist_libimaevm_la_SOURCES = hash_info.h
> @@ -31,6 +35,10 @@ if CONFIG_SIGV1
>   evmctl_CFLAGS = -DCONFIG_SIGV1
>   endif
>   
> +# Enable "--engine" support
> +if CONFIG_IMA_EVM_ENGINE
> +evmctl_CFLAGS = -DCONFIG_IMA_EVM_ENGINE
> +endif
>   
>   # USE_PCRTSS uses the Intel TSS
>   if USE_PCRTSS
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 66eea481d115..6f253bfcba19 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -65,7 +65,9 @@
>   #include <openssl/hmac.h>
>   #include <openssl/err.h>
>   #include <openssl/rsa.h>
> +#if CONFIG_IMA_EVM_ENGINE
>   #include <openssl/engine.h>
> +#endif
>   #include <openssl/x509v3.h>
>   #include "hash_info.h"
>   #include "pcr.h"
> @@ -2709,7 +2711,9 @@ static void usage(void)
>   		"      --selinux      use custom Selinux label for EVM\n"
>   		"      --caps         use custom Capabilities for EVM(unspecified: from FS, empty: do not use)\n"
>   		"      --verify-sig   verify measurement list signatures\n"
> -		"      --engine e     preload OpenSSL engine e (such as: gost)\n"
> +#if CONFIG_IMA_EVM_ENGINE
> +		"      --engine e     preload OpenSSL engine e (such as: gost) is deprecated\n"
> +#endif
>   		"      --ignore-violations ignore ToMToU measurement violations\n"
>   		"  -v                 increase verbosity level\n"
>   		"  -h, --help         display this help and exit\n"
> @@ -2771,7 +2775,9 @@ static struct option opts[] = {
>   	{"selinux", 1, 0, 136},
>   	{"caps", 2, 0, 137},
>   	{"verify-sig", 0, 0, 138},
> +#if CONFIG_IMA_EVM_ENGINE
>   	{"engine", 1, 0, 139},
> +#endif
>   	{"xattr-user", 0, 0, 140},
>   	{"ignore-violations", 0, 0, 141},
>   	{"pcrs", 1, 0, 142},
> @@ -2824,9 +2830,11 @@ static char *get_password(void)
>   	return password;
>   }
>   
> +#if CONFIG_IMA_EVM_ENGINE
>   static ENGINE *setup_engine(const char *engine_id)
>   {
>   	ENGINE *eng = ENGINE_by_id(engine_id);
> +
>   	if (!eng) {
>   		log_err("engine %s isn't available\n", optarg);
>   		ERR_print_errors_fp(stderr);
> @@ -2840,6 +2848,7 @@ static ENGINE *setup_engine(const char *engine_id)
>   		ENGINE_set_default(eng, ENGINE_METHOD_ALL);
>   	return eng;
>   }
> +#endif
>   
>   int main(int argc, char *argv[])
>   {
> @@ -2965,11 +2974,13 @@ int main(int argc, char *argv[])
>   		case 138:
>   			verify_list_sig = 1;
>   			break;
> +#if CONFIG_IMA_EVM_ENGINE
>   		case 139: /* --engine e */
>   			imaevm_params.eng = setup_engine(optarg);
>   			if (!imaevm_params.eng)
>   				goto error;
>   			break;
> +#endif
>   		case 140: /* --xattr-user */
>   			xattr_ima = "user.ima";
>   			xattr_evm = "user.evm";
> @@ -3028,7 +3039,9 @@ int main(int argc, char *argv[])
>   	if (imaevm_params.keyfile != NULL &&
>   	    imaevm_params.eng == NULL &&
>   	    !strncmp(imaevm_params.keyfile, "pkcs11:", 7)) {
> +#if CONFIG_IMA_EVM_ENGINE
>   		imaevm_params.eng = setup_engine("pkcs11");
> +#endif
>   		if (!imaevm_params.eng)
>   			goto error;
>   	}
> @@ -3054,6 +3067,7 @@ int main(int argc, char *argv[])
>   	}
>   
>   error:
> +#if CONFIG_IMA_EVM_ENGINE
>   	if (imaevm_params.eng) {
>   		ENGINE_finish(imaevm_params.eng);
>   		ENGINE_free(imaevm_params.eng);
> @@ -3061,6 +3075,7 @@ error:
>   		ENGINE_cleanup();
>   #endif
>   	}
> +#endif
>   	ERR_free_strings();
>   	EVP_cleanup();
>   	BIO_free(NULL);
> diff --git a/src/imaevm.h b/src/imaevm.h
> index afcf1e042014..884321670fa7 100644
> --- a/src/imaevm.h
> +++ b/src/imaevm.h
> @@ -48,7 +48,13 @@
>   #include <errno.h>
>   #include <sys/types.h>
>   #include <openssl/rsa.h>
> +#ifdef CONFIG_IMA_EVM_ENGINE
>   #include <openssl/engine.h>
> +#endif
> +
> +#if defined(OPENSSL_NO_ENGINE) || defined(OPENSSL_NO_DYNAMIC_ENGINE)
> +#undef CONFIG_IMA_EVM_ENGINE
> +#endif
>   
>   #ifdef USE_FPRINTF
>   #define do_log(level, fmt, args...)	\
> diff --git a/src/libimaevm.c b/src/libimaevm.c
> index b12b7ff14d95..8070ffd61a2c 100644
> --- a/src/libimaevm.c
> +++ b/src/libimaevm.c
> @@ -953,9 +953,10 @@ uint32_t imaevm_read_keyid(const char *certfile)
>   static EVP_PKEY *read_priv_pkey(const char *keyfile, const char *keypass)
>   {
>   	FILE *fp;
> -	EVP_PKEY *pkey;
> +	EVP_PKEY *pkey = NULL;
>   
>   	if (!strncmp(keyfile, "pkcs11:", 7)) {
> +#ifdef CONFIG_IMA_EVM_ENGINE
>   		if (!imaevm_params.keyid) {
>   			log_err("When using a pkcs11 URI you must provide the keyid with an option\n");
>   			return NULL;
> @@ -972,6 +973,10 @@ static EVP_PKEY *read_priv_pkey(const char *keyfile, const char *keypass)
>   			log_err("Failed to load private key %s\n", keyfile);
>   			goto err_engine;
>   		}
> +#else
> +		log_err("OpenSSL \"engine\" support is disabled\n");
> +		goto err_engine;
> +#endif
>   	} else {
>   		fp = fopen(keyfile, "r");
>   		if (!fp) {