* [PATCH v8 1/1] plainmount: Support plain encryption mode
@ 2022-10-29 17:40 Maxim Fomin
2022-11-29 17:13 ` Daniel Kiper
2022-12-01 21:00 ` Glenn Washburn
0 siblings, 2 replies; 12+ messages in thread
From: Maxim Fomin @ 2022-10-29 17:40 UTC (permalink / raw)
To: grub-devel, development, ps, dkiper
From 2b1d2deb3f2416cbc3e7d25cbc4141a3078eaf68 Mon Sep 17 00:00:00 2001
From: Maxim Fomin <maxim@fomin.one>
Date: Sat, 29 Oct 2022 18:18:58 +0100
Subject: [PATCH v8 1/1] plainmount: Support plain encryption mode
This patch adds support for plain encryption mode (plain dm-crypt) via
new module/command named 'plainmount'.
Signed-off-by: Maxim Fomin <maxim@fomin.one>
Difference with v7:
diff --git a/docs/grub.texi b/docs/grub.texi
index 377969984..34ca6b4f1 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -5138,13 +5138,13 @@ to generate password hashes. @xref{Security}.
Setup access to the encrypted device in plain mode. Offset of the encrypted
-data at the device is specified in terms of 512 byte sectors with the blocklist
+data at the device is specified in terms of 512 byte sectors using the blocklist
syntax and loopback device. The following example shows how to specify 1MiB
offset:
@example
loopback node (hd0,gpt1)2048+
-plainmount node
+plainmount node @var{...}
@end example
The @command{plainmount} command can be used to open LUKS encrypted volume
@@ -5155,13 +5155,14 @@ The keyfile path parameter has higher priority than the secret passphrase
parameter and is specified with the option @option{-d}. Password data obtained
from keyfiles is not hashed and is used directly as a cipher key. An optional
offset of password data in the keyfile can be specified with the option
-@option{-O} or directly with the option @option{-d} and GRUB blocklist syntax.
+@option{-O} or directly with the option @option{-d} and GRUB blocklist syntax,
+if the keyfile data can be accessed from a device and is 512 byte aligned.
The following example shows both methods to specify password data in the
keyfile at offset 1MiB:
@example
-plainmount -d (hd0,gpt1)2048+
-plainmount -d (hd0,gpt1)+ -O 1048576
+plainmount -d (hd0,gpt1)2048+ @var{...}
+plainmount -d (hd0,gpt1)+ -O 1048576 @var{...}
@end example
If no keyfile is specified then the password is set to the string specified
diff --git a/grub-core/disk/plainmount.c b/grub-core/disk/plainmount.c
index 656c5d09f..85ada25bc 100644
--- a/grub-core/disk/plainmount.c
+++ b/grub-core/disk/plainmount.c
@@ -146,8 +146,12 @@ plainmount_configure_password (grub_cryptodisk_t dev, const char *hash,
dev->hash = grub_crypto_lookup_md_by_name (hash);
len = dev->hash->mdlen;
- alloc_size = password_size >= key_size ? password_size : key_size;
- p = grub_zalloc (alloc_size + (alloc_size / len));
+ alloc_size = grub_max (password_size, key_size);
+ /*
+ * Allocate buffer for the password and for an added prefix character
+ * for each hash round ('alloc_size' may not be a multiple of 'len').
+ */
+ p = grub_zalloc (alloc_size + (alloc_size / len) + 1);
derived_hash = grub_zalloc (GRUB_CRYPTODISK_MAX_KEYLEN * 2);
if (p == NULL || derived_hash == NULL)
{
@@ -170,9 +174,10 @@ plainmount_configure_password (grub_cryptodisk_t dev, const char *hash,
if (len > size)
len = size;
- grub_crypto_hash (dev->hash, dh, p, password_size);
+ grub_crypto_hash (dev->hash, dh, p, password_size + round);
}
- grub_memcpy (key_data, derived_hash, GRUB_CRYPTODISK_MAX_KEYLEN * 2);
+ grub_memcpy (key_data, derived_hash, key_size);
+
exit:
grub_free (p);
grub_free (derived_hash);
---
docs/grub.texi | 81 +++++++
grub-core/Makefile.core.def | 5 +
grub-core/disk/plainmount.c | 462 ++++++++++++++++++++++++++++++++++++
3 files changed, 548 insertions(+)
create mode 100644 grub-core/disk/plainmount.c
diff --git a/docs/grub.texi b/docs/grub.texi
index 2d6cd8358..34ca6b4f1 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -4271,6 +4271,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
* play:: Play a tune
* probe:: Retrieve device info
* rdmsr:: Read values from model-specific registers
@@ -4558,6 +4559,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{@pxref{plainmount}} command.
@end deffn
@node cutmem
@@ -5120,6 +5129,78 @@ 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. Offset of the encrypted
+data at the device is specified in terms of 512 byte sectors using the blocklist
+syntax and loopback device. The following example shows how to specify 1MiB
+offset:
+
+@example
+loopback node (hd0,gpt1)2048+
+plainmount node @var{...}
+@end example
+
+The @command{plainmount} command can be used to open LUKS encrypted volume
+if its master key and parameters (key size, cipher, offset, etc) are known.
+
+There are two ways to specify a password: a keyfile and a secret passphrase.
+The keyfile path parameter has higher priority than the secret passphrase
+parameter and is specified with the option @option{-d}. Password data obtained
+from keyfiles is not hashed and is used directly as a cipher key. An optional
+offset of password data in the keyfile can be specified with the option
+@option{-O} or directly with the option @option{-d} and GRUB blocklist syntax,
+if the keyfile data can be accessed from a device and is 512 byte aligned.
+The following example shows both methods to specify password data in the
+keyfile at offset 1MiB:
+
+@example
+plainmount -d (hd0,gpt1)2048+ @var{...}
+plainmount -d (hd0,gpt1)+ -O 1048576 @var{...}
+@end example
+
+If no keyfile is specified then the password is set to the string specified
+by option @option{-p} or is requested interactively from the console. In both
+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 @samp{plain} which means that no hashing is done 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. Cipher must be specified
+with the mode separated by a dash (for example, @samp{aes-xts-plain64}). Key size
+option @option{-s} is the key size of the cipher in bits, not to be confused with
+the offset of the key data in a keyfile specified with the @option{-O} option. It
+must not exceed 1024 bits, so a 32 byte key would be specified as 256 bits
+
+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 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 the filesystem driver can detect the presence
+of a filesystem, but nevertheless will refuse to mount it).
+
+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.
+
+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 98714c68d..f4153608c 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -1184,6 +1184,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..85ada25bc
--- /dev/null
+++ b/grub-core/disk/plainmount.c
@@ -0,0 +1,462 @@
+/*
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2022 Free Software Foundation, Inc.
+ *
+ * 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/>.
+ */
+
+/* plaimount.c - Open device encrypted in plain mode. */
+
+#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 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", "failed to set cipher key with code: %d\n", 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);
+ }
+ 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)
+{
+ 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);
+ 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 the cipher. */
+static grub_err_t
+plainmount_configure_password (grub_cryptodisk_t dev, const char *hash,
+ grub_uint8_t *key_data, grub_size_t key_size,
+ grub_size_t password_size)
+{
+ grub_uint8_t *derived_hash, *dh;
+ char *p;
+ unsigned int round, i, len, size;
+ grub_size_t alloc_size;
+ grub_err_t err = GRUB_ERR_NONE;
+
+ /* Support none (plain) hash */
+ if (grub_strcmp (hash, "plain") == 0)
+ {
+ dev->hash = NULL;
+ return err;
+ }
+
+ /* Hash argument was checked at main func */
+ dev->hash = grub_crypto_lookup_md_by_name (hash);
+ len = dev->hash->mdlen;
+
+ alloc_size = grub_max (password_size, key_size);
+ /*
+ * Allocate buffer for the password and for an added prefix character
+ * for each hash round ('alloc_size' may not be a multiple of 'len').
+ */
+ p = grub_zalloc (alloc_size + (alloc_size / len) + 1);
+ derived_hash = grub_zalloc (GRUB_CRYPTODISK_MAX_KEYLEN * 2);
+ if (p == NULL || derived_hash == NULL)
+ {
+ err = grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
+ goto exit;
+ }
+ dh = derived_hash;
+
+ /*
+ * Hash password. Adapted from cryptsetup.
+ * https://gitlab.com/cryptsetup/cryptsetup/-/blob/main/lib/crypt_plain.c
+ */
+ for (round = 0, size = alloc_size; size; round++, dh += len, size -= len)
+ {
+ for (i = 0; i < round; i++)
+ p[i] = 'A';
+
+ grub_memcpy (p + i, (char*) key_data, password_size);
+
+ if (len > size)
+ len = size;
+
+ grub_crypto_hash (dev->hash, dh, p, password_size + round);
+ }
+ grub_memcpy (key_data, derived_hash, key_size);
+
+exit:
+ grub_free (p);
+ grub_free (derived_hash);
+ return err;
+}
+
+
+/* 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;
+ const gcry_md_spec_t *gcry_hash;
+ char *diskname, *disklast = NULL, *cipher, *mode, *hash, *keyfile, *uuid;
+ grub_size_t len, key_size, sector_size, keyfile_offset = 0, password_size = 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 hash */
+ if (!state[OPTION_KEYFILE].set)
+ {
+ gcry_hash = grub_crypto_lookup_md_by_name (state[OPTION_HASH].arg);
+ if (!gcry_hash)
+ return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("couldn't load hash %s"),
+ state[OPTION_HASH].arg);
+
+ 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 * GRUB_CHAR_BIT,
+ GRUB_CRYPTODISK_MAX_KEYLEN * GRUB_CHAR_BIT);
+ }
+
+ /* 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 % GRUB_CHAR_BIT) != 0)
+ return grub_error (GRUB_ERR_BAD_ARGUMENT,
+ N_("key size is not multiple of %d bits"), GRUB_CHAR_BIT);
+ key_size = key_size / GRUB_CHAR_BIT;
+ 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 * GRUB_CHAR_BIT,
+ GRUB_CRYPTODISK_MAX_KEYLEN * GRUB_CHAR_BIT);
+
+ /* 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);
+
+ /* 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 == NULL && state[OPTION_HASH].set) || cipher == NULL || dev == NULL ||
+ (keyfile == NULL && state[OPTION_KEYFILE].set) || key_data == NULL ||
+ (uuid == NULL && state[OPTION_UUID].set))
+ {
+ err = grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
+ goto exit;
+ }
+
+ /* Copy user password from -p option */
+ if (state[OPTION_PASSWORD].set)
+ {
+ /*
+ * Password from the '-p' option is limited to C-string.
+ * Arbitrary data keys are supported via keyfiles.
+ */
+ password_size = grub_strlen (state[OPTION_PASSWORD].arg);
+ grub_memcpy (key_data, state[OPTION_PASSWORD].arg, password_size);
+ }
+
+ /* 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;
+ }
+
+ /* Get password from console */
+ if (!state[OPTION_KEYFILE].set && 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))
+ {
+ err = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("error reading password"));
+ goto exit;
+ }
+ /*
+ * Password from interactive console is limited to C-string.
+ * Arbitrary data keys are supported via keyfiles.
+ */
+ password_size = grub_strlen (key_data);
+ }
+
+ /* Warn if hash and keyfile are both provided */
+ if (state[OPTION_KEYFILE].set && 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 (state[OPTION_KEYFILE].set)
+ err = plainmount_configure_keyfile (keyfile, key_data, key_size, keyfile_offset);
+ else
+ err = plainmount_configure_password (dev, hash, key_data, key_size, password_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.38.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v8 1/1] plainmount: Support plain encryption mode
2022-10-29 17:40 [PATCH v8 1/1] plainmount: Support plain encryption mode Maxim Fomin
@ 2022-11-29 17:13 ` Daniel Kiper
2022-12-01 5:58 ` Glenn Washburn
2022-12-02 16:41 ` Maxim Fomin
2022-12-01 21:00 ` Glenn Washburn
1 sibling, 2 replies; 12+ messages in thread
From: Daniel Kiper @ 2022-11-29 17:13 UTC (permalink / raw)
To: Maxim Fomin; +Cc: grub-devel, development, ps
First of all, sorry for late reply. I hope I will be able to review next
version of this patch much faster.
On Sat, Oct 29, 2022 at 05:40:42PM +0000, Maxim Fomin wrote:
> From 2b1d2deb3f2416cbc3e7d25cbc4141a3078eaf68 Mon Sep 17 00:00:00 2001
> From: Maxim Fomin <maxim@fomin.one>
> Date: Sat, 29 Oct 2022 18:18:58 +0100
> Subject: [PATCH v8 1/1] plainmount: Support plain encryption mode
>
> This patch adds support for plain encryption mode (plain dm-crypt) via
> new module/command named 'plainmount'.
>
> Signed-off-by: Maxim Fomin <maxim@fomin.one>
Please put "---" behind SOB and before "Difference with..." next time...
> Difference with v7:
[...]
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 2d6cd8358..34ca6b4f1 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -4271,6 +4271,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
> * play:: Play a tune
> * probe:: Retrieve device info
> * rdmsr:: Read values from model-specific registers
> @@ -4558,6 +4559,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{@pxref{plainmount}} command.
> @end deffn
>
> @node cutmem
> @@ -5120,6 +5129,78 @@ 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. Offset of the encrypted
> +data at the device is specified in terms of 512 byte sectors using the blocklist
> +syntax and loopback device. The following example shows how to specify 1MiB
> +offset:
> +
> +@example
> +loopback node (hd0,gpt1)2048+
> +plainmount node @var{...}
> +@end example
> +
> +The @command{plainmount} command can be used to open LUKS encrypted volume
> +if its master key and parameters (key size, cipher, offset, etc) are known.
> +
> +There are two ways to specify a password: a keyfile and a secret passphrase.
> +The keyfile path parameter has higher priority than the secret passphrase
> +parameter and is specified with the option @option{-d}. Password data obtained
> +from keyfiles is not hashed and is used directly as a cipher key. An optional
> +offset of password data in the keyfile can be specified with the option
> +@option{-O} or directly with the option @option{-d} and GRUB blocklist syntax,
> +if the keyfile data can be accessed from a device and is 512 byte aligned.
> +The following example shows both methods to specify password data in the
> +keyfile at offset 1MiB:
> +
> +@example
> +plainmount -d (hd0,gpt1)2048+ @var{...}
> +plainmount -d (hd0,gpt1)+ -O 1048576 @var{...}
> +@end example
> +
> +If no keyfile is specified then the password is set to the string specified
> +by option @option{-p} or is requested interactively from the console. In both
> +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 @samp{plain} which means that no hashing is done 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. Cipher must be specified
> +with the mode separated by a dash (for example, @samp{aes-xts-plain64}). Key size
> +option @option{-s} is the key size of the cipher in bits, not to be confused with
> +the offset of the key data in a keyfile specified with the @option{-O} option. It
> +must not exceed 1024 bits, so a 32 byte key would be specified as 256 bits
> +
> +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 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 the filesystem driver can detect the presence
> +of a filesystem, but nevertheless will refuse to mount it).
> +
> +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.
> +
> +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.
Hmmm... Why do you say "Writing data" here if the GRUB does not support
filesystems writes except grubenv file? I think it should be clarified
what you mean or dropped if it is wrong in this context. Otherwise it is
confusing.
> +@end deffn
> +
> +
> @node play
> @subsection play
>
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 98714c68d..f4153608c 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1184,6 +1184,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..85ada25bc
> --- /dev/null
> +++ b/grub-core/disk/plainmount.c
> @@ -0,0 +1,462 @@
> +/*
> + * GRUB -- GRand Unified Bootloader
> + * Copyright (C) 2022 Free Software Foundation, Inc.
> + *
> + * 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/>.
> + */
> +
> +/* plaimount.c - Open device encrypted in plain mode. */
> +
> +#include <grub/cryptodisk.h>
> +#include <grub/dl.h>
> +#include <grub/err.h>
> +#include <grub/extcmd.h>
> +#include <grub/partition.h>
> +#include <grub/file.h>
> +
> +
Please use one line instead of two to separate things. Same comment
applies to double empty lines below.
> +GRUB_MOD_LICENSE ("GPLv3+");
> +
> +#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", "failed to set cipher key with code: %d\n", 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));
I think grub_strcpy() instead of grub_memcpy() would be more natural in
string contexts. And then you do not need grub_strlen().
Is it safe to assume here that dev->uuid has been zeroed earlier?
> + 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);
s/sizeof (dev->uuid)-1/sizeof (dev->uuid) - 1/
s/dev->id+1/dev->id + 1/
> + while (dev->uuid[++pos] == ' ');
> + grub_memcpy (dev->uuid, PLAINMOUNT_DEFAULT_UUID, pos);
> + }
> + 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)
> +{
> + 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);
> + 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 the cipher. */
> +static grub_err_t
> +plainmount_configure_password (grub_cryptodisk_t dev, const char *hash,
> + grub_uint8_t *key_data, grub_size_t key_size,
> + grub_size_t password_size)
> +{
> + grub_uint8_t *derived_hash, *dh;
> + char *p;
> + unsigned int round, i, len, size;
> + grub_size_t alloc_size;
> + grub_err_t err = GRUB_ERR_NONE;
> +
> + /* Support none (plain) hash */
> + if (grub_strcmp (hash, "plain") == 0)
> + {
> + dev->hash = NULL;
> + return err;
> + }
> +
> + /* Hash argument was checked at main func */
> + dev->hash = grub_crypto_lookup_md_by_name (hash);
> + len = dev->hash->mdlen;
> +
> + alloc_size = grub_max (password_size, key_size);
> + /*
> + * Allocate buffer for the password and for an added prefix character
> + * for each hash round ('alloc_size' may not be a multiple of 'len').
> + */
> + p = grub_zalloc (alloc_size + (alloc_size / len) + 1);
> + derived_hash = grub_zalloc (GRUB_CRYPTODISK_MAX_KEYLEN * 2);
> + if (p == NULL || derived_hash == NULL)
> + {
> + err = grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
> + goto exit;
> + }
> + dh = derived_hash;
> +
> + /*
> + * Hash password. Adapted from cryptsetup.
> + * https://gitlab.com/cryptsetup/cryptsetup/-/blob/main/lib/crypt_plain.c
> + */
> + for (round = 0, size = alloc_size; size; round++, dh += len, size -= len)
> + {
> + for (i = 0; i < round; i++)
> + p[i] = 'A';
> +
> + grub_memcpy (p + i, (char*) key_data, password_size);
> +
> + if (len > size)
> + len = size;
> +
> + grub_crypto_hash (dev->hash, dh, p, password_size + round);
> + }
> + grub_memcpy (key_data, derived_hash, key_size);
> +
> +exit:
I prefer "fail" instead of "exit" label in that context. And labels
should be prefixed with one space. Same applies to "exit" labels below.
> + grub_free (p);
> + grub_free (derived_hash);
> + return err;
> +}
> +
> +
> +/* 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)
s/(grub_off_t)-1/(grub_off_t) -1/
Casts require a space after ")"...
> + 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;
> + const gcry_md_spec_t *gcry_hash;
> + char *diskname, *disklast = NULL, *cipher, *mode, *hash, *keyfile, *uuid;
> + grub_size_t len, key_size, sector_size, keyfile_offset = 0, password_size = 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 hash */
> + if (!state[OPTION_KEYFILE].set)
> + {
> + gcry_hash = grub_crypto_lookup_md_by_name (state[OPTION_HASH].arg);
> + if (!gcry_hash)
> + return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("couldn't load hash %s"),
> + state[OPTION_HASH].arg);
> +
> + 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 * GRUB_CHAR_BIT,
> + GRUB_CRYPTODISK_MAX_KEYLEN * GRUB_CHAR_BIT);
> + }
> +
> + /* 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"));
What will happen if somebody passes shorter, probably non-valid, UUID?
I think we should check UUID is not shorter than something too.
> + /* 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 % GRUB_CHAR_BIT) != 0)
> + return grub_error (GRUB_ERR_BAD_ARGUMENT,
> + N_("key size is not multiple of %d bits"), GRUB_CHAR_BIT);
> + key_size = key_size / GRUB_CHAR_BIT;
> + 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 * GRUB_CHAR_BIT,
> + GRUB_CRYPTODISK_MAX_KEYLEN * GRUB_CHAR_BIT);
> +
> + /* 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);
> +
> + /* 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 == NULL && state[OPTION_HASH].set) || cipher == NULL || dev == NULL ||
I think it would be more natural if you do "(state[OPTION_HASH].set && hash == NULL)..."
instead of "(hash == NULL && state[OPTION_HASH].set)...".
> + (keyfile == NULL && state[OPTION_KEYFILE].set) || key_data == NULL ||
Ditto.
> + (uuid == NULL && state[OPTION_UUID].set))
Ditto.
> + {
> + err = grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
> + goto exit;
> + }
> +
> + /* Copy user password from -p option */
> + if (state[OPTION_PASSWORD].set)
> + {
> + /*
> + * Password from the '-p' option is limited to C-string.
> + * Arbitrary data keys are supported via keyfiles.
> + */
> + password_size = grub_strlen (state[OPTION_PASSWORD].arg);
> + grub_memcpy (key_data, state[OPTION_PASSWORD].arg, password_size);
> + }
> +
> + /* Copy user UUID from -u option */
> + if (state[OPTION_UUID].set)
> + grub_memcpy (uuid, state[OPTION_UUID].arg,
> + grub_strlen (state[OPTION_UUID].arg));
This seems duplicate. grub_strdup() did work for you above.
> +
> + /* 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;
> + }
> +
> + /* Get password from console */
> + if (!state[OPTION_KEYFILE].set && 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))
s/(char*)key_data/(char *) key_data/
s/GRUB_CRYPTODISK_MAX_PASSPHRASE-1/GRUB_CRYPTODISK_MAX_PASSPHRASE - 1/
> + {
> + err = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("error reading password"));
> + goto exit;
> + }
> + /*
> + * Password from interactive console is limited to C-string.
> + * Arbitrary data keys are supported via keyfiles.
> + */
> + password_size = grub_strlen (key_data);
> + }
> +
> + /* Warn if hash and keyfile are both provided */
> + if (state[OPTION_KEYFILE].set && 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",
s/"PRIuGRUB_SIZE"/" PRIuGRUB_SIZE "/
> + 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 (state[OPTION_KEYFILE].set)
> + err = plainmount_configure_keyfile (keyfile, key_data, key_size, keyfile_offset);
> + else
> + err = plainmount_configure_password (dev, hash, key_data, key_size, password_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)
s/disk/disk != NULL/
> + grub_disk_close (disk);
> + if (err != GRUB_ERR_NONE && dev)
You can drop "dev" check from here.
> + grub_free (dev);
> + return err;
> +}
Daniel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v8 1/1] plainmount: Support plain encryption mode
2022-11-29 17:13 ` Daniel Kiper
@ 2022-12-01 5:58 ` Glenn Washburn
2022-12-01 19:38 ` Daniel Kiper
2022-12-02 16:41 ` Maxim Fomin
1 sibling, 1 reply; 12+ messages in thread
From: Glenn Washburn @ 2022-12-01 5:58 UTC (permalink / raw)
To: Daniel Kiper; +Cc: Maxim Fomin, grub-devel, ps
On Tue, 29 Nov 2022 18:13:11 +0100
Daniel Kiper <dkiper@net-space.pl> wrote:
> First of all, sorry for late reply. I hope I will be able to review
> next version of this patch much faster.
>
> On Sat, Oct 29, 2022 at 05:40:42PM +0000, Maxim Fomin wrote:
> > From 2b1d2deb3f2416cbc3e7d25cbc4141a3078eaf68 Mon Sep 17 00:00:00
> > 2001 From: Maxim Fomin <maxim@fomin.one>
> > Date: Sat, 29 Oct 2022 18:18:58 +0100
> > Subject: [PATCH v8 1/1] plainmount: Support plain encryption mode
> >
> > This patch adds support for plain encryption mode (plain dm-crypt)
> > via new module/command named 'plainmount'.
> >
> > Signed-off-by: Maxim Fomin <maxim@fomin.one>
>
> Please put "---" behind SOB and before "Difference with..." next
> time...
[...]
> > +
> > +/* 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));
>
> I think grub_strcpy() instead of grub_memcpy() would be more natural
> in string contexts. And then you do not need grub_strlen().
>
> Is it safe to assume here that dev->uuid has been zeroed earlier?
Yes, its zalloc'd in grub_cmd_plainmount below.
>
> > + 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);
>
> s/sizeof (dev->uuid)-1/sizeof (dev->uuid) - 1/
> s/dev->id+1/dev->id + 1/
>
> > + while (dev->uuid[++pos] == ' ');
> > + grub_memcpy (dev->uuid, PLAINMOUNT_DEFAULT_UUID, pos);
> > + }
> > + 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)
> > +{
> > + 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);
> > + 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 the cipher. */
> > +static grub_err_t
> > +plainmount_configure_password (grub_cryptodisk_t dev, const char
> > *hash,
> > + grub_uint8_t *key_data, grub_size_t
> > key_size,
> > + grub_size_t password_size)
> > +{
> > + grub_uint8_t *derived_hash, *dh;
> > + char *p;
> > + unsigned int round, i, len, size;
> > + grub_size_t alloc_size;
> > + grub_err_t err = GRUB_ERR_NONE;
> > +
> > + /* Support none (plain) hash */
> > + if (grub_strcmp (hash, "plain") == 0)
> > + {
> > + dev->hash = NULL;
> > + return err;
> > + }
> > +
> > + /* Hash argument was checked at main func */
> > + dev->hash = grub_crypto_lookup_md_by_name (hash);
> > + len = dev->hash->mdlen;
> > +
> > + alloc_size = grub_max (password_size, key_size);
> > + /*
> > + * Allocate buffer for the password and for an added prefix
> > character
> > + * for each hash round ('alloc_size' may not be a multiple of
> > 'len').
> > + */
> > + p = grub_zalloc (alloc_size + (alloc_size / len) + 1);
> > + derived_hash = grub_zalloc (GRUB_CRYPTODISK_MAX_KEYLEN * 2);
> > + if (p == NULL || derived_hash == NULL)
> > + {
> > + err = grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
> > + goto exit;
> > + }
> > + dh = derived_hash;
> > +
> > + /*
> > + * Hash password. Adapted from cryptsetup.
> > + *
> > https://gitlab.com/cryptsetup/cryptsetup/-/blob/main/lib/crypt_plain.c
> > + */
> > + for (round = 0, size = alloc_size; size; round++, dh += len,
> > size -= len)
> > + {
> > + for (i = 0; i < round; i++)
> > + p[i] = 'A';
> > +
> > + grub_memcpy (p + i, (char*) key_data, password_size);
> > +
> > + if (len > size)
> > + len = size;
> > +
> > + grub_crypto_hash (dev->hash, dh, p, password_size + round);
> > + }
> > + grub_memcpy (key_data, derived_hash, key_size);
> > +
> > +exit:
>
> I prefer "fail" instead of "exit" label in that context. And labels
> should be prefixed with one space. Same applies to "exit" labels
> below.
>
> > + grub_free (p);
> > + grub_free (derived_hash);
> > + return err;
> > +}
> > +
> > +
> > +/* 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)
>
> s/(grub_off_t)-1/(grub_off_t) -1/
> Casts require a space after ")"...
>
> > + 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;
> > + const gcry_md_spec_t *gcry_hash;
> > + char *diskname, *disklast = NULL, *cipher, *mode, *hash,
> > *keyfile, *uuid;
> > + grub_size_t len, key_size, sector_size, keyfile_offset = 0,
> > password_size = 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 hash */
> > + if (!state[OPTION_KEYFILE].set)
> > + {
> > + gcry_hash = grub_crypto_lookup_md_by_name
> > (state[OPTION_HASH].arg);
> > + if (!gcry_hash)
> > + return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("couldn't
> > load hash %s"),
> > + state[OPTION_HASH].arg);
> > +
> > + 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 * GRUB_CHAR_BIT,
> > + GRUB_CRYPTODISK_MAX_KEYLEN *
> > GRUB_CHAR_BIT);
> > + }
> > +
> > + /* 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"));
>
> What will happen if somebody passes shorter, probably non-valid, UUID?
> I think we should check UUID is not shorter than something too.
Afaik, we don't really care what the UUID string is, just that its
unique. It doesn't have to be a real UUID either. A minimum length
check of 1 might be good, otherwise '--uuid ""' will create an empty
uuid, but its not a big deal either. We might think about a character
validation check so that characters like ')' can't be in the string.
None of this will cause real problems as far as I can tell, but some
features may not work right. For example, having ')' in the uuid will
make it so that you can't access the crypto device via the
'(cruuid/<uuid>)' syntax. I also don't mind not doing the validation
and letting the user shoot themselves in the foot if they so choose.
Glenn
>
> > + /* 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 % GRUB_CHAR_BIT) != 0)
> > + return grub_error (GRUB_ERR_BAD_ARGUMENT,
> > + N_("key size is not multiple of %d bits"),
> > GRUB_CHAR_BIT);
> > + key_size = key_size / GRUB_CHAR_BIT;
> > + 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 * GRUB_CHAR_BIT,
> > + GRUB_CRYPTODISK_MAX_KEYLEN * GRUB_CHAR_BIT);
> > +
> > + /* 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);
> > +
> > + /* 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 == NULL && state[OPTION_HASH].set) || cipher == NULL
> > || dev == NULL ||
>
> I think it would be more natural if you do "(state[OPTION_HASH].set
> && hash == NULL)..." instead of "(hash == NULL &&
> state[OPTION_HASH].set)...".
>
> > + (keyfile == NULL && state[OPTION_KEYFILE].set) || key_data
> > == NULL ||
>
> Ditto.
>
> > + (uuid == NULL && state[OPTION_UUID].set))
>
> Ditto.
>
> > + {
> > + err = grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
> > + goto exit;
> > + }
> > +
> > + /* Copy user password from -p option */
> > + if (state[OPTION_PASSWORD].set)
> > + {
> > + /*
> > + * Password from the '-p' option is limited to C-string.
> > + * Arbitrary data keys are supported via keyfiles.
> > + */
> > + password_size = grub_strlen (state[OPTION_PASSWORD].arg);
> > + grub_memcpy (key_data, state[OPTION_PASSWORD].arg,
> > password_size);
> > + }
> > +
> > + /* Copy user UUID from -u option */
> > + if (state[OPTION_UUID].set)
> > + grub_memcpy (uuid, state[OPTION_UUID].arg,
> > + grub_strlen (state[OPTION_UUID].arg));
>
> This seems duplicate. grub_strdup() did work for you above.
>
> > +
> > + /* 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;
> > + }
> > +
> > + /* Get password from console */
> > + if (!state[OPTION_KEYFILE].set && 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))
>
> s/(char*)key_data/(char *) key_data/
> s/GRUB_CRYPTODISK_MAX_PASSPHRASE-1/GRUB_CRYPTODISK_MAX_PASSPHRASE - 1/
>
> > + {
> > + err = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("error reading
> > password"));
> > + goto exit;
> > + }
> > + /*
> > + * Password from interactive console is limited to C-string.
> > + * Arbitrary data keys are supported via keyfiles.
> > + */
> > + password_size = grub_strlen (key_data);
> > + }
> > +
> > + /* Warn if hash and keyfile are both provided */
> > + if (state[OPTION_KEYFILE].set && 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",
>
> s/"PRIuGRUB_SIZE"/" PRIuGRUB_SIZE "/
>
> > + 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 (state[OPTION_KEYFILE].set)
> > + err = plainmount_configure_keyfile (keyfile, key_data,
> > key_size, keyfile_offset);
> > + else
> > + err = plainmount_configure_password (dev, hash, key_data,
> > key_size, password_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)
>
> s/disk/disk != NULL/
>
> > + grub_disk_close (disk);
> > + if (err != GRUB_ERR_NONE && dev)
>
> You can drop "dev" check from here.
>
> > + grub_free (dev);
> > + return err;
> > +}
>
> Daniel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v8 1/1] plainmount: Support plain encryption mode
2022-12-01 5:58 ` Glenn Washburn
@ 2022-12-01 19:38 ` Daniel Kiper
0 siblings, 0 replies; 12+ messages in thread
From: Daniel Kiper @ 2022-12-01 19:38 UTC (permalink / raw)
To: Glenn Washburn; +Cc: Maxim Fomin, grub-devel, ps
On Wed, Nov 30, 2022 at 11:58:33PM -0600, Glenn Washburn wrote:
> On Tue, 29 Nov 2022 18:13:11 +0100
> Daniel Kiper <dkiper@net-space.pl> wrote:
>
> > First of all, sorry for late reply. I hope I will be able to review
> > next version of this patch much faster.
> >
> > On Sat, Oct 29, 2022 at 05:40:42PM +0000, Maxim Fomin wrote:
> > > From 2b1d2deb3f2416cbc3e7d25cbc4141a3078eaf68 Mon Sep 17 00:00:00
> > > 2001 From: Maxim Fomin <maxim@fomin.one>
> > > Date: Sat, 29 Oct 2022 18:18:58 +0100
> > > Subject: [PATCH v8 1/1] plainmount: Support plain encryption mode
> > >
> > > This patch adds support for plain encryption mode (plain dm-crypt)
> > > via new module/command named 'plainmount'.
> > >
> > > Signed-off-by: Maxim Fomin <maxim@fomin.one>
> >
> > Please put "---" behind SOB and before "Difference with..." next
> > time...
>
> [...]
>
> > > +
> > > +/* 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));
> >
> > I think grub_strcpy() instead of grub_memcpy() would be more natural
> > in string contexts. And then you do not need grub_strlen().
> >
> > Is it safe to assume here that dev->uuid has been zeroed earlier?
>
> Yes, its zalloc'd in grub_cmd_plainmount below.
OK, cool!
[...]
> > > + /* 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"));
> >
> > What will happen if somebody passes shorter, probably non-valid, UUID?
> > I think we should check UUID is not shorter than something too.
>
> Afaik, we don't really care what the UUID string is, just that its
> unique. It doesn't have to be a real UUID either. A minimum length
> check of 1 might be good, otherwise '--uuid ""' will create an empty
> uuid, but its not a big deal either. We might think about a character
> validation check so that characters like ')' can't be in the string.
> None of this will cause real problems as far as I can tell, but some
> features may not work right. For example, having ')' in the uuid will
> make it so that you can't access the crypto device via the
> '(cruuid/<uuid>)' syntax. I also don't mind not doing the validation
> and letting the user shoot themselves in the foot if they so choose.
I think we should check UUID size and probably it should not be smaller
than 1 and larger than "sizeof(PLAINMOUNT_DEFAULT_UUID) - 1". I do not
think we should check validity/content of the UUID.
Daniel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v8 1/1] plainmount: Support plain encryption mode
2022-10-29 17:40 [PATCH v8 1/1] plainmount: Support plain encryption mode Maxim Fomin
2022-11-29 17:13 ` Daniel Kiper
@ 2022-12-01 21:00 ` Glenn Washburn
2022-12-02 17:11 ` Maxim Fomin
1 sibling, 1 reply; 12+ messages in thread
From: Glenn Washburn @ 2022-12-01 21:00 UTC (permalink / raw)
To: Maxim Fomin; +Cc: grub-devel, ps, dkiper
On Sat, 29 Oct 2022 17:40:42 +0000
Maxim Fomin <maxim@fomin.one> wrote:
> From 2b1d2deb3f2416cbc3e7d25cbc4141a3078eaf68 Mon Sep 17 00:00:00 2001
> From: Maxim Fomin <maxim@fomin.one>
> Date: Sat, 29 Oct 2022 18:18:58 +0100
> Subject: [PATCH v8 1/1] plainmount: Support plain encryption mode
>
> This patch adds support for plain encryption mode (plain dm-crypt) via
> new module/command named 'plainmount'.
>
> Signed-off-by: Maxim Fomin <maxim@fomin.one>
>
> Difference with v7:
Daniel pointed this out, but this isn't a well formed patch. I do very
much appreciate you adding the differences in as it made it easier to
look at this. And my suggestion was to use --interdiff or --range-diff
to the format-patch command, which would properly format things. It
looks like you just copy pasted the output of "git diff v7 v8".
I'm now compiling this patch and found a few compile issues below.
You're compile testing this right?
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 377969984..34ca6b4f1 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -5138,13 +5138,13 @@ to generate password hashes. @xref{Security}.
>
> Setup access to the encrypted device in plain mode. Offset of the
> encrypted -data at the device is specified in terms of 512 byte
> sectors with the blocklist +data at the device is specified in terms
> of 512 byte sectors using the blocklist syntax and loopback device.
> The following example shows how to specify 1MiB offset:
>
> @example
> loopback node (hd0,gpt1)2048+
> -plainmount node
> +plainmount node @var{...}
> @end example
>
> The @command{plainmount} command can be used to open LUKS encrypted
> volume @@ -5155,13 +5155,14 @@ The keyfile path parameter has higher
> priority than the secret passphrase parameter and is specified with
> the option @option{-d}. Password data obtained from keyfiles is not
> hashed and is used directly as a cipher key. An optional offset of
> password data in the keyfile can be specified with the option
> -@option{-O} or directly with the option @option{-d} and GRUB
> blocklist syntax. +@option{-O} or directly with the option
> @option{-d} and GRUB blocklist syntax, +if the keyfile data can be
> accessed from a device and is 512 byte aligned. The following example
> shows both methods to specify password data in the keyfile at offset
> 1MiB:
>
> @example
> -plainmount -d (hd0,gpt1)2048+
> -plainmount -d (hd0,gpt1)+ -O 1048576
> +plainmount -d (hd0,gpt1)2048+ @var{...}
> +plainmount -d (hd0,gpt1)+ -O 1048576 @var{...}
> @end example
>
> If no keyfile is specified then the password is set to the string
> specified diff --git a/grub-core/disk/plainmount.c
> b/grub-core/disk/plainmount.c index 656c5d09f..85ada25bc 100644
> --- a/grub-core/disk/plainmount.c
> +++ b/grub-core/disk/plainmount.c
> @@ -146,8 +146,12 @@ plainmount_configure_password (grub_cryptodisk_t
> dev, const char *hash, dev->hash = grub_crypto_lookup_md_by_name
> (hash); len = dev->hash->mdlen;
>
> - alloc_size = password_size >= key_size ? password_size : key_size;
> - p = grub_zalloc (alloc_size + (alloc_size / len));
> + alloc_size = grub_max (password_size, key_size);
> + /*
> + * Allocate buffer for the password and for an added prefix
> character
> + * for each hash round ('alloc_size' may not be a multiple of
> 'len').
> + */
> + p = grub_zalloc (alloc_size + (alloc_size / len) + 1);
> derived_hash = grub_zalloc (GRUB_CRYPTODISK_MAX_KEYLEN * 2);
> if (p == NULL || derived_hash == NULL)
> {
> @@ -170,9 +174,10 @@ plainmount_configure_password (grub_cryptodisk_t
> dev, const char *hash, if (len > size)
> len = size;
>
> - grub_crypto_hash (dev->hash, dh, p, password_size);
> + grub_crypto_hash (dev->hash, dh, p, password_size + round);
> }
> - grub_memcpy (key_data, derived_hash, GRUB_CRYPTODISK_MAX_KEYLEN *
> 2);
> + grub_memcpy (key_data, derived_hash, key_size);
> +
> exit:
> grub_free (p);
> grub_free (derived_hash);
> ---
> docs/grub.texi | 81 +++++++
> grub-core/Makefile.core.def | 5 +
> grub-core/disk/plainmount.c | 462
> ++++++++++++++++++++++++++++++++++++ 3 files changed, 548
> insertions(+) create mode 100644 grub-core/disk/plainmount.c
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 2d6cd8358..34ca6b4f1 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -4271,6 +4271,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
> * play:: Play a tune
> * probe:: Retrieve device info
> * rdmsr:: Read values from model-specific
> registers @@ -4558,6 +4559,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{@pxref{plainmount}} command.
> @end deffn
>
> @node cutmem
> @@ -5120,6 +5129,78 @@ 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. Offset of the
> encrypted +data at the device is specified in terms of 512 byte
> sectors using the blocklist +syntax and loopback device. The
> following example shows how to specify 1MiB +offset:
> +
> +@example
> +loopback node (hd0,gpt1)2048+
> +plainmount node @var{...}
> +@end example
> +
> +The @command{plainmount} command can be used to open LUKS encrypted
> volume +if its master key and parameters (key size, cipher, offset,
> etc) are known. +
> +There are two ways to specify a password: a keyfile and a secret
> passphrase. +The keyfile path parameter has higher priority than the
> secret passphrase +parameter and is specified with the option
> @option{-d}. Password data obtained +from keyfiles is not hashed and
> is used directly as a cipher key. An optional +offset of password
> data in the keyfile can be specified with the option +@option{-O} or
> directly with the option @option{-d} and GRUB blocklist syntax, +if
> the keyfile data can be accessed from a device and is 512 byte
> aligned. +The following example shows both methods to specify
> password data in the +keyfile at offset 1MiB: +
> +@example
> +plainmount -d (hd0,gpt1)2048+ @var{...}
> +plainmount -d (hd0,gpt1)+ -O 1048576 @var{...}
> +@end example
> +
> +If no keyfile is specified then the password is set to the string
> specified +by option @option{-p} or is requested interactively from
> the console. In both +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
> @samp{plain} which means that no hashing is done 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. Cipher must be specified +with the mode separated by a dash
> (for example, @samp{aes-xts-plain64}). Key size +option @option{-s}
> is the key size of the cipher in bits, not to be confused with +the
> offset of the key data in a keyfile specified with the @option{-O}
> option. It +must not exceed 1024 bits, so a 32 byte key would be
> specified as 256 bits + +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 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
> the filesystem driver can detect the presence +of a filesystem, but
> nevertheless will refuse to mount it). + +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. + +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 98714c68d..f4153608c 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1184,6 +1184,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..85ada25bc
> --- /dev/null
> +++ b/grub-core/disk/plainmount.c
> @@ -0,0 +1,462 @@
> +/*
> + * GRUB -- GRand Unified Bootloader
> + * Copyright (C) 2022 Free Software Foundation, Inc.
> + *
> + * 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/>.
> + */
> +
> +/* plaimount.c - Open device encrypted in plain mode. */
> +
> +#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 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", "failed to set cipher key with
> code: %d\n", 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);
> + }
> + 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)
> +{
> + 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);
> + 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);
s/PRIuGRUB_SIZE/PRIuGRUB_UINT64_T/
This compiles fine on x86_64 as is, but fails to compile on i386.
> + return GRUB_ERR_NONE;
> +}
> +
> +
> +/* Hashes a password into a key and stores it with the cipher. */
> +static grub_err_t
> +plainmount_configure_password (grub_cryptodisk_t dev, const char
> *hash,
> + grub_uint8_t *key_data, grub_size_t
> key_size,
> + grub_size_t password_size)
> +{
> + grub_uint8_t *derived_hash, *dh;
> + char *p;
> + unsigned int round, i, len, size;
> + grub_size_t alloc_size;
> + grub_err_t err = GRUB_ERR_NONE;
> +
> + /* Support none (plain) hash */
> + if (grub_strcmp (hash, "plain") == 0)
> + {
> + dev->hash = NULL;
> + return err;
> + }
> +
> + /* Hash argument was checked at main func */
> + dev->hash = grub_crypto_lookup_md_by_name (hash);
> + len = dev->hash->mdlen;
> +
> + alloc_size = grub_max (password_size, key_size);
> + /*
> + * Allocate buffer for the password and for an added prefix
> character
> + * for each hash round ('alloc_size' may not be a multiple of
> 'len').
> + */
> + p = grub_zalloc (alloc_size + (alloc_size / len) + 1);
> + derived_hash = grub_zalloc (GRUB_CRYPTODISK_MAX_KEYLEN * 2);
> + if (p == NULL || derived_hash == NULL)
> + {
> + err = grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
> + goto exit;
> + }
> + dh = derived_hash;
> +
> + /*
> + * Hash password. Adapted from cryptsetup.
> + *
> https://gitlab.com/cryptsetup/cryptsetup/-/blob/main/lib/crypt_plain.c
> + */
> + for (round = 0, size = alloc_size; size; round++, dh += len, size
> -= len)
> + {
> + for (i = 0; i < round; i++)
> + p[i] = 'A';
> +
> + grub_memcpy (p + i, (char*) key_data, password_size);
> +
> + if (len > size)
> + len = size;
> +
> + grub_crypto_hash (dev->hash, dh, p, password_size + round);
> + }
> + grub_memcpy (key_data, derived_hash, key_size);
> +
> +exit:
> + grub_free (p);
> + grub_free (derived_hash);
> + return err;
> +}
> +
> +
> +/* 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")"),
Also, this compiles fine on x86_64 as is, but fails to compile on i386.
The format code for g_keyfile->size should be PRIuGRUB_UINT64_T.
> + 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;
> + const gcry_md_spec_t *gcry_hash;
> + char *diskname, *disklast = NULL, *cipher, *mode, *hash, *keyfile,
> *uuid;
> + grub_size_t len, key_size, sector_size, keyfile_offset = 0,
> password_size = 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 hash */
> + if (!state[OPTION_KEYFILE].set)
> + {
> + gcry_hash = grub_crypto_lookup_md_by_name
> (state[OPTION_HASH].arg);
> + if (!gcry_hash)
> + return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("couldn't load
> hash %s"),
> + state[OPTION_HASH].arg);
> +
> + 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 * GRUB_CHAR_BIT,
> + GRUB_CRYPTODISK_MAX_KEYLEN * GRUB_CHAR_BIT);
> + }
> +
> + /* 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 % GRUB_CHAR_BIT) != 0)
> + return grub_error (GRUB_ERR_BAD_ARGUMENT,
> + N_("key size is not multiple of %d bits"),
> GRUB_CHAR_BIT);
> + key_size = key_size / GRUB_CHAR_BIT;
> + 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 * GRUB_CHAR_BIT,
> + GRUB_CRYPTODISK_MAX_KEYLEN * GRUB_CHAR_BIT);
> +
> + /* 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);
> +
> + /* 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 == NULL && state[OPTION_HASH].set) || cipher == NULL ||
> dev == NULL ||
> + (keyfile == NULL && state[OPTION_KEYFILE].set) || key_data ==
> NULL ||
> + (uuid == NULL && state[OPTION_UUID].set))
> + {
> + err = grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
> + goto exit;
> + }
> +
> + /* Copy user password from -p option */
> + if (state[OPTION_PASSWORD].set)
> + {
> + /*
> + * Password from the '-p' option is limited to C-string.
> + * Arbitrary data keys are supported via keyfiles.
> + */
> + password_size = grub_strlen (state[OPTION_PASSWORD].arg);
> + grub_memcpy (key_data, state[OPTION_PASSWORD].arg,
> password_size);
> + }
> +
> + /* 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;
> + }
> +
> + /* Get password from console */
> + if (!state[OPTION_KEYFILE].set && 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))
Space between cast and key_data.
> + {
> + err = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("error reading
> password"));
> + goto exit;
> + }
> + /*
> + * Password from interactive console is limited to C-string.
> + * Arbitrary data keys are supported via keyfiles.
> + */
> + password_size = grub_strlen (key_data);
This caused x86_64 to fail to compile with sign mismatch. Should
probably cast to char * as above.
Glenn
> + }
> +
> + /* Warn if hash and keyfile are both provided */
> + if (state[OPTION_KEYFILE].set && 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 (state[OPTION_KEYFILE].set)
> + err = plainmount_configure_keyfile (keyfile, key_data, key_size,
> keyfile_offset);
> + else
> + err = plainmount_configure_password (dev, hash, key_data,
> key_size, password_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);
> +}
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v8 1/1] plainmount: Support plain encryption mode
2022-11-29 17:13 ` Daniel Kiper
2022-12-01 5:58 ` Glenn Washburn
@ 2022-12-02 16:41 ` Maxim Fomin
1 sibling, 0 replies; 12+ messages in thread
From: Maxim Fomin @ 2022-12-02 16:41 UTC (permalink / raw)
To: Daniel Kiper, grub-devel, development, ps
------- Original Message -------
On Tuesday, November 29th, 2022 at 20:13, Daniel Kiper <dkiper@net-space.pl> wrote:
> Hmmm... Why do you say "Writing data" here if the GRUB does not support
> filesystems writes except grubenv file? I think it should be clarified
> what you mean or dropped if it is wrong in this context. Otherwise it is
> confusing.
I do not know 100% of GRUB to be sure that none of modules can write data
(can be used to write data) to unformatted disk partitions. Note, the risk
here is not to write to some proper fs, but to write data at arbitrary disk
or partition location which happens to be incorrectly decrypted fs containing
some data (wrongly decrypted plain mode device looks like random data). If
there is nothing in GRUB which can write data at arbitrary disk locations,
then the sentence can be indeed removed. Although writing data at arbitrary
locations is a strange thing to do, doing so on unformatted disk/partition is
safe, while doing so on incorrectly decrypted in plain mode device will damage
encrypted data. If using plain mode considered expert level, then the warning
may be not needed because it is assumed that a user knows what he is doing.
> > +/ 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));
>
>
> I think grub_strcpy() instead of grub_memcpy() would be more natural in
> string contexts. And then you do not need grub_strlen().
OK.
> Is it safe to assume here that dev->uuid has been zeroed earlier?
It seems Glenn has already answered here.
> > + / 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"));
>
>
> What will happen if somebody passes shorter, probably non-valid, UUID?
> I think we should check UUID is not shorter than something too.
It seems there is agreement to check that UUID size is longer than 1 and less
than PLAINMOUNT_UUID - 1.
Best regards,
Maxim Fomin
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v8 1/1] plainmount: Support plain encryption mode
2022-12-01 21:00 ` Glenn Washburn
@ 2022-12-02 17:11 ` Maxim Fomin
2022-12-24 1:54 ` Glenn Washburn
0 siblings, 1 reply; 12+ messages in thread
From: Maxim Fomin @ 2022-12-02 17:11 UTC (permalink / raw)
To: development, grub-devel, dkiper, ps
------- Original Message -------
On Friday, December 2nd, 2022 at 0:00, Glenn Washburn <development@efficientek.com> wrote:
>
>
> On Sat, 29 Oct 2022 17:40:42 +0000
> Maxim Fomin maxim@fomin.one wrote:
>
> > From 2b1d2deb3f2416cbc3e7d25cbc4141a3078eaf68 Mon Sep 17 00:00:00 2001
> > From: Maxim Fomin maxim@fomin.one
> > Date: Sat, 29 Oct 2022 18:18:58 +0100
> > Subject: [PATCH v8 1/1] plainmount: Support plain encryption mode
> >
> > This patch adds support for plain encryption mode (plain dm-crypt) via
> > new module/command named 'plainmount'.
> >
> > Signed-off-by: Maxim Fomin maxim@fomin.one
> >
> > Difference with v7:
>
>
> Daniel pointed this out, but this isn't a well formed patch. I do very
> much appreciate you adding the differences in as it made it easier to
> look at this. And my suggestion was to use --interdiff or --range-diff
> to the format-patch command, which would properly format things. It
> looks like you just copy pasted the output of "git diff v7 v8".
I will try to fix these issues in v9.
> I'm now compiling this patch and found a few compile issues below.
> You're compile testing this right?
First versions of the patch were tested in pure grub src directory.
Later I switched to directly making and installing GRUB package for
my distro using its source script syntax. It seems this process was
affected by environment options which hided these errors/warnings.
I test the patch on my two old laptops - one with UEFI BIOS (x86_64-efi)
and one is pre-UEFI (i386-pc). I was compiling i386-pc target too,
because otherwise the second laptop was unbootable. During i386-pc
compilation I noticed some warnings related to 'PRIuGRUB_XXX'
macros which were absent during efi target compilation. I noticed
that there are similar warnings in other modules and decided that
there are issues with 'PRIuGRUB_XXX' macros at i386-pc platform at
global level. In any case, these issues didn't cause compilation
fail in my working environment because I would not be able to compile
and boot pre-UEFI lap. Do you use -Werror?
P.S. Also thanks for suggested fixes.
Best regards,
Maxim Fomin
> > diff --git a/docs/grub.texi b/docs/grub.texi
> > index 377969984..34ca6b4f1 100644
> > --- a/docs/grub.texi
> > +++ b/docs/grub.texi
> > @@ -5138,13 +5138,13 @@ to generate password hashes. @xref{Security}.
> >
> > Setup access to the encrypted device in plain mode. Offset of the
> > encrypted -data at the device is specified in terms of 512 byte
> > sectors with the blocklist +data at the device is specified in terms
> > of 512 byte sectors using the blocklist syntax and loopback device.
> > The following example shows how to specify 1MiB offset:
> >
> > @example
> > loopback node (hd0,gpt1)2048+
> > -plainmount node
> > +plainmount node @var{...}
> > @end example
> >
> > The @command{plainmount} command can be used to open LUKS encrypted
> > volume @@ -5155,13 +5155,14 @@ The keyfile path parameter has higher
> > priority than the secret passphrase parameter and is specified with
> > the option @option{-d}. Password data obtained from keyfiles is not
> > hashed and is used directly as a cipher key. An optional offset of
> > password data in the keyfile can be specified with the option
> > -@option{-O} or directly with the option @option{-d} and GRUB
> > blocklist syntax. +@option{-O} or directly with the option
> > @option{-d} and GRUB blocklist syntax, +if the keyfile data can be
> > accessed from a device and is 512 byte aligned. The following example
> > shows both methods to specify password data in the keyfile at offset
> > 1MiB:
> >
> > @example
> > -plainmount -d (hd0,gpt1)2048+
> > -plainmount -d (hd0,gpt1)+ -O 1048576
> > +plainmount -d (hd0,gpt1)2048+ @var{...}
> > +plainmount -d (hd0,gpt1)+ -O 1048576 @var{...}
> > @end example
> >
> > If no keyfile is specified then the password is set to the string
> > specified diff --git a/grub-core/disk/plainmount.c
> > b/grub-core/disk/plainmount.c index 656c5d09f..85ada25bc 100644
> > --- a/grub-core/disk/plainmount.c
> > +++ b/grub-core/disk/plainmount.c
> > @@ -146,8 +146,12 @@ plainmount_configure_password (grub_cryptodisk_t
> > dev, const char *hash, dev->hash = grub_crypto_lookup_md_by_name
> > (hash); len = dev->hash->mdlen;
> >
> > - alloc_size = password_size >= key_size ? password_size : key_size;
> > - p = grub_zalloc (alloc_size + (alloc_size / len));
> > + alloc_size = grub_max (password_size, key_size);
> > + /*
> > + * Allocate buffer for the password and for an added prefix
> > character
> > + * for each hash round ('alloc_size' may not be a multiple of
> > 'len').
> > + */
> > + p = grub_zalloc (alloc_size + (alloc_size / len) + 1);
> > derived_hash = grub_zalloc (GRUB_CRYPTODISK_MAX_KEYLEN * 2);
> > if (p == NULL || derived_hash == NULL)
> > {
> > @@ -170,9 +174,10 @@ plainmount_configure_password (grub_cryptodisk_t
> > dev, const char *hash, if (len > size)
> > len = size;
> >
> > - grub_crypto_hash (dev->hash, dh, p, password_size);
> > + grub_crypto_hash (dev->hash, dh, p, password_size + round);
> > }
> > - grub_memcpy (key_data, derived_hash, GRUB_CRYPTODISK_MAX_KEYLEN *
> > 2);
> > + grub_memcpy (key_data, derived_hash, key_size);
> > +
> > exit:
> > grub_free (p);
> > grub_free (derived_hash);
> > ---
> > docs/grub.texi | 81 +++++++
> > grub-core/Makefile.core.def | 5 +
> > grub-core/disk/plainmount.c | 462
> > ++++++++++++++++++++++++++++++++++++ 3 files changed, 548
> > insertions(+) create mode 100644 grub-core/disk/plainmount.c
> >
> > diff --git a/docs/grub.texi b/docs/grub.texi
> > index 2d6cd8358..34ca6b4f1 100644
> > --- a/docs/grub.texi
> > +++ b/docs/grub.texi
> > @@ -4271,6 +4271,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
> > * play:: Play a tune
> > * probe:: Retrieve device info
> > * rdmsr:: Read values from model-specific
> > registers @@ -4558,6 +4559,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{@pxref{plainmount}} command.
> > @end deffn
> >
> > @node cutmem
> > @@ -5120,6 +5129,78 @@ 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. Offset of the
> > encrypted +data at the device is specified in terms of 512 byte
> > sectors using the blocklist +syntax and loopback device. The
> > following example shows how to specify 1MiB +offset:
> > +
> > +@example
> > +loopback node (hd0,gpt1)2048+
> > +plainmount node @var{...}
> > +@end example
> > +
> > +The @command{plainmount} command can be used to open LUKS encrypted
> > volume +if its master key and parameters (key size, cipher, offset,
> > etc) are known. +
> > +There are two ways to specify a password: a keyfile and a secret
> > passphrase. +The keyfile path parameter has higher priority than the
> > secret passphrase +parameter and is specified with the option
> > @option{-d}. Password data obtained +from keyfiles is not hashed and
> > is used directly as a cipher key. An optional +offset of password
> > data in the keyfile can be specified with the option +@option{-O} or
> > directly with the option @option{-d} and GRUB blocklist syntax, +if
> > the keyfile data can be accessed from a device and is 512 byte
> > aligned. +The following example shows both methods to specify
> > password data in the +keyfile at offset 1MiB: +
> > +@example
> > +plainmount -d (hd0,gpt1)2048+ @var{...}
> > +plainmount -d (hd0,gpt1)+ -O 1048576 @var{...}
> > +@end example
> > +
> > +If no keyfile is specified then the password is set to the string
> > specified +by option @option{-p} or is requested interactively from
> > the console. In both +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
> > @samp{plain} which means that no hashing is done 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. Cipher must be specified +with the mode separated by a dash
> > (for example, @samp{aes-xts-plain64}). Key size +option @option{-s}
> > is the key size of the cipher in bits, not to be confused with +the
> > offset of the key data in a keyfile specified with the @option{-O}
> > option. It +must not exceed 1024 bits, so a 32 byte key would be
> > specified as 256 bits + +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 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
> > the filesystem driver can detect the presence +of a filesystem, but
> > nevertheless will refuse to mount it). + +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. + +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 98714c68d..f4153608c 100644
> > --- a/grub-core/Makefile.core.def
> > +++ b/grub-core/Makefile.core.def
> > @@ -1184,6 +1184,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..85ada25bc
> > --- /dev/null
> > +++ b/grub-core/disk/plainmount.c
> > @@ -0,0 +1,462 @@
> > +/*
> > + * GRUB -- GRand Unified Bootloader
> > + * Copyright (C) 2022 Free Software Foundation, Inc.
> > + *
> > + * 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/.
> > + /
> > +
> > +/ plaimount.c - Open device encrypted in plain mode. /
> > +
> > +#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 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", "failed to set cipher key with
> > code: %d\n", 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);
> > + }
> > + 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)
> > +{
> > + 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);
> > + 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);
>
>
> s/PRIuGRUB_SIZE/PRIuGRUB_UINT64_T/
>
> This compiles fine on x86_64 as is, but fails to compile on i386.
>
> > + return GRUB_ERR_NONE;
> > +}
> > +
> > +
> > +/* Hashes a password into a key and stores it with the cipher. */
> > +static grub_err_t
> > +plainmount_configure_password (grub_cryptodisk_t dev, const char
> > *hash,
> > + grub_uint8_t *key_data, grub_size_t
> > key_size,
> > + grub_size_t password_size)
> > +{
> > + grub_uint8_t *derived_hash, dh;
> > + char p;
> > + unsigned int round, i, len, size;
> > + grub_size_t alloc_size;
> > + grub_err_t err = GRUB_ERR_NONE;
> > +
> > + / Support none (plain) hash /
> > + if (grub_strcmp (hash, "plain") == 0)
> > + {
> > + dev->hash = NULL;
> > + return err;
> > + }
> > +
> > + / Hash argument was checked at main func /
> > + dev->hash = grub_crypto_lookup_md_by_name (hash);
> > + len = dev->hash->mdlen;
> > +
> > + alloc_size = grub_max (password_size, key_size);
> > + /
> > + * Allocate buffer for the password and for an added prefix
> > character
> > + * for each hash round ('alloc_size' may not be a multiple of
> > 'len').
> > + /
> > + p = grub_zalloc (alloc_size + (alloc_size / len) + 1);
> > + derived_hash = grub_zalloc (GRUB_CRYPTODISK_MAX_KEYLEN * 2);
> > + if (p == NULL || derived_hash == NULL)
> > + {
> > + err = grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
> > + goto exit;
> > + }
> > + dh = derived_hash;
> > +
> > + /
> > + * Hash password. Adapted from cryptsetup.
> > + *
> > https://gitlab.com/cryptsetup/cryptsetup/-/blob/main/lib/crypt_plain.c
> > + /
> > + for (round = 0, size = alloc_size; size; round++, dh += len, size
> > -= len)
> > + {
> > + for (i = 0; i < round; i++)
> > + p[i] = 'A';
> > +
> > + grub_memcpy (p + i, (char) key_data, password_size);
> > +
> > + if (len > size)
> > + len = size;
> > +
> > + grub_crypto_hash (dev->hash, dh, p, password_size + round);
> > + }
> > + grub_memcpy (key_data, derived_hash, key_size);
> > +
> > +exit:
> > + grub_free (p);
> > + grub_free (derived_hash);
> > + return err;
> > +}
> > +
> > +
> > +/ 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")"),
>
>
> Also, this compiles fine on x86_64 as is, but fails to compile on i386.
>
> The format code for g_keyfile->size should be PRIuGRUB_UINT64_T.
>
> > + 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;
> > + const gcry_md_spec_t *gcry_hash;
> > + char *diskname, *disklast = NULL, *cipher, *mode, *hash, *keyfile,
> > *uuid;
> > + grub_size_t len, key_size, sector_size, keyfile_offset = 0,
> > password_size = 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 hash /
> > + if (!state[OPTION_KEYFILE].set)
> > + {
> > + gcry_hash = grub_crypto_lookup_md_by_name
> > (state[OPTION_HASH].arg);
> > + if (!gcry_hash)
> > + return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("couldn't load
> > hash %s"),
> > + state[OPTION_HASH].arg);
> > +
> > + 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 * GRUB_CHAR_BIT,
> > + GRUB_CRYPTODISK_MAX_KEYLEN * GRUB_CHAR_BIT);
> > + }
> > +
> > + / 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 % GRUB_CHAR_BIT) != 0)
> > + return grub_error (GRUB_ERR_BAD_ARGUMENT,
> > + N_("key size is not multiple of %d bits"),
> > GRUB_CHAR_BIT);
> > + key_size = key_size / GRUB_CHAR_BIT;
> > + 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 * GRUB_CHAR_BIT,
> > + GRUB_CRYPTODISK_MAX_KEYLEN * GRUB_CHAR_BIT);
> > +
> > + / 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);
> > +
> > + / 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 == NULL && state[OPTION_HASH].set) || cipher == NULL ||
> > dev == NULL ||
> > + (keyfile == NULL && state[OPTION_KEYFILE].set) || key_data ==
> > NULL ||
> > + (uuid == NULL && state[OPTION_UUID].set))
> > + {
> > + err = grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
> > + goto exit;
> > + }
> > +
> > + / Copy user password from -p option /
> > + if (state[OPTION_PASSWORD].set)
> > + {
> > + /
> > + * Password from the '-p' option is limited to C-string.
> > + * Arbitrary data keys are supported via keyfiles.
> > + /
> > + password_size = grub_strlen (state[OPTION_PASSWORD].arg);
> > + grub_memcpy (key_data, state[OPTION_PASSWORD].arg,
> > password_size);
> > + }
> > +
> > + / 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;
> > + }
> > +
> > + / Get password from console */
> > + if (!state[OPTION_KEYFILE].set && 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))
>
>
> Space between cast and key_data.
>
> > + {
> > + err = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("error reading
> > password"));
> > + goto exit;
> > + }
> > + /*
> > + * Password from interactive console is limited to C-string.
> > + * Arbitrary data keys are supported via keyfiles.
> > + */
> > + password_size = grub_strlen (key_data);
>
>
> This caused x86_64 to fail to compile with sign mismatch. Should
> probably cast to char * as above.
>
> Glenn
>
> > + }
> > +
> > + /* Warn if hash and keyfile are both provided /
> > + if (state[OPTION_KEYFILE].set && 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 (state[OPTION_KEYFILE].set)
> > + err = plainmount_configure_keyfile (keyfile, key_data, key_size,
> > keyfile_offset);
> > + else
> > + err = plainmount_configure_password (dev, hash, key_data,
> > key_size, password_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);
> > +}
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v8 1/1] plainmount: Support plain encryption mode
2022-12-02 17:11 ` Maxim Fomin
@ 2022-12-24 1:54 ` Glenn Washburn
2022-12-24 2:09 ` Glenn Washburn
0 siblings, 1 reply; 12+ messages in thread
From: Glenn Washburn @ 2022-12-24 1:54 UTC (permalink / raw)
To: Maxim Fomin; +Cc: grub-devel, dkiper, ps
On Fri, 02 Dec 2022 17:11:23 +0000
Maxim Fomin <maxim@fomin.one> wrote:
> ------- Original Message -------
> On Friday, December 2nd, 2022 at 0:00, Glenn Washburn
> <development@efficientek.com> wrote:
> > I'm now compiling this patch and found a few compile issues below.
> > You're compile testing this right?
>
> First versions of the patch were tested in pure grub src directory.
> Later I switched to directly making and installing GRUB package for
> my distro using its source script syntax. It seems this process was
> affected by environment options which hided these errors/warnings.
>
> I test the patch on my two old laptops - one with UEFI BIOS
> (x86_64-efi) and one is pre-UEFI (i386-pc). I was compiling i386-pc
> target too, because otherwise the second laptop was unbootable.
> During i386-pc compilation I noticed some warnings related to
> 'PRIuGRUB_XXX' macros which were absent during efi target
> compilation. I noticed that there are similar warnings in other
> modules and decided that there are issues with 'PRIuGRUB_XXX' macros
> at i386-pc platform at global level. In any case, these issues didn't
> cause compilation fail in my working environment because I would not
> be able to compile and boot pre-UEFI lap. Do you use -Werror?
I didn't see this until just now. In case you're still interested, no I
don't use -Werror or any special compiler flags. And I'm using gcc
version 10.2.1 from a Debian 11 container.
Glenn
>
> P.S. Also thanks for suggested fixes.
>
> Best regards,
> Maxim Fomin
>
> > > diff --git a/docs/grub.texi b/docs/grub.texi
> > > index 377969984..34ca6b4f1 100644
> > > --- a/docs/grub.texi
> > > +++ b/docs/grub.texi
> > > @@ -5138,13 +5138,13 @@ to generate password hashes.
> > > @xref{Security}.
> > >
> > > Setup access to the encrypted device in plain mode. Offset of the
> > > encrypted -data at the device is specified in terms of 512 byte
> > > sectors with the blocklist +data at the device is specified in
> > > terms of 512 byte sectors using the blocklist syntax and loopback
> > > device. The following example shows how to specify 1MiB offset:
> > >
> > > @example
> > > loopback node (hd0,gpt1)2048+
> > > -plainmount node
> > > +plainmount node @var{...}
> > > @end example
> > >
> > > The @command{plainmount} command can be used to open LUKS
> > > encrypted volume @@ -5155,13 +5155,14 @@ The keyfile path
> > > parameter has higher priority than the secret passphrase
> > > parameter and is specified with the option @option{-d}. Password
> > > data obtained from keyfiles is not hashed and is used directly as
> > > a cipher key. An optional offset of password data in the keyfile
> > > can be specified with the option -@option{-O} or directly with
> > > the option @option{-d} and GRUB blocklist syntax. +@option{-O} or
> > > directly with the option @option{-d} and GRUB blocklist syntax,
> > > +if the keyfile data can be accessed from a device and is 512
> > > byte aligned. The following example shows both methods to specify
> > > password data in the keyfile at offset 1MiB:
> > >
> > > @example
> > > -plainmount -d (hd0,gpt1)2048+
> > > -plainmount -d (hd0,gpt1)+ -O 1048576
> > > +plainmount -d (hd0,gpt1)2048+ @var{...}
> > > +plainmount -d (hd0,gpt1)+ -O 1048576 @var{...}
> > > @end example
> > >
> > > If no keyfile is specified then the password is set to the string
> > > specified diff --git a/grub-core/disk/plainmount.c
> > > b/grub-core/disk/plainmount.c index 656c5d09f..85ada25bc 100644
> > > --- a/grub-core/disk/plainmount.c
> > > +++ b/grub-core/disk/plainmount.c
> > > @@ -146,8 +146,12 @@ plainmount_configure_password
> > > (grub_cryptodisk_t dev, const char *hash, dev->hash =
> > > grub_crypto_lookup_md_by_name (hash); len = dev->hash->mdlen;
> > >
> > > - alloc_size = password_size >= key_size ? password_size :
> > > key_size;
> > > - p = grub_zalloc (alloc_size + (alloc_size / len));
> > > + alloc_size = grub_max (password_size, key_size);
> > > + /*
> > > + * Allocate buffer for the password and for an added prefix
> > > character
> > > + * for each hash round ('alloc_size' may not be a multiple of
> > > 'len').
> > > + */
> > > + p = grub_zalloc (alloc_size + (alloc_size / len) + 1);
> > > derived_hash = grub_zalloc (GRUB_CRYPTODISK_MAX_KEYLEN * 2);
> > > if (p == NULL || derived_hash == NULL)
> > > {
> > > @@ -170,9 +174,10 @@ plainmount_configure_password
> > > (grub_cryptodisk_t dev, const char *hash, if (len > size)
> > > len = size;
> > >
> > > - grub_crypto_hash (dev->hash, dh, p, password_size);
> > > + grub_crypto_hash (dev->hash, dh, p, password_size + round);
> > > }
> > > - grub_memcpy (key_data, derived_hash, GRUB_CRYPTODISK_MAX_KEYLEN
> > > * 2);
> > > + grub_memcpy (key_data, derived_hash, key_size);
> > > +
> > > exit:
> > > grub_free (p);
> > > grub_free (derived_hash);
> > > ---
> > > docs/grub.texi | 81 +++++++
> > > grub-core/Makefile.core.def | 5 +
> > > grub-core/disk/plainmount.c | 462
> > > ++++++++++++++++++++++++++++++++++++ 3 files changed, 548
> > > insertions(+) create mode 100644 grub-core/disk/plainmount.c
> > >
> > > diff --git a/docs/grub.texi b/docs/grub.texi
> > > index 2d6cd8358..34ca6b4f1 100644
> > > --- a/docs/grub.texi
> > > +++ b/docs/grub.texi
> > > @@ -4271,6 +4271,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
> > > * play:: Play a tune
> > > * probe:: Retrieve device info
> > > * rdmsr:: Read values from model-specific
> > > registers @@ -4558,6 +4559,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{@pxref{plainmount}} command.
> > > @end deffn
> > >
> > > @node cutmem
> > > @@ -5120,6 +5129,78 @@ 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. Offset of the
> > > encrypted +data at the device is specified in terms of 512 byte
> > > sectors using the blocklist +syntax and loopback device. The
> > > following example shows how to specify 1MiB +offset:
> > > +
> > > +@example
> > > +loopback node (hd0,gpt1)2048+
> > > +plainmount node @var{...}
> > > +@end example
> > > +
> > > +The @command{plainmount} command can be used to open LUKS
> > > encrypted volume +if its master key and parameters (key size,
> > > cipher, offset, etc) are known. +
> > > +There are two ways to specify a password: a keyfile and a secret
> > > passphrase. +The keyfile path parameter has higher priority than
> > > the secret passphrase +parameter and is specified with the option
> > > @option{-d}. Password data obtained +from keyfiles is not hashed
> > > and is used directly as a cipher key. An optional +offset of
> > > password data in the keyfile can be specified with the option
> > > +@option{-O} or directly with the option @option{-d} and GRUB
> > > blocklist syntax, +if the keyfile data can be accessed from a
> > > device and is 512 byte aligned. +The following example shows both
> > > methods to specify password data in the +keyfile at offset 1MiB: +
> > > +@example
> > > +plainmount -d (hd0,gpt1)2048+ @var{...}
> > > +plainmount -d (hd0,gpt1)+ -O 1048576 @var{...}
> > > +@end example
> > > +
> > > +If no keyfile is specified then the password is set to the string
> > > specified +by option @option{-p} or is requested interactively
> > > from the console. In both +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 @samp{plain} which means that no hashing is done 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. Cipher must be specified +with the mode separated by a
> > > dash (for example, @samp{aes-xts-plain64}). Key size +option
> > > @option{-s} is the key size of the cipher in bits, not to be
> > > confused with +the offset of the key data in a keyfile specified
> > > with the @option{-O} option. It +must not exceed 1024 bits, so a
> > > 32 byte key would be specified as 256 bits + +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
> > > 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 the
> > > filesystem driver can detect the presence +of a filesystem, but
> > > nevertheless will refuse to mount it). + +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. + +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 98714c68d..f4153608c 100644
> > > --- a/grub-core/Makefile.core.def
> > > +++ b/grub-core/Makefile.core.def
> > > @@ -1184,6 +1184,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..85ada25bc
> > > --- /dev/null
> > > +++ b/grub-core/disk/plainmount.c
> > > @@ -0,0 +1,462 @@
> > > +/*
> > > + * GRUB -- GRand Unified Bootloader
> > > + * Copyright (C) 2022 Free Software Foundation, Inc.
> > > + *
> > > + * 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/.
> > > + /
> > > +
> > > +/ plaimount.c - Open device encrypted in plain mode. /
> > > +
> > > +#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 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", "failed to set cipher key with
> > > code: %d\n", 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);
> > > + }
> > > + 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)
> > > +{
> > > + 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);
> > > + 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);
> >
> >
> > s/PRIuGRUB_SIZE/PRIuGRUB_UINT64_T/
> >
> > This compiles fine on x86_64 as is, but fails to compile on i386.
> >
> > > + return GRUB_ERR_NONE;
> > > +}
> > > +
> > > +
> > > +/* Hashes a password into a key and stores it with the cipher. */
> > > +static grub_err_t
> > > +plainmount_configure_password (grub_cryptodisk_t dev, const char
> > > *hash,
> > > + grub_uint8_t *key_data, grub_size_t
> > > key_size,
> > > + grub_size_t password_size)
> > > +{
> > > + grub_uint8_t *derived_hash, dh;
> > > + char p;
> > > + unsigned int round, i, len, size;
> > > + grub_size_t alloc_size;
> > > + grub_err_t err = GRUB_ERR_NONE;
> > > +
> > > + / Support none (plain) hash /
> > > + if (grub_strcmp (hash, "plain") == 0)
> > > + {
> > > + dev->hash = NULL;
> > > + return err;
> > > + }
> > > +
> > > + / Hash argument was checked at main func /
> > > + dev->hash = grub_crypto_lookup_md_by_name (hash);
> > > + len = dev->hash->mdlen;
> > > +
> > > + alloc_size = grub_max (password_size, key_size);
> > > + /
> > > + * Allocate buffer for the password and for an added prefix
> > > character
> > > + * for each hash round ('alloc_size' may not be a multiple of
> > > 'len').
> > > + /
> > > + p = grub_zalloc (alloc_size + (alloc_size / len) + 1);
> > > + derived_hash = grub_zalloc (GRUB_CRYPTODISK_MAX_KEYLEN * 2);
> > > + if (p == NULL || derived_hash == NULL)
> > > + {
> > > + err = grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
> > > + goto exit;
> > > + }
> > > + dh = derived_hash;
> > > +
> > > + /
> > > + * Hash password. Adapted from cryptsetup.
> > > + *
> > > https://gitlab.com/cryptsetup/cryptsetup/-/blob/main/lib/crypt_plain.c
> > > + /
> > > + for (round = 0, size = alloc_size; size; round++, dh += len,
> > > size -= len)
> > > + {
> > > + for (i = 0; i < round; i++)
> > > + p[i] = 'A';
> > > +
> > > + grub_memcpy (p + i, (char) key_data, password_size);
> > > +
> > > + if (len > size)
> > > + len = size;
> > > +
> > > + grub_crypto_hash (dev->hash, dh, p, password_size + round);
> > > + }
> > > + grub_memcpy (key_data, derived_hash, key_size);
> > > +
> > > +exit:
> > > + grub_free (p);
> > > + grub_free (derived_hash);
> > > + return err;
> > > +}
> > > +
> > > +
> > > +/ 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")"),
> >
> >
> > Also, this compiles fine on x86_64 as is, but fails to compile on
> > i386.
> >
> > The format code for g_keyfile->size should be PRIuGRUB_UINT64_T.
> >
> > > + 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;
> > > + const gcry_md_spec_t *gcry_hash;
> > > + char *diskname, *disklast = NULL, *cipher, *mode, *hash,
> > > *keyfile, *uuid;
> > > + grub_size_t len, key_size, sector_size, keyfile_offset = 0,
> > > password_size = 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 hash /
> > > + if (!state[OPTION_KEYFILE].set)
> > > + {
> > > + gcry_hash = grub_crypto_lookup_md_by_name
> > > (state[OPTION_HASH].arg);
> > > + if (!gcry_hash)
> > > + return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("couldn't load
> > > hash %s"),
> > > + state[OPTION_HASH].arg);
> > > +
> > > + 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 * GRUB_CHAR_BIT,
> > > + GRUB_CRYPTODISK_MAX_KEYLEN * GRUB_CHAR_BIT);
> > > + }
> > > +
> > > + / 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 % GRUB_CHAR_BIT) != 0)
> > > + return grub_error (GRUB_ERR_BAD_ARGUMENT,
> > > + N_("key size is not multiple of %d bits"),
> > > GRUB_CHAR_BIT);
> > > + key_size = key_size / GRUB_CHAR_BIT;
> > > + 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 * GRUB_CHAR_BIT,
> > > + GRUB_CRYPTODISK_MAX_KEYLEN * GRUB_CHAR_BIT);
> > > +
> > > + / 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);
> > > +
> > > + / 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 == NULL && state[OPTION_HASH].set) || cipher == NULL
> > > || dev == NULL ||
> > > + (keyfile == NULL && state[OPTION_KEYFILE].set) || key_data ==
> > > NULL ||
> > > + (uuid == NULL && state[OPTION_UUID].set))
> > > + {
> > > + err = grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
> > > + goto exit;
> > > + }
> > > +
> > > + / Copy user password from -p option /
> > > + if (state[OPTION_PASSWORD].set)
> > > + {
> > > + /
> > > + * Password from the '-p' option is limited to C-string.
> > > + * Arbitrary data keys are supported via keyfiles.
> > > + /
> > > + password_size = grub_strlen (state[OPTION_PASSWORD].arg);
> > > + grub_memcpy (key_data, state[OPTION_PASSWORD].arg,
> > > password_size);
> > > + }
> > > +
> > > + / 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;
> > > + }
> > > +
> > > + / Get password from console */
> > > + if (!state[OPTION_KEYFILE].set && 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))
> >
> >
> > Space between cast and key_data.
> >
> > > + {
> > > + err = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("error reading
> > > password"));
> > > + goto exit;
> > > + }
> > > + /*
> > > + * Password from interactive console is limited to C-string.
> > > + * Arbitrary data keys are supported via keyfiles.
> > > + */
> > > + password_size = grub_strlen (key_data);
> >
> >
> > This caused x86_64 to fail to compile with sign mismatch. Should
> > probably cast to char * as above.
> >
> > Glenn
> >
> > > + }
> > > +
> > > + /* Warn if hash and keyfile are both provided /
> > > + if (state[OPTION_KEYFILE].set && 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 (state[OPTION_KEYFILE].set)
> > > + err = plainmount_configure_keyfile (keyfile, key_data, key_size,
> > > keyfile_offset);
> > > + else
> > > + err = plainmount_configure_password (dev, hash, key_data,
> > > key_size, password_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);
> > > +}
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v8 1/1] plainmount: Support plain encryption mode
2022-12-24 1:54 ` Glenn Washburn
@ 2022-12-24 2:09 ` Glenn Washburn
2022-12-28 18:05 ` Maxim Fomin
0 siblings, 1 reply; 12+ messages in thread
From: Glenn Washburn @ 2022-12-24 2:09 UTC (permalink / raw)
To: Maxim Fomin; +Cc: grub-devel, dkiper
On Fri, 23 Dec 2022 19:54:47 -0600
Glenn Washburn <development@efficientek.com> wrote:
> On Fri, 02 Dec 2022 17:11:23 +0000
> Maxim Fomin <maxim@fomin.one> wrote:
>
> > ------- Original Message -------
> > On Friday, December 2nd, 2022 at 0:00, Glenn Washburn
> > <development@efficientek.com> wrote:
> > > I'm now compiling this patch and found a few compile issues below.
> > > You're compile testing this right?
> >
> > First versions of the patch were tested in pure grub src directory.
> > Later I switched to directly making and installing GRUB package for
> > my distro using its source script syntax. It seems this process was
> > affected by environment options which hided these errors/warnings.
> >
> > I test the patch on my two old laptops - one with UEFI BIOS
> > (x86_64-efi) and one is pre-UEFI (i386-pc). I was compiling i386-pc
> > target too, because otherwise the second laptop was unbootable.
> > During i386-pc compilation I noticed some warnings related to
> > 'PRIuGRUB_XXX' macros which were absent during efi target
> > compilation. I noticed that there are similar warnings in other
> > modules and decided that there are issues with 'PRIuGRUB_XXX' macros
> > at i386-pc platform at global level. In any case, these issues
> > didn't cause compilation fail in my working environment because I
> > would not be able to compile and boot pre-UEFI lap. Do you use
> > -Werror?
>
> I didn't see this until just now. In case you're still interested, no
> I don't use -Werror or any special compiler flags. And I'm using gcc
> version 10.2.1 from a Debian 11 container.
Correction, -Werror is being used. Perhaps that's a default compiler
flag on Debian systems.
Glenn
> > P.S. Also thanks for suggested fixes.
> >
> > Best regards,
> > Maxim Fomin
> >
> > > > diff --git a/docs/grub.texi b/docs/grub.texi
> > > > index 377969984..34ca6b4f1 100644
> > > > --- a/docs/grub.texi
> > > > +++ b/docs/grub.texi
> > > > @@ -5138,13 +5138,13 @@ to generate password hashes.
> > > > @xref{Security}.
> > > >
> > > > Setup access to the encrypted device in plain mode. Offset of
> > > > the encrypted -data at the device is specified in terms of 512
> > > > byte sectors with the blocklist +data at the device is
> > > > specified in terms of 512 byte sectors using the blocklist
> > > > syntax and loopback device. The following example shows how to
> > > > specify 1MiB offset:
> > > >
> > > > @example
> > > > loopback node (hd0,gpt1)2048+
> > > > -plainmount node
> > > > +plainmount node @var{...}
> > > > @end example
> > > >
> > > > The @command{plainmount} command can be used to open LUKS
> > > > encrypted volume @@ -5155,13 +5155,14 @@ The keyfile path
> > > > parameter has higher priority than the secret passphrase
> > > > parameter and is specified with the option @option{-d}. Password
> > > > data obtained from keyfiles is not hashed and is used directly
> > > > as a cipher key. An optional offset of password data in the
> > > > keyfile can be specified with the option -@option{-O} or
> > > > directly with the option @option{-d} and GRUB blocklist syntax.
> > > > +@option{-O} or directly with the option @option{-d} and GRUB
> > > > blocklist syntax, +if the keyfile data can be accessed from a
> > > > device and is 512 byte aligned. The following example shows
> > > > both methods to specify password data in the keyfile at offset
> > > > 1MiB:
> > > >
> > > > @example
> > > > -plainmount -d (hd0,gpt1)2048+
> > > > -plainmount -d (hd0,gpt1)+ -O 1048576
> > > > +plainmount -d (hd0,gpt1)2048+ @var{...}
> > > > +plainmount -d (hd0,gpt1)+ -O 1048576 @var{...}
> > > > @end example
> > > >
> > > > If no keyfile is specified then the password is set to the
> > > > string specified diff --git a/grub-core/disk/plainmount.c
> > > > b/grub-core/disk/plainmount.c index 656c5d09f..85ada25bc 100644
> > > > --- a/grub-core/disk/plainmount.c
> > > > +++ b/grub-core/disk/plainmount.c
> > > > @@ -146,8 +146,12 @@ plainmount_configure_password
> > > > (grub_cryptodisk_t dev, const char *hash, dev->hash =
> > > > grub_crypto_lookup_md_by_name (hash); len = dev->hash->mdlen;
> > > >
> > > > - alloc_size = password_size >= key_size ? password_size :
> > > > key_size;
> > > > - p = grub_zalloc (alloc_size + (alloc_size / len));
> > > > + alloc_size = grub_max (password_size, key_size);
> > > > + /*
> > > > + * Allocate buffer for the password and for an added prefix
> > > > character
> > > > + * for each hash round ('alloc_size' may not be a multiple of
> > > > 'len').
> > > > + */
> > > > + p = grub_zalloc (alloc_size + (alloc_size / len) + 1);
> > > > derived_hash = grub_zalloc (GRUB_CRYPTODISK_MAX_KEYLEN * 2);
> > > > if (p == NULL || derived_hash == NULL)
> > > > {
> > > > @@ -170,9 +174,10 @@ plainmount_configure_password
> > > > (grub_cryptodisk_t dev, const char *hash, if (len > size)
> > > > len = size;
> > > >
> > > > - grub_crypto_hash (dev->hash, dh, p, password_size);
> > > > + grub_crypto_hash (dev->hash, dh, p, password_size + round);
> > > > }
> > > > - grub_memcpy (key_data, derived_hash,
> > > > GRUB_CRYPTODISK_MAX_KEYLEN
> > > > * 2);
> > > > + grub_memcpy (key_data, derived_hash, key_size);
> > > > +
> > > > exit:
> > > > grub_free (p);
> > > > grub_free (derived_hash);
> > > > ---
> > > > docs/grub.texi | 81 +++++++
> > > > grub-core/Makefile.core.def | 5 +
> > > > grub-core/disk/plainmount.c | 462
> > > > ++++++++++++++++++++++++++++++++++++ 3 files changed, 548
> > > > insertions(+) create mode 100644 grub-core/disk/plainmount.c
> > > >
> > > > diff --git a/docs/grub.texi b/docs/grub.texi
> > > > index 2d6cd8358..34ca6b4f1 100644
> > > > --- a/docs/grub.texi
> > > > +++ b/docs/grub.texi
> > > > @@ -4271,6 +4271,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
> > > > * play:: Play a tune
> > > > * probe:: Retrieve device info
> > > > * rdmsr:: Read values from model-specific
> > > > registers @@ -4558,6 +4559,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{@pxref{plainmount}} command.
> > > > @end deffn
> > > >
> > > > @node cutmem
> > > > @@ -5120,6 +5129,78 @@ 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. Offset of
> > > > the encrypted +data at the device is specified in terms of 512
> > > > byte sectors using the blocklist +syntax and loopback device.
> > > > The following example shows how to specify 1MiB +offset:
> > > > +
> > > > +@example
> > > > +loopback node (hd0,gpt1)2048+
> > > > +plainmount node @var{...}
> > > > +@end example
> > > > +
> > > > +The @command{plainmount} command can be used to open LUKS
> > > > encrypted volume +if its master key and parameters (key size,
> > > > cipher, offset, etc) are known. +
> > > > +There are two ways to specify a password: a keyfile and a
> > > > secret passphrase. +The keyfile path parameter has higher
> > > > priority than the secret passphrase +parameter and is specified
> > > > with the option @option{-d}. Password data obtained +from
> > > > keyfiles is not hashed and is used directly as a cipher key. An
> > > > optional +offset of password data in the keyfile can be
> > > > specified with the option +@option{-O} or directly with the
> > > > option @option{-d} and GRUB blocklist syntax, +if the keyfile
> > > > data can be accessed from a device and is 512 byte aligned.
> > > > +The following example shows both methods to specify password
> > > > data in the +keyfile at offset 1MiB: + +@example
> > > > +plainmount -d (hd0,gpt1)2048+ @var{...}
> > > > +plainmount -d (hd0,gpt1)+ -O 1048576 @var{...}
> > > > +@end example
> > > > +
> > > > +If no keyfile is specified then the password is set to the
> > > > string specified +by option @option{-p} or is requested
> > > > interactively from the console. In both +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 @samp{plain} which means that
> > > > no hashing is done 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. Cipher must be specified +with the mode
> > > > separated by a dash (for example, @samp{aes-xts-plain64}). Key
> > > > size +option @option{-s} is the key size of the cipher in bits,
> > > > not to be confused with +the offset of the key data in a
> > > > keyfile specified with the @option{-O} option. It +must not
> > > > exceed 1024 bits, so a 32 byte key would be specified as 256
> > > > bits + +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 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 the filesystem driver can
> > > > detect the presence +of a filesystem, but nevertheless will
> > > > refuse to mount it). + +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. + +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 98714c68d..f4153608c 100644
> > > > --- a/grub-core/Makefile.core.def
> > > > +++ b/grub-core/Makefile.core.def
> > > > @@ -1184,6 +1184,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..85ada25bc
> > > > --- /dev/null
> > > > +++ b/grub-core/disk/plainmount.c
> > > > @@ -0,0 +1,462 @@
> > > > +/*
> > > > + * GRUB -- GRand Unified Bootloader
> > > > + * Copyright (C) 2022 Free Software Foundation, Inc.
> > > > + *
> > > > + * 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/.
> > > > + /
> > > > +
> > > > +/ plaimount.c - Open device encrypted in plain mode. /
> > > > +
> > > > +#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 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", "failed to set cipher key with
> > > > code: %d\n", 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);
> > > > + }
> > > > + 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)
> > > > +{
> > > > + 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);
> > > > + 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);
> > >
> > >
> > > s/PRIuGRUB_SIZE/PRIuGRUB_UINT64_T/
> > >
> > > This compiles fine on x86_64 as is, but fails to compile on i386.
> > >
> > > > + return GRUB_ERR_NONE;
> > > > +}
> > > > +
> > > > +
> > > > +/* Hashes a password into a key and stores it with the cipher.
> > > > */ +static grub_err_t
> > > > +plainmount_configure_password (grub_cryptodisk_t dev, const
> > > > char *hash,
> > > > + grub_uint8_t *key_data, grub_size_t
> > > > key_size,
> > > > + grub_size_t password_size)
> > > > +{
> > > > + grub_uint8_t *derived_hash, dh;
> > > > + char p;
> > > > + unsigned int round, i, len, size;
> > > > + grub_size_t alloc_size;
> > > > + grub_err_t err = GRUB_ERR_NONE;
> > > > +
> > > > + / Support none (plain) hash /
> > > > + if (grub_strcmp (hash, "plain") == 0)
> > > > + {
> > > > + dev->hash = NULL;
> > > > + return err;
> > > > + }
> > > > +
> > > > + / Hash argument was checked at main func /
> > > > + dev->hash = grub_crypto_lookup_md_by_name (hash);
> > > > + len = dev->hash->mdlen;
> > > > +
> > > > + alloc_size = grub_max (password_size, key_size);
> > > > + /
> > > > + * Allocate buffer for the password and for an added prefix
> > > > character
> > > > + * for each hash round ('alloc_size' may not be a multiple of
> > > > 'len').
> > > > + /
> > > > + p = grub_zalloc (alloc_size + (alloc_size / len) + 1);
> > > > + derived_hash = grub_zalloc (GRUB_CRYPTODISK_MAX_KEYLEN * 2);
> > > > + if (p == NULL || derived_hash == NULL)
> > > > + {
> > > > + err = grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
> > > > + goto exit;
> > > > + }
> > > > + dh = derived_hash;
> > > > +
> > > > + /
> > > > + * Hash password. Adapted from cryptsetup.
> > > > + *
> > > > https://gitlab.com/cryptsetup/cryptsetup/-/blob/main/lib/crypt_plain.c
> > > > + /
> > > > + for (round = 0, size = alloc_size; size; round++, dh += len,
> > > > size -= len)
> > > > + {
> > > > + for (i = 0; i < round; i++)
> > > > + p[i] = 'A';
> > > > +
> > > > + grub_memcpy (p + i, (char) key_data, password_size);
> > > > +
> > > > + if (len > size)
> > > > + len = size;
> > > > +
> > > > + grub_crypto_hash (dev->hash, dh, p, password_size + round);
> > > > + }
> > > > + grub_memcpy (key_data, derived_hash, key_size);
> > > > +
> > > > +exit:
> > > > + grub_free (p);
> > > > + grub_free (derived_hash);
> > > > + return err;
> > > > +}
> > > > +
> > > > +
> > > > +/ 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")"),
> > >
> > >
> > > Also, this compiles fine on x86_64 as is, but fails to compile on
> > > i386.
> > >
> > > The format code for g_keyfile->size should be PRIuGRUB_UINT64_T.
> > >
> > > > + 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;
> > > > + const gcry_md_spec_t *gcry_hash;
> > > > + char *diskname, *disklast = NULL, *cipher, *mode, *hash,
> > > > *keyfile, *uuid;
> > > > + grub_size_t len, key_size, sector_size, keyfile_offset = 0,
> > > > password_size = 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 hash /
> > > > + if (!state[OPTION_KEYFILE].set)
> > > > + {
> > > > + gcry_hash = grub_crypto_lookup_md_by_name
> > > > (state[OPTION_HASH].arg);
> > > > + if (!gcry_hash)
> > > > + return grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("couldn't load
> > > > hash %s"),
> > > > + state[OPTION_HASH].arg);
> > > > +
> > > > + 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 * GRUB_CHAR_BIT,
> > > > + GRUB_CRYPTODISK_MAX_KEYLEN * GRUB_CHAR_BIT);
> > > > + }
> > > > +
> > > > + / 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 % GRUB_CHAR_BIT) != 0)
> > > > + return grub_error (GRUB_ERR_BAD_ARGUMENT,
> > > > + N_("key size is not multiple of %d bits"),
> > > > GRUB_CHAR_BIT);
> > > > + key_size = key_size / GRUB_CHAR_BIT;
> > > > + 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 * GRUB_CHAR_BIT,
> > > > + GRUB_CRYPTODISK_MAX_KEYLEN * GRUB_CHAR_BIT);
> > > > +
> > > > + / 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);
> > > > +
> > > > + / 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 == NULL && state[OPTION_HASH].set) || cipher == NULL
> > > > || dev == NULL ||
> > > > + (keyfile == NULL && state[OPTION_KEYFILE].set) || key_data ==
> > > > NULL ||
> > > > + (uuid == NULL && state[OPTION_UUID].set))
> > > > + {
> > > > + err = grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
> > > > + goto exit;
> > > > + }
> > > > +
> > > > + / Copy user password from -p option /
> > > > + if (state[OPTION_PASSWORD].set)
> > > > + {
> > > > + /
> > > > + * Password from the '-p' option is limited to C-string.
> > > > + * Arbitrary data keys are supported via keyfiles.
> > > > + /
> > > > + password_size = grub_strlen (state[OPTION_PASSWORD].arg);
> > > > + grub_memcpy (key_data, state[OPTION_PASSWORD].arg,
> > > > password_size);
> > > > + }
> > > > +
> > > > + / 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;
> > > > + }
> > > > +
> > > > + / Get password from console */
> > > > + if (!state[OPTION_KEYFILE].set && 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))
> > >
> > >
> > > Space between cast and key_data.
> > >
> > > > + {
> > > > + err = grub_error (GRUB_ERR_BAD_ARGUMENT, N_("error reading
> > > > password"));
> > > > + goto exit;
> > > > + }
> > > > + /*
> > > > + * Password from interactive console is limited to C-string.
> > > > + * Arbitrary data keys are supported via keyfiles.
> > > > + */
> > > > + password_size = grub_strlen (key_data);
> > >
> > >
> > > This caused x86_64 to fail to compile with sign mismatch. Should
> > > probably cast to char * as above.
> > >
> > > Glenn
> > >
> > > > + }
> > > > +
> > > > + /* Warn if hash and keyfile are both provided /
> > > > + if (state[OPTION_KEYFILE].set && 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 (state[OPTION_KEYFILE].set)
> > > > + err = plainmount_configure_keyfile (keyfile, key_data,
> > > > key_size, keyfile_offset);
> > > > + else
> > > > + err = plainmount_configure_password (dev, hash, key_data,
> > > > key_size, password_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);
> > > > +}
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v8 1/1] plainmount: Support plain encryption mode
2022-12-24 2:09 ` Glenn Washburn
@ 2022-12-28 18:05 ` Maxim Fomin
2023-01-10 18:19 ` Glenn Washburn
0 siblings, 1 reply; 12+ messages in thread
From: Maxim Fomin @ 2022-12-28 18:05 UTC (permalink / raw)
To: grub-devel; +Cc: development, dkiper
------- Original Message -------
On Saturday, December 24th, 2022 at 2:09 AM, Glenn Washburn <development@efficientek.com> wrote:
>
> On Fri, 23 Dec 2022 19:54:47 -0600
> Glenn Washburn development@efficientek.com wrote:
>
> > On Fri, 02 Dec 2022 17:11:23 +0000
> > Maxim Fomin maxim@fomin.one wrote:
> >
> > > ------- Original Message -------
> > > On Friday, December 2nd, 2022 at 0:00, Glenn Washburn
> > > development@efficientek.com wrote:
> > >
> > > > I'm now compiling this patch and found a few compile issues below.
> > > > You're compile testing this right?
> > >
> > > First versions of the patch were tested in pure grub src directory.
> > > Later I switched to directly making and installing GRUB package for
> > > my distro using its source script syntax. It seems this process was
> > > affected by environment options which hided these errors/warnings.
> > >
> > > I test the patch on my two old laptops - one with UEFI BIOS
> > > (x86_64-efi) and one is pre-UEFI (i386-pc). I was compiling i386-pc
> > > target too, because otherwise the second laptop was unbootable.
> > > During i386-pc compilation I noticed some warnings related to
> > > 'PRIuGRUB_XXX' macros which were absent during efi target
> > > compilation. I noticed that there are similar warnings in other
> > > modules and decided that there are issues with 'PRIuGRUB_XXX' macros
> > > at i386-pc platform at global level. In any case, these issues
> > > didn't cause compilation fail in my working environment because I
> > > would not be able to compile and boot pre-UEFI lap. Do you use
> > > -Werror?
> >
> > I didn't see this until just now. In case you're still interested, no
> > I don't use -Werror or any special compiler flags. And I'm using gcc
> > version 10.2.1 from a Debian 11 container.
>
>
> Correction, -Werror is being used. Perhaps that's a default compiler
> flag on Debian systems.
>
> Glenn
>
This explains why you have found these issues. However, it does not explain
how you can compile grub with -Werror because currently there are following
warnings in x86_64-efi mode:
grub-core/lib/libgcrypt-grub/mpi/mpi-internal.h:150:24: warning: variable ‘_ql’ set but not used [-Wunused-but-set-variable]
grub-core/lib/libgcrypt-grub/mpi/mpih-div.c:53:9: warning: variable ‘dummy’ set but not used [-Wunused-but-set-variable]
When I was working with the patch earlier this year I remember having these
and several more warnings which prevented me from using -Werror. Back then
I have removed the switch and have forgotten about this issue completely.
Best regards,
Maxim Fomin
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v8 1/1] plainmount: Support plain encryption mode
2022-12-28 18:05 ` Maxim Fomin
@ 2023-01-10 18:19 ` Glenn Washburn
2023-01-14 12:07 ` Maxim Fomin
0 siblings, 1 reply; 12+ messages in thread
From: Glenn Washburn @ 2023-01-10 18:19 UTC (permalink / raw)
To: Maxim Fomin; +Cc: grub-devel, dkiper
On Wed, 28 Dec 2022 18:05:11 +0000
Maxim Fomin <maxim@fomin.one> wrote:
> ------- Original Message -------
> On Saturday, December 24th, 2022 at 2:09 AM, Glenn Washburn
> <development@efficientek.com> wrote:
> >
> > On Fri, 23 Dec 2022 19:54:47 -0600
> > Glenn Washburn development@efficientek.com wrote:
> >
> > > On Fri, 02 Dec 2022 17:11:23 +0000
> > > Maxim Fomin maxim@fomin.one wrote:
> > >
> > > > ------- Original Message -------
> > > > On Friday, December 2nd, 2022 at 0:00, Glenn Washburn
> > > > development@efficientek.com wrote:
> > > >
> > > > > I'm now compiling this patch and found a few compile issues
> > > > > below. You're compile testing this right?
> > > >
> > > > First versions of the patch were tested in pure grub src
> > > > directory. Later I switched to directly making and installing
> > > > GRUB package for my distro using its source script syntax. It
> > > > seems this process was affected by environment options which
> > > > hided these errors/warnings.
> > > >
> > > > I test the patch on my two old laptops - one with UEFI BIOS
> > > > (x86_64-efi) and one is pre-UEFI (i386-pc). I was compiling
> > > > i386-pc target too, because otherwise the second laptop was
> > > > unbootable. During i386-pc compilation I noticed some warnings
> > > > related to 'PRIuGRUB_XXX' macros which were absent during efi
> > > > target compilation. I noticed that there are similar warnings
> > > > in other modules and decided that there are issues with
> > > > 'PRIuGRUB_XXX' macros at i386-pc platform at global level. In
> > > > any case, these issues didn't cause compilation fail in my
> > > > working environment because I would not be able to compile and
> > > > boot pre-UEFI lap. Do you use -Werror?
> > >
> > > I didn't see this until just now. In case you're still
> > > interested, no I don't use -Werror or any special compiler flags.
> > > And I'm using gcc version 10.2.1 from a Debian 11 container.
> >
> >
> > Correction, -Werror is being used. Perhaps that's a default compiler
> > flag on Debian systems.
> >
> > Glenn
> >
>
> This explains why you have found these issues. However, it does not
> explain how you can compile grub with -Werror because currently there
> are following warnings in x86_64-efi mode:
> grub-core/lib/libgcrypt-grub/mpi/mpi-internal.h:150:24: warning:
> variable ‘_ql’ set but not used [-Wunused-but-set-variable]
> grub-core/lib/libgcrypt-grub/mpi/mpih-div.c:53:9: warning: variable
> ‘dummy’ set but not used [-Wunused-but-set-variable]
Ok I looked at this a little more, I'm getting these warnings when
compiling the speedtest module. They are not being treated as errors.
By default GRUB will use -Werror when building the target, unless
--disable-werror is specified. However, the gcc command for the
speedtest module doesn't have -Werror but a bunch of -W* and does not
include -Wunused-but-set-variable, which is why the compile doesn't
error. So it seems that -Werror is being changed to constituent -W*
options and some -W* are left out in my case (haven't found where this
happens yet). I'm not setting any CFLAGS that might affect this, are
you? Since, I've not seen anyone else complaining about it here, I
suspect this is something odd about your build environment.
What is your build environment? (distro, GCC version)
>
> When I was working with the patch earlier this year I remember having
> these and several more warnings which prevented me from using
> -Werror. Back then I have removed the switch and have forgotten about
> this issue completely.
Did you remove the switch by using the --disable-werror configure
option? If not, how?
Glenn
>
> Best regards,
> Maxim Fomin
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v8 1/1] plainmount: Support plain encryption mode
2023-01-10 18:19 ` Glenn Washburn
@ 2023-01-14 12:07 ` Maxim Fomin
0 siblings, 0 replies; 12+ messages in thread
From: Maxim Fomin @ 2023-01-14 12:07 UTC (permalink / raw)
To: development; +Cc: grub-devel, dkiper
------- Original Message -------
On Tuesday, January 10th, 2023 at 6:19 PM, Glenn Washburn <development@efficientek.com> wrote:
>
> On Wed, 28 Dec 2022 18:05:11 +0000
> Maxim Fomin maxim@fomin.one wrote:
>
> > ------- Original Message -------
> > On Saturday, December 24th, 2022 at 2:09 AM, Glenn Washburn
> > development@efficientek.com wrote:
> >
> > > On Fri, 23 Dec 2022 19:54:47 -0600
> > > Glenn Washburn development@efficientek.com wrote:
> > >
> > > > On Fri, 02 Dec 2022 17:11:23 +0000
> > > > Maxim Fomin maxim@fomin.one wrote:
> > > >
> > > > > ------- Original Message -------
> > > > > On Friday, December 2nd, 2022 at 0:00, Glenn Washburn
> > > > > development@efficientek.com wrote:
> > > > >
> > > > > > I'm now compiling this patch and found a few compile issues
> > > > > > below. You're compile testing this right?
> > > > >
> > > > > First versions of the patch were tested in pure grub src
> > > > > directory. Later I switched to directly making and installing
> > > > > GRUB package for my distro using its source script syntax. It
> > > > > seems this process was affected by environment options which
> > > > > hided these errors/warnings.
> > > > >
> > > > > I test the patch on my two old laptops - one with UEFI BIOS
> > > > > (x86_64-efi) and one is pre-UEFI (i386-pc). I was compiling
> > > > > i386-pc target too, because otherwise the second laptop was
> > > > > unbootable. During i386-pc compilation I noticed some warnings
> > > > > related to 'PRIuGRUB_XXX' macros which were absent during efi
> > > > > target compilation. I noticed that there are similar warnings
> > > > > in other modules and decided that there are issues with
> > > > > 'PRIuGRUB_XXX' macros at i386-pc platform at global level. In
> > > > > any case, these issues didn't cause compilation fail in my
> > > > > working environment because I would not be able to compile and
> > > > > boot pre-UEFI lap. Do you use -Werror?
> > > >
> > > > I didn't see this until just now. In case you're still
> > > > interested, no I don't use -Werror or any special compiler flags.
> > > > And I'm using gcc version 10.2.1 from a Debian 11 container.
> > >
> > > Correction, -Werror is being used. Perhaps that's a default compiler
> > > flag on Debian systems.
> > >
> > > Glenn
> >
> > This explains why you have found these issues. However, it does not
> > explain how you can compile grub with -Werror because currently there
> > are following warnings in x86_64-efi mode:
> > grub-core/lib/libgcrypt-grub/mpi/mpi-internal.h:150:24: warning:
> > variable ‘_ql’ set but not used [-Wunused-but-set-variable]
> > grub-core/lib/libgcrypt-grub/mpi/mpih-div.c:53:9: warning: variable
> > ‘dummy’ set but not used [-Wunused-but-set-variable]
>
>
> Ok I looked at this a little more, I'm getting these warnings when
> compiling the speedtest module. They are not being treated as errors.
> By default GRUB will use -Werror when building the target, unless
> --disable-werror is specified. However, the gcc command for the
> speedtest module doesn't have -Werror but a bunch of -W* and does not
> include -Wunused-but-set-variable, which is why the compile doesn't
> error. So it seems that -Werror is being changed to constituent -W*
> options and some -W* are left out in my case (haven't found where this
> happens yet). I'm not setting any CFLAGS that might affect this, are
> you? Since, I've not seen anyone else complaining about it here, I
> suspect this is something odd about your build environment.
>
> What is your build environment? (distro, GCC version)
It has two steps. Firstly I compile GRUB sources in local git folder
to see errors and warnings. Then I make GRUB package for my linux
distro. Until October I was building GRUB package with custom archlinux
PKGBUILD which approximately follows this script:
https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=grub-luks-keyfile-git
CFLAGS in local makepkg.conf were CFLAGS="-march=sandybridge -mtune=generic
-O2 -pipe -fno-plt -fexceptions -Wp,-D_FORTIFY_SOURCE=2 -Wformat -Werror=format-security
-fstack-clash-protection -fcf-protection"
Since October I used a local copy of this gentoo ebuild to build GRUB:
https://gitweb.gentoo.org/repo/gentoo.git/tree/sys-boot/grub/grub-9999.ebuild
CFLAGS in make.conf is set to "-O2 -pipe -march=sandybridge"
In both local scripts src uri was changed from mainstream to local git repo
which hosted plainmount commits. Both scripts enable --disable-Werror
configure option, which means several warnings (including 'libgcrypt-grub/mpi/')
were ignored.
gcc -v on current build environment:
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-pc-linux-gnu/12/lto-wrapper
Target: x86_64-pc-linux-gnu
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 12.2.1 20221231 (Gentoo 12.2.1_p20221231 p8)
> > When I was working with the patch earlier this year I remember having
> > these and several more warnings which prevented me from using
> > -Werror. Back then I have removed the switch and have forgotten about
> > this issue completely.
>
>
> Did you remove the switch by using the --disable-werror configure
> option? If not, how?
>
> Glenn
In my build environment --disable-werror is set in local src folder and
in GRUB package scripts.
It seems I have these mpi warnings because in my build environment some
macros are left undefined which makes variables '_ql' and 'dummy' left
unused. I did not investigate these warnings deeper.
Best regards,
Maxim Fomin
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-01-14 12:07 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-29 17:40 [PATCH v8 1/1] plainmount: Support plain encryption mode Maxim Fomin
2022-11-29 17:13 ` Daniel Kiper
2022-12-01 5:58 ` Glenn Washburn
2022-12-01 19:38 ` Daniel Kiper
2022-12-02 16:41 ` Maxim Fomin
2022-12-01 21:00 ` Glenn Washburn
2022-12-02 17:11 ` Maxim Fomin
2022-12-24 1:54 ` Glenn Washburn
2022-12-24 2:09 ` Glenn Washburn
2022-12-28 18:05 ` Maxim Fomin
2023-01-10 18:19 ` Glenn Washburn
2023-01-14 12:07 ` Maxim Fomin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.