Re: [PATCH] ima-evm-utils: remove redundant call to OpenSSL_add_all_algorithms

[James Bottomley <James.Bottomley@HansenPartnership.com>]

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