| Message ID | 20181128200610.21214-5-vt@altlinux.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2,1/7] ima-evm-utils: Fix hash buffer overflow in verify_evm and hmac_evm | expand |
On Wed, 2018-11-28 at 23:06 +0300, Vitaly Chikunov wrote: > @@ -1773,6 +1776,7 @@ static char *get_password(void) > int main(int argc, char *argv[]) > { > int err = 0, c, lind; > + ENGINE *eng = NULL; > > g_argv = argv; > g_argc = argc; > @@ -1883,6 +1887,18 @@ int main(int argc, char *argv[]) > case 138: > measurement_list = 1; > break; > + case 139: /* --engine e */ > + eng = ENGINE_by_id(optarg); The usage is only adding "--engine e" support. Either change the usage or add a test to verify the argument. > + if (!eng) { > + log_err("engine %s isn't available\n", optarg); > + ERR_print_errors_fp(stderr); > + } else if (!ENGINE_init(eng)) { > + log_err("engine %s init failed\n", optarg); > + ERR_print_errors_fp(stderr); > + ENGINE_free(eng); > + eng = NULL; > + } > + break; > case 140: /* --xattr-user */ > xattr_ima = "user.ima"; > xattr_evm = "user.evm";
On Fri, Nov 30, 2018 at 02:21:34PM -0500, Mimi Zohar wrote: > On Wed, 2018-11-28 at 23:06 +0300, Vitaly Chikunov wrote: > > > @@ -1773,6 +1776,7 @@ static char *get_password(void) > > int main(int argc, char *argv[]) > > { > > int err = 0, c, lind; > > + ENGINE *eng = NULL; > > > > g_argv = argv; > > g_argc = argc; > > @@ -1883,6 +1887,18 @@ int main(int argc, char *argv[]) > > case 138: > > measurement_list = 1; > > break; > > + case 139: /* --engine e */ > > + eng = ENGINE_by_id(optarg); > > The usage is only adding "--engine e" support. Either change the > usage or add a test to verify the argument. Could you elaborate what I should do? I didn't understand your suggestion. User should be able to specify anything as engine name and it is tested by ENGINE_by_id call. Also, usage implies that it would load engine with the name e. > > > > + if (!eng) { > > + log_err("engine %s isn't available\n", optarg); > > + ERR_print_errors_fp(stderr); > > + } else if (!ENGINE_init(eng)) { > > + log_err("engine %s init failed\n", optarg); > > + ERR_print_errors_fp(stderr); > > + ENGINE_free(eng); > > + eng = NULL; > > + } > > + break; > > case 140: /* --xattr-user */ > > xattr_ima = "user.ima"; > > xattr_evm = "user.evm";
On Sat, 2018-12-01 at 06:01 +0300, Vitaly Chikunov wrote: > On Fri, Nov 30, 2018 at 02:21:34PM -0500, Mimi Zohar wrote: > > On Wed, 2018-11-28 at 23:06 +0300, Vitaly Chikunov wrote: > > > > > @@ -1773,6 +1776,7 @@ static char *get_password(void) > > > int main(int argc, char *argv[]) > > > { > > > int err = 0, c, lind; > > > + ENGINE *eng = NULL; > > > > > > g_argv = argv; > > > g_argc = argc; > > > @@ -1883,6 +1887,18 @@ int main(int argc, char *argv[]) > > > case 138: > > > measurement_list = 1; > > > break; > > > + case 139: /* --engine e */ > > > + eng = ENGINE_by_id(optarg); > > > > The usage is only adding "--engine e" support. Either change the > > usage or add a test to verify the argument. > > Could you elaborate what I should do? I didn't understand your > suggestion. User should be able to specify anything as engine name and > it is tested by ENGINE_by_id call. Also, usage implies that it would > load engine with the name e. My mistake. Could you update the patch description with example usage? Mimi
diff --git a/src/evmctl.c b/src/evmctl.c index f4b2e7d..03b6132 100644 --- a/src/evmctl.c +++ b/src/evmctl.c @@ -62,6 +62,7 @@ #include <openssl/hmac.h> #include <openssl/err.h> #include <openssl/rsa.h> +#include <openssl/engine.h> #ifndef XATTR_APPAARMOR_SUFFIX #define XATTR_APPARMOR_SUFFIX "apparmor" @@ -1679,6 +1680,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" " --list measurement list verification\n" + " --engine e preload OpenSSL engine e\n" " -v increase verbosity level\n" " -h, --help display this help and exit\n" "\n"); @@ -1731,6 +1733,7 @@ static struct option opts[] = { {"selinux", 1, 0, 136}, {"caps", 2, 0, 137}, {"list", 0, 0, 138}, + {"engine", 1, 0, 139}, {"xattr-user", 0, 0, 140}, {} @@ -1773,6 +1776,7 @@ static char *get_password(void) int main(int argc, char *argv[]) { int err = 0, c, lind; + ENGINE *eng = NULL; g_argv = argv; g_argc = argc; @@ -1883,6 +1887,18 @@ int main(int argc, char *argv[]) case 138: measurement_list = 1; break; + case 139: /* --engine e */ + eng = ENGINE_by_id(optarg); + if (!eng) { + log_err("engine %s isn't available\n", optarg); + ERR_print_errors_fp(stderr); + } else if (!ENGINE_init(eng)) { + log_err("engine %s init failed\n", optarg); + ERR_print_errors_fp(stderr); + ENGINE_free(eng); + eng = NULL; + } + break; case 140: /* --xattr-user */ xattr_ima = "user.ima"; xattr_evm = "user.evm"; @@ -1913,6 +1929,13 @@ int main(int argc, char *argv[]) } } + if (eng) { + ENGINE_finish(eng); + ENGINE_free(eng); +#if OPENSSL_API_COMPAT < 0x10100000L + ENGINE_cleanup(); +#endif + } ERR_free_strings(); EVP_cleanup(); BIO_free(NULL);
Another method of using GOST algorithms (and cryptographic accelerators) is via direct preloading of appropriate engine using '--engine' option. Signed-off-by: Vitaly Chikunov <vt@altlinux.org> --- Changes since v1: - Code split from prevously combined patch. - More verbose OpenSSL error message. src/evmctl.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)