Stefan,
On Thu, May 06, 2021 at 03:29:32AM +0300, Vitaly Chikunov wrote:
> On Wed, May 05, 2021 at 07:29:17PM -0400, Stefan Berger wrote:
> >
> > On 5/5/21 2:48 AM, Vitaly Chikunov wrote:
> > > Allow to have certificate appended to the private key of `--key'
> > > specified (PEM) file (for v2 signing) to facilitate reading of keyid
> > > from the associated cert. This will allow users to have private and
> > > public key as a single file. There is no check that public key form the
> > > cert matches associated private key.
> > >
> > > Signed-off-by: Vitaly Chikunov <vt@xxxxxxxxxxxx>
> > > ---
> > > README | 3 +++
> > > src/libimaevm.c | 8 +++++---
> > > 2 files changed, 8 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/README b/README
> > > index 0e1f6ba..ea11bde 100644
> > > --- a/README
> > > +++ b/README
> > > @@ -127,6 +127,9 @@ for signing and importing the key.
> > > Second key format uses X509 DER encoded public key certificates and uses asymmetric key support
> > > in the kernel (since kernel 3.9). CONFIG_INTEGRITY_ASYMMETRIC_KEYS must be enabled (default).
> > > +For v2 signatures x509 certificate with the public key could be appended to the private
> > > +key (both are in PEM format) to properly determine its Subject Key Identifier (SKID).
> > > +
> > > Integrity keyrings
> > > ----------------
> > > diff --git a/src/libimaevm.c b/src/libimaevm.c
> > > index a22d9bb..ac4bb46 100644
> > > --- a/src/libimaevm.c
> > > +++ b/src/libimaevm.c
> > > @@ -1017,10 +1017,12 @@ static int sign_hash_v2(const char *algo, const unsigned char *hash,
> > > return -1;
> > > }
> > > - if (imaevm_params.keyid)
> > > + if (imaevm_params.keyid) {
> > > hdr->keyid = htonl(imaevm_params.keyid);
> > > - else
> > > - calc_keyid_v2(&hdr->keyid, name, pkey);
> > > + } else {
> > > + if (_ima_read_keyid(keyfile, &hdr->keyid, KEYID_FILE_PEM_KEY) == ULONG_MAX)
> > > + calc_keyid_v2(&hdr->keyid, name, pkey);
> > > + }
> >
> > It might be convenient here to just write the result in network byte order
> > into the header but for a library API I find it not so nice, but then
> > there's calc_keyid_v2 that does that already... I just wouldn't expect that
> > these parameter are in big endian order already, I would expect them in
> > native order.
>
> I expect them in network order, similar to how calc_keyid_v2() already
> writes it one line below. So, why should we mix orders? Both functions
> write keyids, so it's not like completely different parts of API. Also,
> it's documented that ima_read_keyid() writes to the pointer in network
> order (and returns integer in host order), so I don't see the
> problem. Thus, I would prefer not to follow this suggestion.
>
> Thanks,
>
> > So maybe ima_read_keyid should just return ULONG_MAX or the
> > keyid in host order and it call _ima_read_keyid() with a NULL pointer or a
> > dummy variable as keyid that the library API caller doesn't see.
So there will be exported
uint32_t ima_read_keyid(char *certfile);
and internal
static bool _read_keyid(char *certfile, uint32_t *keyid)
ima_read_keyid will wrap read_keyid and return keyid via intermediate
variable. Right?
> >
> > Stefan
> >
> >
> > > st = "EVP_PKEY_CTX_new";
> > > if (!(ctx = EVP_PKEY_CTX_new(pkey, NULL)))
