* [PATCH] ima-evm-utils: remove redundant call to OpenSSL_add_all_algorithms @ 2019-01-27 2:39 Vitaly Chikunov 2019-01-30 12:34 ` Mimi Zohar 0 siblings, 1 reply; 13+ messages in thread From: Vitaly Chikunov @ 2019-01-27 2:39 UTC (permalink / raw) To: Mimi Zohar, Dmitry Kasatkin, linux-integrity Because of call to OPENSSL_add_all_algorithms_conf() calling OpenSSL_add_all_algorithms() is not needed. There was not be any problems though because double initialization is permitted. --- src/libimaevm.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libimaevm.c b/src/libimaevm.c index 7501303..b038d0c 100644 --- a/src/libimaevm.c +++ b/src/libimaevm.c @@ -995,7 +995,6 @@ int sign_hash(const char *hashalgo, const unsigned char *hash, int size, const c static void libinit() { - OpenSSL_add_all_algorithms(); OPENSSL_add_all_algorithms_conf(); ERR_load_crypto_strings(); } -- 2.11.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] ima-evm-utils: remove redundant call to OpenSSL_add_all_algorithms 2019-01-27 2:39 [PATCH] ima-evm-utils: remove redundant call to OpenSSL_add_all_algorithms Vitaly Chikunov @ 2019-01-30 12:34 ` Mimi Zohar 2019-01-30 13:25 ` Vitaly Chikunov 0 siblings, 1 reply; 13+ messages in thread From: Mimi Zohar @ 2019-01-30 12:34 UTC (permalink / raw) To: Vitaly Chikunov, Mimi Zohar, Dmitry Kasatkin, linux-integrity On Sun, 2019-01-27 at 05:39 +0300, Vitaly Chikunov wrote: > Because of call to OPENSSL_add_all_algorithms_conf() calling > OpenSSL_add_all_algorithms() is not needed. There was not be any > problems though because double initialization is permitted. > --- > src/libimaevm.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/src/libimaevm.c b/src/libimaevm.c > index 7501303..b038d0c 100644 > --- a/src/libimaevm.c > +++ b/src/libimaevm.c > @@ -995,7 +995,6 @@ int sign_hash(const char *hashalgo, const unsigned char *hash, int size, const c > > static void libinit() > { > - OpenSSL_add_all_algorithms(); > OPENSSL_add_all_algorithms_conf(); > ERR_load_crypto_strings(); > } The only difference between the two calls seems to be reading the system openssl.cnf file. In the original call that is dependent on OPENSSL_LOAD_CONF being defined. Calling OPENSSL_add_all_algorithms_conf(), forces reading the system openssl.cnf. Mimi ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] ima-evm-utils: remove redundant call to OpenSSL_add_all_algorithms 2019-01-30 12:34 ` Mimi Zohar @ 2019-01-30 13:25 ` Vitaly Chikunov 2019-01-30 13:41 ` Mimi Zohar 2019-01-30 15:35 ` James Bottomley 0 siblings, 2 replies; 13+ messages in thread From: Vitaly Chikunov @ 2019-01-30 13:25 UTC (permalink / raw) To: Mimi Zohar; +Cc: Mimi Zohar, Dmitry Kasatkin, linux-integrity On Wed, Jan 30, 2019 at 07:34:57AM -0500, Mimi Zohar wrote: > On Sun, 2019-01-27 at 05:39 +0300, Vitaly Chikunov wrote: > > Because of call to OPENSSL_add_all_algorithms_conf() calling > > OpenSSL_add_all_algorithms() is not needed. There was not be any > > problems though because double initialization is permitted. > > --- > > src/libimaevm.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/src/libimaevm.c b/src/libimaevm.c > > index 7501303..b038d0c 100644 > > --- a/src/libimaevm.c > > +++ b/src/libimaevm.c > > @@ -995,7 +995,6 @@ int sign_hash(const char *hashalgo, const unsigned char *hash, int size, const c > > > > static void libinit() > > { > > - OpenSSL_add_all_algorithms(); > > OPENSSL_add_all_algorithms_conf(); > > ERR_load_crypto_strings(); > > } > > The only difference between the two calls seems to be reading the > system openssl.cnf file. In the original call that is dependent on > OPENSSL_LOAD_CONF being defined. Calling > OPENSSL_add_all_algorithms_conf(), forces reading the system > openssl.cnf. Yes. OPENSSL_LOAD_CONF is per application define, which is by default undefined. And instead of defining it, we could just call OPENSSL_add_all_algorithms_conf(), which is required for GOST support. Otherwise enabling Streebog via OPENSSL_CONF will not work. Thanks, > > Mimi ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] ima-evm-utils: remove redundant call to OpenSSL_add_all_algorithms 2019-01-30 13:25 ` Vitaly Chikunov @ 2019-01-30 13:41 ` Mimi Zohar 2019-01-30 13:53 ` Vitaly Chikunov 2019-01-30 15:35 ` James Bottomley 1 sibling, 1 reply; 13+ messages in thread From: Mimi Zohar @ 2019-01-30 13:41 UTC (permalink / raw) To: Vitaly Chikunov Cc: Mimi Zohar, Dmitry Kasatkin, linux-integrity, Bruno E. O. Meneguele, Petr Vorel [Cc'ing Bruno, Petr] On Wed, 2019-01-30 at 16:25 +0300, Vitaly Chikunov wrote: > On Wed, Jan 30, 2019 at 07:34:57AM -0500, Mimi Zohar wrote: > > On Sun, 2019-01-27 at 05:39 +0300, Vitaly Chikunov wrote: > > > Because of call to OPENSSL_add_all_algorithms_conf() calling > > > OpenSSL_add_all_algorithms() is not needed. There was not be any > > > problems though because double initialization is permitted. > > > --- > > > src/libimaevm.c | 1 - > > > 1 file changed, 1 deletion(-) > > > > > > diff --git a/src/libimaevm.c b/src/libimaevm.c > > > index 7501303..b038d0c 100644 > > > --- a/src/libimaevm.c > > > +++ b/src/libimaevm.c > > > @@ -995,7 +995,6 @@ int sign_hash(const char *hashalgo, const unsigned char *hash, int size, const c > > > > > > static void libinit() > > > { > > > - OpenSSL_add_all_algorithms(); > > > OPENSSL_add_all_algorithms_conf(); > > > ERR_load_crypto_strings(); > > > } > > > > The only difference between the two calls seems to be reading the > > system openssl.cnf file. In the original call that is dependent on > > OPENSSL_LOAD_CONF being defined. Calling > > OPENSSL_add_all_algorithms_conf(), forces reading the system > > openssl.cnf. > > Yes. OPENSSL_LOAD_CONF is per application define, which is by default > undefined. And instead of defining it, we could just call > OPENSSL_add_all_algorithms_conf(), which is required for GOST support. > Otherwise enabling Streebog via OPENSSL_CONF will not work. I think this is basically a packaging question. Instead of hard coding this change, it could be an autoconf option (eg. enable/disable-gost). Bruno, Petr, any preference? Thanks, Mimi ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] ima-evm-utils: remove redundant call to OpenSSL_add_all_algorithms 2019-01-30 13:41 ` Mimi Zohar @ 2019-01-30 13:53 ` Vitaly Chikunov 2019-01-30 15:59 ` Petr Vorel 0 siblings, 1 reply; 13+ messages in thread From: Vitaly Chikunov @ 2019-01-30 13:53 UTC (permalink / raw) To: Mimi Zohar Cc: Mimi Zohar, Dmitry Kasatkin, linux-integrity, Bruno E. O. Meneguele, Petr Vorel On Wed, Jan 30, 2019 at 08:41:13AM -0500, Mimi Zohar wrote: > [Cc'ing Bruno, Petr] > > On Wed, 2019-01-30 at 16:25 +0300, Vitaly Chikunov wrote: > > On Wed, Jan 30, 2019 at 07:34:57AM -0500, Mimi Zohar wrote: > > > On Sun, 2019-01-27 at 05:39 +0300, Vitaly Chikunov wrote: > > > > Because of call to OPENSSL_add_all_algorithms_conf() calling > > > > OpenSSL_add_all_algorithms() is not needed. There was not be any > > > > problems though because double initialization is permitted. > > > > --- > > > > src/libimaevm.c | 1 - > > > > 1 file changed, 1 deletion(-) > > > > > > > > diff --git a/src/libimaevm.c b/src/libimaevm.c > > > > index 7501303..b038d0c 100644 > > > > --- a/src/libimaevm.c > > > > +++ b/src/libimaevm.c > > > > @@ -995,7 +995,6 @@ int sign_hash(const char *hashalgo, const unsigned char *hash, int size, const c > > > > > > > > static void libinit() > > > > { > > > > - OpenSSL_add_all_algorithms(); > > > > OPENSSL_add_all_algorithms_conf(); > > > > ERR_load_crypto_strings(); > > > > } > > > > > > The only difference between the two calls seems to be reading the > > > system openssl.cnf file. In the original call that is dependent on > > > OPENSSL_LOAD_CONF being defined. Calling > > > OPENSSL_add_all_algorithms_conf(), forces reading the system > > > openssl.cnf. > > > > Yes. OPENSSL_LOAD_CONF is per application define, which is by default > > undefined. And instead of defining it, we could just call > > OPENSSL_add_all_algorithms_conf(), which is required for GOST support. > > Otherwise enabling Streebog via OPENSSL_CONF will not work. > > I think this is basically a packaging question. Instead of hard > coding this change, it could be an autoconf option (eg. > enable/disable-gost). User already have control to load or not engines by editing or not the config. But, if OpenSSL_add_all_algorithms() is used without defining OPENSSL_LOAD_CONF (default) user will lose such option. I think loading config is quite normal operation which is also default for the openssl binary. > > Bruno, Petr, any preference? > > Thanks, > > Mimi ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] ima-evm-utils: remove redundant call to OpenSSL_add_all_algorithms 2019-01-30 13:53 ` Vitaly Chikunov @ 2019-01-30 15:59 ` Petr Vorel 0 siblings, 0 replies; 13+ messages in thread From: Petr Vorel @ 2019-01-30 15:59 UTC (permalink / raw) To: Vitaly Chikunov Cc: Mimi Zohar, Mimi Zohar, Dmitry Kasatkin, linux-integrity, Bruno E. O. Meneguele, Jonathan Davies, Marcus Meissner Hi, [ Cc Marcus and Jonathan ] > On Wed, Jan 30, 2019 at 08:41:13AM -0500, Mimi Zohar wrote: > > [Cc'ing Bruno, Petr] > > On Wed, 2019-01-30 at 16:25 +0300, Vitaly Chikunov wrote: > > > On Wed, Jan 30, 2019 at 07:34:57AM -0500, Mimi Zohar wrote: > > > > On Sun, 2019-01-27 at 05:39 +0300, Vitaly Chikunov wrote: > > > > > Because of call to OPENSSL_add_all_algorithms_conf() calling > > > > > OpenSSL_add_all_algorithms() is not needed. There was not be any > > > > > problems though because double initialization is permitted. > > > > > --- > > > > > src/libimaevm.c | 1 - > > > > > 1 file changed, 1 deletion(-) > > > > > diff --git a/src/libimaevm.c b/src/libimaevm.c > > > > > index 7501303..b038d0c 100644 > > > > > --- a/src/libimaevm.c > > > > > +++ b/src/libimaevm.c > > > > > @@ -995,7 +995,6 @@ int sign_hash(const char *hashalgo, const unsigned char *hash, int size, const c > > > > > static void libinit() > > > > > { > > > > > - OpenSSL_add_all_algorithms(); > > > > > OPENSSL_add_all_algorithms_conf(); > > > > > ERR_load_crypto_strings(); > > > > > } > > > > The only difference between the two calls seems to be reading the > > > > system openssl.cnf file. In the original call that is dependent on > > > > OPENSSL_LOAD_CONF being defined. Calling > > > > OPENSSL_add_all_algorithms_conf(), forces reading the system > > > > openssl.cnf. > > > Yes. OPENSSL_LOAD_CONF is per application define, which is by default > > > undefined. And instead of defining it, we could just call > > > OPENSSL_add_all_algorithms_conf(), which is required for GOST support. > > > Otherwise enabling Streebog via OPENSSL_CONF will not work. > > I think this is basically a packaging question. Instead of hard > > coding this change, it could be an autoconf option (eg. > > enable/disable-gost). > User already have control to load or not engines by editing or not the > config. But, if OpenSSL_add_all_algorithms() is used without defining > OPENSSL_LOAD_CONF (default) user will lose such option. > I think loading config is quite normal operation which is also default > for the openssl binary. > > Bruno, Petr, any preference? Not sure (I Cc more experienced packagers), but allowing distros to control it via autoconf certainly does not harm. BTW OpenSSL_add_all_algorithms() is deprecated since OpenSSL 1.1.0. Kind regards, Petr ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] ima-evm-utils: remove redundant call to OpenSSL_add_all_algorithms 2019-01-30 13:25 ` Vitaly Chikunov 2019-01-30 13:41 ` Mimi Zohar @ 2019-01-30 15:35 ` James Bottomley 2019-01-30 16:12 ` Vitaly Chikunov 1 sibling, 1 reply; 13+ messages in thread From: James Bottomley @ 2019-01-30 15:35 UTC (permalink / raw) To: Vitaly Chikunov, Mimi Zohar; +Cc: Mimi Zohar, Dmitry Kasatkin, linux-integrity On Wed, 2019-01-30 at 16:25 +0300, Vitaly Chikunov wrote: > On Wed, Jan 30, 2019 at 07:34:57AM -0500, Mimi Zohar wrote: > > On Sun, 2019-01-27 at 05:39 +0300, Vitaly Chikunov wrote: > > > Because of call to OPENSSL_add_all_algorithms_conf() calling > > > OpenSSL_add_all_algorithms() is not needed. There was not be any > > > problems though because double initialization is permitted. > > > --- > > > src/libimaevm.c | 1 - > > > 1 file changed, 1 deletion(-) > > > > > > diff --git a/src/libimaevm.c b/src/libimaevm.c > > > index 7501303..b038d0c 100644 > > > --- a/src/libimaevm.c > > > +++ b/src/libimaevm.c > > > @@ -995,7 +995,6 @@ int sign_hash(const char *hashalgo, const > > > unsigned char *hash, int size, const c > > > > > > static void libinit() > > > { > > > - OpenSSL_add_all_algorithms(); > > > OPENSSL_add_all_algorithms_conf(); > > > ERR_load_crypto_strings(); > > > } > > > > The only difference between the two calls seems to be reading the > > system openssl.cnf file. In the original call that is dependent on > > OPENSSL_LOAD_CONF being defined. Calling > > OPENSSL_add_all_algorithms_conf(), forces reading the system > > openssl.cnf. > > Yes. OPENSSL_LOAD_CONF is per application define, which is by default > undefined. And instead of defining it, we could just call > OPENSSL_add_all_algorithms_conf(), which is required for GOST > support. > Otherwise enabling Streebog via OPENSSL_CONF will not work. It will if you call ENGINE_set_default(e, ENGINE_METHOD_ALL); after ENGINE_init That's all the conf file is covering up for. James ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] ima-evm-utils: remove redundant call to OpenSSL_add_all_algorithms 2019-01-30 15:35 ` James Bottomley @ 2019-01-30 16:12 ` Vitaly Chikunov 2019-01-30 16:44 ` James Bottomley 0 siblings, 1 reply; 13+ messages in thread From: Vitaly Chikunov @ 2019-01-30 16:12 UTC (permalink / raw) To: James Bottomley; +Cc: Mimi Zohar, Mimi Zohar, Dmitry Kasatkin, linux-integrity On Wed, Jan 30, 2019 at 07:35:49AM -0800, James Bottomley wrote: > On Wed, 2019-01-30 at 16:25 +0300, Vitaly Chikunov wrote: > > On Wed, Jan 30, 2019 at 07:34:57AM -0500, Mimi Zohar wrote: > > > On Sun, 2019-01-27 at 05:39 +0300, Vitaly Chikunov wrote: > > > > Because of call to OPENSSL_add_all_algorithms_conf() calling > > > > OpenSSL_add_all_algorithms() is not needed. There was not be any > > > > problems though because double initialization is permitted. > > > > --- > > > > src/libimaevm.c | 1 - > > > > 1 file changed, 1 deletion(-) > > > > > > > > diff --git a/src/libimaevm.c b/src/libimaevm.c > > > > index 7501303..b038d0c 100644 > > > > --- a/src/libimaevm.c > > > > +++ b/src/libimaevm.c > > > > @@ -995,7 +995,6 @@ int sign_hash(const char *hashalgo, const > > > > unsigned char *hash, int size, const c > > > > > > > > static void libinit() > > > > { > > > > - OpenSSL_add_all_algorithms(); > > > > OPENSSL_add_all_algorithms_conf(); > > > > ERR_load_crypto_strings(); > > > > } > > > > > > The only difference between the two calls seems to be reading the > > > system openssl.cnf file. In the original call that is dependent on > > > OPENSSL_LOAD_CONF being defined. Calling > > > OPENSSL_add_all_algorithms_conf(), forces reading the system > > > openssl.cnf. > > > > Yes. OPENSSL_LOAD_CONF is per application define, which is by default > > undefined. And instead of defining it, we could just call > > OPENSSL_add_all_algorithms_conf(), which is required for GOST > > support. > > Otherwise enabling Streebog via OPENSSL_CONF will not work. > > It will if you call There is preferred "easy" method of [system wide] loading of gost-engine "by default" just by changing openssl.cnf like this: https://github.com/gost-engine/engine/blob/master/example.conf After that change all openssl (and linked) tools understand GOST algorithms without needing options like `-engine gost`. This works unless tool is compiled without OPENSSL_LOAD_CONF and it calls OpenSSL_add_all_algorithms() instead of OPENSSL_add_all_algorithms_conf(). Which is frequently the default, (because there is too much methods of openssl initialization and people may not understand all intrications for all options.) In that case we try to persuade tool author to change the way openssl is initialized. > > ENGINE_set_default(e, ENGINE_METHOD_ALL); > > after ENGINE_init > > That's all the conf file is covering up for. > > James > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] ima-evm-utils: remove redundant call to OpenSSL_add_all_algorithms 2019-01-30 16:12 ` Vitaly Chikunov @ 2019-01-30 16:44 ` James Bottomley 2019-01-30 17:54 ` Vitaly Chikunov 0 siblings, 1 reply; 13+ messages in thread From: James Bottomley @ 2019-01-30 16:44 UTC (permalink / raw) To: Vitaly Chikunov; +Cc: Mimi Zohar, Mimi Zohar, Dmitry Kasatkin, linux-integrity On Wed, 2019-01-30 at 19:12 +0300, Vitaly Chikunov wrote: > On Wed, Jan 30, 2019 at 07:35:49AM -0800, James Bottomley wrote: > > On Wed, 2019-01-30 at 16:25 +0300, Vitaly Chikunov wrote: > > > On Wed, Jan 30, 2019 at 07:34:57AM -0500, Mimi Zohar wrote: > > > > On Sun, 2019-01-27 at 05:39 +0300, Vitaly Chikunov wrote: > > > > > Because of call to OPENSSL_add_all_algorithms_conf() calling > > > > > OpenSSL_add_all_algorithms() is not needed. There was not be > > > > > any > > > > > problems though because double initialization is permitted. > > > > > --- > > > > > src/libimaevm.c | 1 - > > > > > 1 file changed, 1 deletion(-) > > > > > > > > > > diff --git a/src/libimaevm.c b/src/libimaevm.c > > > > > index 7501303..b038d0c 100644 > > > > > --- a/src/libimaevm.c > > > > > +++ b/src/libimaevm.c > > > > > @@ -995,7 +995,6 @@ int sign_hash(const char *hashalgo, const > > > > > unsigned char *hash, int size, const c > > > > > > > > > > static void libinit() > > > > > { > > > > > - OpenSSL_add_all_algorithms(); > > > > > OPENSSL_add_all_algorithms_conf(); > > > > > ERR_load_crypto_strings(); > > > > > } > > > > > > > > The only difference between the two calls seems to be reading > > > > the > > > > system openssl.cnf file. In the original call that is > > > > dependent on > > > > OPENSSL_LOAD_CONF being defined. Calling > > > > OPENSSL_add_all_algorithms_conf(), forces reading the system > > > > openssl.cnf. > > > > > > Yes. OPENSSL_LOAD_CONF is per application define, which is by > > > default > > > undefined. And instead of defining it, we could just call > > > OPENSSL_add_all_algorithms_conf(), which is required for GOST > > > support. > > > Otherwise enabling Streebog via OPENSSL_CONF will not work. > > > > It will if you call > > There is preferred "easy" method of [system wide] loading of gost- > engine > "by default" just by changing openssl.cnf like this: > > https://github.com/gost-engine/engine/blob/master/example.conf > > After that change all openssl (and linked) tools understand GOST > algorithms without needing options like `-engine gost`. This means that you turn the gost engine on by default in the file > This works unless tool is compiled without OPENSSL_LOAD_CONF and it > calls OpenSSL_add_all_algorithms() instead of > OPENSSL_add_all_algorithms_conf(). Which is frequently the default, > (because there is too much methods of openssl initialization and > people may not understand all intrications for all options.) In that > case we try to persuade tool author to change the way openssl is > initialized. What I'm saying is that modifying config files is really difficult for most users. So, if you want ima-evm-utils to work out of the box with the Streebog hash it needs to have no dependent requirement on config files, which means you need to add the call to ENGINE_set_default() This isn't about how you usually do it, it's about making these additions usable for average users. That doesn't stop you from adding further mods via the config files, but it does mean that if a random user installs evmctl and types evmctl -e gost -a streebog256 ... it will just work instead of failing with an obscure error because the default config file is wrong. I actually think for usability you should tie the hash and the engine together so a user can just type evmctl -a streebog256 ... And the tool will automatically try to load the gost engine and tell you if it's missing. James ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] ima-evm-utils: remove redundant call to OpenSSL_add_all_algorithms 2019-01-30 16:44 ` James Bottomley @ 2019-01-30 17:54 ` Vitaly Chikunov 2019-01-30 18:36 ` James Bottomley 0 siblings, 1 reply; 13+ messages in thread From: Vitaly Chikunov @ 2019-01-30 17:54 UTC (permalink / raw) To: James Bottomley; +Cc: Mimi Zohar, Mimi Zohar, Dmitry Kasatkin, linux-integrity On Wed, Jan 30, 2019 at 08:44:25AM -0800, James Bottomley wrote: > On Wed, 2019-01-30 at 19:12 +0300, Vitaly Chikunov wrote: > > On Wed, Jan 30, 2019 at 07:35:49AM -0800, James Bottomley wrote: > > > On Wed, 2019-01-30 at 16:25 +0300, Vitaly Chikunov wrote: > > > > On Wed, Jan 30, 2019 at 07:34:57AM -0500, Mimi Zohar wrote: > > > > > On Sun, 2019-01-27 at 05:39 +0300, Vitaly Chikunov wrote: > > > > > > Because of call to OPENSSL_add_all_algorithms_conf() calling > > > > > > OpenSSL_add_all_algorithms() is not needed. There was not be > > > > > > any > > > > > > problems though because double initialization is permitted. > > > > > > --- > > > > > > src/libimaevm.c | 1 - > > > > > > 1 file changed, 1 deletion(-) > > > > > > > > > > > > diff --git a/src/libimaevm.c b/src/libimaevm.c > > > > > > index 7501303..b038d0c 100644 > > > > > > --- a/src/libimaevm.c > > > > > > +++ b/src/libimaevm.c > > > > > > @@ -995,7 +995,6 @@ int sign_hash(const char *hashalgo, const > > > > > > unsigned char *hash, int size, const c > > > > > > > > > > > > static void libinit() > > > > > > { > > > > > > - OpenSSL_add_all_algorithms(); > > > > > > OPENSSL_add_all_algorithms_conf(); > > > > > > ERR_load_crypto_strings(); > > > > > > } > > > > > > > > > > The only difference between the two calls seems to be reading > > > > > the > > > > > system openssl.cnf file. In the original call that is > > > > > dependent on > > > > > OPENSSL_LOAD_CONF being defined. Calling > > > > > OPENSSL_add_all_algorithms_conf(), forces reading the system > > > > > openssl.cnf. > > > > > > > > Yes. OPENSSL_LOAD_CONF is per application define, which is by > > > > default > > > > undefined. And instead of defining it, we could just call > > > > OPENSSL_add_all_algorithms_conf(), which is required for GOST > > > > support. > > > > Otherwise enabling Streebog via OPENSSL_CONF will not work. > > > > > > It will if you call > > > > There is preferred "easy" method of [system wide] loading of gost- > > engine > > "by default" just by changing openssl.cnf like this: > > > > https://github.com/gost-engine/engine/blob/master/example.conf > > > > After that change all openssl (and linked) tools understand GOST > > algorithms without needing options like `-engine gost`. > > This means that you turn the gost engine on by default in the file This is how most gost users are suggested to use it. > > This works unless tool is compiled without OPENSSL_LOAD_CONF and it > > calls OpenSSL_add_all_algorithms() instead of > > OPENSSL_add_all_algorithms_conf(). Which is frequently the default, > > (because there is too much methods of openssl initialization and > > people may not understand all intrications for all options.) In that > > case we try to persuade tool author to change the way openssl is > > initialized. > > What I'm saying is that modifying config files is really difficult for > most users. So, if you want ima-evm-utils to work out of the box with > the Streebog hash it needs to have no dependent requirement on config > files, which means you need to add the call to ENGINE_set_default() I implemented two methods of loading engine for evmctl (via config and via --engine option). There is no problem with --engine option for Streebog, AFAIK. We are talking about config method, which would be default for the most users. (Your suggestion is still good for my next patch what implements PKEY instead of RSA, for EC-RDSA signatures. Thanks! I will add it.) > This isn't about how you usually do it, it's about making these > additions usable for average users. Config method of loading gost engine is not how I usually do it, but how most gost users use it (and suggested to use it). This isn't some marginal use case. They just modify `openssl.cnf` once (manually of via script) and all relevant ssl tools "magically" start to understand gost algorithms without any additional options. This would be very not user friendly if for every network tool that supports ssl they should specify `--someoption gost`. `--engine` option is just supplementary for advanced or occasional users. It continues to work (for Streebog) if OpenSSL_add_all_algorithms call is removed, but breaks if OPENSSL_add_all_algorithms_conf is removed, probably, need to call ENGINE_load_builtin_engines in that case. All this make initialization more complicated instead of single call to OPENSSL_add_all_algorithms_conf(). > That doesn't stop you from adding further mods via the config files, > but it does mean that if a random user installs evmctl and types > > evmctl -e gost -a streebog256 ... > > it will just work instead of failing with an obscure error because the > default config file is wrong. I actually think for usability you > should tie the hash and the engine together so a user can just type > > evmctl -a streebog256 ... > > And the tool will automatically try to load the gost engine and tell > you if it's missing. This is interesting idea. (But this probably will fail for GOST keys/certificates.) > > James > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] ima-evm-utils: remove redundant call to OpenSSL_add_all_algorithms 2019-01-30 17:54 ` Vitaly Chikunov @ 2019-01-30 18:36 ` James Bottomley 2019-01-31 9:22 ` Vitaly Chikunov 0 siblings, 1 reply; 13+ messages in thread From: James Bottomley @ 2019-01-30 18:36 UTC (permalink / raw) To: Vitaly Chikunov; +Cc: Mimi Zohar, Mimi Zohar, Dmitry Kasatkin, linux-integrity On Wed, 2019-01-30 at 20:54 +0300, Vitaly Chikunov wrote: > On Wed, Jan 30, 2019 at 08:44:25AM -0800, James Bottomley wrote: > > On Wed, 2019-01-30 at 19:12 +0300, Vitaly Chikunov wrote: [...] > > > There is preferred "easy" method of [system wide] loading of > > > gost-engine "by default" just by changing openssl.cnf like this: > > > > > > https://github.com/gost-engine/engine/blob/master/example.conf > > > > > > After that change all openssl (and linked) tools understand GOST > > > algorithms without needing options like `-engine gost`. > > > > This means that you turn the gost engine on by default in the file > > This is how most gost users are suggested to use it. Well, no, this is how Russian Government people use it because gost implements the mandatory aspects of their encryption standard. If you work for them you want gost all the time for everything. So it's the suggested way for that class of users. However, that's not how a casual non-Russian user would want it. They'd only want gost if they specified the streebog hash. And if we advertise the hash (as we do because you added it to the help) they should have a reasonable expectation of its working easily. > > > This works unless tool is compiled without OPENSSL_LOAD_CONF and > > > it calls OpenSSL_add_all_algorithms() instead of > > > OPENSSL_add_all_algorithms_conf(). Which is frequently the > > > default, (because there is too much methods of openssl > > > initialization and people may not understand all intrications for > > > all options.) In that case we try to persuade tool author to > > > change the way openssl is initialized. > > > > What I'm saying is that modifying config files is really difficult > > for most users. So, if you want ima-evm-utils to work out of the > > box with the Streebog hash it needs to have no dependent > > requirement on config files, which means you need to add the call > > to ENGINE_set_default() > > I implemented two methods of loading engine for evmctl (via config > and via --engine option). There is no problem with --engine option > for Streebog, AFAIK. Can you try it with a vanilla (non gost modified) openssl.cnf file to verify? I think you require the ENGINE_set_default() call but it may be that a non-standard hash name will cause a search of the engine added hashes. OpenSSL has badly documented defaults, so I usually chase that through the code, but in this case a simple experiment will tell us. > We are talking about config method, which would be > default for the most users. > > (Your suggestion is still good for my next patch what implements PKEY > instead of RSA, for EC-RDSA signatures. Thanks! I will add it.) > > > This isn't about how you usually do it, it's about making these > > additions usable for average users. > > Config method of loading gost engine is not how I usually do it, but > how most gost users use it (and suggested to use it). This isn't some > marginal use case. They just modify `openssl.cnf` once (manually of > via script) and all relevant ssl tools "magically" start to > understand gost algorithms without any additional options. This would > be very not user friendly if for every network tool that supports ssl > they should specify `--someoption gost`. Right, I'm not saying disallow that. The Russian crypto case is certainly a valid class of user. What I'm saying is *don't* require this config setup for other non-Russian crypto users because they won't be familiar with it. > `--engine` option is just supplementary for advanced or occasional > users. It continues to work (for Streebog) if > OpenSSL_add_all_algorithms call is removed, but breaks if > OPENSSL_add_all_algorithms_conf is removed, probably, need to call > ENGINE_load_builtin_engines in that case. > > All this make initialization more complicated instead of single call > to OPENSSL_add_all_algorithms_conf(). Yes, but it's acceptable to make our life more complex if it makes life simpler for the end user. > > That doesn't stop you from adding further mods via the config > > files, but it does mean that if a random user installs evmctl and > > types > > > > evmctl -e gost -a streebog256 ... > > > > it will just work instead of failing with an obscure error because > > the default config file is wrong. I actually think for usability > > you should tie the hash and the engine together so a user can just > > type > > > > evmctl -a streebog256 ... > > > > And the tool will automatically try to load the gost engine and > > tell you if it's missing. > > This is interesting idea. (But this probably will fail for GOST > keys/certificates.) Gost certs should identify the streebog hash, so in theory we can examine the OIDs and see we need the gost engine to process them. I'm certainly not saying do this, I'm just giving it as an example of how to make life easier for users. James ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] ima-evm-utils: remove redundant call to OpenSSL_add_all_algorithms 2019-01-30 18:36 ` James Bottomley @ 2019-01-31 9:22 ` Vitaly Chikunov 2019-01-31 20:29 ` James Bottomley 0 siblings, 1 reply; 13+ messages in thread From: Vitaly Chikunov @ 2019-01-31 9:22 UTC (permalink / raw) To: James Bottomley; +Cc: Mimi Zohar, Mimi Zohar, Dmitry Kasatkin, linux-integrity On Wed, Jan 30, 2019 at 10:36:00AM -0800, James Bottomley wrote: > On Wed, 2019-01-30 at 20:54 +0300, Vitaly Chikunov wrote: > > On Wed, Jan 30, 2019 at 08:44:25AM -0800, James Bottomley wrote: > > > On Wed, 2019-01-30 at 19:12 +0300, Vitaly Chikunov wrote: > [...] > > > > There is preferred "easy" method of [system wide] loading of > > > > gost-engine "by default" just by changing openssl.cnf like this: > > > > > > > > https://github.com/gost-engine/engine/blob/master/example.conf > > > > > > > > After that change all openssl (and linked) tools understand GOST > > > > algorithms without needing options like `-engine gost`. > > > > > > This means that you turn the gost engine on by default in the file > > > > This is how most gost users are suggested to use it. > > Well, no, this is how Russian Government people use it because gost > implements the mandatory aspects of their encryption standard. If you > work for them you want gost all the time for everything. So it's the > suggested way for that class of users. Well, no, this is not "how Russian Government people use it", but is how normal people use it, and gost-engine does not make GOST everything, everywhere, always, but just enables GOST algorithms to use when they need. They may need to access gov portals, APIs, tax and communal systems, banks, etc. The root of the problem is GOST algorithms was split from mainstream OpenSSL project into separate subproject gost-engine. Unlike, for example, to SM2 or whatever you can use natively just by linking with openssl. Linking openssl does not make you work for Chinese Government and use SM algorithms all the time for everything. Just to make GOST algorithms "automatically" accessible by software which is linked to openssl, openssl.cnf trick is used. From the user point of view these algorithms just become "inside" of openssl. And there is no other way of extending openssl besides engines. And there are no other ways of loading engine except by 1) making software and their users aware of openssl engines, 2) config trick. > However, that's not how a casual non-Russian user would want it. > They'd only want gost if they specified the streebog hash. And if we > advertise the hash (as we do because you added it to the help) they > should have a reasonable expectation of its working easily. It will. I support both methods of use. For occasional user there is option --engine and for the frequent user there is config trick. > > > > This works unless tool is compiled without OPENSSL_LOAD_CONF and > > > > it calls OpenSSL_add_all_algorithms() instead of > > > > OPENSSL_add_all_algorithms_conf(). Which is frequently the > > > > default, (because there is too much methods of openssl > > > > initialization and people may not understand all intrications for > > > > all options.) In that case we try to persuade tool author to > > > > change the way openssl is initialized. > > > > > > What I'm saying is that modifying config files is really difficult > > > for most users. So, if you want ima-evm-utils to work out of the > > > box with the Streebog hash it needs to have no dependent > > > requirement on config files, which means you need to add the call > > > to ENGINE_set_default() > > > > I implemented two methods of loading engine for evmctl (via config > > and via --engine option). There is no problem with --engine option > > for Streebog, AFAIK. > > Can you try it with a vanilla (non gost modified) openssl.cnf file to > verify? I think you require the ENGINE_set_default() call but it may > be that a non-standard hash name will cause a search of the engine > added hashes. OpenSSL has badly documented defaults, so I usually > chase that through the code, but in this case a simple experiment will > tell us. Of course, I tried and tested that both ways are working independently. Just for Streebog ENGINE_set_default is not required, but to support GOST signatures (patch is RFCed) it will be required. > > We are talking about config method, which would be > > default for the most users. > > > > (Your suggestion is still good for my next patch what implements PKEY > > instead of RSA, for EC-RDSA signatures. Thanks! I will add it.) > > > > > This isn't about how you usually do it, it's about making these > > > additions usable for average users. > > > > Config method of loading gost engine is not how I usually do it, but > > how most gost users use it (and suggested to use it). This isn't some > > marginal use case. They just modify `openssl.cnf` once (manually of > > via script) and all relevant ssl tools "magically" start to > > understand gost algorithms without any additional options. This would > > be very not user friendly if for every network tool that supports ssl > > they should specify `--someoption gost`. > > Right, I'm not saying disallow that. The Russian crypto case is > certainly a valid class of user. What I'm saying is *don't* require > this config setup for other non-Russian crypto users because they won't > be familiar with it. Config method is not required as there are two ways of use. But engine does not load properly without a call to OPENSSL_add_all_algorithms_conf. So, I am curious why to disallow it. Also, calling OPENSSL_add_all_algorithms_conf will not cause any engine load if a user does not edit config on her system. Thanks, > > `--engine` option is just supplementary for advanced or occasional > > users. It continues to work (for Streebog) if > > OpenSSL_add_all_algorithms call is removed, but breaks if > > OPENSSL_add_all_algorithms_conf is removed, probably, need to call > > ENGINE_load_builtin_engines in that case. > > > > All this make initialization more complicated instead of single call > > to OPENSSL_add_all_algorithms_conf(). > > Yes, but it's acceptable to make our life more complex if it makes life > simpler for the end user. > > > > That doesn't stop you from adding further mods via the config > > > files, but it does mean that if a random user installs evmctl and > > > types > > > > > > evmctl -e gost -a streebog256 ... > > > > > > it will just work instead of failing with an obscure error because > > > the default config file is wrong. I actually think for usability > > > you should tie the hash and the engine together so a user can just > > > type > > > > > > evmctl -a streebog256 ... > > > > > > And the tool will automatically try to load the gost engine and > > > tell you if it's missing. > > > > This is interesting idea. (But this probably will fail for GOST > > keys/certificates.) > > Gost certs should identify the streebog hash, so in theory we can > examine the OIDs and see we need the gost engine to process them. I'm > certainly not saying do this, I'm just giving it as an example of how > to make life easier for users. > > James ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] ima-evm-utils: remove redundant call to OpenSSL_add_all_algorithms 2019-01-31 9:22 ` Vitaly Chikunov @ 2019-01-31 20:29 ` James Bottomley 0 siblings, 0 replies; 13+ messages in thread From: James Bottomley @ 2019-01-31 20:29 UTC (permalink / raw) To: Vitaly Chikunov; +Cc: Mimi Zohar, Mimi Zohar, Dmitry Kasatkin, linux-integrity On Thu, 2019-01-31 at 12:22 +0300, Vitaly Chikunov wrote: > On Wed, Jan 30, 2019 at 10:36:00AM -0800, James Bottomley wrote: [...] > > However, that's not how a casual non-Russian user would want it. > > They'd only want gost if they specified the streebog hash. And if > > we advertise the hash (as we do because you added it to the help) > > they should have a reasonable expectation of its working easily. > > It will. I support both methods of use. For occasional user there is > option --engine and for the frequent user there is config trick. OK, as long as users can use it without modifying the config file, I'm happy. [...] > > > I implemented two methods of loading engine for evmctl (via > > > config and via --engine option). There is no problem with -- > > > engine option for Streebog, AFAIK. > > > > Can you try it with a vanilla (non gost modified) openssl.cnf file > > to verify? I think you require the ENGINE_set_default() call but > > it may be that a non-standard hash name will cause a search of the > > engine added hashes. OpenSSL has badly documented defaults, so I > > usually chase that through the code, but in this case a simple > > experiment will tell us. > > Of course, I tried and tested that both ways are working > independently. Just for Streebog ENGINE_set_default is not required, > but to support GOST signatures (patch is RFCed) it will be required. I agree, I tried it with the openssl gost engine and you get this weird behaviour (I have to use md_gost94 because 1.0.2 gost doesn't have streebog): jejb@mulgrave:~/git/ima-evm-utils/src> ./evmctl -n --hashalgo md_gost94 ima_hash ~/tmp.ppt 01945d562c031c262563b026d8cc53e070140ad101 jejb@mulgrave:~/git/ima-evm-utils/src> ./evmctl -n --engine gost --hashalgo md_gost94 ima_hash ~/tmp.ppt 01a930a87289b548c2744fbb183a22196b1f651a727d84021d0eeb80cb4dddbb5d Because IMA silently falls back on sha1 if it can't find the hash. But the test proves it will use the gost hash when the engine is provided without ENGINE_set_default(). James ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-01-31 20:29 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-01-27 2:39 [PATCH] ima-evm-utils: remove redundant call to OpenSSL_add_all_algorithms Vitaly Chikunov 2019-01-30 12:34 ` Mimi Zohar 2019-01-30 13:25 ` Vitaly Chikunov 2019-01-30 13:41 ` Mimi Zohar 2019-01-30 13:53 ` Vitaly Chikunov 2019-01-30 15:59 ` Petr Vorel 2019-01-30 15:35 ` James Bottomley 2019-01-30 16:12 ` Vitaly Chikunov 2019-01-30 16:44 ` James Bottomley 2019-01-30 17:54 ` Vitaly Chikunov 2019-01-30 18:36 ` James Bottomley 2019-01-31 9:22 ` Vitaly Chikunov 2019-01-31 20:29 ` James Bottomley
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.