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

Message ID 20190725061306.30515-1-vt@altlinux.org
State New
Headers show
Series
  • [1/2] ima-evm-utils: Do not allow fallback and unknown hash algos
Related show

Commit Message

Vitaly Chikunov July 25, 2019, 6:13 a.m. UTC
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(-)

Comments

Mimi Zohar July 25, 2019, 1:44 p.m. UTC | #1
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
Vitaly Chikunov July 25, 2019, 2:08 p.m. UTC | #2
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,
Mimi Zohar July 25, 2019, 2:29 p.m. UTC | #3
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

Patch
diff mbox series

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;