* [RFC PATCH ima-evm-utils 01/11] travis: use the distro OpenSSL version on jammy
2022-08-30 0:59 [RFC PATCH ima-evm-utils 00/11] address deprecated warnings Mimi Zohar
@ 2022-08-30 0:59 ` Mimi Zohar
2022-08-30 11:30 ` Petr Vorel
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 02/11] travis: update dist=focal Mimi Zohar
` (9 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Mimi Zohar @ 2022-08-30 0:59 UTC (permalink / raw)
To: linux-integrity; +Cc: Mimi Zohar, Petr Vorel, Vitaly Chikunov, Stefan Berger
Use the distro OpenSSL version on jammy, which is newer than
OpenSSL 3-beta.
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
.travis.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/.travis.yml b/.travis.yml
index 5741116e418a..b18c871be200 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -40,7 +40,7 @@ matrix:
compiler: gcc
- os: linux
- env: DISTRO=ubuntu:jammy TSS=ibmtss COMPILE_SSL=openssl-3.0.0-beta1
+ env: DISTRO=ubuntu:jammy TSS=ibmtss
compiler: gcc
- os: linux
--
2.31.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [RFC PATCH ima-evm-utils 02/11] travis: update dist=focal
2022-08-30 0:59 [RFC PATCH ima-evm-utils 00/11] address deprecated warnings Mimi Zohar
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 01/11] travis: use the distro OpenSSL version on jammy Mimi Zohar
@ 2022-08-30 0:59 ` Mimi Zohar
2022-08-30 11:31 ` Petr Vorel
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 03/11] Update configure.ac to address a couple of obsolete warnings Mimi Zohar
` (8 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Mimi Zohar @ 2022-08-30 0:59 UTC (permalink / raw)
To: linux-integrity; +Cc: Mimi Zohar, Petr Vorel, Vitaly Chikunov, Stefan Berger
Although Github Actions is GA within Github Enterprise Server 3.x single
server edition, it is not available in Github Enterprise Server 3.x
cluster edition[1]. Continue to support travis.
[1] https://docs.github.com/en/enterprise-server@3.0/admin/release-notes#github-packages
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
.travis.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/.travis.yml b/.travis.yml
index b18c871be200..cc76c0adb312 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,6 +1,6 @@
# Copyright (c) 2017-2021 Petr Vorel <pvorel@suse.cz>
-dist: bionic
+dist: focal
language: C
services:
- docker
--
2.31.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [RFC PATCH ima-evm-utils 03/11] Update configure.ac to address a couple of obsolete warnings
2022-08-30 0:59 [RFC PATCH ima-evm-utils 00/11] address deprecated warnings Mimi Zohar
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 01/11] travis: use the distro OpenSSL version on jammy Mimi Zohar
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 02/11] travis: update dist=focal Mimi Zohar
@ 2022-08-30 0:59 ` Mimi Zohar
2022-08-30 11:32 ` Petr Vorel
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 04/11] Deprecate IMA signature version 1 Mimi Zohar
` (7 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Mimi Zohar @ 2022-08-30 0:59 UTC (permalink / raw)
To: linux-integrity; +Cc: Mimi Zohar, Petr Vorel, Vitaly Chikunov, Stefan Berger
Remove AC_PROG_LIBTOOL and AC_HEAD_STDC. Replace AC_HELP_STRING with
AS_HELP_STRING.
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
acinclude.m4 | 2 +-
configure.ac | 4 ----
m4/manpage-docbook-xsl.m4 | 2 +-
3 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/acinclude.m4 b/acinclude.m4
index dd430d4f0565..bb962f81a9c0 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -2,7 +2,7 @@
AC_DEFUN([PKG_ARG_ENABLE],
[
AC_MSG_CHECKING(whether to enable $1)
- AC_ARG_ENABLE([$1], AC_HELP_STRING([--enable-$1], [enable $1 (default is $2)]),
+ AC_ARG_ENABLE([$1], AS_HELP_STRING([--enable-$1], [enable $1 (default is $2)]),
[pkg_cv_enable_$1=$enableval],
[AC_CACHE_VAL([pkg_cv_enable_$1], [pkg_cv_enable_$1=$2])])
if test $pkg_cv_enable_$1 = yes; then
diff --git a/configure.ac b/configure.ac
index 1a0f093df562..9d3b23ff8def 100644
--- a/configure.ac
+++ b/configure.ac
@@ -15,16 +15,12 @@ AM_PROG_CC_C_O
#AC_PROG_CXX
#AC_PROG_CPP
AC_PROG_INSTALL
-AC_PROG_LIBTOOL
#AC_PROG_LN_S
LT_INIT
# FIXME: Replace `main' with a function in `-lpthread':
#AC_CHECK_LIB([pthread], [main])
-# Checks for header files.
-AC_HEADER_STDC
-
PKG_CHECK_MODULES(LIBCRYPTO, [libcrypto >= 0.9.8 ])
AC_SUBST(KERNEL_HEADERS)
AC_CHECK_HEADER(unistd.h)
diff --git a/m4/manpage-docbook-xsl.m4 b/m4/manpage-docbook-xsl.m4
index 25c8ce54b068..f2ee912ed1be 100644
--- a/m4/manpage-docbook-xsl.m4
+++ b/m4/manpage-docbook-xsl.m4
@@ -7,7 +7,7 @@ AC_DEFUN([EVMCTL_MANPAGE_DOCBOOK_XSL], [
AC_PATH_PROGS(XMLCATALOG, xmlcatalog)
AC_ARG_WITH([xml-catalog],
- AC_HELP_STRING([--with-xml-catalog=CATALOG],
+ AS_HELP_STRING([--with-xml-catalog=CATALOG],
[path to xml catalog to use]),,
[with_xml_catalog=/etc/xml/catalog])
XML_CATALOG_FILE="$with_xml_catalog"
--
2.31.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [RFC PATCH ima-evm-utils 04/11] Deprecate IMA signature version 1
2022-08-30 0:59 [RFC PATCH ima-evm-utils 00/11] address deprecated warnings Mimi Zohar
` (2 preceding siblings ...)
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 03/11] Update configure.ac to address a couple of obsolete warnings Mimi Zohar
@ 2022-08-30 0:59 ` Mimi Zohar
2022-08-30 11:55 ` Petr Vorel
2022-08-30 12:12 ` Stefan Berger
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 05/11] Replace the low level SHA1 calls when calculating the TPM 1.2 PCRs Mimi Zohar
` (6 subsequent siblings)
10 siblings, 2 replies; 31+ messages in thread
From: Mimi Zohar @ 2022-08-30 0:59 UTC (permalink / raw)
To: linux-integrity; +Cc: Mimi Zohar, Petr Vorel, Vitaly Chikunov, Stefan Berger
The original IMA file signatures were based on a SHA1 hash. Kernel
support for other hash algorithms was subsequently upstreamed. Deprecate
"--rsa" support.
Define "--enable-sigv1" option to configure signature v1 support.
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
configure.ac | 6 ++++++
src/Makefile.am | 10 ++++++++++
src/evmctl.c | 16 ++++++++++++----
src/libimaevm.c | 24 ++++++++++++++++++++++--
tests/sign_verify.test | 18 ++++++++++++------
5 files changed, 62 insertions(+), 12 deletions(-)
diff --git a/configure.ac b/configure.ac
index 9d3b23ff8def..dc666f2bb1fa 100644
--- a/configure.ac
+++ b/configure.ac
@@ -49,6 +49,11 @@ AC_ARG_ENABLE([openssl_conf],
AC_DEFINE(DISABLE_OPENSSL_CONF, 1, [Define to disable loading of openssl config by evmctl.])
fi], [enable_openssl_conf=yes])
+AC_ARG_ENABLE(sigv1,
+ AS_HELP_STRING([--enable-sigv1], [Build ima-evm-utils with signature v1 support]))
+ AM_CONDITIONAL([CONFIG_SIGV1], [test "x$enable_sigv1" = "xyes"])
+ AS_IF([test "$enable_sigv1" != "yes"], [enable_sigv1="no"])
+
#debug support - yes for a while
PKG_ARG_ENABLE(debug, "yes", DEBUG, [Enable Debug support])
if test $pkg_cv_enable_debug = yes; then
@@ -83,5 +88,6 @@ echo " openssl-conf: $enable_openssl_conf"
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 " doc: $have_doc"
echo
diff --git a/src/Makefile.am b/src/Makefile.am
index 396496bb439d..90c7249020cf 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -7,6 +7,10 @@ libimaevm_la_CPPFLAGS = $(AM_CPPFLAGS) $(LIBCRYPTO_CFLAGS)
libimaevm_la_LDFLAGS = -version-info 3:0:0
libimaevm_la_LIBADD = $(LIBCRYPTO_LIBS)
+if CONFIG_SIGV1
+libimaevm_la_CFLAGS = -DCONFIG_SIGV1
+endif
+
include_HEADERS = imaevm.h
nodist_libimaevm_la_SOURCES = hash_info.h
@@ -22,6 +26,12 @@ evmctl_CPPFLAGS = $(AM_CPPFLAGS) $(LIBCRYPTO_CFLAGS)
evmctl_LDFLAGS = $(LDFLAGS_READLINE)
evmctl_LDADD = $(LIBCRYPTO_LIBS) -lkeyutils libimaevm.la
+# Enable IMA signature version 1
+if CONFIG_SIGV1
+evmctl_CFLAGS = -DCONFIG_SIGV1
+endif
+
+
# USE_PCRTSS uses the Intel TSS
if USE_PCRTSS
evmctl_SOURCES += pcr_tss.c
diff --git a/src/evmctl.c b/src/evmctl.c
index 76e2561798fa..621136b5b85f 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -987,7 +987,6 @@ static int cmd_verify_ima(struct command *cmd)
init_public_keys("/etc/keys/x509_evm.der");
}
- errno = 0;
if (!file) {
log_err("Parameters missing\n");
print_usage(cmd);
@@ -1006,6 +1005,7 @@ static int cmd_verify_ima(struct command *cmd)
static int cmd_convert(struct command *cmd)
{
+#if CONFIG_SIGV1
char *inkey;
unsigned char _pub[1024], *pub = _pub;
int len, err = 0;
@@ -1033,6 +1033,8 @@ static int cmd_convert(struct command *cmd)
RSA_free(key);
return err;
+#endif
+ return 77;
}
static int cmd_import(struct command *cmd)
@@ -1088,6 +1090,7 @@ static int cmd_import(struct command *cmd)
calc_keyid_v2((uint32_t *)keyid, name, pkey);
EVP_PKEY_free(pkey);
} else {
+#if CONFIG_SIGV1
RSA *key = read_pub_key(inkey, imaevm_params.x509);
if (!key)
@@ -1095,6 +1098,10 @@ static int cmd_import(struct command *cmd)
len = key2bin(key, pub);
calc_keyid_v1(keyid, name, pub, len);
RSA_free(key);
+#else
+ log_info("Importing public RSA key not supported\n");
+ return 1;
+#endif
}
log_info("Importing public key %s from file %s into keyring %d\n", name, inkey, id);
@@ -2598,7 +2605,8 @@ static void usage(void)
" -d, --imahash make IMA hash\n"
" -f, --sigfile store IMA signature in .sig file instead of xattr\n"
" --xattr-user store xattrs in user namespace (for testing purposes)\n"
- " --rsa use RSA key type and signing scheme v1\n"
+
+ " --rsa use RSA key type and signing scheme v1 (deprecated)\n"
" -k, --key path to signing key (default: /etc/keys/{privkey,pubkey}_evm.pem)\n"
" or a pkcs11 URI\n"
" --keyid n overwrite signature keyid with a 32-bit value in hex (for signing)\n"
@@ -2637,8 +2645,8 @@ static void usage(void)
struct command cmds[] = {
{"--version", NULL, 0, ""},
{"help", cmd_help, 0, "<command>"},
- {"import", cmd_import, 0, "[--rsa] pubkey keyring", "Import public key into the keyring.\n"},
- {"convert", cmd_convert, 0, "key", "convert public key into the keyring.\n"},
+ {"import", cmd_import, 0, "[--rsa] pubkey keyring", "Import public key into the keyring. (deprecated)\n"},
+ {"convert", cmd_convert, 0, "key", "convert public key into the keyring. (deprecated)\n"},
{"sign", cmd_sign_evm, 0, "[-r] [--imahash | --imasig ] [--key key] [--pass [password] file", "Sign file metadata.\n"},
{"verify", cmd_verify_evm, 0, "file", "Verify EVM signature (for debugging).\n"},
{"ima_sign", cmd_sign_ima, 0, "[--sigfile] [--key key] [--pass [password] file", "Make file content signature.\n"},
diff --git a/src/libimaevm.c b/src/libimaevm.c
index e4b62b4989b2..cb815f953a80 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -294,8 +294,9 @@ out:
RSA *read_pub_key(const char *keyfile, int x509)
{
+ RSA *key = NULL;
+#if CONFIG_SIGV1
EVP_PKEY *pkey;
- RSA *key;
pkey = read_pub_pkey(keyfile, x509);
if (!pkey)
@@ -307,9 +308,11 @@ RSA *read_pub_key(const char *keyfile, int x509)
output_openssl_errors();
return NULL;
}
+#endif
return key;
}
+#if CONFIG_SIGV1
static int verify_hash_v1(const char *file, const unsigned char *hash, int size,
unsigned char *sig, int siglen, const char *keyfile)
{
@@ -351,6 +354,7 @@ static int verify_hash_v1(const char *file, const unsigned char *hash, int size,
return 0;
}
+#endif
struct public_key_entry {
struct public_key_entry *next;
@@ -686,6 +690,7 @@ int verify_hash(const char *file, const unsigned char *hash, int size,
{
/* Get signature type from sig header */
if (sig[1] == DIGSIG_VERSION_1) {
+#if CONFIG_SIGV1
const char *key = NULL;
/* Read pubkey from RSA key */
@@ -695,6 +700,10 @@ int verify_hash(const char *file, const unsigned char *hash, int size,
key = imaevm_params.keyfile;
return verify_hash_v1(file, hash, size, sig + 1, siglen - 1,
key);
+#else
+ log_info("Signature version 1 deprecated.");
+ return -1;
+#endif
} else if (sig[1] == DIGSIG_VERSION_2) {
return verify_hash_v2(file, hash, size, sig, siglen);
} else if (sig[1] == DIGSIG_VERSION_3) {
@@ -747,6 +756,7 @@ int ima_verify_signature(const char *file, unsigned char *sig, int siglen,
*/
int key2bin(RSA *key, unsigned char *pub)
{
+#if CONFIG_SIGV1
int len, b, offset = 0;
struct pubkey_hdr *pkh = (struct pubkey_hdr *)pub;
const BIGNUM *n, *e;
@@ -781,10 +791,14 @@ int key2bin(RSA *key, unsigned char *pub)
offset += len;
return offset;
+#else
+ return 77; /* SKIP */
+#endif
}
void calc_keyid_v1(uint8_t *keyid, char *str, const unsigned char *pkey, int len)
{
+#if CONFIG_SIGV1
uint8_t sha1[SHA_DIGEST_LENGTH];
uint64_t id;
@@ -799,6 +813,7 @@ void calc_keyid_v1(uint8_t *keyid, char *str, const unsigned char *pkey, int len
if (imaevm_params.verbose > LOG_INFO)
log_info("keyid-v1: %s\n", str);
+#endif
}
/*
@@ -990,10 +1005,11 @@ err_engine:
return NULL;
}
+#if CONFIG_SIGV1
static RSA *read_priv_key(const char *keyfile, const char *keypass)
{
+ RSA *key = NULL;
EVP_PKEY *pkey;
- RSA *key;
pkey = read_priv_pkey(keyfile, keypass);
if (!pkey)
@@ -1018,10 +1034,12 @@ static int get_hash_algo_v1(const char *algo)
return -1;
}
+#endif
static int sign_hash_v1(const char *hashalgo, const unsigned char *hash,
int size, const char *keyfile, unsigned char *sig)
{
+#if CONFIG_SIGV1
int len = -1, hashalgo_idx;
SHA_CTX ctx;
unsigned char pub[1024];
@@ -1099,6 +1117,8 @@ static int sign_hash_v1(const char *hashalgo, const unsigned char *hash,
out:
RSA_free(key);
return len;
+#endif
+ return 77; /* SKIP */
}
/*
diff --git a/tests/sign_verify.test b/tests/sign_verify.test
index c56290aa4932..948892759424 100755
--- a/tests/sign_verify.test
+++ b/tests/sign_verify.test
@@ -17,6 +17,7 @@
cd "$(dirname "$0")" || exit 1
PATH=../src:$PATH
+SIGV1=0
source ./functions.sh
_require cmp evmctl getfattr openssl xxd
@@ -368,13 +369,18 @@ try_different_sigs() {
## Test v1 signatures
# Signature v1 only supports sha1 and sha256 so any other should fail
-expect_fail \
- check_sign TYPE=ima KEY=rsa1024 ALG=md5 PREFIX=0x0301 OPTS=--rsa
+if [ $SIGV1 -eq 0 ]; then
+ __skip() { echo "IMA signature v1 tests are skipped: not supported"; return $SKIP; }
+ expect_pass __skip
+else
+ expect_fail \
+ check_sign TYPE=ima KEY=rsa1024 ALG=md5 PREFIX=0x0301 OPTS=--rsa
-sign_verify rsa1024 sha1 0x0301 --rsa
-sign_verify rsa1024 sha256 0x0301 --rsa
- try_different_keys
- try_different_sigs
+ sign_verify rsa1024 sha1 0x0301 --rsa
+ sign_verify rsa1024 sha256 0x0301 --rsa
+ try_different_keys
+ try_different_sigs
+fi
## Test v2 signatures with RSA PKCS#1
# List of allowed hashes much greater but not all are supported.
--
2.31.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [RFC PATCH ima-evm-utils 04/11] Deprecate IMA signature version 1
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 04/11] Deprecate IMA signature version 1 Mimi Zohar
@ 2022-08-30 11:55 ` Petr Vorel
2022-08-31 18:58 ` Mimi Zohar
2022-08-30 12:12 ` Stefan Berger
1 sibling, 1 reply; 31+ messages in thread
From: Petr Vorel @ 2022-08-30 11:55 UTC (permalink / raw)
To: Mimi Zohar; +Cc: linux-integrity, Vitaly Chikunov, Stefan Berger
> The original IMA file signatures were based on a SHA1 hash. Kernel
> support for other hash algorithms was subsequently upstreamed. Deprecate
> "--rsa" support.
> Define "--enable-sigv1" option to configure signature v1 support.
LGTM, few minor comments below.
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Kind regards,
Petr
...
> +++ b/configure.ac
...
> static int cmd_convert(struct command *cmd)
> {
> +#if CONFIG_SIGV1
> char *inkey;
> unsigned char _pub[1024], *pub = _pub;
> int len, err = 0;
> @@ -1033,6 +1033,8 @@ static int cmd_convert(struct command *cmd)
> RSA_free(key);
> return err;
> +#endif
> + return 77;
What is this this magic number? EBADFD?
Well, git grep shows many places with 77, so it's just a tip for next cleanup :).
...
> log_info("Importing public key %s from file %s into keyring %d\n", name, inkey, id);
> @@ -2598,7 +2605,8 @@ static void usage(void)
> " -d, --imahash make IMA hash\n"
> " -f, --sigfile store IMA signature in .sig file instead of xattr\n"
> " --xattr-user store xattrs in user namespace (for testing purposes)\n"
> - " --rsa use RSA key type and signing scheme v1\n"
> +
nit: was this blank line intentional?
> + " --rsa use RSA key type and signing scheme v1 (deprecated)\n"
> " -k, --key path to signing key (default: /etc/keys/{privkey,pubkey}_evm.pem)\n"
> " or a pkcs11 URI\n"
> " --keyid n overwrite signature keyid with a 32-bit value in hex (for signing)\n"
> @@ -2637,8 +2645,8 @@ static void usage(void)
> struct command cmds[] = {
> {"--version", NULL, 0, ""},
> {"help", cmd_help, 0, "<command>"},
> - {"import", cmd_import, 0, "[--rsa] pubkey keyring", "Import public key into the keyring.\n"},
> - {"convert", cmd_convert, 0, "key", "convert public key into the keyring.\n"},
> + {"import", cmd_import, 0, "[--rsa] pubkey keyring", "Import public key into the keyring. (deprecated)\n"},
> + {"convert", cmd_convert, 0, "key", "convert public key into the keyring. (deprecated)\n"},
> {"sign", cmd_sign_evm, 0, "[-r] [--imahash | --imasig ] [--key key] [--pass [password] file", "Sign file metadata.\n"},
> {"verify", cmd_verify_evm, 0, "file", "Verify EVM signature (for debugging).\n"},
> {"ima_sign", cmd_sign_ima, 0, "[--sigfile] [--key key] [--pass [password] file", "Make file content signature.\n"},
...
> +++ b/src/libimaevm.c
...
> +#if CONFIG_SIGV1
> static RSA *read_priv_key(const char *keyfile, const char *keypass)
> {
> + RSA *key = NULL;
nit: NULL is safe, I wonder if it is necessary (was needed before).
> EVP_PKEY *pkey;
> - RSA *key;
> pkey = read_priv_pkey(keyfile, keypass);
> if (!pkey)
> @@ -1018,10 +1034,12 @@ static int get_hash_algo_v1(const char *algo)
> return -1;
> }
> +#endif
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [RFC PATCH ima-evm-utils 04/11] Deprecate IMA signature version 1
2022-08-30 11:55 ` Petr Vorel
@ 2022-08-31 18:58 ` Mimi Zohar
0 siblings, 0 replies; 31+ messages in thread
From: Mimi Zohar @ 2022-08-31 18:58 UTC (permalink / raw)
To: Petr Vorel; +Cc: linux-integrity, Vitaly Chikunov, Stefan Berger
Hi Petr,
On Tue, 2022-08-30 at 13:55 +0200, Petr Vorel wrote:
> > The original IMA file signatures were based on a SHA1 hash. Kernel
> > support for other hash algorithms was subsequently upstreamed. Deprecate
> > "--rsa" support.
>
> > Define "--enable-sigv1" option to configure signature v1 support.
>
> LGTM, few minor comments below.
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
Thank you for all the reviews!
>
> ...
> > +++ b/configure.ac
> ...
>
> > static int cmd_convert(struct command *cmd)
> > {
> > +#if CONFIG_SIGV1
> > char *inkey;
> > unsigned char _pub[1024], *pub = _pub;
> > int len, err = 0;
> > @@ -1033,6 +1033,8 @@ static int cmd_convert(struct command *cmd)
>
> > RSA_free(key);
> > return err;
> > +#endif
> > + return 77;
> What is this this magic number? EBADFD?
> Well, git grep shows many places with 77, so it's just a tip for next cleanup :).
SKIP is defined as 77 in the tests/ directory. Using 77 in src/*.c is
incorrect. v2 ifdefs all of cmd_convert(), so this is going away.
I'll remove the other occurance of 77 in the src/ directory.
>
> ...
> > log_info("Importing public key %s from file %s into keyring %d\n", name, inkey, id);
> > @@ -2598,7 +2605,8 @@ static void usage(void)
> > " -d, --imahash make IMA hash\n"
> > " -f, --sigfile store IMA signature in .sig file instead of xattr\n"
> > " --xattr-user store xattrs in user namespace (for testing purposes)\n"
> > - " --rsa use RSA key type and signing scheme v1\n"
> > +
> nit: was this blank line intentional?
> > + " --rsa use RSA key type and signing scheme v1 (deprecated)\n"
> > " -k, --key path to signing key (default: /etc/keys/{privkey,pubkey}_evm.pem)\n"
> > " or a pkcs11 URI\n"
> > " --keyid n overwrite signature keyid with a 32-bit value in hex (for signing)\n"
> > @@ -2637,8 +2645,8 @@ static void usage(void)
> > struct command cmds[] = {
> > {"--version", NULL, 0, ""},
> > {"help", cmd_help, 0, "<command>"},
> > - {"import", cmd_import, 0, "[--rsa] pubkey keyring", "Import public key into the keyring.\n"},
> > - {"convert", cmd_convert, 0, "key", "convert public key into the keyring.\n"},
> > + {"import", cmd_import, 0, "[--rsa] pubkey keyring", "Import public key into the keyring. (deprecated)\n"},
> > + {"convert", cmd_convert, 0, "key", "convert public key into the keyring. (deprecated)\n"},
> > {"sign", cmd_sign_evm, 0, "[-r] [--imahash | --imasig ] [--key key] [--pass [password] file", "Sign file metadata.\n"},
> > {"verify", cmd_verify_evm, 0, "file", "Verify EVM signature (for debugging).\n"},
> > {"ima_sign", cmd_sign_ima, 0, "[--sigfile] [--key key] [--pass [password] file", "Make file content signature.\n"},
> ...
> > +++ b/src/libimaevm.c
> ...
>
> > +#if CONFIG_SIGV1
> > static RSA *read_priv_key(const char *keyfile, const char *keypass)
> > {
> > + RSA *key = NULL;
> nit: NULL is safe, I wonder if it is necessary (was needed before).
Looking at this again, it isn't needed. On failure to set either pkey
or key, NULL is returned.
> > EVP_PKEY *pkey;
> > - RSA *key;
>
> > pkey = read_priv_pkey(keyfile, keypass);
> > if (!pkey)
> > @@ -1018,10 +1034,12 @@ static int get_hash_algo_v1(const char *algo)
>
> > return -1;
> > }
> > +#endif
--
thanks,
Mimi
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC PATCH ima-evm-utils 04/11] Deprecate IMA signature version 1
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 04/11] Deprecate IMA signature version 1 Mimi Zohar
2022-08-30 11:55 ` Petr Vorel
@ 2022-08-30 12:12 ` Stefan Berger
2022-08-31 15:17 ` Mimi Zohar
1 sibling, 1 reply; 31+ messages in thread
From: Stefan Berger @ 2022-08-30 12:12 UTC (permalink / raw)
To: Mimi Zohar, linux-integrity; +Cc: Petr Vorel, Vitaly Chikunov
On 8/29/22 20:59, Mimi Zohar wrote:
> The original IMA file signatures were based on a SHA1 hash. Kernel
> support for other hash algorithms was subsequently upstreamed. Deprecate
> "--rsa" support.
>
> Define "--enable-sigv1" option to configure signature v1 support.
>
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
> configure.ac | 6 ++++++
> src/Makefile.am | 10 ++++++++++
> src/evmctl.c | 16 ++++++++++++----
> src/libimaevm.c | 24 ++++++++++++++++++++++--
> tests/sign_verify.test | 18 ++++++++++++------
> 5 files changed, 62 insertions(+), 12 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 9d3b23ff8def..dc666f2bb1fa 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -49,6 +49,11 @@ AC_ARG_ENABLE([openssl_conf],
> AC_DEFINE(DISABLE_OPENSSL_CONF, 1, [Define to disable loading of openssl config by evmctl.])
> fi], [enable_openssl_conf=yes])
>
> +AC_ARG_ENABLE(sigv1,
> + AS_HELP_STRING([--enable-sigv1], [Build ima-evm-utils with signature v1 support]))
> + AM_CONDITIONAL([CONFIG_SIGV1], [test "x$enable_sigv1" = "xyes"])
> + AS_IF([test "$enable_sigv1" != "yes"], [enable_sigv1="no"])
> +
> #debug support - yes for a while
> PKG_ARG_ENABLE(debug, "yes", DEBUG, [Enable Debug support])
> if test $pkg_cv_enable_debug = yes; then
> @@ -83,5 +88,6 @@ echo " openssl-conf: $enable_openssl_conf"
> 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 " doc: $have_doc"
> echo
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 396496bb439d..90c7249020cf 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -7,6 +7,10 @@ libimaevm_la_CPPFLAGS = $(AM_CPPFLAGS) $(LIBCRYPTO_CFLAGS)
> libimaevm_la_LDFLAGS = -version-info 3:0:0
> libimaevm_la_LIBADD = $(LIBCRYPTO_LIBS)
>
> +if CONFIG_SIGV1
> +libimaevm_la_CFLAGS = -DCONFIG_SIGV1
> +endif
> +
> include_HEADERS = imaevm.h
>
> nodist_libimaevm_la_SOURCES = hash_info.h
> @@ -22,6 +26,12 @@ evmctl_CPPFLAGS = $(AM_CPPFLAGS) $(LIBCRYPTO_CFLAGS)
> evmctl_LDFLAGS = $(LDFLAGS_READLINE)
> evmctl_LDADD = $(LIBCRYPTO_LIBS) -lkeyutils libimaevm.la
>
> +# Enable IMA signature version 1
> +if CONFIG_SIGV1
> +evmctl_CFLAGS = -DCONFIG_SIGV1
> +endif
> +
> +
> # USE_PCRTSS uses the Intel TSS
> if USE_PCRTSS
> evmctl_SOURCES += pcr_tss.c
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 76e2561798fa..621136b5b85f 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -987,7 +987,6 @@ static int cmd_verify_ima(struct command *cmd)
> init_public_keys("/etc/keys/x509_evm.der");
> }
>
> - errno = 0;
> if (!file) {
> log_err("Parameters missing\n");
> print_usage(cmd);
> @@ -1006,6 +1005,7 @@ static int cmd_verify_ima(struct command *cmd)
>
> static int cmd_convert(struct command *cmd)
> {
> +#if CONFIG_SIGV1
> char *inkey;
> unsigned char _pub[1024], *pub = _pub;
> int len, err = 0;
> @@ -1033,6 +1033,8 @@ static int cmd_convert(struct command *cmd)
>
> RSA_free(key);
> return err;
> +#endif
> + return 77;
> }
>
> static int cmd_import(struct command *cmd)
> @@ -1088,6 +1090,7 @@ static int cmd_import(struct command *cmd)
> calc_keyid_v2((uint32_t *)keyid, name, pkey);
> EVP_PKEY_free(pkey);
> } else {
> +#if CONFIG_SIGV1
> RSA *key = read_pub_key(inkey, imaevm_params.x509);
>
> if (!key)
> @@ -1095,6 +1098,10 @@ static int cmd_import(struct command *cmd)
> len = key2bin(key, pub);
> calc_keyid_v1(keyid, name, pub, len);
> RSA_free(key);
> +#else
> + log_info("Importing public RSA key not supported\n");
.. key *is* not supported
> + return 1;
> +#endif
> }
>
> log_info("Importing public key %s from file %s into keyring %d\n", name, inkey, id);
> @@ -2598,7 +2605,8 @@ static void usage(void)
> " -d, --imahash make IMA hash\n"
> " -f, --sigfile store IMA signature in .sig file instead of xattr\n"
> " --xattr-user store xattrs in user namespace (for testing purposes)\n"
> - " --rsa use RSA key type and signing scheme v1\n"
> +
> + " --rsa use RSA key type and signing scheme v1 (deprecated)\n"
> " -k, --key path to signing key (default: /etc/keys/{privkey,pubkey}_evm.pem)\n"
> " or a pkcs11 URI\n"
> " --keyid n overwrite signature keyid with a 32-bit value in hex (for signing)\n"
> @@ -2637,8 +2645,8 @@ static void usage(void)
> struct command cmds[] = {
> {"--version", NULL, 0, ""},
> {"help", cmd_help, 0, "<command>"},
> - {"import", cmd_import, 0, "[--rsa] pubkey keyring", "Import public key into the keyring.\n"},
> - {"convert", cmd_convert, 0, "key", "convert public key into the keyring.\n"},
> + {"import", cmd_import, 0, "[--rsa] pubkey keyring", "Import public key into the keyring. (deprecated)\n"},
> + {"convert", cmd_convert, 0, "key", "convert public key into the keyring. (deprecated)\n"},
Unless CONFIG_SIGV1 is defined it looks like cmd_convert doesn't do
anything anymore except return 77? In this case you could just ifdef
this command out.
> {"sign", cmd_sign_evm, 0, "[-r] [--imahash | --imasig ] [--key key] [--pass [password] file", "Sign file metadata.\n"},
> {"verify", cmd_verify_evm, 0, "file", "Verify EVM signature (for debugging).\n"},
> {"ima_sign", cmd_sign_ima, 0, "[--sigfile] [--key key] [--pass [password] file", "Make file content signature.\n"},
> diff --git a/src/libimaevm.c b/src/libimaevm.c
> index e4b62b4989b2..cb815f953a80 100644
> --- a/src/libimaevm.c
> +++ b/src/libimaevm.c
> @@ -294,8 +294,9 @@ out:
>
> RSA *read_pub_key(const char *keyfile, int x509)
> {
> + RSA *key = NULL;
> +#if CONFIG_SIGV1
> EVP_PKEY *pkey;
> - RSA *key;
>
> pkey = read_pub_pkey(keyfile, x509);
> if (!pkey)
> @@ -307,9 +308,11 @@ RSA *read_pub_key(const char *keyfile, int x509)
> output_openssl_errors();
> return NULL;
> }
> +#endif
> return key;
> }
>
> +#if CONFIG_SIGV1
> static int verify_hash_v1(const char *file, const unsigned char *hash, int size,
> unsigned char *sig, int siglen, const char *keyfile)
> {
> @@ -351,6 +354,7 @@ static int verify_hash_v1(const char *file, const unsigned char *hash, int size,
>
> return 0;
> }
> +#endif
>
> struct public_key_entry {
> struct public_key_entry *next;
> @@ -686,6 +690,7 @@ int verify_hash(const char *file, const unsigned char *hash, int size,
> {
> /* Get signature type from sig header */
> if (sig[1] == DIGSIG_VERSION_1) {
> +#if CONFIG_SIGV1
> const char *key = NULL;
>
> /* Read pubkey from RSA key */
> @@ -695,6 +700,10 @@ int verify_hash(const char *file, const unsigned char *hash, int size,
> key = imaevm_params.keyfile;
> return verify_hash_v1(file, hash, size, sig + 1, siglen - 1,
> key);
> +#else
> + log_info("Signature version 1 deprecated.");
> + return -1;
> +#endif
> } else if (sig[1] == DIGSIG_VERSION_2) {
> return verify_hash_v2(file, hash, size, sig, siglen);
> } else if (sig[1] == DIGSIG_VERSION_3) {
> @@ -747,6 +756,7 @@ int ima_verify_signature(const char *file, unsigned char *sig, int siglen,
> */
> int key2bin(RSA *key, unsigned char *pub)
> {
> +#if CONFIG_SIGV1
> int len, b, offset = 0;
> struct pubkey_hdr *pkh = (struct pubkey_hdr *)pub;
> const BIGNUM *n, *e;
> @@ -781,10 +791,14 @@ int key2bin(RSA *key, unsigned char *pub)
> offset += len;
>
> return offset;
> +#else
> + return 77; /* SKIP */
> +#endif
> }
This function has no callers if CONFIG_SIGV1 is not set and otherwise
it's useless also if someone was a user of the library only. I would
consider ifdef'ing the whole function...
>
> void calc_keyid_v1(uint8_t *keyid, char *str, const unsigned char *pkey, int len)
> {
> +#if CONFIG_SIGV1
> uint8_t sha1[SHA_DIGEST_LENGTH];
> uint64_t id;
>
> @@ -799,6 +813,7 @@ void calc_keyid_v1(uint8_t *keyid, char *str, const unsigned char *pkey, int len
>
> if (imaevm_params.verbose > LOG_INFO)
> log_info("keyid-v1: %s\n", str);
> +#endif
> }
>
> /*
> @@ -990,10 +1005,11 @@ err_engine:
> return NULL;
> }
>
> +#if CONFIG_SIGV1
> static RSA *read_priv_key(const char *keyfile, const char *keypass)
> {
> + RSA *key = NULL;
> EVP_PKEY *pkey;
> - RSA *key;
>
> pkey = read_priv_pkey(keyfile, keypass);
> if (!pkey)
> @@ -1018,10 +1034,12 @@ static int get_hash_algo_v1(const char *algo)
>
> return -1;
> }
> +#endif
>
> static int sign_hash_v1(const char *hashalgo, const unsigned char *hash,
> int size, const char *keyfile, unsigned char *sig)
> {
> +#if CONFIG_SIGV1
> int len = -1, hashalgo_idx;
> SHA_CTX ctx;
> unsigned char pub[1024];
> @@ -1099,6 +1117,8 @@ static int sign_hash_v1(const char *hashalgo, const unsigned char *hash,
> out:
> RSA_free(key);
> return len;
> +#endif
> + return 77; /* SKIP */
> }
>
> /*
> diff --git a/tests/sign_verify.test b/tests/sign_verify.test
> index c56290aa4932..948892759424 100755
> --- a/tests/sign_verify.test
> +++ b/tests/sign_verify.test
> @@ -17,6 +17,7 @@
>
> cd "$(dirname "$0")" || exit 1
> PATH=../src:$PATH
> +SIGV1=0
> source ./functions.sh
>
> _require cmp evmctl getfattr openssl xxd
> @@ -368,13 +369,18 @@ try_different_sigs() {
>
> ## Test v1 signatures
> # Signature v1 only supports sha1 and sha256 so any other should fail
> -expect_fail \
> - check_sign TYPE=ima KEY=rsa1024 ALG=md5 PREFIX=0x0301 OPTS=--rsa
> +if [ $SIGV1 -eq 0 ]; then
> + __skip() { echo "IMA signature v1 tests are skipped: not supported"; return $SKIP; }
> + expect_pass __skip
> +else
> + expect_fail \
> + check_sign TYPE=ima KEY=rsa1024 ALG=md5 PREFIX=0x0301 OPTS=--rsa
>
> -sign_verify rsa1024 sha1 0x0301 --rsa
> -sign_verify rsa1024 sha256 0x0301 --rsa
> - try_different_keys
> - try_different_sigs
> + sign_verify rsa1024 sha1 0x0301 --rsa
> + sign_verify rsa1024 sha256 0x0301 --rsa
> + try_different_keys
> + try_different_sigs
> +fi
>
> ## Test v2 signatures with RSA PKCS#1
> # List of allowed hashes much greater but not all are supported.
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [RFC PATCH ima-evm-utils 04/11] Deprecate IMA signature version 1
2022-08-30 12:12 ` Stefan Berger
@ 2022-08-31 15:17 ` Mimi Zohar
0 siblings, 0 replies; 31+ messages in thread
From: Mimi Zohar @ 2022-08-31 15:17 UTC (permalink / raw)
To: Stefan Berger, linux-integrity; +Cc: Petr Vorel, Vitaly Chikunov
On Tue, 2022-08-30 at 08:12 -0400, Stefan Berger wrote:
> > @@ -747,6 +756,7 @@ int ima_verify_signature(const char *file, unsigned char *sig, int siglen,
> > */
> > int key2bin(RSA *key, unsigned char *pub)
> > {
> > +#if CONFIG_SIGV1
> > int len, b, offset = 0;
> > struct pubkey_hdr *pkh = (struct pubkey_hdr *)pub;
> > const BIGNUM *n, *e;
> > @@ -781,10 +791,14 @@ int key2bin(RSA *key, unsigned char *pub)
> > offset += len;
> >
> > return offset;
> > +#else
> > + return 77; /* SKIP */
> > +#endif
> > }
>
> This function has no callers if CONFIG_SIGV1 is not set and otherwise
> it's useless also if someone was a user of the library only. I would
> consider ifdef'ing the whole function...
Agreed. key2bin() and calc_keyid_v1() are now fully commented out in
the next version, as well as cmd_convert() and read_pub_key() as you
suggested.
>
> >
> > void calc_keyid_v1(uint8_t *keyid, char *str, const unsigned char *pkey, int len)
> > {
> > +#if CONFIG_SIGV1
> > uint8_t sha1[SHA_DIGEST_LENGTH];
> > uint64_t id;
> >
> > @@ -799,6 +813,7 @@ void calc_keyid_v1(uint8_t *keyid, char *str, const unsigned char *pkey, int len
> >
> > if (imaevm_params.verbose > LOG_INFO)
> > log_info("keyid-v1: %s\n", str);
> > +#endif
> > }
--
thanks,
Mimi
^ permalink raw reply [flat|nested] 31+ messages in thread
* [RFC PATCH ima-evm-utils 05/11] Replace the low level SHA1 calls when calculating the TPM 1.2 PCRs
2022-08-30 0:59 [RFC PATCH ima-evm-utils 00/11] address deprecated warnings Mimi Zohar
` (3 preceding siblings ...)
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 04/11] Deprecate IMA signature version 1 Mimi Zohar
@ 2022-08-30 0:59 ` Mimi Zohar
2022-08-30 12:55 ` Petr Vorel
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 06/11] Replace the low level HMAC calls when calculating the EVM HMAC Mimi Zohar
` (5 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Mimi Zohar @ 2022-08-30 0:59 UTC (permalink / raw)
To: linux-integrity; +Cc: Mimi Zohar, Petr Vorel, Vitaly Chikunov, Stefan Berger
OpenSSL v3 emits deprecated warnings for SHA1 functions. Use the
EVP_ functions when walking the TPM 1.2 binary bios measurements
to calculate the TPM 1.2 PCRs.
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
src/evmctl.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 59 insertions(+), 6 deletions(-)
diff --git a/src/evmctl.c b/src/evmctl.c
index 621136b5b85f..6534950f3683 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -2296,6 +2296,11 @@ static int cmd_ima_measurement(struct command *cmd)
return ima_measurement(file);
}
+/*
+ * read_binary_bios_measurements - read the TPM 1.2 event log
+ *
+ * Returns 0 on success, 1 on failure.
+ */
#define MAX_EVENT_DATA_SIZE 200000
static int read_binary_bios_measurements(char *file, struct tpm_bank_info *bank)
{
@@ -2308,12 +2313,19 @@ static int read_binary_bios_measurements(char *file, struct tpm_bank_info *bank)
} header;
unsigned char data[MAX_EVENT_DATA_SIZE];
} event;
+ EVP_MD_CTX *mdctx;
+ const EVP_MD *md;
+ unsigned int mdlen;
+ int evp_err = 1; /* success */
struct stat s;
FILE *fp;
- SHA_CTX c;
int err = 0;
int len;
int i;
+#if OPENSSL_VERSION_NUMBER < 0x10100000
+ EVP_MD_CTX ctx;
+ mdctx = &ctx;
+#endif
if (stat(file, &s) == -1) {
errno = 0;
@@ -2335,6 +2347,23 @@ static int read_binary_bios_measurements(char *file, struct tpm_bank_info *bank)
if (imaevm_params.verbose > LOG_INFO)
log_info("Reading the TPM 1.2 event log %s.\n", file);
+ md = EVP_get_digestbyname(bank->algo_name);
+ if (!md) {
+ log_errno("Unknown message digest %s\n", bank->algo_name);
+ errno = 0;
+ fclose(fp);
+ return 1;
+ }
+
+#if OPENSSL_VERSION_NUMBER >= 0x10100000
+ mdctx = EVP_MD_CTX_new();
+ if (!mdctx) {
+ log_err("EVP_MD_CTX_new failed\n");
+ fclose(fp);
+ return 1;
+ }
+#endif
+
/* Extend the pseudo TPM PCRs with the event digest */
while (fread(&event, sizeof(event.header), 1, fp) == 1) {
if (imaevm_params.verbose > LOG_INFO) {
@@ -2343,13 +2372,30 @@ static int read_binary_bios_measurements(char *file, struct tpm_bank_info *bank)
}
if (event.header.pcr >= NUM_PCRS) {
log_err("Invalid PCR %d.\n", event.header.pcr);
- err = 1;
break;
}
- SHA1_Init(&c);
- SHA1_Update(&c, bank->pcr[event.header.pcr], 20);
- SHA1_Update(&c, event.header.digest, 20);
- SHA1_Final(bank->pcr[event.header.pcr], &c);
+
+ evp_err = EVP_DigestInit(mdctx, md);
+ if (evp_err == 0) {
+ log_err("EVP_DigestInit() failed\n");
+ break;
+ }
+
+ evp_err = EVP_DigestUpdate(mdctx, bank->pcr[event.header.pcr], 20);
+ if (evp_err == 0) {
+ log_err("EVP_DigestUpdate() failed\n");
+ break;
+ }
+ evp_err = EVP_DigestUpdate(mdctx, event.header.digest, 20);
+ if (evp_err == 0) {
+ log_err("EVP_DigestUpdate() failed\n");
+ break;
+ }
+ evp_err = EVP_DigestFinal(mdctx, bank->pcr[event.header.pcr], &mdlen);
+ if (evp_err == 0) {
+ log_err("EVP_DigestFinal() failed\n");
+ break;
+ }
if (event.header.len > MAX_EVENT_DATA_SIZE) {
log_err("Event data event too long.\n");
err = 1;
@@ -2358,10 +2404,17 @@ static int read_binary_bios_measurements(char *file, struct tpm_bank_info *bank)
len = fread(event.data, event.header.len, 1, fp);
if (len != 1) {
log_errno("Failed reading event data (short read)\n");
+ err = 1;
break;
}
}
+
+ if (evp_err == 0) /* EVP_ functions return 1 on success, 0 on failure */
+ err = 1;
fclose(fp);
+#if OPENSSL_VERSION_NUMBER >= 0x10100000
+ EVP_MD_CTX_free(mdctx);
+#endif
if (imaevm_params.verbose <= LOG_INFO)
return err;
--
2.31.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [RFC PATCH ima-evm-utils 06/11] Replace the low level HMAC calls when calculating the EVM HMAC
2022-08-30 0:59 [RFC PATCH ima-evm-utils 00/11] address deprecated warnings Mimi Zohar
` (4 preceding siblings ...)
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 05/11] Replace the low level SHA1 calls when calculating the TPM 1.2 PCRs Mimi Zohar
@ 2022-08-30 0:59 ` Mimi Zohar
2022-08-30 12:59 ` Petr Vorel
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 07/11] Add missing EVP_MD_CTX_free() call in calc_evm_hash() Mimi Zohar
` (4 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Mimi Zohar @ 2022-08-30 0:59 UTC (permalink / raw)
To: linux-integrity; +Cc: Mimi Zohar, Petr Vorel, Vitaly Chikunov, Stefan Berger
Calculating the EVM HMAC and labeling the filesystem was originally
included in ima-evm-utils for debugging purposes only. For now,
instead of removing EVM HMAC support just replace the low level
HMAC_ calls with EVP_ calls.
The '-a, --hashalgo' specifies the IMA hash or signature algorithm.
The kernel EVM HMAC is limited to SHA1. Fix ima-evm-utils by hard
coding the EVM HMAC algorithm to SHA1.
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
src/evmctl.c | 57 +++++++++++++++++++++++++++++-----------------------
1 file changed, 32 insertions(+), 25 deletions(-)
diff --git a/src/evmctl.c b/src/evmctl.c
index 6534950f3683..913b85f6b620 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -1160,12 +1160,12 @@ static int cmd_setxattr_ima(struct command *cmd)
static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *hash)
{
- const EVP_MD *md;
+ size_t mdlen;
+ EVP_MD_CTX *pctx;
+ EVP_PKEY *pkey = NULL;
struct stat st;
int err = -1;
uint32_t generation = 0;
- HMAC_CTX *pctx;
- unsigned int mdlen;
char **xattrname;
unsigned char xattr_value[1024];
unsigned char *key;
@@ -1176,10 +1176,8 @@ static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *h
struct h_misc_64 hmac_misc;
int hmac_size;
#if OPENSSL_VERSION_NUMBER < 0x10100000
- HMAC_CTX ctx;
+ EVP_MD_CTX ctx;
pctx = &ctx;
-#else
- pctx = HMAC_CTX_new();
#endif
key = file2bin(keyfile, NULL, &keylen);
@@ -1227,19 +1225,26 @@ static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *h
goto out;
}
- md = EVP_get_digestbyname(imaevm_params.hash_algo);
- if (!md) {
- log_err("EVP_get_digestbyname(%s) failed\n",
- imaevm_params.hash_algo);
+#if OPENSSL_VERSION_NUMBER >= 0x10100000
+ pctx = EVP_MD_CTX_new();
+ if (!pctx) {
+ log_err("EVP_MD_CTX_new failed\n");
goto out;
}
+#endif
- err = !HMAC_Init_ex(pctx, evmkey, sizeof(evmkey), md, NULL);
- if (err) {
+ pkey = EVP_PKEY_new_mac_key(EVP_PKEY_HMAC, NULL, evmkey, sizeof(evmkey));
+ if (!pkey) {
log_err("HMAC_Init() failed\n");
goto out;
}
+ err = EVP_DigestSignInit(pctx, NULL, EVP_sha1(), NULL, pkey);
+ if (err != 1) {
+ log_err("EVP_DigestSignInit() failed\n");
+ goto out;
+ }
+
for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) {
err = lgetxattr(file, *xattrname, xattr_value, sizeof(xattr_value));
if (err < 0) {
@@ -1250,12 +1255,12 @@ static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *h
log_info("skipping xattr: %s\n", *xattrname);
continue;
}
- /*log_debug("name: %s, value: %s, size: %d\n", *xattrname, xattr_value, err);*/
log_info("name: %s, size: %d\n", *xattrname, err);
log_debug_dump(xattr_value, err);
- err = !HMAC_Update(pctx, xattr_value, err);
- if (err) {
- log_err("HMAC_Update() failed\n");
+
+ err = EVP_DigestSignUpdate(pctx, xattr_value, err);
+ if (err != 1) {
+ log_err("EVP_DigestSignUpdate() failed\n");
goto out_ctx_cleanup;
}
}
@@ -1294,23 +1299,24 @@ static int calc_evm_hmac(const char *file, const char *keyfile, unsigned char *h
log_debug("hmac_misc (%d): ", hmac_size);
log_debug_dump(&hmac_misc, hmac_size);
- err = !HMAC_Update(pctx, (const unsigned char *)&hmac_misc, hmac_size);
- if (err) {
+ err = EVP_DigestSignUpdate(pctx, &hmac_misc, hmac_size);
+ if (err != 1) {
log_err("HMAC_Update() failed\n");
goto out_ctx_cleanup;
}
- err = !HMAC_Final(pctx, hash, &mdlen);
- if (err)
+ err = EVP_DigestSignFinal(pctx, hash, &mdlen);
+ if (err != 1)
log_err("HMAC_Final() failed\n");
out_ctx_cleanup:
-#if OPENSSL_VERSION_NUMBER < 0x10100000
- HMAC_CTX_cleanup(pctx);
-#else
- HMAC_CTX_free(pctx);
+ EVP_PKEY_free(pkey);
+#if OPENSSL_VERSION_NUMBER >= 0x10100000
+ EVP_MD_CTX_free(pctx);
#endif
out:
free(key);
- return err ?: mdlen;
+ if (err == 1)
+ return mdlen;
+ return err;
}
static int hmac_evm(const char *file, const char *key)
@@ -1334,6 +1340,7 @@ static int hmac_evm(const char *file, const char *key)
err = lsetxattr(file, xattr_evm, sig, len + 1, 0);
if (err < 0) {
log_err("setxattr failed: %s\n", file);
+ errno = 0;
return err;
}
}
--
2.31.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [RFC PATCH ima-evm-utils 07/11] Add missing EVP_MD_CTX_free() call in calc_evm_hash()
2022-08-30 0:59 [RFC PATCH ima-evm-utils 00/11] address deprecated warnings Mimi Zohar
` (5 preceding siblings ...)
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 06/11] Replace the low level HMAC calls when calculating the EVM HMAC Mimi Zohar
@ 2022-08-30 0:59 ` Mimi Zohar
2022-08-30 13:02 ` Petr Vorel
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 08/11] Deprecate use of OpenSSL 3 "engine" support Mimi Zohar
` (3 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Mimi Zohar @ 2022-08-30 0:59 UTC (permalink / raw)
To: linux-integrity; +Cc: Mimi Zohar, Petr Vorel, Vitaly Chikunov, Stefan Berger
When EVP_MD_CTX_new() call was added, the corresponding EVP_MD_CTX_free()
was never called. Properly free it.
Fixes: 81010f0d87ef ("ima-evm-utils: Add backward compatible support for openssl 1.1")
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
src/evmctl.c | 57 ++++++++++++++++++++++++++++++++++++----------------
1 file changed, 40 insertions(+), 17 deletions(-)
diff --git a/src/evmctl.c b/src/evmctl.c
index 913b85f6b620..490d355188d0 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -331,11 +331,17 @@ err:
return -1;
}
+/*
+ * calc_evm_hash - calculate the file metadata hash
+ *
+ * Returns 0 for EVP_ function failures. Return -1 for other failures.
+ * Return hash algorithm size on success.
+ */
static int calc_evm_hash(const char *file, unsigned char *hash)
{
const EVP_MD *md;
struct stat st;
- int err;
+ int err = -1;
uint32_t generation = 0;
EVP_MD_CTX *pctx;
unsigned int mdlen;
@@ -349,12 +355,11 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
#if OPENSSL_VERSION_NUMBER < 0x10100000
EVP_MD_CTX ctx;
pctx = &ctx;
-#else
- pctx = EVP_MD_CTX_new();
#endif
if (lstat(file, &st)) {
log_err("Failed to stat: %s\n", file);
+ errno = 0;
return -1;
}
@@ -391,20 +396,30 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
list_size = llistxattr(file, list, sizeof(list));
if (list_size < 0) {
log_err("llistxattr() failed\n");
+ errno = 0;
return -1;
}
+#if OPENSSL_VERSION_NUMBER >= 0x10100000
+ pctx = EVP_MD_CTX_new();
+ if (!pctx) {
+ log_err("EVP_MD_CTX_new() failed\n");
+ return 0;
+ }
+#endif
+
md = EVP_get_digestbyname(imaevm_params.hash_algo);
if (!md) {
log_err("EVP_get_digestbyname(%s) failed\n",
imaevm_params.hash_algo);
- return 1;
+ err = 0;
+ goto out;
}
err = EVP_DigestInit(pctx, md);
if (!err) {
log_err("EVP_DigestInit() failed\n");
- return 1;
+ goto out;
}
for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) {
@@ -415,7 +430,8 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
if (err > sizeof(xattr_value)) {
log_err("selinux[%u] value is too long to fit into xattr[%zu]\n",
err, sizeof(xattr_value));
- return -1;
+ err = -1;
+ goto out;
}
strcpy(xattr_value, selinux_str);
} else if (!strcmp(*xattrname, XATTR_NAME_IMA) && ima_str) {
@@ -423,7 +439,8 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
if (err > sizeof(xattr_value)) {
log_err("ima[%u] value is too long to fit into xattr[%zu]\n",
err, sizeof(xattr_value));
- return -1;
+ err = -1;
+ goto out;
}
hex2bin(xattr_value, ima_str, err);
} else if (!strcmp(*xattrname, XATTR_NAME_IMA) && evm_portable){
@@ -432,7 +449,7 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
if (err < 0) {
log_err("EVM portable sig: %s required\n",
xattr_ima);
- return -1;
+ goto out;
}
use_xattr_ima = 1;
} else if (!strcmp(*xattrname, XATTR_NAME_CAPS) && (hmac_flags & HMAC_FLAG_CAPS_SET)) {
@@ -442,7 +459,8 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
if (err >= sizeof(xattr_value)) {
log_err("caps[%u] value is too long to fit into xattr[%zu]\n",
err + 1, sizeof(xattr_value));
- return -1;
+ err = -1;
+ goto out;
}
strcpy(xattr_value, caps_str);
} else {
@@ -463,7 +481,7 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
err = EVP_DigestUpdate(pctx, xattr_value, err);
if (!err) {
log_err("EVP_DigestUpdate() failed\n");
- return 1;
+ goto out;
}
}
@@ -517,29 +535,33 @@ static int calc_evm_hash(const char *file, unsigned char *hash)
err = EVP_DigestUpdate(pctx, &hmac_misc, hmac_size);
if (!err) {
log_err("EVP_DigestUpdate() failed\n");
- return 1;
+ goto out;
}
if (!evm_immutable && !evm_portable &&
!(hmac_flags & HMAC_FLAG_NO_UUID)) {
err = get_uuid(&st, uuid);
if (err)
- return -1;
+ goto out;
err = EVP_DigestUpdate(pctx, (const unsigned char *)uuid, sizeof(uuid));
if (!err) {
log_err("EVP_DigestUpdate() failed\n");
- return 1;
+ goto out;
}
}
err = EVP_DigestFinal(pctx, hash, &mdlen);
- if (!err) {
+ if (!err)
log_err("EVP_DigestFinal() failed\n");
- return 1;
- }
- return mdlen;
+out:
+#if OPENSSL_VERSION_NUMBER >= 0x10100000
+ EVP_MD_CTX_free(pctx);
+#endif
+ if (err == 1)
+ return mdlen;
+ return err;
}
static int sign_evm(const char *file, const char *key)
@@ -575,6 +597,7 @@ static int sign_evm(const char *file, const char *key)
err = lsetxattr(file, xattr_evm, sig, len, 0);
if (err < 0) {
log_err("setxattr failed: %s\n", file);
+ errno = 0;
return err;
}
}
--
2.31.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [RFC PATCH ima-evm-utils 08/11] Deprecate use of OpenSSL 3 "engine" support
2022-08-30 0:59 [RFC PATCH ima-evm-utils 00/11] address deprecated warnings Mimi Zohar
` (6 preceding siblings ...)
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 07/11] Add missing EVP_MD_CTX_free() call in calc_evm_hash() Mimi Zohar
@ 2022-08-30 0:59 ` Mimi Zohar
2022-08-30 3:03 ` Vitaly Chikunov
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 09/11] Fix potential use after free in read_tpm_banks() Mimi Zohar
` (2 subsequent siblings)
10 siblings, 1 reply; 31+ messages in thread
From: Mimi Zohar @ 2022-08-30 0:59 UTC (permalink / raw)
To: linux-integrity; +Cc: Mimi Zohar, Petr Vorel, Vitaly Chikunov, Stefan Berger
OpenSSL 3 "engine" support is deprecated, which results in deprecated
build warning messages. In preparation for OpenSSL "engine" support
to be removed, define a "--enable-engine" configuration option. If not
specified, disable engine support on systems with OpenSSL v3.
When ima-evm-utils engine support is disabled, don't execute the tests
requiring it.
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
configure.ac | 15 +++++++++++++++
src/Makefile.am | 8 ++++++++
src/evmctl.c | 17 +++++++++++++++--
src/imaevm.h | 2 ++
src/libimaevm.c | 5 +++++
tests/functions.sh | 11 ++++++++++-
tests/ima_hash.test | 9 +++++++++
tests/sign_verify.test | 10 ++++++++++
8 files changed, 74 insertions(+), 3 deletions(-)
diff --git a/configure.ac b/configure.ac
index dc666f2bb1fa..5e0b78eb651b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -54,6 +54,19 @@ 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([--enable-engine], [build ima-evm-utils with OpenSSL engine support])])
+
+ssl_version=$(openssl version | sed -e 's/^OpenSSL //' | sed -e 's/ .*//')
+if test -z "$enable_engine"; then
+ if test "${ssl_version::1}" = "3"; then
+ enable_engine="no"
+ else
+ enable_engine="yes"
+ fi
+fi
+AM_CONDITIONAL([CONFIG_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 +102,7 @@ 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 " ssl: $ssl_version"
echo " doc: $have_doc"
echo
diff --git a/src/Makefile.am b/src/Makefile.am
index 90c7249020cf..a810d6e0acff 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -11,6 +11,10 @@ if CONFIG_SIGV1
libimaevm_la_CFLAGS = -DCONFIG_SIGV1
endif
+if CONFIG_ENGINE
+libimaevm_la_CFLAGS = -DCONFIG_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_ENGINE
+evmctl_CFLAGS = -DCONFIG_ENGINE
+endif
# USE_PCRTSS uses the Intel TSS
if USE_PCRTSS
diff --git a/src/evmctl.c b/src/evmctl.c
index 490d355188d0..ad96789f1984 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -64,7 +64,9 @@
#include <openssl/hmac.h>
#include <openssl/err.h>
#include <openssl/rsa.h>
+#if CONFIG_ENGINE
#include <openssl/engine.h>
+#endif
#include <openssl/x509v3.h>
#include "hash_info.h"
#include "pcr.h"
@@ -2715,7 +2717,7 @@ 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"
+ " --engine e preload OpenSSL engine e (such as: gost) id deprecated\n"
" --ignore-violations ignore ToMToU measurement violations\n"
" -v increase verbosity level\n"
" -h, --help display this help and exit\n"
@@ -2828,7 +2830,9 @@ static char *get_password(void)
static ENGINE *setup_engine(const char *engine_id)
{
+#if CONFIG_ENGINE
ENGINE *eng = ENGINE_by_id(engine_id);
+
if (!eng) {
log_err("engine %s isn't available\n", optarg);
ERR_print_errors_fp(stderr);
@@ -2841,6 +2845,9 @@ static ENGINE *setup_engine(const char *engine_id)
if (eng)
ENGINE_set_default(eng, ENGINE_METHOD_ALL);
return eng;
+#endif
+ log_err("OpenSSL 3 \"engine\" support is deprecated\n");
+ return NULL;
}
int main(int argc, char *argv[])
@@ -2969,8 +2976,12 @@ int main(int argc, char *argv[])
break;
case 139: /* --engine e */
imaevm_params.eng = setup_engine(optarg);
- if (!imaevm_params.eng)
+ if (!imaevm_params.eng) {
+#ifndef CONFIG_ENGINE
+ err = 77; /* SKIP */
+#endif
goto error;
+ }
break;
case 140: /* --xattr-user */
xattr_ima = "user.ima";
@@ -3056,6 +3067,7 @@ int main(int argc, char *argv[])
}
error:
+#if CONFIG_ENGINE
if (imaevm_params.eng) {
ENGINE_finish(imaevm_params.eng);
ENGINE_free(imaevm_params.eng);
@@ -3063,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..ebe8c20d566a 100644
--- a/src/imaevm.h
+++ b/src/imaevm.h
@@ -48,7 +48,9 @@
#include <errno.h>
#include <sys/types.h>
#include <openssl/rsa.h>
+#ifdef CONFIG_ENGINE
#include <openssl/engine.h>
+#endif
#ifdef USE_FPRINTF
#define do_log(level, fmt, args...) \
diff --git a/src/libimaevm.c b/src/libimaevm.c
index cb815f953a80..d81bdbb85250 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -965,6 +965,7 @@ static EVP_PKEY *read_priv_pkey(const char *keyfile, const char *keypass)
EVP_PKEY *pkey;
if (!strncmp(keyfile, "pkcs11:", 7)) {
+#ifdef CONFIG_ENGINE
if (!imaevm_params.keyid) {
log_err("When using a pkcs11 URI you must provide the keyid with an option\n");
return NULL;
@@ -981,6 +982,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 3 \"engine\" support is deprecated\n");
+ goto err_engine;
+#endif
} else {
fp = fopen(keyfile, "r");
if (!fp) {
diff --git a/tests/functions.sh b/tests/functions.sh
index 8f6f02dfcd95..ddc8fe9e5ea6 100755
--- a/tests/functions.sh
+++ b/tests/functions.sh
@@ -312,4 +312,13 @@ _softhsm_teardown() {
rm -rf "${SOFTHSM_SETUP_CONFIGDIR}"
unset SOFTHSM_SETUP_CONFIGDIR SOFTHSM2_CONF PKCS11_KEYURI \
EVMCTL_ENGINE OPENSSL_ENGINE OPENSSL_KEYFORM
-}
\ No newline at end of file
+}
+
+# OpenSSL 3 engine support still works, but is deprecated. In preparation
+# for it being removed, a new ima-evm-utils configuration option
+# "--enable-engine" is defined.`
+_is_engine_supported() {
+ cmd="evmctl --engine pkcs11"
+ $cmd &>/dev/null
+ ENGINE_SUPPORTED=$?
+}
diff --git a/tests/ima_hash.test b/tests/ima_hash.test
index e88fd59cc359..0de9e6808af9 100755
--- a/tests/ima_hash.test
+++ b/tests/ima_hash.test
@@ -71,6 +71,15 @@ expect_pass check sha384 0x0405 38b060a751ac96384cd9327eb1b1e36a21fdb71114b
expect_pass check sha512 0x0406 cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e
expect_pass check rmd160 0x0403 9c1185a5c5e9fc54612808977ee8f548b2258d31
expect_pass check sm3 0x0411 1ab21d8355cfa17f8e61194831e81a8f22bec8c728fefb747ed035eb5082aa2b
+
+# Remaining tests require engine support
+_is_engine_supported
+if [ $ENGINE_SUPPORTED -eq 77 ]; then
+ __skip() { echo "Tests requiring engine support are skipped (not supported)"; return $SKIP; }
+ expect_pass __skip
+ exit 0
+fi
+
_enable_gost_engine
expect_pass check md_gost12_256 0x0412 3f539a213e97c802cc229d474c6aa32a825a360b2a933a949fd925208d9ce1bb
expect_pass check streebog256 0x0412 3f539a213e97c802cc229d474c6aa32a825a360b2a933a949fd925208d9ce1bb
diff --git a/tests/sign_verify.test b/tests/sign_verify.test
index 948892759424..8c005b741916 100755
--- a/tests/sign_verify.test
+++ b/tests/sign_verify.test
@@ -18,6 +18,8 @@
cd "$(dirname "$0")" || exit 1
PATH=../src:$PATH
SIGV1=0
+ENGINE_SUPPORTED=0
+
source ./functions.sh
_require cmp evmctl getfattr openssl xxd
@@ -418,6 +420,14 @@ if [ -x /opt/openssl3/bin/openssl ]; then
sign_verify sm2 sm3 0x030211:K:004[345678]
fi
+# Remaining tests require engine support
+_is_engine_supported
+if [ $ENGINE_SUPPORTED -eq 77 ]; then
+ __skip() { echo "Tests requiring engine support are skipped (not supported)"; return $SKIP; }
+ expect_pass __skip
+ exit 0
+fi
+
# Test v2 signatures with EC-RDSA
_enable_gost_engine
sign_verify gost2012_256-A md_gost12_256 0x030212:K:0040
--
2.31.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* Re: [RFC PATCH ima-evm-utils 08/11] Deprecate use of OpenSSL 3 "engine" support
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 08/11] Deprecate use of OpenSSL 3 "engine" support Mimi Zohar
@ 2022-08-30 3:03 ` Vitaly Chikunov
2022-08-30 11:46 ` Mimi Zohar
0 siblings, 1 reply; 31+ messages in thread
From: Vitaly Chikunov @ 2022-08-30 3:03 UTC (permalink / raw)
To: Mimi Zohar; +Cc: linux-integrity, Petr Vorel, Stefan Berger
Mimi,
On Mon, Aug 29, 2022 at 08:59:33PM -0400, Mimi Zohar wrote:
> OpenSSL 3 "engine" support is deprecated, which results in deprecated
> build warning messages. In preparation for OpenSSL "engine" support
> to be removed, define a "--enable-engine" configuration option. If not
> specified, disable engine support on systems with OpenSSL v3.
This seems to change default behavior.
Many distributions (AFAIK) still contain openssl1, and some contain both
openssl1 and openssl3, including devel packages. So they will suddenly
drop --engine support on ima-evm-utils update, even though their OpenSSL
supports it.
Maybe --disable-engine should be added instead if user wants to avoid
compile time deprecation warnings?
(Also, engines aren't just deprecated, they are deprecated in favor of
providers.)
>
> When ima-evm-utils engine support is disabled, don't execute the tests
> requiring it.
>
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
> configure.ac | 15 +++++++++++++++
> src/Makefile.am | 8 ++++++++
> src/evmctl.c | 17 +++++++++++++++--
> src/imaevm.h | 2 ++
> src/libimaevm.c | 5 +++++
> tests/functions.sh | 11 ++++++++++-
> tests/ima_hash.test | 9 +++++++++
> tests/sign_verify.test | 10 ++++++++++
> 8 files changed, 74 insertions(+), 3 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index dc666f2bb1fa..5e0b78eb651b 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -54,6 +54,19 @@ 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([--enable-engine], [build ima-evm-utils with OpenSSL engine support])])
> +
> +ssl_version=$(openssl version | sed -e 's/^OpenSSL //' | sed -e 's/ .*//')
> +if test -z "$enable_engine"; then
> + if test "${ssl_version::1}" = "3"; then
> + enable_engine="no"
> + else
> + enable_engine="yes"
> + fi
> +fi
> +AM_CONDITIONAL([CONFIG_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 +102,7 @@ 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 " ssl: $ssl_version"
> echo " doc: $have_doc"
> echo
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 90c7249020cf..a810d6e0acff 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -11,6 +11,10 @@ if CONFIG_SIGV1
> libimaevm_la_CFLAGS = -DCONFIG_SIGV1
> endif
>
> +if CONFIG_ENGINE
> +libimaevm_la_CFLAGS = -DCONFIG_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_ENGINE
> +evmctl_CFLAGS = -DCONFIG_ENGINE
> +endif
>
> # USE_PCRTSS uses the Intel TSS
> if USE_PCRTSS
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 490d355188d0..ad96789f1984 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -64,7 +64,9 @@
> #include <openssl/hmac.h>
> #include <openssl/err.h>
> #include <openssl/rsa.h>
> +#if CONFIG_ENGINE
> #include <openssl/engine.h>
> +#endif
> #include <openssl/x509v3.h>
> #include "hash_info.h"
> #include "pcr.h"
> @@ -2715,7 +2717,7 @@ 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"
> + " --engine e preload OpenSSL engine e (such as: gost) id deprecated\n"
Typo.
> " --ignore-violations ignore ToMToU measurement violations\n"
> " -v increase verbosity level\n"
> " -h, --help display this help and exit\n"
> @@ -2828,7 +2830,9 @@ static char *get_password(void)
>
> static ENGINE *setup_engine(const char *engine_id)
> {
> +#if CONFIG_ENGINE
> ENGINE *eng = ENGINE_by_id(engine_id);
> +
> if (!eng) {
> log_err("engine %s isn't available\n", optarg);
> ERR_print_errors_fp(stderr);
> @@ -2841,6 +2845,9 @@ static ENGINE *setup_engine(const char *engine_id)
> if (eng)
> ENGINE_set_default(eng, ENGINE_METHOD_ALL);
> return eng;
> +#endif
> + log_err("OpenSSL 3 \"engine\" support is deprecated\n");
> + return NULL;
> }
>
> int main(int argc, char *argv[])
> @@ -2969,8 +2976,12 @@ int main(int argc, char *argv[])
> break;
> case 139: /* --engine e */
> imaevm_params.eng = setup_engine(optarg);
> - if (!imaevm_params.eng)
> + if (!imaevm_params.eng) {
> +#ifndef CONFIG_ENGINE
> + err = 77; /* SKIP */
> +#endif
> goto error;
> + }
> break;
> case 140: /* --xattr-user */
> xattr_ima = "user.ima";
> @@ -3056,6 +3067,7 @@ int main(int argc, char *argv[])
> }
>
> error:
> +#if CONFIG_ENGINE
> if (imaevm_params.eng) {
> ENGINE_finish(imaevm_params.eng);
> ENGINE_free(imaevm_params.eng);
> @@ -3063,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..ebe8c20d566a 100644
> --- a/src/imaevm.h
> +++ b/src/imaevm.h
> @@ -48,7 +48,9 @@
> #include <errno.h>
> #include <sys/types.h>
> #include <openssl/rsa.h>
> +#ifdef CONFIG_ENGINE
> #include <openssl/engine.h>
> +#endif
>
> #ifdef USE_FPRINTF
> #define do_log(level, fmt, args...) \
> diff --git a/src/libimaevm.c b/src/libimaevm.c
> index cb815f953a80..d81bdbb85250 100644
> --- a/src/libimaevm.c
> +++ b/src/libimaevm.c
> @@ -965,6 +965,7 @@ static EVP_PKEY *read_priv_pkey(const char *keyfile, const char *keypass)
> EVP_PKEY *pkey;
>
> if (!strncmp(keyfile, "pkcs11:", 7)) {
> +#ifdef CONFIG_ENGINE
> if (!imaevm_params.keyid) {
> log_err("When using a pkcs11 URI you must provide the keyid with an option\n");
> return NULL;
> @@ -981,6 +982,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 3 \"engine\" support is deprecated\n");
> + goto err_engine;
> +#endif
> } else {
> fp = fopen(keyfile, "r");
> if (!fp) {
> diff --git a/tests/functions.sh b/tests/functions.sh
> index 8f6f02dfcd95..ddc8fe9e5ea6 100755
> --- a/tests/functions.sh
> +++ b/tests/functions.sh
> @@ -312,4 +312,13 @@ _softhsm_teardown() {
> rm -rf "${SOFTHSM_SETUP_CONFIGDIR}"
> unset SOFTHSM_SETUP_CONFIGDIR SOFTHSM2_CONF PKCS11_KEYURI \
> EVMCTL_ENGINE OPENSSL_ENGINE OPENSSL_KEYFORM
> -}
> \ No newline at end of file
> +}
> +
> +# OpenSSL 3 engine support still works, but is deprecated. In preparation
> +# for it being removed, a new ima-evm-utils configuration option
> +# "--enable-engine" is defined.`
> +_is_engine_supported() {
> + cmd="evmctl --engine pkcs11"
I think pkcs11 engine is optional (and could be in additional package)
even if other engines are present.
Also engine could be loaded via openssl.cnf/OPENSSL_CONF, in that case
--engine option is not needed but engine is still there to use/test.
> + $cmd &>/dev/null
> + ENGINE_SUPPORTED=$?
> +}
> diff --git a/tests/ima_hash.test b/tests/ima_hash.test
> index e88fd59cc359..0de9e6808af9 100755
> --- a/tests/ima_hash.test
> +++ b/tests/ima_hash.test
> @@ -71,6 +71,15 @@ expect_pass check sha384 0x0405 38b060a751ac96384cd9327eb1b1e36a21fdb71114b
> expect_pass check sha512 0x0406 cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e
> expect_pass check rmd160 0x0403 9c1185a5c5e9fc54612808977ee8f548b2258d31
> expect_pass check sm3 0x0411 1ab21d8355cfa17f8e61194831e81a8f22bec8c728fefb747ed035eb5082aa2b
> +
> +# Remaining tests require engine support
> +_is_engine_supported
> +if [ $ENGINE_SUPPORTED -eq 77 ]; then
> + __skip() { echo "Tests requiring engine support are skipped (not supported)"; return $SKIP; }
> + expect_pass __skip
> + exit 0
> +fi
GOST tests try to handle absence of algorithms (can work w/o --engine
option if configured via openssl config) and skip gracefully.
Perhaps this check should be moved below them just for pkcs11 tests
if they are so sensitive.
Thanks,
> +
> _enable_gost_engine
> expect_pass check md_gost12_256 0x0412 3f539a213e97c802cc229d474c6aa32a825a360b2a933a949fd925208d9ce1bb
> expect_pass check streebog256 0x0412 3f539a213e97c802cc229d474c6aa32a825a360b2a933a949fd925208d9ce1bb
> diff --git a/tests/sign_verify.test b/tests/sign_verify.test
> index 948892759424..8c005b741916 100755
> --- a/tests/sign_verify.test
> +++ b/tests/sign_verify.test
> @@ -18,6 +18,8 @@
> cd "$(dirname "$0")" || exit 1
> PATH=../src:$PATH
> SIGV1=0
> +ENGINE_SUPPORTED=0
> +
> source ./functions.sh
>
> _require cmp evmctl getfattr openssl xxd
> @@ -418,6 +420,14 @@ if [ -x /opt/openssl3/bin/openssl ]; then
> sign_verify sm2 sm3 0x030211:K:004[345678]
> fi
>
> +# Remaining tests require engine support
> +_is_engine_supported
> +if [ $ENGINE_SUPPORTED -eq 77 ]; then
> + __skip() { echo "Tests requiring engine support are skipped (not supported)"; return $SKIP; }
> + expect_pass __skip
> + exit 0
> +fi
> +
> # Test v2 signatures with EC-RDSA
> _enable_gost_engine
> sign_verify gost2012_256-A md_gost12_256 0x030212:K:0040
> --
> 2.31.1
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [RFC PATCH ima-evm-utils 08/11] Deprecate use of OpenSSL 3 "engine" support
2022-08-30 3:03 ` Vitaly Chikunov
@ 2022-08-30 11:46 ` Mimi Zohar
2022-08-30 20:52 ` Vitaly Chikunov
0 siblings, 1 reply; 31+ messages in thread
From: Mimi Zohar @ 2022-08-30 11:46 UTC (permalink / raw)
To: Vitaly Chikunov; +Cc: linux-integrity, Petr Vorel, Stefan Berger
Hi Vitaly,
On Tue, 2022-08-30 at 06:03 +0300, Vitaly Chikunov wrote:
> Mimi,
>
> On Mon, Aug 29, 2022 at 08:59:33PM -0400, Mimi Zohar wrote:
> > OpenSSL 3 "engine" support is deprecated, which results in deprecated
> > build warning messages. In preparation for OpenSSL "engine" support
> > to be removed, define a "--enable-engine" configuration option. If not
> > specified, disable engine support on systems with OpenSSL v3.
>
> This seems to change default behavior.
>
> Many distributions (AFAIK) still contain openssl1, and some contain both
> openssl1 and openssl3, including devel packages. So they will suddenly
> drop --engine support on ima-evm-utils update, even though their OpenSSL
> supports it.
>
> Maybe --disable-engine should be added instead if user wants to avoid
> compile time deprecation warnings?
No, engine support is not disabled for previous versions of OpenSSL.
It's only disabled for OpenSSL v3, with the --enable-engine option
allowing it to be configured. The "-Wno-deprecated-declarations"
option could even be added.
>
> (Also, engines aren't just deprecated, they are deprecated in favor of
> providers.)
Right. At some point, OpenSSL engine support will be removed. This
is a first step to removing it in ima-evm-utils. I searched but didn't
find any gost providers. Feel free to add it.
> >
> > When ima-evm-utils engine support is disabled, don't execute the tests
> > requiring it.
> >
> > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> > ---
> > configure.ac | 15 +++++++++++++++
> > src/Makefile.am | 8 ++++++++
> > src/evmctl.c | 17 +++++++++++++++--
> > src/imaevm.h | 2 ++
> > src/libimaevm.c | 5 +++++
> > tests/functions.sh | 11 ++++++++++-
> > tests/ima_hash.test | 9 +++++++++
> > tests/sign_verify.test | 10 ++++++++++
> > 8 files changed, 74 insertions(+), 3 deletions(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index dc666f2bb1fa..5e0b78eb651b 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -54,6 +54,19 @@ 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([--enable-engine], [build ima-evm-utils with OpenSSL engine support])])
> > +
> > +ssl_version=$(openssl version | sed -e 's/^OpenSSL //' | sed -e 's/ .*//')
> > +if test -z "$enable_engine"; then
> > + if test "${ssl_version::1}" = "3"; then
> > + enable_engine="no"
> > + else
> > + enable_engine="yes"
> > + fi
> > +fi
> > +AM_CONDITIONAL([CONFIG_ENGINE], [test "x$enable_engine" = "xyes"])
Above is the code to conditionally disable engine support based on the
OpenSSL version.
> > +
> > #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 +102,7 @@ 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 " ssl: $ssl_version"
> > echo " doc: $have_doc"
> > echo
> > diff --git a/src/Makefile.am b/src/Makefile.am
> > index 90c7249020cf..a810d6e0acff 100644
> > --- a/src/Makefile.am
> > +++ b/src/Makefile.am
> > @@ -11,6 +11,10 @@ if CONFIG_SIGV1
> > libimaevm_la_CFLAGS = -DCONFIG_SIGV1
> > endif
> >
> > +if CONFIG_ENGINE
> > +libimaevm_la_CFLAGS = -DCONFIG_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_ENGINE
> > +evmctl_CFLAGS = -DCONFIG_ENGINE
> > +endif
> >
> > # USE_PCRTSS uses the Intel TSS
> > if USE_PCRTSS
> > diff --git a/src/evmctl.c b/src/evmctl.c
> > index 490d355188d0..ad96789f1984 100644
> > --- a/src/evmctl.c
> > +++ b/src/evmctl.c
> > @@ -64,7 +64,9 @@
> > #include <openssl/hmac.h>
> > #include <openssl/err.h>
> > #include <openssl/rsa.h>
> > +#if CONFIG_ENGINE
> > #include <openssl/engine.h>
> > +#endif
> > #include <openssl/x509v3.h>
> > #include "hash_info.h"
> > #include "pcr.h"
> > @@ -2715,7 +2717,7 @@ 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"
> > + " --engine e preload OpenSSL engine e (such as: gost) id deprecated\n"
>
> Typo.
Thanks.
>
> > " --ignore-violations ignore ToMToU measurement violations\n"
> > " -v increase verbosity level\n"
> > " -h, --help display this help and exit\n"
> > @@ -2828,7 +2830,9 @@ static char *get_password(void)
> >
> > static ENGINE *setup_engine(const char *engine_id)
> > {
> > +#if CONFIG_ENGINE
> > ENGINE *eng = ENGINE_by_id(engine_id);
> > +
> > if (!eng) {
> > log_err("engine %s isn't available\n", optarg);
> > ERR_print_errors_fp(stderr);
> > @@ -2841,6 +2845,9 @@ static ENGINE *setup_engine(const char *engine_id)
> > if (eng)
> > ENGINE_set_default(eng, ENGINE_METHOD_ALL);
> > return eng;
> > +#endif
> > + log_err("OpenSSL 3 \"engine\" support is deprecated\n");
> > + return NULL;
> > }
> >
> > int main(int argc, char *argv[])
> > @@ -2969,8 +2976,12 @@ int main(int argc, char *argv[])
> > break;
> > case 139: /* --engine e */
> > imaevm_params.eng = setup_engine(optarg);
> > - if (!imaevm_params.eng)
> > + if (!imaevm_params.eng) {
> > +#ifndef CONFIG_ENGINE
> > + err = 77; /* SKIP */
> > +#endif
> > goto error;
> > + }
> > break;
> > case 140: /* --xattr-user */
> > xattr_ima = "user.ima";
> > @@ -3056,6 +3067,7 @@ int main(int argc, char *argv[])
> > }
> >
> > error:
> > +#if CONFIG_ENGINE
> > if (imaevm_params.eng) {
> > ENGINE_finish(imaevm_params.eng);
> > ENGINE_free(imaevm_params.eng);
> > @@ -3063,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..ebe8c20d566a 100644
> > --- a/src/imaevm.h
> > +++ b/src/imaevm.h
> > @@ -48,7 +48,9 @@
> > #include <errno.h>
> > #include <sys/types.h>
> > #include <openssl/rsa.h>
> > +#ifdef CONFIG_ENGINE
> > #include <openssl/engine.h>
> > +#endif
> >
> > #ifdef USE_FPRINTF
> > #define do_log(level, fmt, args...) \
> > diff --git a/src/libimaevm.c b/src/libimaevm.c
> > index cb815f953a80..d81bdbb85250 100644
> > --- a/src/libimaevm.c
> > +++ b/src/libimaevm.c
> > @@ -965,6 +965,7 @@ static EVP_PKEY *read_priv_pkey(const char *keyfile, const char *keypass)
> > EVP_PKEY *pkey;
> >
> > if (!strncmp(keyfile, "pkcs11:", 7)) {
> > +#ifdef CONFIG_ENGINE
> > if (!imaevm_params.keyid) {
> > log_err("When using a pkcs11 URI you must provide the keyid with an option\n");
> > return NULL;
> > @@ -981,6 +982,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 3 \"engine\" support is deprecated\n");
> > + goto err_engine;
> > +#endif
> > } else {
> > fp = fopen(keyfile, "r");
> > if (!fp) {
> > diff --git a/tests/functions.sh b/tests/functions.sh
> > index 8f6f02dfcd95..ddc8fe9e5ea6 100755
> > --- a/tests/functions.sh
> > +++ b/tests/functions.sh
> > @@ -312,4 +312,13 @@ _softhsm_teardown() {
> > rm -rf "${SOFTHSM_SETUP_CONFIGDIR}"
> > unset SOFTHSM_SETUP_CONFIGDIR SOFTHSM2_CONF PKCS11_KEYURI \
> > EVMCTL_ENGINE OPENSSL_ENGINE OPENSSL_KEYFORM
> > -}
> > \ No newline at end of file
> > +}
> > +
> > +# OpenSSL 3 engine support still works, but is deprecated. In preparation
> > +# for it being removed, a new ima-evm-utils configuration option
> > +# "--enable-engine" is defined.`
> > +_is_engine_supported() {
> > + cmd="evmctl --engine pkcs11"
>
> I think pkcs11 engine is optional (and could be in additional package)
> even if other engines are present.
We need to move away from OpenSSL engine support towards providers.
>
> Also engine could be loaded via openssl.cnf/OPENSSL_CONF, in that case
> --engine option is not needed but engine is still there to use/test.
Thank you for reminding me that engine support based on the OpenSSL
configuration also needs to be deprecated.
>
>
> > + $cmd &>/dev/null
> > + ENGINE_SUPPORTED=$?
> > +}
> > diff --git a/tests/ima_hash.test b/tests/ima_hash.test
> > index e88fd59cc359..0de9e6808af9 100755
> > --- a/tests/ima_hash.test
> > +++ b/tests/ima_hash.test
> > @@ -71,6 +71,15 @@ expect_pass check sha384 0x0405 38b060a751ac96384cd9327eb1b1e36a21fdb71114b
> > expect_pass check sha512 0x0406 cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e
> > expect_pass check rmd160 0x0403 9c1185a5c5e9fc54612808977ee8f548b2258d31
> > expect_pass check sm3 0x0411 1ab21d8355cfa17f8e61194831e81a8f22bec8c728fefb747ed035eb5082aa2b
> > +
> > +# Remaining tests require engine support
> > +_is_engine_supported
> > +if [ $ENGINE_SUPPORTED -eq 77 ]; then
> > + __skip() { echo "Tests requiring engine support are skipped (not supported)"; return $SKIP; }
> > + expect_pass __skip
> > + exit 0
> > +fi
>
> GOST tests try to handle absence of algorithms (can work w/o --engine
> option if configured via openssl config) and skip gracefully.
> Perhaps this check should be moved below them just for pkcs11 tests
> if they are so sensitive.
Does OpenSSL v3 differentiate how engines are configured? I assume
when engine support is removed, all of it will be removed.
>
> Thanks,
>
> > +
> > _enable_gost_engine
> > expect_pass check md_gost12_256 0x0412 3f539a213e97c802cc229d474c6aa32a825a360b2a933a949fd925208d9ce1bb
> > expect_pass check streebog256 0x0412 3f539a213e97c802cc229d474c6aa32a825a360b2a933a949fd925208d9ce1bb
> > diff --git a/tests/sign_verify.test b/tests/sign_verify.test
> > index 948892759424..8c005b741916 100755
> > --- a/tests/sign_verify.test
> > +++ b/tests/sign_verify.test
> > @@ -18,6 +18,8 @@
> > cd "$(dirname "$0")" || exit 1
> > PATH=../src:$PATH
> > SIGV1=0
> > +ENGINE_SUPPORTED=0
> > +
> > source ./functions.sh
> >
> > _require cmp evmctl getfattr openssl xxd
> > @@ -418,6 +420,14 @@ if [ -x /opt/openssl3/bin/openssl ]; then
> > sign_verify sm2 sm3 0x030211:K:004[345678]
> > fi
> >
> > +# Remaining tests require engine support
> > +_is_engine_supported
> > +if [ $ENGINE_SUPPORTED -eq 77 ]; then
> > + __skip() { echo "Tests requiring engine support are skipped (not supported)"; return $SKIP; }
> > + expect_pass __skip
> > + exit 0
> > +fi
> > +
> > # Test v2 signatures with EC-RDSA
> > _enable_gost_engine
> > sign_verify gost2012_256-A md_gost12_256 0x030212:K:0040
> > --
> > 2.31.1
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [RFC PATCH ima-evm-utils 08/11] Deprecate use of OpenSSL 3 "engine" support
2022-08-30 11:46 ` Mimi Zohar
@ 2022-08-30 20:52 ` Vitaly Chikunov
2022-08-30 22:54 ` Vitaly Chikunov
2022-08-31 12:02 ` Mimi Zohar
0 siblings, 2 replies; 31+ messages in thread
From: Vitaly Chikunov @ 2022-08-30 20:52 UTC (permalink / raw)
To: Mimi Zohar; +Cc: linux-integrity, Petr Vorel, Stefan Berger
Mimi,
On Tue, Aug 30, 2022 at 07:46:40AM -0400, Mimi Zohar wrote:
> On Tue, 2022-08-30 at 06:03 +0300, Vitaly Chikunov wrote:
> > Mimi,
> >
> > On Mon, Aug 29, 2022 at 08:59:33PM -0400, Mimi Zohar wrote:
> > > OpenSSL 3 "engine" support is deprecated, which results in deprecated
> > > build warning messages. In preparation for OpenSSL "engine" support
> > > to be removed, define a "--enable-engine" configuration option. If not
> > > specified, disable engine support on systems with OpenSSL v3.
> >
> > This seems to change default behavior.
> >
> > Many distributions (AFAIK) still contain openssl1, and some contain both
> > openssl1 and openssl3, including devel packages. So they will suddenly
> > drop --engine support on ima-evm-utils update, even though their OpenSSL
> > supports it.
> >
> > Maybe --disable-engine should be added instead if user wants to avoid
> > compile time deprecation warnings?
>
> No, engine support is not disabled for previous versions of OpenSSL.
> It's only disabled for OpenSSL v3, with the --enable-engine option
> allowing it to be configured. The "-Wno-deprecated-declarations"
> option could even be added.
IC, but many distributions switched to openssl3 - engine support will
suddenly disappear for them if they aren't attentive enough.
OpenSSL can be compiled w/o engine support, I think engine support in
evmctl should depend on that and not on ima-evm-utils configure option. If
distribution wants to disable engine support they will disable it in
OpenSSL and it will be disabled in ima-evm-utils as consequence.
Or at least this should be default behavior, and to deviate from it
additional --disable-engine option in ima-evm-utils.
> >
> > (Also, engines aren't just deprecated, they are deprecated in favor of
> > providers.)
>
> Right. At some point, OpenSSL engine support will be removed. This
> is a first step to removing it in ima-evm-utils. I searched but didn't
> find any gost providers. Feel free to add it.
gost-engine already compiles to a provider:
https://github.com/gost-engine/engine#provider
> > > diff --git a/tests/functions.sh b/tests/functions.sh
> > > index 8f6f02dfcd95..ddc8fe9e5ea6 100755
> > > --- a/tests/functions.sh
> > > +++ b/tests/functions.sh
> > > @@ -312,4 +312,13 @@ _softhsm_teardown() {
> > > rm -rf "${SOFTHSM_SETUP_CONFIGDIR}"
> > > unset SOFTHSM_SETUP_CONFIGDIR SOFTHSM2_CONF PKCS11_KEYURI \
> > > EVMCTL_ENGINE OPENSSL_ENGINE OPENSSL_KEYFORM
> > > -}
> > > \ No newline at end of file
> > > +}
> > > +
> > > +# OpenSSL 3 engine support still works, but is deprecated. In preparation
> > > +# for it being removed, a new ima-evm-utils configuration option
> > > +# "--enable-engine" is defined.`
> > > +_is_engine_supported() {
> > > + cmd="evmctl --engine pkcs11"
> >
> > I think pkcs11 engine is optional (and could be in additional package)
> > even if other engines are present.
>
> We need to move away from OpenSSL engine support towards providers.
Perhaps they will remove engines (it's not certain) in openssl4,
but how many years will pass before that? I don't see why we should
hurry in that.
>
> >
> > Also engine could be loaded via openssl.cnf/OPENSSL_CONF, in that case
> > --engine option is not needed but engine is still there to use/test.
>
> Thank you for reminding me that engine support based on the OpenSSL
> configuration also needs to be deprecated.
I'm curious - how would you deprecate that? As this should be
transparent to the libcrypto clients - new functionality and algorithms
just appear.
>
> >
> >
> > > + $cmd &>/dev/null
> > > + ENGINE_SUPPORTED=$?
> > > +}
> > > diff --git a/tests/ima_hash.test b/tests/ima_hash.test
> > > index e88fd59cc359..0de9e6808af9 100755
> > > --- a/tests/ima_hash.test
> > > +++ b/tests/ima_hash.test
> > > @@ -71,6 +71,15 @@ expect_pass check sha384 0x0405 38b060a751ac96384cd9327eb1b1e36a21fdb71114b
> > > expect_pass check sha512 0x0406 cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e
> > > expect_pass check rmd160 0x0403 9c1185a5c5e9fc54612808977ee8f548b2258d31
> > > expect_pass check sm3 0x0411 1ab21d8355cfa17f8e61194831e81a8f22bec8c728fefb747ed035eb5082aa2b
> > > +
> > > +# Remaining tests require engine support
> > > +_is_engine_supported
> > > +if [ $ENGINE_SUPPORTED -eq 77 ]; then
> > > + __skip() { echo "Tests requiring engine support are skipped (not supported)"; return $SKIP; }
> > > + expect_pass __skip
> > > + exit 0
> > > +fi
> >
> > GOST tests try to handle absence of algorithms (can work w/o --engine
> > option if configured via openssl config) and skip gracefully.
> > Perhaps this check should be moved below them just for pkcs11 tests
> > if they are so sensitive.
>
> Does OpenSSL v3 differentiate how engines are configured? I assume
> when engine support is removed, all of it will be removed.
Perhaps. But providers are configured similarly via config - there
config examples for some gost-engine Perl tests:
as engine https://github.com/gost-engine/engine/blob/master/test/engine.cnf
as provider https://github.com/gost-engine/engine/blob/master/test/provider.cnf
Thanks,
^ permalink raw reply [flat|nested] 31+ messages in thread* Re: [RFC PATCH ima-evm-utils 08/11] Deprecate use of OpenSSL 3 "engine" support
2022-08-30 20:52 ` Vitaly Chikunov
@ 2022-08-30 22:54 ` Vitaly Chikunov
2022-08-31 11:43 ` Mimi Zohar
2022-08-31 12:02 ` Mimi Zohar
1 sibling, 1 reply; 31+ messages in thread
From: Vitaly Chikunov @ 2022-08-30 22:54 UTC (permalink / raw)
To: Mimi Zohar; +Cc: linux-integrity, Petr Vorel, Stefan Berger
Mimi,
On Tue, Aug 30, 2022 at 11:52:54PM +0300, Vitaly Chikunov wrote:
> On Tue, Aug 30, 2022 at 07:46:40AM -0400, Mimi Zohar wrote:
> > On Tue, 2022-08-30 at 06:03 +0300, Vitaly Chikunov wrote:
> > We need to move away from OpenSSL engine support towards providers.
>
> Perhaps they will remove engines (it's not certain) in openssl4,
> but how many years will pass before that? I don't see why we should
> hurry in that.
https://www.openssl.org/policies/releasestrat.html
Version 3.0 will be supported until 2026-09-07 (LTS).
So all that time there will be engines support, even if they decide to
remove it in next major release. So it doesn't look like we need to
delete it ASAP.
Thanks,
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC PATCH ima-evm-utils 08/11] Deprecate use of OpenSSL 3 "engine" support
2022-08-30 22:54 ` Vitaly Chikunov
@ 2022-08-31 11:43 ` Mimi Zohar
0 siblings, 0 replies; 31+ messages in thread
From: Mimi Zohar @ 2022-08-31 11:43 UTC (permalink / raw)
To: Vitaly Chikunov; +Cc: linux-integrity, Petr Vorel, Stefan Berger
Hi Vitaly,
On Wed, 2022-08-31 at 01:54 +0300, Vitaly Chikunov wrote:
> Mimi,
>
> On Tue, Aug 30, 2022 at 11:52:54PM +0300, Vitaly Chikunov wrote:
> > On Tue, Aug 30, 2022 at 07:46:40AM -0400, Mimi Zohar wrote:
> > > On Tue, 2022-08-30 at 06:03 +0300, Vitaly Chikunov wrote:
> > > We need to move away from OpenSSL engine support towards providers.
> >
> > Perhaps they will remove engines (it's not certain) in openssl4,
> > but how many years will pass before that? I don't see why we should
> > hurry in that.
>
> https://www.openssl.org/policies/releasestrat.html
> Version 3.0 will be supported until 2026-09-07 (LTS).
>
> So all that time there will be engines support, even if they decide to
> remove it in next major release. So it doesn't look like we need to
> delete it ASAP.
Agreed there is no rush. The original intent of this patch set,
however, was to address as many deprecated warnings as possible without
turning them off. Once they're turned off, they're out of sight, out
of mind. The only remaining deprecated warnings, without this patch,
are the engine related ones.
The two alternatives both require disabling deprecated warnings:
- invert "--enable-engine" to "--disable-engine" and disable the
deprecated warnings.
- drop this patch and turn off the deprecated warnings, if compiled
with OpenSSL v3.
Mimi
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [RFC PATCH ima-evm-utils 08/11] Deprecate use of OpenSSL 3 "engine" support
2022-08-30 20:52 ` Vitaly Chikunov
2022-08-30 22:54 ` Vitaly Chikunov
@ 2022-08-31 12:02 ` Mimi Zohar
1 sibling, 0 replies; 31+ messages in thread
From: Mimi Zohar @ 2022-08-31 12:02 UTC (permalink / raw)
To: Vitaly Chikunov; +Cc: linux-integrity, Petr Vorel, Stefan Berger
On Tue, 2022-08-30 at 23:52 +0300, Vitaly Chikunov wrote:
> > > Also engine could be loaded via openssl.cnf/OPENSSL_CONF, in that case
> > > --engine option is not needed but engine is still there to use/test.
> >
> > Thank you for reminding me that engine support based on the OpenSSL
> > configuration also needs to be deprecated.
>
> I'm curious - how would you deprecate that? As this should be
> transparent to the libcrypto clients - new functionality and algorithms
> just appear.
I'm referring to commit 782224f33cd7 ("ima-evm-utils: Rework openssl
init"), which introduced "--disable-openssl-conf".
>
> > > GOST tests try to handle absence of algorithms (can work w/o --engine
> > > option if configured via openssl config) and skip gracefully.
> > > Perhaps this check should be moved below them just for pkcs11 tests
> > > if they are so sensitive.
> >
> > Does OpenSSL v3 differentiate how engines are configured? I assume
> > when engine support is removed, all of it will be removed.
>
> Perhaps. But providers are configured similarly via config - there
> config examples for some gost-engine Perl tests:
> as engine https://github.com/gost-engine/engine/blob/master/test/engine.cnf
> as provider https://github.com/gost-engine/engine/blob/master/test/provider.cnf
Ok. Thank you for the reference.
Mimi
^ permalink raw reply [flat|nested] 31+ messages in thread
* [RFC PATCH ima-evm-utils 09/11] Fix potential use after free in read_tpm_banks()
2022-08-30 0:59 [RFC PATCH ima-evm-utils 00/11] address deprecated warnings Mimi Zohar
` (7 preceding siblings ...)
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 08/11] Deprecate use of OpenSSL 3 "engine" support Mimi Zohar
@ 2022-08-30 0:59 ` Mimi Zohar
2022-08-30 13:04 ` Petr Vorel
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 10/11] Limit the file hash algorithm name length Mimi Zohar
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 11/11] Missing template data size lower bounds checking Mimi Zohar
10 siblings, 1 reply; 31+ messages in thread
From: Mimi Zohar @ 2022-08-30 0:59 UTC (permalink / raw)
To: linux-integrity; +Cc: Mimi Zohar, Petr Vorel, Vitaly Chikunov, Stefan Berger
On failure to read TPM 2.0 bank PCRs 'errmsg' is not properly set to
NULL after being freed. Fix potential use after free.
Fixes: 3472f9ba9c05 ("ima-evm-utils: read the PCRs for the requested TPM banks")
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
src/evmctl.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/evmctl.c b/src/evmctl.c
index ad96789f1984..4bdc62d2e2e4 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -2076,6 +2076,7 @@ static int read_tpm_banks(int num_banks, struct tpm_bank_info *bank)
log_debug("Failed to read %s PCRs: (%s)\n",
bank[i].algo_name, errmsg);
free(errmsg);
+ errmsg = NULL;
bank[i].supported = 0;
}
}
--
2.31.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [RFC PATCH ima-evm-utils 10/11] Limit the file hash algorithm name length
2022-08-30 0:59 [RFC PATCH ima-evm-utils 00/11] address deprecated warnings Mimi Zohar
` (8 preceding siblings ...)
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 09/11] Fix potential use after free in read_tpm_banks() Mimi Zohar
@ 2022-08-30 0:59 ` Mimi Zohar
2022-08-30 13:04 ` Petr Vorel
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 11/11] Missing template data size lower bounds checking Mimi Zohar
10 siblings, 1 reply; 31+ messages in thread
From: Mimi Zohar @ 2022-08-30 0:59 UTC (permalink / raw)
To: linux-integrity; +Cc: Mimi Zohar, Petr Vorel, Vitaly Chikunov, Stefan Berger
Instead of assuming the file hash algorithm is a properly NULL terminated
string, properly limit the "algo:<hash>" field size.
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
src/evmctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/evmctl.c b/src/evmctl.c
index 4bdc62d2e2e4..4619bb433926 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -1603,7 +1603,7 @@ void ima_ng_show(struct template_entry *entry)
total_len -= sizeof(field_len);
algo = (char *)fieldp;
- len = strlen(algo) + 1;
+ len = strnlen(algo, field_len - 1) + 1;
digest_len = field_len - len;
digest = fieldp + len;
--
2.31.1
^ permalink raw reply related [flat|nested] 31+ messages in thread* [RFC PATCH ima-evm-utils 11/11] Missing template data size lower bounds checking
2022-08-30 0:59 [RFC PATCH ima-evm-utils 00/11] address deprecated warnings Mimi Zohar
` (9 preceding siblings ...)
2022-08-30 0:59 ` [RFC PATCH ima-evm-utils 10/11] Limit the file hash algorithm name length Mimi Zohar
@ 2022-08-30 0:59 ` Mimi Zohar
2022-08-30 13:06 ` Petr Vorel
10 siblings, 1 reply; 31+ messages in thread
From: Mimi Zohar @ 2022-08-30 0:59 UTC (permalink / raw)
To: linux-integrity; +Cc: Mimi Zohar, Petr Vorel, Vitaly Chikunov, Stefan Berger
Each record in the IMA measurement list must contain some template data.
Ensure the template data is not zero length.
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
src/evmctl.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/evmctl.c b/src/evmctl.c
index 4619bb433926..0d8f93bf5d26 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -2189,6 +2189,10 @@ static int ima_measurement(const char *file)
log_err("Unable to read template length\n");
goto out;
}
+ if (entry.template_len == 0) {
+ log_err("Invalid template data len\n");
+ goto out;
+ }
} else {
entry.template_len = SHA_DIGEST_LENGTH +
TCG_EVENT_NAME_LEN_MAX + 1;
--
2.31.1
^ permalink raw reply related [flat|nested] 31+ messages in thread