Mimi, On Mon, Jul 05, 2021 at 04:04:52PM -0400, Mimi Zohar wrote: > On Thu, 2021-07-01 at 04:13 +0300, Vitaly Chikunov wrote: > > > > +/** > > + * read_keyid_from_key() - Read 32-bit keyid from the key file > > + * @keyid_be: Pointer to 32-bit value in network order (BE, unaligned). > > + * @keyfile: PEM file with private key with optionally appended x509 cert. > > + * Return: 0 on success and keyid_be is written; > > + * -1 on error, logged error message, and keyid_be isn't written. > > + */ > > +static int read_keyid_from_key(uint32_t *keyid_be, const char *keyfile) > > (With the new option "--keyid-from-cert" is this patch really still > needed?) Yes. Key+cert is a nice option and should be handy for users. Key is stored together with the cert which will verify them. Otherwise key format doesn't store keyid which is error prone. > The function name is a bit off. Both imaevm_read_keyid() and this > function are getting the keyid from a cert. There's also quite a bit > of code duplication between them. Refactoring the code might help. > For example, perhaps imaevm_read_keyid() could be a wrapper for > read_keyid_from_cert(). They have important difference too. Thus, its hard to refactor them into nested function that looked good and simple. This is third attempt. And it's like solving a unsolvable puzzle. imaevm_read_keyid() reads from cert-only file where cert could be PEM or DER encoded. It's failure if there is no cert, because user intended to read a cert. imaevm_read_keyid() reads from private key optionally combined with a cert (both are PEM-only). It's not failure if there is no cert. We want to save "duplicated" call to PEM_read_X509() but really it causes more convoluted internal logic. I tried to de-duplicate them as much as possible while remaining understandability. > Otherwise renaming this function to read_keyid_from_keyfile(), > read_appended_keyid() or read_appended_keyid_from_cert(), which is > really wordy, would be better. Ok. read_keyid_from_keyfile looks good. > > thanks, > > Mimi
