Re: [PATCH v4][ 5/6] cryptodisk: enable the backends to implement key files

[Patrick Steinhardt]

On Thu, Jun 04, 2020 at 05:43:23AM +0200, Denis 'GNUtoo' Carikli wrote:
> @@ -1112,6 +1117,88 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int 
> argc, char **args)
>      hdr = NULL;
>  
>    have_it = 0;
> +  key = NULL;
> +
> +  if (state[4].set) /* keyfile */
> +    {
> +      const char *p = NULL;
> +      grub_file_t keyfile;
> +      int keyfile_offset;
> +      grub_size_t requested_keyfile_size = 0;
> +
> +
> +      if (state[5].set) /* keyfile-offset */
> +     {
> +       keyfile_offset = grub_strtoul (state[5].arg, &p, 0);
> +
> +       if (grub_errno != GRUB_ERR_NONE)
> +         return grub_errno;
> +
> +       if (*p != '\0')
> +         return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +                            N_("unrecognized number"));
> +     }
> +      else
> +     {
> +       keyfile_offset = 0;
> +     }
> +
> +      if (state[6].set) /* keyfile-size */
> +     {
> +       requested_keyfile_size = grub_strtoul(state[6].arg, &p, 0);

Nit: missing space after `grub_strtoul`.

> +       if (*p != '\0')
> +         return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +                            N_("unrecognized number"));
> +
> +       if (grub_errno != GRUB_ERR_NONE)
> +         return grub_errno;
> +
> +       if (requested_keyfile_size > GRUB_CRYPTODISK_MAX_KEYFILE_SIZE)
> +         return grub_error(GRUB_ERR_OUT_OF_RANGE,
> +                           N_("Key file size exceeds maximum (%"     \
> +                              PRIuGRUB_SIZE ")\n"),
> +                           GRUB_CRYPTODISK_MAX_KEYFILE_SIZE);

Here too, ...

> +       if (requested_keyfile_size == 0)
> +         return grub_error(GRUB_ERR_OUT_OF_RANGE,
> +                           N_("Key file size is 0\n"));
> +     }

and here. There's also two newlines instead of one, which also
applies...

> +      keyfile = grub_file_open (state[4].arg,
> +                             GRUB_FILE_TYPE_CRYPTODISK_ENCRYPTION_KEY);
> +      if (!keyfile)
> +     return grub_errno;
> +
> +      if (grub_file_seek (keyfile, keyfile_offset) == (grub_off_t)-1)
> +     return grub_errno;
> +
> +

here.

> +      if (requested_keyfile_size)
> +     {
> +       if (requested_keyfile_size > (keyfile->size - keyfile_offset))
> +         return grub_error (GRUB_ERR_FILE_READ_ERROR,
> +                            N_("Keyfile is too small: "                \
> +                               "requested %" PRIuGRUB_SIZE " bytes, "  \
> +                               "but the file only has %" PRIuGRUB_SIZE \
> +                               " bytes.\n"),

These trailing backslashes aren't required. C handles strings split over
multiple lines without any need for escaping.

Patrick

signature.asc
Description: PGP signature