ima-evm-utils: remove redundant call to OpenSSL_add_all_algorithms

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(-)

src/libimaevm.c +0 -0

index 7501303..b038d0c 100644

Quoted 13 lines · Vitaly Chikunov

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(-)

src/libimaevm.c +0 -0

index 7501303..b038d0c 100644
@@ -995,7 +995,6 @@ int sign_hash(const char *hashalgo, const unsigned char *hash, int size, const c
 

src/libimaevm.c int sign_hash(const char *hashalgo, const unsigned char *hash, int size, const c · line 1000

Quoted 6 lines

src/libimaevm.c +0 -1

 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

Quoted 14 lines · Mimi Zohar

Quoted 13 lines · Vitaly Chikunov

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(-)

src/libimaevm.c +0 -0

index 7501303..b038d0c 100644
@@ -995,7 +995,6 @@ int sign_hash(const char *hashalgo, const unsigned char *hash, int size, const c
 

Quoted 12 lines

src/libimaevm.c int sign_hash(const char *hashalgo, const unsigned char *hash, int size, const c · line 1000

Quoted 6 lines

src/libimaevm.c +0 -1

 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.

[Cc'ing Bruno, Petr]

Quoted 28 lines · Vitaly Chikunov

Quoted 26 lines · Mimi Zohar

Quoted 12 lines · Vitaly Chikunov

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(-)

src/libimaevm.c +0 -0

index 7501303..b038d0c 100644

src/libimaevm.c int sign_hash(const char *hashalgo, const unsigned char *hash, int size, const c · line 1000

Quoted 7 lines

src/libimaevm.c +0 -1

@@ -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.

Quoted 4 lines

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

Quoted 31 lines

[Cc'ing Bruno, Petr]

Quoted 27 lines · Vitaly Chikunov

Quoted 26 lines · Mimi Zohar

Quoted 12 lines · Vitaly Chikunov

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(-)

src/libimaevm.c +0 -0

index 7501303..b038d0c 100644

src/libimaevm.c int sign_hash(const char *hashalgo, const unsigned char *hash, int size, const c · line 1000

Quoted 7 lines

src/libimaevm.c +0 -1

@@ -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.

Quoted 8 lines

Quoted 4 lines

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.

Quoted 1 line

Bruno, Petr, any preference?

Quoted 15 lines · Mimi Zohar

Quoted 14 lines · Vitaly Chikunov

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(-)

src/libimaevm.c +0 -0

index 7501303..b038d0c 100644
@@ -995,7 +995,6 @@ int sign_hash(const char *hashalgo, const

unsigned char *hash, int size, const c

Quoted 12 lines

src/libimaevm.c int sign_hash(const char *hashalgo, const unsigned char *hash, int size, const c · line 1000

Quoted 6 lines

static void libinit()
{

src/libimaevm.c +0 -1

-	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.

Quoted 29 lines · Vitaly Chikunov

Quoted 27 lines · Mimi Zohar

Quoted 16 lines · Vitaly Chikunov

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(-)

src/libimaevm.c +0 -0

index 7501303..b038d0c 100644
@@ -995,7 +995,6 @@ int sign_hash(const char *hashalgo, const

unsigned char *hash, int size, const c

static void libinit()
{

src/libimaevm.c int sign_hash(const char *hashalgo, const unsigned char *hash, int size, const c · line 1000

Quoted 4 lines

src/libimaevm.c +0 -1

-	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.

Quoted 5 lines

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

Quoted 33 lines · James Bottomley

Quoted 32 lines · Vitaly Chikunov

Quoted 30 lines · Mimi Zohar

Quoted 17 lines · Vitaly Chikunov

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(-)

src/libimaevm.c +0 -0

index 7501303..b038d0c 100644
@@ -995,7 +995,6 @@ int sign_hash(const char *hashalgo, const

unsigned char *hash, int size, const c

static void libinit()
{

src/libimaevm.c int sign_hash(const char *hashalgo, const unsigned char *hash, int size, const c · line 1000

Quoted 4 lines

src/libimaevm.c +0 -1

-	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.

Quoted 8 lines

Quoted 6 lines

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.

Quoted 47 lines · Vitaly Chikunov

Quoted 33 lines · James Bottomley

Quoted 32 lines · Vitaly Chikunov

Quoted 30 lines · Mimi Zohar

Quoted 17 lines · Vitaly Chikunov

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(-)

src/libimaevm.c +0 -0

index 7501303..b038d0c 100644
@@ -995,7 +995,6 @@ int sign_hash(const char *hashalgo, const

unsigned char *hash, int size, const c

static void libinit()
{

src/libimaevm.c int sign_hash(const char *hashalgo, const unsigned char *hash, int size, const c · line 1000

Quoted 4 lines

src/libimaevm.c +0 -1

-	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.

Quoted 8 lines

Quoted 6 lines

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:

Quoted 4 lines

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

Quoted 7 lines

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

Quoted 1 line · James Bottomley

On Wed, 2019-01-30 at 19:12 +0300, Vitaly Chikunov wrote:

Quoted 9 lines

Quoted 7 lines

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.

Quoted 8 lines

Quoted 7 lines

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.

Quoted 5 lines

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.)

Quoted 2 lines

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().

Quoted 11 lines

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

Quoted 4 lines

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.)

Quoted 2 lines · Vitaly Chikunov

Quoted 1 line · James Bottomley

On Wed, 2019-01-30 at 19:12 +0300, Vitaly Chikunov wrote:

[...]

Quoted 8 lines

Quoted 8 lines

Quoted 7 lines

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.

Quoted 3 lines

Quoted 1 line

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.

Quoted 14 lines

Quoted 8 lines

Quoted 7 lines

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.

Quoted 5 lines

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()

Quoted 3 lines

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.

Quoted 9 lines

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.)

Quoted 2 lines

This isn't about how you usually do it, it's about making these additions usable for average users.

Quoted 7 lines

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.

Quoted 8 lines

--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.

Quoted 11 lines

Quoted 11 lines

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

Quoted 7 lines

Quoted 4 lines

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

On Wed, Jan 30, 2019 at 10:36:00AM -0800, James Bottomley wrote:

Quoted 4 lines

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.

Quoted 10 lines

Quoted 3 lines

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.