From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1p04Ax-0000P1-S9 for mharc-grub-devel@gnu.org; Tue, 29 Nov 2022 12:13:43 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1p04Av-0000Ok-FN for grub-devel@gnu.org; Tue, 29 Nov 2022 12:13:41 -0500 Received: from dibed.net-space.pl ([84.10.22.86]) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_3DES_EDE_CBC_SHA1:192) (Exim 4.90_1) (envelope-from ) id 1p04As-0002yF-GA for grub-devel@gnu.org; Tue, 29 Nov 2022 12:13:41 -0500 Received: from router-fw.i.net-space.pl ([192.168.52.1]:45754 "EHLO tomti.i.net-space.pl") by router-fw-old.i.net-space.pl with ESMTP id S2161637AbiK2RNN (ORCPT ); Tue, 29 Nov 2022 18:13:13 +0100 X-Comment: RFC 2476 MSA function at dibed.net-space.pl logged sender identity as: dkiper Date: Tue, 29 Nov 2022 18:13:11 +0100 From: Daniel Kiper To: Maxim Fomin Cc: "grub-devel@gnu.org" , "development@efficientek.com" , "ps@pks.im" Subject: Re: [PATCH v8 1/1] plainmount: Support plain encryption mode Message-ID: <20221129171311.k5yrlmmt4c4bo4r5@tomti.i.net-space.pl> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Received-SPF: pass client-ip=84.10.22.86; envelope-from=dkiper@net-space.pl; helo=dibed.net-space.pl X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 29 Nov 2022 17:13:41 -0000 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 > 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 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 . > + */ > + > +/* plaimount.c - Open device encrypted in plain mode. */ > + > +#include > +#include > +#include > +#include > +#include > +#include > + > + 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