Closed tdaitx closed 5 years ago
Looks good to me, but I was there when you typed it. :)
I don't have any problem with any of these, though it for the third one, it might also be nice to be able to export:
@vathpela thanks for the review!
Are these additional exports a requirement to get this PR through or can they be dealt with later on? There is nothing else in the code dealing with these other default keys so a new PR might be cleaner and could be done on top of this one.
Also, what do you mean about exporting the Modes (eg. Audit, Setup, etc)? They are not x509 keys/certs, so I'm not entirely sure what one would expect to be there.
Thanks!
Sorry for the late reply. Just back from vacation. Instead of adding a new argument for each variable, I'm thinking about adding a new "--filepath" argument for "--export" so that it can be more flexible. Does it sound good to you?
How would --filepath work? You'd have to pass it the path to the sysfs variable?
My idea was to reuse --mok, --pk, etc. except it requires some small reworking of the way arguments are parsed. Those seem to be at least useful shorthand versions to pointing mokutil to an arbitrary path (plus, doesn't require the kind of validation you'd need if passing a path to a sysfs).
The only issue is how to export lists of hashes from db, dbx, mok, mokx.
I see your point. The arguments could reduce the typing errors from the users. On the other hand, I sometimes have to check db/mok files from someone else, and that is why I feel "filepath" could be useful for me. We can do that parallelly anyway since there is no conflict.
As for exporting hashes, I don't have a good idea now. What would be your use case for the exported hashes?
This PR adds the option to export the other keys beyond MOK.
I reused the existing arguments for the other dbs, so for example exporting pks only requires. --export --pk, and I added a --mok argument for symmetry.
I tried to preserve the existing behavior, such as setting only the --mokx/--pk/--kek/--db/--dbx will end up calling list-enrolled, but I had to move that logic to after the getopt_long block so I can be sure no other command was specified.