| Message ID | 20190707234837.4866-1-vt@altlinux.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | ima-evm-utils: Assorted fixes and improvements | expand |
[Cc'ing Roberto, Petr, Thiago, Prakhar] Hi Vitaly, On Mon, 2019-07-08 at 02:48 +0300, Vitaly Chikunov wrote: > There is small fixes and improvements to ima-evm-utils. > Tested on x86_64. > > Vitaly Chikunov (5): > ima-evm-utils: Fix EVP_MD_CTX leak in ima_calc_hash > ima-evm-utils: Fix memory leak in init_public_keys > ima-evm-utils: Preload public keys for ima_verify > ima-evm-utils: Allow multiple files in ima_verify > ima-evm-utils: Fix clang warning about possible unaligned pointer for > hdr->keyid > > src/evmctl.c | 11 ++++++++--- > src/libimaevm.c | 38 ++++++++++++++++++++++++++------------ > 2 files changed, 34 insertions(+), 15 deletions(-) Thanks, this patch set looks good. These patches, the "ima-evm-utils: Convert v2 signatures from RSA to EVP_PKEY AP", and the two patches I posted today are now in #next, but I'd really appreciate some additional Review's/Tested's on these patches. Now that we're including ALL the kernel exported hash_info algorithms, a colleague suggested defining a list of deprecated hash algorithms. Instead of preventing the usage of these deprecated hash algorithms, initially I would start out with a warning. It would be helpful to indicate which standard deprecated the hash algorithm and year. At some point, we might want to prevent their usage in signing files, but not verifying file signatures. evmctl "ima_measurement" doesn't support custom template definitions. Also missing is support for verifying the "ima-buf" kexec command boot command line and the "ima-modsig" template appended signature. David Jacobson started writing a regression framework and posted a v2 version. I'd really appreciate help with cleaning up that code. Any other comments/suggestions/ideas? thanks, Mimi
Mimi, On Mon, Jul 08, 2019 at 11:30:50AM -0400, Mimi Zohar wrote: > [Cc'ing Roberto, Petr, Thiago, Prakhar] > Now that we're including ALL the kernel exported hash_info algorithms, > a colleague suggested defining a list of deprecated hash algorithms. > Instead of preventing the usage of these deprecated hash algorithms, > initially I would start out with a warning. It would be helpful to > indicate which standard deprecated the hash algorithm and year. At > some point, we might want to prevent their usage in signing files, but > not verifying file signatures. I think this is not a problem, because user explicitly states which hash algorithm he wants to use. Except for SHA1, which is also silent fallback algorithm. I think this fallback mechanism should be removed. Also, return values of sign_hash/ima_calc_hash/etc are not defined clearly and callers have weird checks such as `if (len <= 1)`. I think this should be conceptually simplified and made them `return -1` on any error. > evmctl "ima_measurement" doesn't support custom template definitions. > Also missing is support for verifying the "ima-buf" kexec command boot > command line and the "ima-modsig" template appended signature. > > David Jacobson started writing a regression framework and posted a v2 > version. I'd really appreciate help with cleaning up that code. Maybe tests should be integrated into ima-evm-utils too. Thanks,
On Tue, 2019-07-09 at 18:43 +0300, Vitaly Chikunov wrote: > On Mon, Jul 08, 2019 at 11:30:50AM -0400, Mimi Zohar wrote: > > [Cc'ing Roberto, Petr, Thiago, Prakhar] > > Now that we're including ALL the kernel exported hash_info algorithms, > > a colleague suggested defining a list of deprecated hash algorithms. > > Instead of preventing the usage of these deprecated hash algorithms, > > initially I would start out with a warning. It would be helpful to > > indicate which standard deprecated the hash algorithm and year. At > > some point, we might want to prevent their usage in signing files, but > > not verifying file signatures. > > I think this is not a problem, because user explicitly states which hash > algorithm he wants to use. Except for SHA1, which is also silent > fallback algorithm. I think this fallback mechanism should be removed. I don't see a problem with informing whoever is using ima-evm-utils, that the requested hash algorithm has been deprecated. Just as NIST has deprecated the use of sha1 for most usecases, certain versions of the gost standard have also been deprecated. Someone verifying the measurement list or the filesystem signatures should be informed that although the verification succeeded, or not, some of the signatures verified were based on deprecated hash algorithms. Maybe something along the lines of "<algorithm> hash algorithm was deprecated (<year> - <standard>)"? In verbose mode, the message could include the filename. The EVM hmac is still sha1 based, but that shouldn't affect ima-evm- utils. So yes we should explicitly require a valid hash algorithm and not fall back to using sha1. > > Also, return values of sign_hash/ima_calc_hash/etc are not defined > clearly and callers have weird checks such as `if (len <= 1)`. I think > this should be conceptually simplified and made them `return -1` on any > error. Agreed. Mostly these functions are returning "-1" on error. There are a few places returning 1 instead. > > > > evmctl "ima_measurement" doesn't support custom template definitions. > > Also missing is support for verifying the "ima-buf" kexec command boot > > command line and the "ima-modsig" template appended signature. > > > > David Jacobson started writing a regression framework and posted a v2 > > version. I'd really appreciate help with cleaning up that code. > > Maybe tests should be integrated into ima-evm-utils too. Including regression tests in ima-evm-utils is the plan. :) Mimi
Hi Mimi, Vitaly, > Mimi, > On Mon, Jul 08, 2019 at 11:30:50AM -0400, Mimi Zohar wrote: > > [Cc'ing Roberto, Petr, Thiago, Prakhar] > > Now that we're including ALL the kernel exported hash_info algorithms, > > a colleague suggested defining a list of deprecated hash algorithms. > > Instead of preventing the usage of these deprecated hash algorithms, > > initially I would start out with a warning. It would be helpful to > > indicate which standard deprecated the hash algorithm and year. At > > some point, we might want to prevent their usage in signing files, but > > not verifying file signatures. Looks useful to me. > I think this is not a problem, because user explicitly states which hash > algorithm he wants to use. Except for SHA1, which is also silent > fallback algorithm. I think this fallback mechanism should be removed. Agree with removing fallback mechanism. > Also, return values of sign_hash/ima_calc_hash/etc are not defined > clearly and callers have weird checks such as `if (len <= 1)`. I think > this should be conceptually simplified and made them `return -1` on any > error. > > evmctl "ima_measurement" doesn't support custom template definitions. > > Also missing is support for verifying the "ima-buf" kexec command boot > > command line and the "ima-modsig" template appended signature. > > David Jacobson started writing a regression framework and posted a v2 > > version. I'd really appreciate help with cleaning up that code. > Maybe tests should be integrated into ima-evm-utils too. https://patchwork.kernel.org/project/linux-integrity/list/?series=95303 > Thanks, Kind regards, Petr
There is small fixes and improvements to ima-evm-utils. Tested on x86_64. Vitaly Chikunov (5): ima-evm-utils: Fix EVP_MD_CTX leak in ima_calc_hash ima-evm-utils: Fix memory leak in init_public_keys ima-evm-utils: Preload public keys for ima_verify ima-evm-utils: Allow multiple files in ima_verify ima-evm-utils: Fix clang warning about possible unaligned pointer for hdr->keyid src/evmctl.c | 11 ++++++++--- src/libimaevm.c | 38 ++++++++++++++++++++++++++------------ 2 files changed, 34 insertions(+), 15 deletions(-)