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.
> Then you could extract "hash_algo_name[]" from crypto/hash_info.c
> directly.
I could add this, BUT this will mean that developer with kernel tree
(very small amount of users) will have slightly different algos list
than a common person with a kernel-headers package. And I don't know a
single distro which packs full kernel source (with crypto/hash_info.c)
as easy to install package, and which would link appropriately into
/lib/modules/. So this is highly doubtful that user will be inclined to
install kernel source via usually complicated procedure just to compile
evmctl.
I already tried to address this possible difference issue by algocmp()
which would not compare `_` and `-` chars, which is the only difference
from hash_algo_name in the kernel and in ima-evm-utils and OpenSSL. So,
even without this feature (of parsing crypto/hash_info.c) added, user
already can specify any algo from crypto/hash_info.c and it will just
work.
> > +echo "enum hash_algo {"
> > +grep HASH_ALGO_.*, $HASH_INFO
> > +printf "\tHASH_ALGO__LAST\n"
> > +echo "};"
> > +
> > +echo "const char *const hash_algo_name[HASH_ALGO__LAST] = {"
> > +sed -n 's/HASH_ALGO_\(.*\),/[HASH_ALGO_\1] = "\L\1\E",/p' $HASH_INFO
> > +echo "};"
>
> Almost perfectly matches crypto/hash_crypto.c! Waiting to see if/how
> the next patch addresses the differences...
Could you elaborate again on what I should do with this suggestion?
I guessed crypto/hash_crypto.c is crypto/hash_info.c, but what
differences you expecting?
> > + /* 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.
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.
Thanks,
