On Thu, 2021-01-07 at 16:15 +0300, Vitaly Chikunov wrote:

On Thu, Jan 07, 2021 at 04:08:16PM +0300, Vitaly Chikunov wrote:

quoted

Patrick, Mimi,

On Thu, Jan 07, 2021 at 07:24:43AM -0500, Mimi Zohar wrote:

quoted

On Wed, 2021-01-06 at 10:43 +0100, Patrick Uiterwijk wrote:

quoted

This fixes sign_hash not using the correct algorithm for creating the
signature, by ensuring it uses the passed in variable value.

Signed-off-by: Patrick Uiterwijk <redacted>

Thank you.  This is a regression first introduced in commit
07e623b60848 ("ima-evm-utils: Convert sign_hash_v2 to EVP_PKEY API").

Ah, when sign_hash() is used not via evmctl, but as a library
imaevm_params may be not set.

Thinking about imaevm_params -- it's still belong to a library and
extensively used in libimaevm.c, so I wonder if the fix is perfect.
Since, now there is hash_algo and algo duplication which one to prefer
and why?

Maybe, it should be [also] set like keypass in sign_hash?

  int sign_hash(const char *hashalgo, const unsigned char *hash, int size, const char *keyfile, const char *keypass, unsigned char *sig)
  {
	  if (keypass)
		  imaevm_params.keypass = keypass;

+	  if (hashalgo)
+		  imaevm_params.hash_algo = hashalgo;

	  return imaevm_params.x509 ?
		  sign_hash_v2(hashalgo, hash, size, keyfile, sig) :
		  sign_hash_v1(hashalgo, hash, size, keyfile, sig);
  }

As seen above, the main difference between keypass and hashalgo is that
hashalgo is being passed as an argument, while keypass isn't.  Instead
of changing the function arguments, it looks like keypass was stored as
global variable.  For this reason, the priority should be the function
argument, not the global variable.

thanks,

Mimi