EXTERNAL - [PATCH v5 1/6] tools: mkeficapsule: Add support for pkcs11

[Ilias Apalodimas]

Hi Wojciech

[...]

> >
> > Can we do strlen() instead of 7 ?
>
> Will do in the next iteration.

Thanks

> >
> > > +           pkcs11_cert = true;
> > >
> > > -   ret = read_bin_file(ctx->key_file, &key.data, &file_size);
> > > -   if (ret < 0)
> > > -           return -1;
> > > -   if (file_size > UINT_MAX)
> > > -           return -1;
> > > -   key.size = file_size;
> > > +   if (!strncmp(ctx->key_file, "pkcs11:", 7))
> >
> > Same
> >
> > > +           pkcs11_key = true;
> > > +
> > > +   if (pkcs11_cert || pkcs11_key) {
> >
> > Don't you need both the cert & key to sign the capsule?
> > I'd simplify the logic here. Instead of having both a pkcs_key and a pkcs_cert,
> > replace the variables with is_pcks and have that set to true if both the key
> > and cert have been found.
> This is what I have done in the first iteration. Later I have learned that there
> is a need for mixed pkcs11/local file usage. The HSM devices are very expensive
> (at least some of them) and have limited memory. It's quite common to use private
> key from HSM over pkcs11 protocol and all the public stuff locally.
>
> The test is implemented so at the moment.

Yea that was one of the things I was thinking to justify why it's
implemented like that.
Since that's a requirement the code is fine as is.

Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>

>
> Regards,
> Wojtek
> >
> > Then the if/else cases later will become a bit easier to read since you'll have
> > to load the private key & crt on a single if/else cases depending on is_pkcs.
> >
> > > +           lib = getenv("PKCS11_MODULE_PATH");
> > > +           if (!lib) {
> >
> > [...]
> >
> > Thanks
> > /Ilias