grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v5 1/1] plainmount: Support plain encryption mode


From: Glenn Washburn
Subject: Re: [PATCH v5 1/1] plainmount: Support plain encryption mode
Date: Thu, 21 Jul 2022 22:51:21 -0500

On Sat, 16 Jul 2022 18:58:39 +0000
Maxim Fomin <maxim@fomin.one> wrote:

> From 06219fde309f1f87b9d52a70a7e992eaeb168449 Mon Sep 17 00:00:00 2001
> From: Maxim Fomin <maxim@fomin.one>
> Date: Sat, 16 Jul 2022 19:52:13 +0100
> Subject: [PATCH v5 1/1] plainmount: Support plain encryption mode
> 
> This patch adds support for plain encryption mode (plain dm-crypt) via
> new module/command named 'plainmount'.

There's no SOB here.

> ---
>  docs/grub.texi              |  82 +++++++
>  grub-core/Makefile.core.def |   5 +
>  grub-core/disk/plainmount.c | 429 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 516 insertions(+)
>  create mode 100644 grub-core/disk/plainmount.c
> 
> diff --git a/docs/grub.texi b/docs/grub.texi
> index af119dea3..b5ca222ac 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -4265,6 +4265,7 @@ you forget a command, you can run the command 
> @command{help}
>  * parttool::                    Modify partition table entries
>  * password::                    Set a clear-text password
>  * password_pbkdf2::             Set a hashed password
> +* plainmount::                  Open device encrypted in plain mode.

Remove period to be consistent with other entries.

>  * play::                        Play a tune
>  * probe::                       Retrieve device info
>  * rdmsr::                       Read values from model-specific registers
> @@ -4552,6 +4553,14 @@ function is supported, as Argon2 is not yet supported.
> 
>  Also, note that, unlike filesystem UUIDs, UUIDs for encrypted devices must be
>  specified without dash separators.
> +
> +Successfully decrypted disks are named as (cryptoX) and have increasing 
> numeration
> +suffix for each new decrypted disk. If the encrypted disk hosts some higher 
> level
> +of abstraction (like LVM2 or MDRAID) it will be created under a separate 
> device
> +namespace in addition to the cryptodisk namespace.
> +
> +Support for plain encryption mode (plain dm-crypt) is provided via separate
> +@command{plainmount} command.

Thinking about it more, I think it would be better for this to be
@command{@pxref{plainmount}}, so that it will be a link that can be
easily clicked to go to plainmount section.

>  @end deffn
> 
>  @node cutmem
> @@ -5060,6 +5069,79 @@ to generate password hashes.  @xref{Security}.
>  @end deffn
> 
> 
> +@node plainmount
> +@subsection plainmount
> +
> +@deffn Command plainmount device @option{-c} cipher @option{-s} key size 
> [@option{-h} hash]
> +[@option{-S} sector size] [@option{-p} password] [@option{-u} uuid]
> +[[@option{-d} keyfile] [@option{-O} keyfile offset]]
> +
> +
> +Setup access to the encrypted device in plain mode. The device argument can 
> point
> +to a disk, partition or to a loopback file. Offset of the encrypted data at 
> the
> +device is specified in terms of 512 byte sectors with the blocklist syntax 
> and
> +loopback file.

I'd say remove the sentence "The device argument can point to a disk,
partition or to a loopback file." A partition is a device. And a what is
a "loopback file"? Isn't that the feature that was removed? I'm not
sure the second sentence is needed as it should be described on the
section on blocklist syntax. I'm okay with the sentence being included
if you really want it though.

> +Example:

Instead, have "The following example specifies encrypted data at offset
1MiB:"

> +
> +@example
> +loopback node (hd0,gpt1)2048+
> +plainmount node
> +@end example
> +
> +specifies encrypted data at offset 1MiB. The @command{plainmount} plainmount 
> command

Remove this incomplete sentence. Also "@command{plainmount} plainmount"
will be rendered as "plainmount plainmount", so remove the bare
"plainmount".

> +can be used to open LUKS encrypted volume if its master key and parameters 
> (key size,
> +cipher, offset, etc) are known.
> +
> +Password can be specified in two ways: as a password data from a keyfile or 
> as a secret

s/a password/password/

