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
>
