On Fri, 2020-10-23 at 21:23 -0700, Eric Biggers wrote:
> On Thu, Oct 22, 2020 at 06:21:55PM +0100, luca.boccassi@xxxxxxxxx wrote:
> > +/* Compute a file's fs-verity measurement, then print it in hex format. */
> > +int fsverity_cmd_digest(const struct fsverity_command *cmd,
> > + int argc, char *argv[])
> > +{
> > + struct filedes file = { .fd = -1 };
> > + u8 *salt = NULL;
> > + struct libfsverity_merkle_tree_params tree_params = { .version = 1 };
> > + struct libfsverity_digest *digest = NULL;
> > + struct fsverity_signed_digest *d = NULL;
> > + char digest_hex[FS_VERITY_MAX_DIGEST_SIZE * 2 + sizeof(struct fsverity_signed_digest) * 2 + 1];
> > + bool compact = false;
> > + int status;
> > + int c;
> > +
> > + while ((c = getopt_long(argc, argv, "", longopts, NULL)) != -1) {
> > + switch (c) {
> > + case OPT_HASH_ALG:
> > + if (!parse_hash_alg_option(optarg,
> > + &tree_params.hash_algorithm))
> > + goto out_usage;
> > + break;
> > + case OPT_BLOCK_SIZE:
> > + if (!parse_block_size_option(optarg,
> > + &tree_params.block_size))
> > + goto out_usage;
> > + break;
> > + case OPT_SALT:
> > + if (!parse_salt_option(optarg, &salt,
> > + &tree_params.salt_size))
> > + goto out_usage;
> > + tree_params.salt = salt;
> > + break;
> > + case OPT_COMPACT:
> > + compact = true;
> > + break;
> > + default:
> > + goto out_usage;
> > + }
> > + }
> > +
> > + argv += optind;
> > + argc -= optind;
> > +
> > + if (argc != 1)
> > + goto out_usage;
> > +
> > + if (tree_params.hash_algorithm == 0)
> > + tree_params.hash_algorithm = FS_VERITY_HASH_ALG_DEFAULT;
> > +
> > + if (tree_params.block_size == 0)
> > + tree_params.block_size = get_default_block_size();
> > +
> > + if (!open_file(&file, argv[0], O_RDONLY, 0))
> > + goto out_err;
> > +
> > + if (!get_file_size(&file, &tree_params.file_size))
> > + goto out_err;
> > +
> > + if (libfsverity_compute_digest(&file, read_callback,
> > + &tree_params, &digest) != 0) {
> > + error_msg("failed to compute digest");
> > + goto out_err;
> > + }
> > +
> > + ASSERT(digest->digest_size <= FS_VERITY_MAX_DIGEST_SIZE);
> > +
> > + d = xzalloc(sizeof(*d) + digest->digest_size);
> > + if (!d)
> > + goto out_err;
> > + memcpy(d->magic, "FSVerity", 8);
> > + d->digest_algorithm = cpu_to_le16(digest->digest_algorithm);
> > + d->digest_size = cpu_to_le16(digest->digest_size);
> > + memcpy(d->digest, digest->digest, digest->digest_size);
> > +
> > + bin2hex((const u8 *)d, sizeof(*d) + digest->digest_size, digest_hex);
> > +
> > + if (compact)
> > + printf("%s", digest_hex);
> > + else
> > + printf("File '%s' (%s:%s)\n", argv[0],
> > + libfsverity_get_hash_name(tree_params.hash_algorithm),
> > + digest_hex);
>
> Can you make this command format its output in the same way as
> 'fsverity measure' by default, and put the 'struct fsverity_signed_digest'
> formatted output behind an option, like --for-builtin-sig?
Sure, done in v2.
> The 'struct fsverity_signed_digest' is specific to the builtin (in-kernel)
> signature support, which isn't the only way to use fs-verity. The signature
> verification can also be done in userspace, which is more flexible. (And you
> should consider doing it that way, if you haven't already. I'm not sure exactly
> what your use case is.)
Our use case is to ultimately add kernel-side policy enforcement of
integrity via the IPE LSM: https://microsoft.github.io/ipe/ so we can't
do without kernel verification. I need the new command as I've got some
teams building and signing on Windows, using whatever native API is
available there, so can't use the 'fsverity sign' command.
> So when possible, I'd like to have the default be the basic fs-verity feature.
> If someone then specifically wants to use the builtin signature support on top
> of that, as opposed to using fs-verity in another way, then they can provide the
> option they need to do that.
>
> Separately, it would also be nice to share more code with cmd_sign.c, as they
> both have to parse a lot of the same options. Maybe it doesn't work out,
> though.
I did have a look at this at the beginning, but I was not happy with
how it looked like - both are using APIs from the public library and
are compact enough, so it felt a bit awkward to add yet another shim
layer, and opted to avoid it. If you feel strongly about it let me know
and I can revise.
--
Kind regards,
Luca Boccassi
|