> +passphrase. The keyfile parameter @option{-d} specifies path to the keyfile 
> containing

s/path/a path/

> +password data and has higher priority than the other password method. 
> Specified password

s/the other password method/the @option{-p} option/

> +data in this mode is not hashed and is used directly as a cipher key. The 
> option
> +@option{-O} specifies offset in terms of bytes of the password data from the 
> start of the

s/offset in terms of bytes of the password data/offset of the password
data in bytes/

> +keyfile. This option has no effect without the option @option{-d}. Offset of 
> password data
> +in the keyfile can be specified directly with option @option{-d} and using 
> GRUB blocklist

s/and/or/

> +syntax. For example,

I don't like having an example split a sentence. I'd do "In the
following example, " and then move "both command ..." below to up here.

> +
> +@example
> +plainmount -s 512 -d (hd0,gpt1)2048+
> +plainmount -s 512 -d (hd0,gpt1) -O 1048576
> +@end example
> +
> +both commands specify password data from the keyfile at the offset at 1MiB 
> and 64 byte
> +(512 bits) long.
> +
> +If no keyfile is specified then the password supplied with the option 
> @option{-p} is used
> +or the password is requested interactively from the console. In both cases 
> provided password

s/console/console, if no option @option{-p} is used/

s/cases provided password/cases the provided password/

> +is hashed with the algorithm specified by the option @option{-h}. This 
> option is mandatory
> +if no keyfile is specified, but it can be set to 'plain' which means that no 
> hashing is done

s/'plain'/@code{plain}/

> +and such password is used directly as a key.
> +
> +Cipher @option{-c} and keysize @option{-s} options specify the cipher 
> algorithm and the
> +key size respectively and are mandatory options (apply to all methods to 
> specify the

s/apply to all methods to specify/apply regardless of method specifying/

> +password). Cipher must be specified with the mode (for example, 
> 'aes-xts-plain64'). Key size

s/mode/mode separated by a dash/

> +must not exceed 128 bytes and must be specified in bits.

Instead of the above sentence have this:

Key size is the key size of the cipher, not to be confused with the
size of the key data in a keyfile specified with the @option{-O} option.
It must not exceed 128 bytes and must be specified in bits (so a 256 bit
key would be specified as 32).

> +
> +The optional parameter @option{-S} specifies encrypted device sector size. 
> It must be
> +at least 512 bytes long (default value) and a power of 2. @footnote{Current 
> implementation
> +of cryptsetup supports only 512/1024/2048/4096 byte sectors}. Disk sector 
> size is
> +configured when creating the encrypted volume. Attempting to decrypt such 
> volume with

s/such volume with/volumes with a/

> +different sector size than it was created with will not result in an error, 
> but will
> +decrypt to random bytes and thus prevent accessing the volume (in some cases 
> filesystem
> +driver can detect the filesystem presense, but nevertheless will refuse to 
> mount it).
> +
> +By default new cryptodisk node will have uuid 
> '109fea84-a6b7-34a8-4bd1-1c506305a401'
> +where last digits are incremented for each subsequently created node up to 
> 2^10 nodes.

By default new plainmount devices will be given a UUID starting with
'109fea84-a6b7-34a8-4bd1-1c506305a401' where the last digits are
incremented by one for each plainmounted device beyond the first up to
2^10 devices.

