* [PATCH] ima-evm-utils: simplify digest alias handling
@ 2019-02-12 0:14 Vitaly Chikunov
2019-02-12 16:23 ` Mimi Zohar
0 siblings, 1 reply; 5+ messages in thread
From: Vitaly Chikunov @ 2019-02-12 0:14 UTC (permalink / raw)
To: Mimi Zohar, Dmitry Kasatkin, linux-integrity
- Make digest name search work just with simple strcmp() and two arrays.
- Make newer name "streebogX" alias of older and backward compatible name
"md_gost12_X".
- Remove get_digestbyname() which was doing two hash name resolving
attempts, reverting to use plain EVP_get_digestbyname(). This will
force the user to specify the proper hash name depending on what
OpenSSL provides, allowing to specify older hash name in older
OpenSSL. (This may be improved later, for example, by resolving
hash name to a numerical id and the id to a proper hash name. Or
we could resolve a hash name to the OpenSSL hash NID and resolve
the NID to a internal hash name. In this case we don't need to keep
hash alias names at all.)
---
src/libimaevm.c | 89 +++++++++------------------------------------------------
1 file changed, 13 insertions(+), 76 deletions(-)
diff --git a/src/libimaevm.c b/src/libimaevm.c
index d9ffa13..ed77211 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -63,7 +63,7 @@
#include "imaevm.h"
#include "hash_info.h"
-const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
+const char *pkey_hash_algo[PKEY_HASH__LAST] = {
[PKEY_HASH_MD4] = "md4",
[PKEY_HASH_MD5] = "md5",
[PKEY_HASH_SHA1] = "sha1",
@@ -72,8 +72,13 @@ const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
[PKEY_HASH_SHA384] = "sha384",
[PKEY_HASH_SHA512] = "sha512",
[PKEY_HASH_SHA224] = "sha224",
- [PKEY_HASH_STREEBOG_256] = "md_gost12_256,streebog256",
- [PKEY_HASH_STREEBOG_512] = "md_gost12_512,streebog512",
+ [PKEY_HASH_STREEBOG_256] = "md_gost12_256",
+ [PKEY_HASH_STREEBOG_512] = "md_gost12_512",
+};
+
+const char *pkey_hash_algo_alias[PKEY_HASH__LAST] = {
+ [PKEY_HASH_STREEBOG_256] = "streebog256",
+ [PKEY_HASH_STREEBOG_512] = "streebog512",
};
/*
@@ -161,8 +166,6 @@ const char *get_hash_algo_by_id(int algo)
{
if (algo < PKEY_HASH__LAST)
return pkey_hash_algo[algo];
- if (algo < HASH_ALGO__LAST)
- return hash_algo_name[algo];
log_err("digest %d not found\n", algo);
return "unknown";
@@ -286,35 +289,6 @@ static int add_dev_hash(struct stat *st, EVP_MD_CTX *ctx)
return !EVP_DigestUpdate(ctx, &dev, sizeof(dev));
}
-/* Call EVP_get_digestbyname with provided name and with alias,
- * which is first name in the comma separated list of names
- * in pkey_hash_algo.
- */
-static const EVP_MD *get_digestbyname(const char *name)
-{
- const EVP_MD *md;
-
- md = EVP_get_digestbyname(params.hash_algo);
- if (!md) {
- char alias[MAX_DIGEST_NAME];
- int len;
- int algo_id;
- const char *algo_alias;
-
- /* try to get algo by its alias */
- algo_id = get_hash_algo(params.hash_algo);
- algo_alias = get_hash_algo_by_id(algo_id);
- len = strchrnul(algo_alias, ',') - algo_alias;
- if (len < sizeof(alias)) {
- memcpy(alias, algo_alias, len);
- alias[len] = '\0';
- if (strcmp(params.hash_algo, alias))
- md = EVP_get_digestbyname(alias);
- }
- }
- return md;
-}
-
int ima_calc_hash(const char *file, uint8_t *hash)
{
const EVP_MD *md;
@@ -336,7 +310,7 @@ int ima_calc_hash(const char *file, uint8_t *hash)
return err;
}
- md = get_digestbyname(params.hash_algo);
+ md = EVP_get_digestbyname(params.hash_algo);
if (!md) {
log_err("EVP_get_digestbyname(%s) failed\n", params.hash_algo);
return 1;
@@ -572,42 +546,6 @@ int verify_hash_v2(const char *file, const unsigned char *hash, int size,
return 0;
}
-/* compare algo names case insensitively and ignoring separators */
-static int algocmp(const char *a, const char *b)
-{
- while (*a && *b) {
- int cha, chb;
-
- cha = tolower((unsigned char)*a++);
- if (!isalnum(cha))
- continue;
- chb = tolower((unsigned char)*b++);
- if (!isalnum(chb)) {
- a--;
- continue;
- }
- if (cha != chb)
- return -1;
- }
- return *a || *b;
-}
-
-static int strmatch(const char *needle, const char *haystack)
-{
- int len = strlen(needle);
- const char *p = haystack;
- const char *t;
-
- for (t = strchrnul(p, ','); *p; p = t, t = strchrnul(p, ',')) {
- if (t - p == len &&
- !strncmp(needle, p, len))
- return 0;
- if (!*t++)
- break;
- }
- return 1;
-}
-
int get_hash_algo(const char *algo)
{
int i;
@@ -615,15 +553,14 @@ int get_hash_algo(const char *algo)
/* first iterate over builtin algorithms */
for (i = 0; i < PKEY_HASH__LAST; i++)
if (pkey_hash_algo[i] &&
- !strmatch(algo, 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]))
+ for (i = 0; i < PKEY_HASH__LAST; i++)
+ if (pkey_hash_algo_alias[i] &&
+ !strcmp(algo, pkey_hash_algo_alias[i]))
return i;
- }
log_info("digest %s not found, fall back to sha1\n", algo);
return PKEY_HASH_SHA1;
--
2.11.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] ima-evm-utils: simplify digest alias handling
2019-02-12 0:14 [PATCH] ima-evm-utils: simplify digest alias handling Vitaly Chikunov
@ 2019-02-12 16:23 ` Mimi Zohar
2019-02-12 17:23 ` Vitaly Chikunov
0 siblings, 1 reply; 5+ messages in thread
From: Mimi Zohar @ 2019-02-12 16:23 UTC (permalink / raw)
To: Vitaly Chikunov, Mimi Zohar, Dmitry Kasatkin, linux-integrity
Hi Vitaly,
Definitely a lot better.
> diff --git a/src/libimaevm.c b/src/libimaevm.c
> index d9ffa13..ed77211 100644
> --- a/src/libimaevm.c
> +++ b/src/libimaevm.c
> @@ -63,7 +63,7 @@
> #include "imaevm.h"
> #include "hash_info.h"
>
> -const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
> +const char *pkey_hash_algo[PKEY_HASH__LAST] = {
Dropping the "const"?
> [PKEY_HASH_MD4] = "md4",
> [PKEY_HASH_MD5] = "md5",
> [PKEY_HASH_SHA1] = "sha1",
> @@ -72,8 +72,13 @@ const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
> [PKEY_HASH_SHA384] = "sha384",
> [PKEY_HASH_SHA512] = "sha512",
> [PKEY_HASH_SHA224] = "sha224",
> - [PKEY_HASH_STREEBOG_256] = "md_gost12_256,streebog256",
> - [PKEY_HASH_STREEBOG_512] = "md_gost12_512,streebog512",
> + [PKEY_HASH_STREEBOG_256] = "md_gost12_256",
> + [PKEY_HASH_STREEBOG_512] = "md_gost12_512",
> +};
> +
> +const char *pkey_hash_algo_alias[PKEY_HASH__LAST] = {
> + [PKEY_HASH_STREEBOG_256] = "streebog256",
> + [PKEY_HASH_STREEBOG_512] = "streebog512",
> };
>
If the upstream kernel name is defined as "streebog", the alias would
be "gost".
> @@ -615,15 +553,14 @@ int get_hash_algo(const char *algo)
> /* first iterate over builtin algorithms */
> for (i = 0; i < PKEY_HASH__LAST; i++)
> if (pkey_hash_algo[i] &&
> - !strmatch(algo, 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]))
> + for (i = 0; i < PKEY_HASH__LAST; i++)
> + if (pkey_hash_algo_alias[i] &&
> + !strcmp(algo, pkey_hash_algo_alias[i]))
> return i;
> - }
>
This doesn't look right. The comments don't reflect the code.
Shouldn't there be 3 loops - pkey_hash_algo, pkey_hash_algo_alias, and
then the kernel header?
Mimi
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] ima-evm-utils: simplify digest alias handling
2019-02-12 16:23 ` Mimi Zohar
@ 2019-02-12 17:23 ` Vitaly Chikunov
2019-02-12 19:17 ` Mimi Zohar
0 siblings, 1 reply; 5+ messages in thread
From: Vitaly Chikunov @ 2019-02-12 17:23 UTC (permalink / raw)
To: Mimi Zohar; +Cc: Mimi Zohar, Dmitry Kasatkin, linux-integrity
Mimi,
On Tue, Feb 12, 2019 at 11:23:40AM -0500, Mimi Zohar wrote:
> Definitely a lot better.
>
> > diff --git a/src/libimaevm.c b/src/libimaevm.c
> > index d9ffa13..ed77211 100644
> > --- a/src/libimaevm.c
> > +++ b/src/libimaevm.c
> > @@ -63,7 +63,7 @@
> > #include "imaevm.h"
> > #include "hash_info.h"
> >
> > -const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
> > +const char *pkey_hash_algo[PKEY_HASH__LAST] = {
>
> Dropping the "const"?
I will undo this change.
> > [PKEY_HASH_MD4] = "md4",
> > [PKEY_HASH_MD5] = "md5",
> > [PKEY_HASH_SHA1] = "sha1",
> > @@ -72,8 +72,13 @@ const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
> > [PKEY_HASH_SHA384] = "sha384",
> > [PKEY_HASH_SHA512] = "sha512",
> > [PKEY_HASH_SHA224] = "sha224",
> > - [PKEY_HASH_STREEBOG_256] = "md_gost12_256,streebog256",
> > - [PKEY_HASH_STREEBOG_512] = "md_gost12_512,streebog512",
> > + [PKEY_HASH_STREEBOG_256] = "md_gost12_256",
> > + [PKEY_HASH_STREEBOG_512] = "md_gost12_512",
> > +};
> > +
> > +const char *pkey_hash_algo_alias[PKEY_HASH__LAST] = {
> > + [PKEY_HASH_STREEBOG_256] = "streebog256",
> > + [PKEY_HASH_STREEBOG_512] = "streebog512",
> > };
> >
>
> If the upstream kernel name is defined as "streebog", the alias would
> be "gost".
To me it seems that meaning of what here is alias is really not
important. There is two names in OpenSSL md_gost12_X and streebogX,
where streebogX is newer and less accessible, and md_gost12_X is
more accessible in older version. While in the kernel name is streebogX.
Probably it's better to distinguish as openssl and kernel names, and not
the name and alias.
Since libimaevm primarily works with OpenSSL API, I keep backward compatible
names md_gost12_X in pkey_hash_algo, so when algo id is resolved to the name
it resolved to the name which is present in the older OpenSSLs, thus we don't
need to request EVP_get_digestbyname another time if user specified name is
not found in OpenSSL.
> > @@ -615,15 +553,14 @@ int get_hash_algo(const char *algo)
> > /* first iterate over builtin algorithms */
> > for (i = 0; i < PKEY_HASH__LAST; i++)
> > if (pkey_hash_algo[i] &&
> > - !strmatch(algo, 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]))
> > + for (i = 0; i < PKEY_HASH__LAST; i++)
> > + if (pkey_hash_algo_alias[i] &&
> > + !strcmp(algo, pkey_hash_algo_alias[i]))
> > return i;
> > - }
> >
>
> This doesn't look right. The comments don't reflect the code.
> Shouldn't there be 3 loops - pkey_hash_algo, pkey_hash_algo_alias, and
> then the kernel header?
My mistake. I forgot about this feature.
Thanks!
>
> Mimi
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] ima-evm-utils: simplify digest alias handling
2019-02-12 17:23 ` Vitaly Chikunov
@ 2019-02-12 19:17 ` Mimi Zohar
2019-02-12 19:31 ` Vitaly Chikunov
0 siblings, 1 reply; 5+ messages in thread
From: Mimi Zohar @ 2019-02-12 19:17 UTC (permalink / raw)
To: Vitaly Chikunov; +Cc: Mimi Zohar, Dmitry Kasatkin, linux-integrity
On Tue, 2019-02-12 at 20:23 +0300, Vitaly Chikunov wrote:
> Mimi,
>
> On Tue, Feb 12, 2019 at 11:23:40AM -0500, Mimi Zohar wrote:
> > Definitely a lot better.
> >
> > > diff --git a/src/libimaevm.c b/src/libimaevm.c
> > > index d9ffa13..ed77211 100644
> > > --- a/src/libimaevm.c
> > > +++ b/src/libimaevm.c
> > > @@ -63,7 +63,7 @@
> > > #include "imaevm.h"
> > > #include "hash_info.h"
> > >
> > > -const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
> > > +const char *pkey_hash_algo[PKEY_HASH__LAST] = {
> >
> > Dropping the "const"?
>
> I will undo this change.
>
> > > [PKEY_HASH_MD4] = "md4",
> > > [PKEY_HASH_MD5] = "md5",
> > > [PKEY_HASH_SHA1] = "sha1",
> > > @@ -72,8 +72,13 @@ const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
> > > [PKEY_HASH_SHA384] = "sha384",
> > > [PKEY_HASH_SHA512] = "sha512",
> > > [PKEY_HASH_SHA224] = "sha224",
> > > - [PKEY_HASH_STREEBOG_256] = "md_gost12_256,streebog256",
> > > - [PKEY_HASH_STREEBOG_512] = "md_gost12_512,streebog512",
> > > + [PKEY_HASH_STREEBOG_256] = "md_gost12_256",
> > > + [PKEY_HASH_STREEBOG_512] = "md_gost12_512",
> > > +};
> > > +
> > > +const char *pkey_hash_algo_alias[PKEY_HASH__LAST] = {
> > > + [PKEY_HASH_STREEBOG_256] = "streebog256",
> > > + [PKEY_HASH_STREEBOG_512] = "streebog512",
> > > };
> > >
> >
> > If the upstream kernel name is defined as "streebog", the alias would
> > be "gost".
>
> To me it seems that meaning of what here is alias is really not
> important. There is two names in OpenSSL md_gost12_X and streebogX,
> where streebogX is newer and less accessible, and md_gost12_X is
> more accessible in older version. While in the kernel name is streebogX.
>
> Probably it's better to distinguish as openssl and kernel names, and not
> the name and alias.
The original design matched the kernel haah_info.h.
Mimi
>
> Since libimaevm primarily works with OpenSSL API, I keep backward compatible
> names md_gost12_X in pkey_hash_algo, so when algo id is resolved to the name
> it resolved to the name which is present in the older OpenSSLs, thus we don't
> need to request EVP_get_digestbyname another time if user specified name is
> not found in OpenSSL.
>
> > > @@ -615,15 +553,14 @@ int get_hash_algo(const char *algo)
> > > /* first iterate over builtin algorithms */
> > > for (i = 0; i < PKEY_HASH__LAST; i++)
> > > if (pkey_hash_algo[i] &&
> > > - !strmatch(algo, 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]))
> > > + for (i = 0; i < PKEY_HASH__LAST; i++)
> > > + if (pkey_hash_algo_alias[i] &&
> > > + !strcmp(algo, pkey_hash_algo_alias[i]))
> > > return i;
> > > - }
> > >
> >
> > This doesn't look right. The comments don't reflect the code.
> > Shouldn't there be 3 loops - pkey_hash_algo, pkey_hash_algo_alias, and
> > then the kernel header?
>
> My mistake. I forgot about this feature.
>
> Thanks!
>
> >
> > Mimi
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] ima-evm-utils: simplify digest alias handling
2019-02-12 19:17 ` Mimi Zohar
@ 2019-02-12 19:31 ` Vitaly Chikunov
0 siblings, 0 replies; 5+ messages in thread
From: Vitaly Chikunov @ 2019-02-12 19:31 UTC (permalink / raw)
To: Mimi Zohar; +Cc: Mimi Zohar, Dmitry Kasatkin, linux-integrity
Mimi,
On Tue, Feb 12, 2019 at 02:17:19PM -0500, Mimi Zohar wrote:
> On Tue, 2019-02-12 at 20:23 +0300, Vitaly Chikunov wrote:
> > On Tue, Feb 12, 2019 at 11:23:40AM -0500, Mimi Zohar wrote:
> > > Definitely a lot better.
> > >
> > > > diff --git a/src/libimaevm.c b/src/libimaevm.c
> > > > index d9ffa13..ed77211 100644
> > > > --- a/src/libimaevm.c
> > > > +++ b/src/libimaevm.c
> > > > @@ -63,7 +63,7 @@
> > > > #include "imaevm.h"
> > > > #include "hash_info.h"
> > > >
> > > > -const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
> > > > +const char *pkey_hash_algo[PKEY_HASH__LAST] = {
> > >
> > > Dropping the "const"?
> >
> > I will undo this change.
> >
> > > > [PKEY_HASH_MD4] = "md4",
> > > > [PKEY_HASH_MD5] = "md5",
> > > > [PKEY_HASH_SHA1] = "sha1",
> > > > @@ -72,8 +72,13 @@ const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
> > > > [PKEY_HASH_SHA384] = "sha384",
> > > > [PKEY_HASH_SHA512] = "sha512",
> > > > [PKEY_HASH_SHA224] = "sha224",
> > > > - [PKEY_HASH_STREEBOG_256] = "md_gost12_256,streebog256",
> > > > - [PKEY_HASH_STREEBOG_512] = "md_gost12_512,streebog512",
> > > > + [PKEY_HASH_STREEBOG_256] = "md_gost12_256",
> > > > + [PKEY_HASH_STREEBOG_512] = "md_gost12_512",
> > > > +};
> > > > +
> > > > +const char *pkey_hash_algo_alias[PKEY_HASH__LAST] = {
> > > > + [PKEY_HASH_STREEBOG_256] = "streebog256",
> > > > + [PKEY_HASH_STREEBOG_512] = "streebog512",
> > > > };
> > > >
> > >
> > > If the upstream kernel name is defined as "streebog", the alias would
> > > be "gost".
> >
> > To me it seems that meaning of what here is alias is really not
> > important. There is two names in OpenSSL md_gost12_X and streebogX,
> > where streebogX is newer and less accessible, and md_gost12_X is
> > more accessible in older version. While in the kernel name is streebogX.
> >
> > Probably it's better to distinguish as openssl and kernel names, and not
> > the name and alias.
>
> The original design matched the kernel haah_info.h.
But for libimaevm we have openssl as crypto library not the kernel. That
pkey_hash_algo/hash_algo_name lists names does not need to reflect kernel
names at all for things to work, since only openssl is interfaced. It is
question of user interface and compatibility with older libs.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-02-12 19:31 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-12 0:14 [PATCH] ima-evm-utils: simplify digest alias handling Vitaly Chikunov
2019-02-12 16:23 ` Mimi Zohar
2019-02-12 17:23 ` Vitaly Chikunov
2019-02-12 19:17 ` Mimi Zohar
2019-02-12 19:31 ` Vitaly Chikunov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).