| Message ID | 20190725061306.30515-1-vt@altlinux.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] ima-evm-utils: Do not allow fallback and unknown hash algos | expand |
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
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,
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
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;
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(-)