> +Custom uuid can be specified with the option @option{-u}. Decrypted disks 
> have the
> +same naming rules as those decrypted by @command{cryptomount}.
> +
> +All encryption arguments (cipher, hash, key size, disk offset and disk sector
> +size) must match the parameters used to create the volume. If any of them 
> does not
> +match the actual arguments used during the initial encryption, plainmount 
> will create
> +virtual device with the garbage data and GRUB will report unknown filesystem 
> for such
> +device. Writing data to such virtual device will result in the data loss if 
> the
> +underlying partition contained desired data.
> +@end deffn
> +
> +
>  @node play
>  @subsection play
> 
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 715994872..3910b7670 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1174,6 +1174,11 @@ module = {
>    common = disk/cryptodisk.c;
>  };
> 
> +module = {
> +  name = plainmount;
> +  common = disk/plainmount.c;
> +};
> +
>  module = {
>    name = json;
>    common = lib/json/json.c;
> diff --git a/grub-core/disk/plainmount.c b/grub-core/disk/plainmount.c
> new file mode 100644
> index 000000000..154f79a4a
> --- /dev/null
> +++ b/grub-core/disk/plainmount.c
> @@ -0,0 +1,429 @@
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2003,2007,2010,2011,2019  Free Software Foundation, Inc.

s/2003,2007,2010,2011,2019/2022/

> + *
> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.

Daniel will probably ask for a line here with a one sentence
description of the module as done in grub-core/kern/efi/sb.c.
I think I like how grub-core/commands/efi/efifwsetup.c does it better
though having it as a separate comment at the start of the file.

> + */
> +
> +
> +#include <grub/cryptodisk.h>
> +#include <grub/dl.h>
> +#include <grub/err.h>
> +#include <grub/extcmd.h>
> +#include <grub/partition.h>
> +#include <grub/file.h>
> +
> +
> +GRUB_MOD_LICENSE ("GPLv3+");
> +#define BITS_PER_BYTE                  8

I'm remembering now that instead of this define we should use
GRUB_CHAR_BIT and remove this define.

> +#define PLAINMOUNT_DEFAULT_SECTOR_SIZE 512
> +#define PLAINMOUNT_DEFAULT_UUID        "109fea84-a6b7-34a8-4bd1-1c506305a400"
> +
> +
> +enum PLAINMOUNT_OPTION
> +  {
> +    OPTION_HASH,
> +    OPTION_CIPHER,
> +    OPTION_KEY_SIZE,
> +    OPTION_SECTOR_SIZE,
> +    OPTION_PASSWORD,
> +    OPTION_KEYFILE,
> +    OPTION_KEYFILE_OFFSET,
> +    OPTION_UUID
> +  };
> +
> +
> +static const struct grub_arg_option options[] =
> +  {
> +    /* TRANSLATORS: It's still restricted to this module only.  */
> +    {"hash", 'h', 0, N_("Password hash"), 0, ARG_TYPE_STRING},
> +    {"cipher", 'c', 0, N_("Password cipher"), 0, ARG_TYPE_STRING},
> +    {"key-size", 's', 0, N_("Key size (in bits)"), 0, ARG_TYPE_INT},
> +    {"sector-size", 'S', 0, N_("Device sector size"), 0, ARG_TYPE_INT},
> +    {"password", 'p', 0, N_("Password (key)"), 0, ARG_TYPE_STRING},
> +    {"keyfile", 'd', 0, N_("Keyfile path"), 0, ARG_TYPE_STRING},
> +    {"keyfile-offset", 'O', 0, N_("Keyfile offset"), 0, ARG_TYPE_INT},
> +    {"uuid", 'u', 0, N_("Set device UUID"), 0, ARG_TYPE_STRING},
> +    {0, 0, 0, 0, 0, 0}
> +  };
> +
> +
> +/* Cryptodisk setkey() function wrapper */
> +static grub_err_t
> +plainmount_setkey (grub_cryptodisk_t dev, grub_uint8_t *key,
> +                   grub_size_t size)
> +{
> +  gcry_err_code_t code = grub_cryptodisk_setkey (dev, key, size);
> +  if (code != GPG_ERR_NO_ERROR)
> +    {
> +      grub_dprintf ("plainmount", "password crypto status is %d\n", code);

s/password crypto status is/failed to set cipher key with code:/

> +      return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("cannot set specified 
> key"));
> +    }
> +  return GRUB_ERR_NONE;
> +}
> +
> +
> +/* Configure cryptodisk uuid */
> +static void plainmount_set_uuid (grub_cryptodisk_t dev, const char 
> *user_uuid)
> +{
> +  grub_size_t pos = 0;
> +
> +  /* Size of user_uuid is checked in main func */
> +  if (user_uuid != NULL)
> +      grub_memcpy (dev->uuid, user_uuid, grub_strlen (user_uuid));
> +  else
> +    {
> +      /*
> +       * Set default UUID. Last digits start from 1 and are incremented for
> +       * each new plainmount device by snprintf().
> +       */
> +      grub_snprintf (dev->uuid, sizeof (dev->uuid)-1, "%36lx", dev->id+1);
> +      while (dev->uuid[pos++] == ' ');
> +      grub_memcpy (dev->uuid, PLAINMOUNT_DEFAULT_UUID, pos-1);

Use ++pos in the while and just "pos" for the 3rd arg to grub_memcpy,
seems more natural and simpler

> +    }
> +  COMPILE_TIME_ASSERT (sizeof (dev->uuid) >= sizeof 
> (PLAINMOUNT_DEFAULT_UUID));
> +}
> +
> +
> +/* Configure cryptodevice sector size (-S option) */
> +static grub_err_t
> +plainmount_configure_sectors (grub_cryptodisk_t dev, grub_disk_t disk,
> +                              grub_size_t sector_size)

Spaces should be converted to tabs.

> +{
> +  dev->total_sectors = grub_disk_native_sectors (disk);
> +  if (dev->total_sectors == GRUB_DISK_SIZE_UNKNOWN)
> +    return grub_error (GRUB_ERR_BAD_DEVICE, N_("cannot determine disk %s 
> size"),
> +                       disk->name);
> +
> +  /* Convert size to sectors */
> +  dev->log_sector_size = grub_log2ull (sector_size);
> +  dev->total_sectors = grub_convert_sector (dev->total_sectors,
> +                                         GRUB_DISK_SECTOR_BITS,
> +                                            dev->log_sector_size);

Spaces should be converted to tabs here too. I imagine there are more
places this needs to happen too. Please check to make sure the usage of
spacing conforms to the GRUB standard. I'm noticing that in a prior
review I thought the indentation was wrong but I think I was wrong and
confused by the way tabs were getting converted to spaces by my email
client.

> +  if (dev->total_sectors == 0)
> +    return grub_error (GRUB_ERR_BAD_DEVICE,
> +                    N_("cannot set specified sector size on disk %s"),
> +                    disk->name);
> +
> +  grub_dprintf ("plainmount", "log_sector_size=%d, total_sectors=%"
> +             PRIuGRUB_SIZE"\n", dev->log_sector_size, dev->total_sectors);
> +  return GRUB_ERR_NONE;
> +}
> +
> +
> +/* Hashes a password into a key and stores it with cipher. */
> +static grub_err_t
> +plainmount_configure_password (grub_cryptodisk_t dev, grub_disk_t disk,
> +                               const char *hash, grub_uint8_t *key_data,
> +                            grub_size_t key_size)
> +{
> +  const gcry_md_spec_t *gcry_hash;
> +  grub_uint8_t derived_hash[GRUB_CRYPTODISK_MAX_KEYLEN * 2], *dh = 
> derived_hash;
> +  char *p;
> +  unsigned int round, i;
> +  unsigned int len, size;
> +
> +  /* Option -p was not set */
> +  if (key_data[0] == '\0')
> +  {
> +    char *part = grub_partition_get_name (disk->partition);
> +    grub_printf_ (N_("Enter passphrase for %s%s%s: "), disk->name,
> +                  disk->partition != NULL ? "," : "",
> +                  part != NULL ? part : N_("UNKNOWN"));
> +    grub_free (part);
> +
> +    if (!grub_password_get ((char*)key_data, 
> GRUB_CRYPTODISK_MAX_PASSPHRASE-1))
> +        grub_error (GRUB_ERR_BAD_ARGUMENT, N_("error reading password"));
> +  }
> +
> +  /* Support none (plain) hash */
> +  if (grub_strcmp (hash, "plain") == 0)
> +    {
> +      dev->hash = NULL;
> +      return plainmount_setkey (dev, key_data, key_size);

Let's not do setkey here either. It looks to me that a "return
GRUB_ERR_NONE" will suffice.

> +    }
> +
> +  /* Check hash */
> +  gcry_hash = grub_crypto_lookup_md_by_name (hash);
> +  if (!gcry_hash)
> +    return grub_error (GRUB_ERR_FILE_NOT_FOUND,
> +                       N_("couldn't load %s hash"),
> +                       hash);

I think Daniel will probably want this all on one line since its barely
over 80 chars.

> +
> +  if (gcry_hash->mdlen > GRUB_CRYPTODISK_MAX_KEYLEN)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +                       N_("hash length %"PRIuGRUB_SIZE" exceeds maximum %d 
> bits"),
> +                       gcry_hash->mdlen * BITS_PER_BYTE,
> +                       GRUB_CRYPTODISK_MAX_KEYLEN * BITS_PER_BYTE);

Use GRUB_CHAR_BIT instead of BITS_PER_BYTE as noted above.

> +
> +  dev->hash = gcry_hash;
> +  len = dev->hash->mdlen;
> +  p = grub_malloc (key_size + 2 + (key_size / len));
> +  if (p == NULL)
> +    return GRUB_ERR_OUT_OF_MEMORY;
> +
> +  /*
> +   * Hash password. Adapted from cryptsetup.
> +   * https://gitlab.com/cryptsetup/cryptsetup/-/blob/main/lib/crypt_plain.c
> +   */
> +  for (round = 0, size = key_size; size; round++, dh += len, size -= len)
> +    {
> +      for (i = 0; i < round; i++)
> +     p[i] = 'A';
> +
> +      grub_strcpy (p + i, (char*) key_data);
> +
> +      if (len > size)
> +     len = size;
> +
> +      grub_crypto_hash (dev->hash, dh, p, grub_strlen (p));
> +    }
> +  grub_free (p);
> +  return GRUB_ERR_NONE;
> +}
> +
> +
> +/* Read key material from keyfile */
> +static grub_err_t
> +plainmount_configure_keyfile (char *keyfile, grub_uint8_t *key_data,
> +                              grub_size_t key_size, grub_size_t 
> keyfile_offset)
> +{
> +  grub_file_t g_keyfile = grub_file_open (keyfile, GRUB_FILE_TYPE_NONE);
> +  if (g_keyfile == NULL)
> +    return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("cannot open keyfile %s"),
> +                       keyfile);
> +
> +  if (grub_file_seek (g_keyfile, keyfile_offset) == (grub_off_t)-1)
> +    return grub_error (GRUB_ERR_FILE_READ_ERROR,
> +                       N_("cannot seek keyfile at offset %"PRIuGRUB_SIZE),
> +                    keyfile_offset);
> +
> +  if (key_size > (g_keyfile->size - keyfile_offset))
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Specified key size (%"
> +                       PRIuGRUB_SIZE") is too small for keyfile size (%"
> +                       PRIuGRUB_SIZE") and offset (%"PRIuGRUB_SIZE")"),
> +                       key_size, g_keyfile->size, keyfile_offset);
> +
> +  if (grub_file_read (g_keyfile, key_data, key_size) != (grub_ssize_t) 
> key_size)
> +    return grub_error (GRUB_ERR_FILE_READ_ERROR, N_("error reading key 
> file"));
> +  return GRUB_ERR_NONE;
> +}
> +
> +
> +/* Plainmount command entry point */
> +static grub_err_t
> +grub_cmd_plainmount (grub_extcmd_context_t ctxt, int argc, char **args)
> +{
> +  struct grub_arg_list *state = ctxt->state;
> +  grub_cryptodisk_t dev = NULL;
> +  grub_disk_t disk = NULL;
> +  char *diskname, *disklast = NULL, *cipher, *mode, *hash, *keyfile, *uuid;
> +  grub_size_t len, key_size, sector_size, keyfile_offset = 0;
> +  grub_err_t err;
> +  const char *p;
> +  grub_uint8_t *key_data;
> +
> +  if (argc < 1)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("device name required"));
> +
> +  /* Check whether required arguments are specified */
> +  if (!state[OPTION_CIPHER].set || !state[OPTION_KEY_SIZE].set)
> +      return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +                      "cipher and key size must be set");
> +  if (!state[OPTION_HASH].set && !state[OPTION_KEYFILE].set)
> +      return grub_error (GRUB_ERR_BAD_ARGUMENT, "hash algorithm must be 
> set");
> +
> +  /* Check cipher mode */
> +  if (!grub_strchr (state[OPTION_CIPHER].arg,'-'))
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +                       N_("invalid cipher mode, must be of format 
> cipher-mode"));
> +
> +  /* Check password size */
> +  if (state[OPTION_PASSWORD].set && grub_strlen (state[OPTION_PASSWORD].arg) 
> >
> +                                              GRUB_CRYPTODISK_MAX_PASSPHRASE)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +                       N_("password exceeds maximium size"));
> +
> +  /* Check uuid length */
> +  if (state[OPTION_UUID].set && grub_strlen (state[OPTION_UUID].arg) >
> +                                sizeof (PLAINMOUNT_DEFAULT_UUID))
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +                       N_("specified UUID exceeds maximum size"));
> +
> +  /* Parse plainmount arguments */
> +  grub_errno = GRUB_ERR_NONE;
> +  keyfile_offset = state[OPTION_KEYFILE_OFFSET].set ?
> +                   grub_strtoull (state[OPTION_KEYFILE_OFFSET].arg, &p, 0) : 
> 0;
> +  if (state[OPTION_KEYFILE_OFFSET].set &&
> +     (state[OPTION_KEYFILE_OFFSET].arg[0] == '\0' || *p != '\0' ||
> +      grub_errno != GRUB_ERR_NONE))
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("unrecognized keyfile 
> offset"));
> +
> +  sector_size = state[OPTION_SECTOR_SIZE].set ?
> +             grub_strtoull (state[OPTION_SECTOR_SIZE].arg, &p, 0) :
> +                PLAINMOUNT_DEFAULT_SECTOR_SIZE;
> +  if (state[OPTION_SECTOR_SIZE].set && (state[OPTION_SECTOR_SIZE].arg[0] == 
> '\0' ||
> +                                     *p != '\0' || grub_errno != 
> GRUB_ERR_NONE))
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("unrecognized sector 
> size"));
> +
> +  /* Check key size */
> +  key_size = grub_strtoull (state[OPTION_KEY_SIZE].arg, &p, 0);
> +  if (state[OPTION_KEY_SIZE].arg[0] == '\0' || *p != '\0' ||
> +      grub_errno != GRUB_ERR_NONE)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("unrecognized key size"));
> +  if (key_size % BITS_PER_BYTE != 0)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +                       N_("key size is not multiple of bits"));

s/of bits/of %d bits/, and use constant as arg.

> +  key_size = key_size / BITS_PER_BYTE;
> +  if (key_size > GRUB_CRYPTODISK_MAX_KEYLEN)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +                       N_("key size %"PRIuGRUB_SIZE" exceeds maximum %d 
> bits"),
> +                       key_size * BITS_PER_BYTE,
> +                    GRUB_CRYPTODISK_MAX_KEYLEN * BITS_PER_BYTE);
> +
> +  /* Check disk sector size */
> +  if (sector_size < GRUB_DISK_SECTOR_SIZE)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT,
> +                    N_("sector size -S must be at least %d"),
> +                    GRUB_DISK_SECTOR_SIZE);
> +  if ((sector_size & (sector_size - 1)) != 0)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("sector size -S %"
> +                       PRIuGRUB_SIZE" is not power of 2"), sector_size);

