On Sat, 2018-12-01 at 06:41 +0300, Vitaly Chikunov wrote:
> On Fri, Nov 30, 2018 at 02:22:28PM -0500, Mimi Zohar wrote:
> > On Thu, 2018-11-29 at 15:27 +0300, Vitaly Chikunov wrote:
> > > If configured with "--with-kernel-headers=PATH" try to extract hash
> > > algorithms from "hash_info.h" from the kernel source tree or
> > > kernel-headers package located in the specified path. (Otherwise, it
> > > will be tried to get from the installed kernel.)
> > >
> > > This also introduces two algorithm lists, one is built-in and another is
> > > from the kernel source. (They should never contain conflicting algorithm
> > > IDs by their append-only nature.) If the digest is not found in the
> > > built-in list it will be searched in the list from kernel's
> > > "hash_info.h".
> > >
> > > This patch will allow evmctl to be just recompiled to work with digest
> > > algorithms introduced in the newer kernels.
> > >
> > > Suggested-by: Mimi Zohar <zohar@xxxxxxxxxxxxx>
> > > Signed-off-by: Vitaly Chikunov <vt@xxxxxxxxxxxx>
> > > ---
> > > Changes since v1:
> > > - New patch.
> > > Changes since v2:
> > > - Mark PATH portion of "--with-kernel-headers=PATH" non-optional and
> > > change description to reflect correct behavior.
> >
> > Defaulting to the currently running kernel build tree would have been
> > nice.
>
> It already does. As description states "(Otherwise, it will be tried to get
> from the installed kernel.)" Also:
>
> > > +AC_ARG_WITH(kernel_headers, [AS_HELP_STRING([--with-kernel-headers=PATH],
> > > + [specifies the Linux kernel-headers package location or kernel root directory you want to use])],
> > > + [KERNEL_HEADERS="$withval"],
> > > + [KERNEL_HEADERS=/lib/modules/$(uname -r)/source])
>
> if "--with-kernel-headers" is not specified it will default to
> `/lib/modules/$(uname -r)/source` which points to the kernel-headers or
> kernel source tree root.
I missed that.
<snip>
> > > + /* first iterate over builtin algorithms */
> > > for (i = 0; i < PKEY_HASH__LAST; i++)
> > > if (pkey_hash_algo[i] &&
> > > !strcmp(algo, pkey_hash_algo[i]))
> > > return i;
> > >
> > > + /* iterate over algorithms provided by kernel-headers */
> > > + for (i = 0; i < HASH_ALGO__LAST; i++) {
> > > + if (hash_algo_name[i] &&
> > > + !algocmp(algo, hash_algo_name[i]))
> > > + return i;
> > > + }
> >
> > Assuming the two lists are in sync, which they should be, "i" could be
> > set to PKEY_HASH__LAST.
>
> Can not do that, since pkey_hash_algo is currently sparse array and could
> skip entries which are present in hash_algo_name.
Missed that too.
>
> Note, that since "[PATCH v2 7/7] ima-evm-utils: Try to load digest by
> its alias" lists will have different semantics: the pkey_hash_algo list is
> compared algo names with strmatch which supports algo aliases, and the
> hash_algo_name list is compared using algocmp which does not compare
> non-alphanumeric chars such as `_` and `-`, so streebog-512, streebog_512
> or just streebog512 all would work (to match algo id).
>
> Concluding, if you would not insist on parsing crypto/hash_info.c, I
> don't need fixing anything in this patch.
Agreed, the patch looks fine as is.
Mimi
