Re: [fsverity-utils PATCH v4] Implement PKCS#11 opaque keys support through OpenSSL pkcs11 engine

Aleksander Adamowski <olo@xxxxxx> · Thu, 9 Sep 2021 21:21:52 +0000

On Wed, Sep 08, 2021 at 12:24AM, Eric Biggers wrote:
> Taking a closer look at this patch, I don't think we should be overloading the
> '--key' option and 'keyfile' field like this, as it's confusing.  It's also not
> really necessary for 'fsverity sign' to do all this option validation itself; I
> think we should keep it simple and just rely on libfsverity.
> 
> Also, I think this feature could use clearer documentation that clearly explains
> that there are now two ways to specify a private key.
> 
> I ended up making the above changes and cleaning up a bunch of other things; can
> you consider the following patch instead?  Thanks!

Your patch looks good to me! I particularly like getting rid of all the
OPENSSL_IS_BORINGSSL ifdefs and instead returning an error from its
implementation of load_pkcs11_private_key().

I ran my tests with PKCS#11 token and regular file-based keys, and everything
including error handling seems to be working as expected.

I'll submit this version as V5.