This is strange line breaking. Maybe have N_() on its own line.

> +
> +  /* Allocate all stuff here */
> +  hash =  state[OPTION_HASH].set ? grub_strdup (state[OPTION_HASH].arg) : 
> NULL;
> +  cipher = grub_strdup (state[OPTION_CIPHER].arg);
> +  keyfile = state[OPTION_KEYFILE].set ?
> +            grub_strdup (state[OPTION_KEYFILE].arg) : NULL;
> +  dev = grub_zalloc (sizeof *dev);
> +  key_data = grub_zalloc (GRUB_CRYPTODISK_MAX_PASSPHRASE);
> +  uuid = state[OPTION_UUID].set ? grub_strdup (state[OPTION_UUID].arg) : 
> NULL;
> +  if ((!hash && state[OPTION_HASH].set) || !cipher ||
> +      (!keyfile && state[OPTION_KEYFILE].set) || !dev || !key_data ||
> +      (!uuid && state[OPTION_UUID].set))

I suspect Daniel will want key_data != NULL, and likewise for other
pointers.

Glenn

> +    {
> +      err = grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
> +      goto exit;
> +    }
> +
> +  /* Copy user password from -p option */
> +  if (state[OPTION_PASSWORD].set)
> +    grub_memcpy (key_data, state[OPTION_PASSWORD].arg,
> +              grub_strlen (state[OPTION_PASSWORD].arg));
> +
> +  /* Copy user UUID from -u option */
> +  if (state[OPTION_UUID].set)
> +    grub_memcpy (uuid, state[OPTION_UUID].arg,
> +                 grub_strlen (state[OPTION_UUID].arg));
> +
> +  /* Set cipher mode (tested above) */
> +  mode = grub_strchr (cipher,'-');
> +  *mode++ = '\0';
> +
> +  /* Check cipher */
> +  if (grub_cryptodisk_setcipher (dev, cipher, mode) != GRUB_ERR_NONE)
> +    {
> +      err = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid cipher %s"), 
> cipher);
> +      goto exit;
> +    }
> +
> +  /* Open SOURCE disk */
> +  diskname = args[0];
> +  len = grub_strlen (diskname);
> +  if (len && diskname[0] == '(' && diskname[len - 1] == ')')
> +    {
> +      disklast = &diskname[len - 1];
> +      *disklast = '\0';
> +      diskname++;
> +    }
> +  disk = grub_disk_open (diskname);
> +  if (disk == NULL)
> +    {
> +      if (disklast)
> +        *disklast = ')';
> +      err = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("cannot open disk %s"),
> +                        diskname);
> +      goto exit;
> +    }
> +
> +  /* Warn if hash and keyfile are both provided */
> +  if (keyfile != NULL && state[OPTION_HASH].arg)
> +    grub_printf_ (N_("warning: hash is ignored if keyfile is specified\n"));
> +
> +  /* Warn if -p option is specified with keyfile */
> +  if (state[OPTION_PASSWORD].set && state[OPTION_KEYFILE].set)
> +    grub_printf_ (N_("warning: password specified with -p option "
> +                     "is ignored if keyfile is provided\n"));
> +
> +  /* Warn of -O is provided without keyfile */
> +  if (state[OPTION_KEYFILE_OFFSET].set && !state[OPTION_KEYFILE].set)
> +    grub_printf_ (N_("warning: keyfile offset option -O "
> +                     "specified without keyfile option -d\n"));
> +
> +  grub_dprintf ("plainmount", "parameters: cipher=%s, hash=%s, key_size=%"
> +             PRIuGRUB_SIZE", keyfile=%s, keyfile offset=%"PRIuGRUB_SIZE"\n",
> +                cipher, hash, key_size, keyfile, keyfile_offset);
> +
> +  err = plainmount_configure_sectors (dev, disk, sector_size);
> +  if (err != GRUB_ERR_NONE)
> +    goto exit;
> +
> +  /* Configure keyfile or password */
> +  if (keyfile != NULL)
> +    err = plainmount_configure_keyfile (keyfile, key_data, key_size,
> +                                     keyfile_offset);
> +  else
> +    err = plainmount_configure_password (dev, disk, hash, key_data, 
> key_size);
> +  if (err != GRUB_ERR_NONE)
> +    goto exit;
> +
> +  err = plainmount_setkey (dev, key_data, key_size);
> +  if (err != GRUB_ERR_NONE)
> +    goto exit;
> +  err = grub_cryptodisk_insert (dev, diskname, disk);
> +  if (err != GRUB_ERR_NONE)
> +    goto exit;
> +
> +  dev->modname = "plainmount";
> +  dev->source_disk = disk;
> +  plainmount_set_uuid (dev, uuid);
> +
> +exit:
> +  grub_free (hash);
> +  grub_free (cipher);
> +  grub_free (keyfile);
> +  grub_free (key_data);
> +  grub_free (uuid);
> +  if (err != GRUB_ERR_NONE && disk)
> +    grub_disk_close (disk);
> +  if (err != GRUB_ERR_NONE && dev)
> +    grub_free (dev);
> +  return err;
> +}
> +
> +static grub_extcmd_t cmd;
> +GRUB_MOD_INIT (plainmount)
> +{
> +  cmd = grub_register_extcmd ("plainmount", grub_cmd_plainmount, 0,
> +                           N_("-c cipher -s key-size [-h hash] [-S 
> sector-size]"
> +                           " [-o offset] [-p password] [-u uuid] "
> +                           " [[-d keyfile] [-O keyfile offset]] <SOURCE>"),
> +                           N_("Open partition encrypted in plain mode."),
> +                           options);
> +}
> +
> +GRUB_MOD_FINI (plainmount)
> +{
> +  grub_unregister_extcmd (cmd);
> +}
> --
> 2.37.1
> 
> 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]