| Message ID | 20190618135623.6861-7-vt@altlinux.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ima-evm-utils: Convert sign v2 from RSA to EVP_PKEY API | expand |
On Tue, 2019-06-18 at 16:56 +0300, Vitaly Chikunov wrote: > New find_keyid_pkey() accepts EVP_PKEY. Old find_keyid() calls > find_keyid_pkey(), but still return RSA key. > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org> With titles starting with "Start converting", it leaves me wondering whether these patches are bisect safe. Does this patch make find_keyid() a wrapper for find_keyid_pkey()? Do all callers of find_keyid() continue to work properly? If so, why are there other changes in this patch? If you haven't already, please make sure that after each patch is applied, the code not only compiles cleanly, but works properly. Mimi > --- > src/libimaevm.c | 24 ++++++++++++++++++++---- > 1 file changed, 20 insertions(+), 4 deletions(-) > > diff --git a/src/libimaevm.c b/src/libimaevm.c > index 707b2e9..ae18005 100644 > --- a/src/libimaevm.c > +++ b/src/libimaevm.c > @@ -452,11 +452,11 @@ struct public_key_entry { > struct public_key_entry *next; > uint32_t keyid; > char name[9]; > - RSA *key; > + EVP_PKEY *key; > }; > static struct public_key_entry *public_keys = NULL; > > -static RSA *find_keyid(uint32_t keyid) > +static EVP_PKEY *find_keyid_pkey(uint32_t keyid) > { > struct public_key_entry *entry; > > @@ -467,6 +467,22 @@ static RSA *find_keyid(uint32_t keyid) > return NULL; > } > > +static RSA *find_keyid(uint32_t keyid) > +{ > + EVP_PKEY *pkey; > + RSA *key; > + > + pkey = find_keyid_pkey(keyid); > + if (!pkey) > + return NULL; > + key = EVP_PKEY_get0_RSA(pkey); > + if (!key) { > + log_err("find_keyid: unsupported key type\n"); > + return NULL; > + } > + return key; > +} > + > void init_public_keys(const char *keyfiles) > { > struct public_key_entry *entry; > @@ -489,13 +505,13 @@ void init_public_keys(const char *keyfiles) > break; > } > > - entry->key = read_pub_key(keyfile, 1); > + entry->key = read_pub_pkey(keyfile, 1); > if (!entry->key) { > free(entry); > continue; > } > > - calc_keyid_v2(&entry->keyid, entry->name, entry->key); > + calc_pkeyid_v2(&entry->keyid, entry->name, entry->key); > sprintf(entry->name, "%x", __be32_to_cpup(&entry->keyid)); > log_info("key %d: %s %s\n", i++, entry->name, keyfile); > entry->next = public_keys;
Mimi, On Wed, Jun 19, 2019 at 08:26:30AM -0400, Mimi Zohar wrote: > On Tue, 2019-06-18 at 16:56 +0300, Vitaly Chikunov wrote: > > New find_keyid_pkey() accepts EVP_PKEY. Old find_keyid() calls > > find_keyid_pkey(), but still return RSA key. > > > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org> > > With titles starting with "Start converting", it leaves me wondering > whether these patches are bisect safe. Does this patch make > find_keyid() a wrapper for find_keyid_pkey()? Yes. > Do all callers of find_keyid() continue to work properly? Yes. > If so, why are there other changes in this patch? There is no other changes beside stated in description. > If you haven't already, please make sure that after each patch is > applied, the code not only compiles cleanly, but works properly. Yes, they compile and work properly after each patch. Thanks,
On Wed, 2019-06-19 at 18:43 +0300, Vitaly Chikunov wrote: > Mimi, > > On Wed, Jun 19, 2019 at 08:26:30AM -0400, Mimi Zohar wrote: > > On Tue, 2019-06-18 at 16:56 +0300, Vitaly Chikunov wrote: > > > New find_keyid_pkey() accepts EVP_PKEY. Old find_keyid() calls > > > find_keyid_pkey(), but still return RSA key. > > > > > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org> > > > > With titles starting with "Start converting", it leaves me wondering > > whether these patches are bisect safe. Does this patch make > > find_keyid() a wrapper for find_keyid_pkey()? > > Yes. > > > Do all callers of find_keyid() continue to work properly? > > Yes. > > > If so, why are there other changes in this patch? > > There is no other changes beside stated in description. Are the changes from read_pub_key() to read_pub_pkey() and calc_keyid_v2() to calc_pkeyid_v2() needed for making find_keyid() a wrapper for find_keyid_pkey()? > > > If you haven't already, please make sure that after each patch is > > applied, the code not only compiles cleanly, but works properly. > > Yes, they compile and work properly after each patch. thanks! Mimi
Mimi, On Wed, Jun 19, 2019 at 12:46:50PM -0400, Mimi Zohar wrote: > On Wed, 2019-06-19 at 18:43 +0300, Vitaly Chikunov wrote: > > On Wed, Jun 19, 2019 at 08:26:30AM -0400, Mimi Zohar wrote: > > > On Tue, 2019-06-18 at 16:56 +0300, Vitaly Chikunov wrote: > > > > New find_keyid_pkey() accepts EVP_PKEY. Old find_keyid() calls > > > > find_keyid_pkey(), but still return RSA key. > > > > > > > > Signed-off-by: Vitaly Chikunov <vt@altlinux.org> > > > > > > With titles starting with "Start converting", it leaves me wondering > > > whether these patches are bisect safe. Does this patch make > > > find_keyid() a wrapper for find_keyid_pkey()? > > > > Yes. > > > > > Do all callers of find_keyid() continue to work properly? > > > > Yes. > > > > > If so, why are there other changes in this patch? > > > > There is no other changes beside stated in description. > > Are the changes from read_pub_key() to read_pub_pkey() and > calc_keyid_v2() to calc_pkeyid_v2() needed for making find_keyid() a > wrapper for find_keyid_pkey()? Of course. `entry->key' now have different type. If we keep old type (RSA) where will be nothing to wrap. Thanks,
Hi Vitaly, > > > > If so, why are there other changes in this patch? > > > > > > There is no other changes beside stated in description. > > > > Are the changes from read_pub_key() to read_pub_pkey() and > > calc_keyid_v2() to calc_pkeyid_v2() needed for making find_keyid() a > > wrapper for find_keyid_pkey()? > > Of course. `entry->key' now have different type. If we keep old type > (RSA) where will be nothing to wrap. The question wasn't if the changes in init_public_keys() need to be made, the question is its correlation to find_keyid(). Unlesss I'm missing something, find_keyid() is only called by verify_hash_v2(), not calc_keyid_v2(). Mimi
On Thu, 2019-06-20 at 09:21 -0400, Mimi Zohar wrote: > Hi Vitaly, > > > > > > If so, why are there other changes in this patch? > > > > > > > > There is no other changes beside stated in description. > > > > > > Are the changes from read_pub_key() to read_pub_pkey() and > > > calc_keyid_v2() to calc_pkeyid_v2() needed for making find_keyid() a > > > wrapper for find_keyid_pkey()? > > > > Of course. `entry->key' now have different type. If we keep old type > > (RSA) where will be nothing to wrap. > > The question wasn't if the changes in init_public_keys() need to be > made, the question is its correlation to find_keyid(). Unlesss I'm > missing something, find_keyid() is only called by verify_hash_v2(), > not calc_keyid_v2(). Ah, the list of keys needs to be in the appropriate format. Perhaps add that info in the patch description. Mimi
Mimi, On Thu, Jun 20, 2019 at 09:40:49AM -0400, Mimi Zohar wrote: > On Thu, 2019-06-20 at 09:21 -0400, Mimi Zohar wrote: > > > > > > If so, why are there other changes in this patch? > > > > > > > > > > There is no other changes beside stated in description. > > > > > > > > Are the changes from read_pub_key() to read_pub_pkey() and > > > > calc_keyid_v2() to calc_pkeyid_v2() needed for making find_keyid() a > > > > wrapper for find_keyid_pkey()? > > > > > > Of course. `entry->key' now have different type. If we keep old type > > > (RSA) where will be nothing to wrap. > > > > The question wasn't if the changes in init_public_keys() need to be > > made, the question is its correlation to find_keyid(). Unlesss I'm > > missing something, find_keyid() is only called by verify_hash_v2(), > > not calc_keyid_v2(). > > Ah, the list of keys needs to be in the appropriate format. Perhaps > add that info in the patch description. Isn't this is implied by find_keyid()? It finds appropriate `entry->key', and it's type is changed to EVP_PKEY, so both init_public_keys() (where it fills `entry->key`) and find_keyid() (where it returns `entry->key`) need to be changed. Vitaly.
diff --git a/src/libimaevm.c b/src/libimaevm.c index 707b2e9..ae18005 100644 --- a/src/libimaevm.c +++ b/src/libimaevm.c @@ -452,11 +452,11 @@ struct public_key_entry { struct public_key_entry *next; uint32_t keyid; char name[9]; - RSA *key; + EVP_PKEY *key; }; static struct public_key_entry *public_keys = NULL; -static RSA *find_keyid(uint32_t keyid) +static EVP_PKEY *find_keyid_pkey(uint32_t keyid) { struct public_key_entry *entry; @@ -467,6 +467,22 @@ static RSA *find_keyid(uint32_t keyid) return NULL; } +static RSA *find_keyid(uint32_t keyid) +{ + EVP_PKEY *pkey; + RSA *key; + + pkey = find_keyid_pkey(keyid); + if (!pkey) + return NULL; + key = EVP_PKEY_get0_RSA(pkey); + if (!key) { + log_err("find_keyid: unsupported key type\n"); + return NULL; + } + return key; +} + void init_public_keys(const char *keyfiles) { struct public_key_entry *entry; @@ -489,13 +505,13 @@ void init_public_keys(const char *keyfiles) break; } - entry->key = read_pub_key(keyfile, 1); + entry->key = read_pub_pkey(keyfile, 1); if (!entry->key) { free(entry); continue; } - calc_keyid_v2(&entry->keyid, entry->name, entry->key); + calc_pkeyid_v2(&entry->keyid, entry->name, entry->key); sprintf(entry->name, "%x", __be32_to_cpup(&entry->keyid)); log_info("key %d: %s %s\n", i++, entry->name, keyfile); entry->next = public_keys;
New find_keyid_pkey() accepts EVP_PKEY. Old find_keyid() calls find_keyid_pkey(), but still return RSA key. Signed-off-by: Vitaly Chikunov <vt@altlinux.org> --- src/libimaevm.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-)