[PATCH 1/2] ima-evm-utils: Do not allow fallback and unknown hash algos

[PATCH 1/2] ima-evm-utils: Do not allow fallback and unknown hash algos

[Vitaly Chikunov]

Falling back and permissiveness could have security implications.

Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
---
 src/evmctl.c    | 4 ++++
 src/libimaevm.c | 7 +++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index 3289061..3766343 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -584,6 +584,10 @@ static int hash_ima(const char *file)
 	int len, err, offset;
 	int algo = get_hash_algo(params.hash_algo);
 
+	if (algo < 0) {
+		log_err("Unknown hash algo: %s\n", params.hash_algo);
+		return -1;
+	}
 	if (algo > PKEY_HASH_SHA1) {
 		hash[0] = IMA_XATTR_DIGEST_NG;
 		hash[1] = algo;
diff --git a/src/libimaevm.c b/src/libimaevm.c
index 2d99570..86c5784 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -571,8 +571,7 @@ int get_hash_algo(const char *algo)
 		    !strcmp(algo, hash_algo_name[i]))
 			return i;
 
-	log_info("digest %s not found, fall back to sha1\n", algo);
-	return PKEY_HASH_SHA1;
+	return -1;
 }
 
 static int get_hash_algo_from_sig(unsigned char *sig)
@@ -920,6 +919,10 @@ int sign_hash_v2(const char *algo, const unsigned char *hash, int size, const ch
 	hdr->version = (uint8_t) DIGSIG_VERSION_2;
 
 	hdr->hash_algo = get_hash_algo(algo);
+	if (hdr->hash_algo == -1) {
+		log_err("sign_hash_v2: hash algo is unknown: %s\n", algo);
+		return -1;
+	}
 
 	calc_keyid_v2(&keyid, name, pkey);
 	hdr->keyid = keyid;
-- 
2.11.0

[PATCH 2/2] ima-evm-utils: Show used hash algo in verbose mode

[Vitaly Chikunov]

This could be useful for users.

Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
---
This will conflict with not accepted `params' rename patch.

 src/evmctl.c    | 2 +-
 src/libimaevm.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/evmctl.c b/src/evmctl.c
index 3766343..75dd163 100644
--- a/src/evmctl.c
+++ b/src/evmctl.c
@@ -605,7 +605,7 @@ static int hash_ima(const char *file)
 	len += offset;
 
 	if (params.verbose >= LOG_INFO)
-		log_info("hash: ");
+		log_info("hash(%s): ", params.hash_algo);
 
 	if (sigdump || params.verbose >= LOG_INFO)
 		dump(hash, len);
diff --git a/src/libimaevm.c b/src/libimaevm.c
index 86c5784..11dbf11 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -822,7 +822,7 @@ int sign_hash_v1(const char *hashalgo, const unsigned char *hash, int size, cons
 		return -1;
 	}
 
-	log_info("hash: ");
+	log_info("hash(%s): ", hashalgo);
 	log_dump(hash, size);
 
 	key = read_priv_key(keyfile, params.keypass);
-- 
2.11.0

Re: [PATCH 1/2] ima-evm-utils: Do not allow fallback and unknown hash algos

[Mimi Zohar]

On Thu, 2019-07-25 at 09:13 +0300, Vitaly Chikunov wrote:
> Falling back and permissiveness could have security implications.
> 
> Signed-off-by: Vitaly Chikunov <vt@altlinux.org>

Thanks!  Please update the README, removing "(default)", and rebase on
top of the "param" changes.

Mimi

Re: [PATCH 2/2] ima-evm-utils: Show used hash algo in verbose mode

[Mimi Zohar]

On Thu, 2019-07-25 at 09:13 +0300, Vitaly Chikunov wrote:
> This could be useful for users.
> 
> Signed-off-by: Vitaly Chikunov <vt@altlinux.org>

Thanks!

Mimi

Re: [PATCH 1/2] ima-evm-utils: Do not allow fallback and unknown hash algos

[Vitaly Chikunov]

Mimi,

On Thu, Jul 25, 2019 at 09:44:02AM -0400, Mimi Zohar wrote:
> On Thu, 2019-07-25 at 09:13 +0300, Vitaly Chikunov wrote:
> > Falling back and permissiveness could have security implications.
> > 
> > Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
> 
> Thanks!  Please update the README, removing "(default)", and rebase on
> top of the "param" changes.

In my understanding this text in README should not be changed, since not
specifying `-a' is the same as `-a sha1', so default holds. Code
handling this is not changed (which is in src/libimaevm.c:87).

What I changed is some other unexpected switching to sha1. Like when
user specify wrong hash name in `-a'.

So I will not resend this (as there is no changes). And I want to rebase
`param' & `imaevm_' prefix patch over these two commits.

Thanks,

Re: [PATCH 1/2] ima-evm-utils: Do not allow fallback and unknown hash algos

[Mimi Zohar]

On Thu, 2019-07-25 at 17:08 +0300, Vitaly Chikunov wrote:
> Mimi,
> 
> On Thu, Jul 25, 2019 at 09:44:02AM -0400, Mimi Zohar wrote:
> > On Thu, 2019-07-25 at 09:13 +0300, Vitaly Chikunov wrote:
> > > Falling back and permissiveness could have security implications.
> > > 
> > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
> > 
> > Thanks!  Please update the README, removing "(default)", and rebase on
> > top of the "param" changes.
> 
> In my understanding this text in README should not be changed, since not
> specifying `-a' is the same as `-a sha1', so default holds. Code
> handling this is not changed (which is in src/libimaevm.c:87).

Agreed

> 
> What I changed is some other unexpected switching to sha1. Like when
> user specify wrong hash name in `-a'.
> 
> So I will not resend this (as there is no changes). And I want to rebase
> `param' & `imaevm_' prefix patch over these two commits.

That works.

thanks,

Mimi