Hi Eric! Thanks for the quick response to the patch! My replies inline below: On Thursday, August 26, 2021 12:11 PM, Eric Biggers wrote: > First, can you make sure to include "[fsverity-utils PATCH]" in the subject like > the fsverity-utils README file suggests? I almost missed this patch as it > initially didn't look relevant to me. Sure! Apologies for missing that part. The V2 of the patch will be appropriately tagged. > I'm not particularly familiar with the OpenSSL PKCS#11 engine, but this patch > looks reasonable at a high level (assuming that you really want to use the > kernel's built-in fs-verity signature verification support -- I've been trying > to encourage people to do userspace signature verification instead). We are currently going forward with in-kernel sig verification (and btrfs), but I'd love to hear more about the userspace support you mention. > Some > comments on the implementation below. > > This comment is incorrect, as your code uses keyfile even in the pkcs11 case. > > Also, keyfile is only optional in the pkcs11 case. Please write a comment that > clearly explains which parameters must be specified and when. Yes, you are entirely right about this. The decision to use the --key argument as an optional PKCS#11 key identified was a later afterthought and I forgot to update some related pieces of code. Will fix in the next patch, as well as address your other comments.
|