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
