Hi Bruno, On 8/16/21 10:58 PM, Bruno Meneguele wrote: > The SHA-1 algorithm is considered a weak hash algorithm and there has been > some movement within certain distros to drop its support completely or at > least drop it from the default behavior. ima-evm-utils uses it as the > default algorithm in case the user doesn't explicitly ask for another > through the --hashalgo/-a option. With that, make SHA-256 the default hash > algorithm instead. I'm really happy to see that patch! I contributed to the patchset https://lore.kernel.org/linux-integrity/20210816081056.24530-1-Simon.THOBY@xxxxxxxxxx/T/#m8372b2b55dc8e1517e37624829fc8cb4361baf4d because I ran into exactly this issue of (hashing files with SHA1 because of the "insecure" default of evmctl). So I'm definitely in favor of switching the default hash to sha256. That said, considering that CONFIG_IMA (in the kernel) doesn't depend on CONFIG_CRYPTO_SHA256, isn't there a risk that files hashes/signed with this patch could break on a kernel where that option wasn't selected? (I also don't know how frequent kernels without CONFIG_CRYPTO_SHA256 may be - given that CONFIG_ENCRYPTED_KEYS require SHA256, probably not so much) I guess this boils down to: "do we know of any distribution not selecting CRYPTO_SHA256?". If we don't, then the backward compatibility impact should be nearly void. If we do, some decision must be taken. > > Signed-off-by: Bruno Meneguele <bmeneg@xxxxxxxxxx> > --- > Changelog: > v1: add ima-evm-utils to the [PATCH] part of the subject > > README | 2 +- > src/evmctl.c | 2 +- > src/libimaevm.c | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/README b/README > index 87cd3b5cd7da..0dc02f551673 100644 > --- a/README > +++ b/README > @@ -41,7 +41,7 @@ COMMANDS > OPTIONS > ------- > > - -a, --hashalgo sha1 (default), sha224, sha256, sha384, sha512 > + -a, --hashalgo sha1, sha224, sha256 (default), sha384, sha512 > -s, --imasig make IMA signature > -d, --imahash make IMA hash > -f, --sigfile store IMA signature in .sig file instead of xattr > diff --git a/src/evmctl.c b/src/evmctl.c > index a8065bbe124a..e0e55bc0b122 100644 > --- a/src/evmctl.c > +++ b/src/evmctl.c > @@ -2496,7 +2496,7 @@ static void usage(void) > > printf( > "\n" > - " -a, --hashalgo sha1 (default), sha224, sha256, sha384, sha512, streebog256, streebog512\n" > + " -a, --hashalgo sha1, sha224, sha256 (default), sha384, sha512, streebog256, streebog512\n" > " -s, --imasig make IMA signature\n" > " -d, --imahash make IMA hash\n" > " -f, --sigfile store IMA signature in .sig file instead of xattr\n" > diff --git a/src/libimaevm.c b/src/libimaevm.c > index 8e9615796153..f6c72b878d88 100644 > --- a/src/libimaevm.c > +++ b/src/libimaevm.c > @@ -88,7 +88,7 @@ static const char *const pkey_hash_algo_kern[PKEY_HASH__LAST] = { > struct libimaevm_params imaevm_params = { > .verbose = LOG_INFO, > .x509 = 1, > - .hash_algo = "sha1", > + .hash_algo = "sha256", > }; > > static void __attribute__ ((constructor)) libinit(void); > No comments on the code change, it looks alright to me. Thanks, Simon
