Re: [PATCH 1/2] EFI: console: Do not set colorstate until the first text output

2022-01-30 Thread Javier Martinez Canillas
Hello Hans,

Thanks for the patch.

On 1/28/22 12:43, Hans de Goede wrote:
> GRUB_MOD_INIT(normal) does an unconditional:
> 
> grub_env_set ("color_normal", "light-gray/black");
> 
> which triggers a grub_term_setcolorstate() call. The original version
> of the "efi/console: Do not set text-mode until we actually need it" patch:
> https://lists.gnu.org/archive/html/grub-devel/2018-03/msg00125.html
> 
> Protected against this by caching the requested state in
> grub_console_setcolorstate () and then only applying it when the first
> text output actually happens. During refactoring to move the
> grub_console_setcolorstate () up higher in the grub-core/term/efi/console.c
> file the code to cache the color-state + bail early was accidentally
> dropped.
> 
> Restore the cache the color-state + bail early behavior from the original.
> 
> Cc: Javier Martinez Canillas 
> Fixes: 2d7c3abd871f ("efi/console: Do not set text-mode until we actually 
> need it")
> Signed-off-by: Hans de Goede 
> ---
>  grub-core/term/efi/console.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/grub-core/term/efi/console.c b/grub-core/term/efi/console.c
> index 2f1ae85ba..c44b2ac31 100644
> --- a/grub-core/term/efi/console.c
> +++ b/grub-core/term/efi/console.c
> @@ -82,6 +82,16 @@ grub_console_setcolorstate (struct grub_term_output *term
>  {
>grub_efi_simple_text_output_interface_t *o;
>  
> +  if (grub_efi_is_finished || text_mode != GRUB_TEXT_MODE_AVAILABLE)
> +{
> +  /*
> +   * Cache colorstate changes before the first text-output, this avoids
> +   * "color_normal" environment writes causing a switch to textmode.
> +   */
> +  text_colorstate = state;
> +  return;
> +}
> +
>if (grub_efi_is_finished)
>  return;
>

Indeed, sorry for messing this when addressing the concerns raised by
Daniel in your original patch. The fix looks good to me.

Reviewed-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Linux Engineering
Red Hat


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH 2/2] EFI: console: Do not set cursor until the first text output

2022-01-30 Thread Javier Martinez Canillas
On 1/28/22 12:43, Hans de Goede wrote:
> To allow flickerfree boot the EFI console code does not call
> grub_efi_set_text_mode (1) until some text is actually output.
> 
> Depending on if the output text is because of an error loading
> e.g. the .cfg file; or because of showing the menu the cursor needs
> to be on or off when the first text is shown.
> 
> So far the cursor was hardcoded to being on, but this is causing
> drawing artifacts + slow drawing of the menu as reported here:
> https://bugzilla.redhat.com/show_bug.cgi?id=1946969
> 
> Handle the cursorstate in the same way as the colorstate to fix this,
> when no text has been output yet, just cache the cursorstate and
> then use the last set value when the first text is output.
> 
> Fixes: 2d7c3abd871f ("efi/console: Do not set text-mode until we actually 
> need it")
> Signed-off-by: Hans de Goede 
> ---

Reviewed-by: Javier Martinez Canillas 

Best regards,
-- 
Javier Martinez Canillas
Linux Engineering
Red Hat


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH 1/2] plainmount: Support decryption of devices encrypted in plain mode.

2022-01-30 Thread Maxim Fomin
This patch introduces support for plain encryption mode (plain dm-crypt) via
new module and command named 'plainmount'. The command allows to open devices
encrypted in plain mode (without LUKS) with following syntax:

plainmount  -h hash -c cipher -o offset -s size -k key-size
   -z sector-size -d keyfile -O keyfile offset -l keyfile-size

 can be some partition or GPT UUID, keyfile can be /some/path.

Despite moving plain mode code into separate module, it still depends on
cryptodisk.c because cryptodisk module defines whole grub infrastructure
of encrypted devices.

Principal feature of this patch is supporting GPT partition UUID to point to
specific partition. The patch also includes two other minor features: using
disk block as a key (instead of keyfile) and support for 1024/2048/4096 byte
blocks in plain mode (cryptsetup added large block support in plain mode after
introducing them in LUKS2 mode - since circa version 2.4).

This is fully reworked version of '0004-Cryptomount-support-plain-dm-crypt'
patch which is member of John Lane crypto enhancement patch series. It also
includes '0007-Add-support-for-using-a-whole-device-as-a-keyfile' patch. They
were sent in grub-devel mailing list as:

From a8f9e3dcece89c179e89414abe89985c7ab1e03f Mon Sep 17 00:00:00 2001
From: John Lane 
Date: Fri, 26 Jun 2015 22:09:52 +0100
Subject: [PATCH 4/7] Cryptomount support plain dm-crypt

From ef720d0d44b8d97a83950ced0df1ce1bcf8cd988 Mon Sep 17 00:00:00 2001
From: Paul Gideon Dann 
Date: Tue, 19 Jul 2016 12:36:37 +0100
Subject: [PATCH 7/7] Add support for using a whole device as a keyfile
---
 grub-core/Makefile.core.def |   5 +
 grub-core/disk/plainmount.c | 678 
 2 files changed, 683 insertions(+)
 create mode 100644 grub-core/disk/plainmount.c

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 8022e1c0a..ce8478055 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -1184,6 +1184,11 @@ module = {
   common = disk/luks.c;
 };

+module = {
+  name = plainmount;
+  common = disk/plainmount.c;
+};
+
 module = {
   name = luks2;
   common = disk/luks2.c;
diff --git a/grub-core/disk/plainmount.c b/grub-core/disk/plainmount.c
new file mode 100644
index 0..4654cec96
--- /dev/null
+++ b/grub-core/disk/plainmount.c
@@ -0,0 +1,678 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2003,2007,2010,2011,2019  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 .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+GRUB_MOD_LICENSE ("GPLv3+");
+
+#define GRUB_PLAINMOUNT_UUID""
+#define GRUB_PLAINMOUNT_CIPHER  "aes-cbc-essiv:sha256"
+#define GRUB_PLAINMOUNT_DIGEST  "ripemd160"
+#define GRUB_PLAINMOUNT_KEY_SIZE256
+#define GRUB_PLAINMOUNT_SECTOR_SIZE 512
+
+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},
+{"offset", 'o', 0, N_("Device offset (512 bit blocks)."), 0, ARG_TYPE_INT},
+{"size", 'b', 0, N_("Size of device (512 byte blocks)."), 0, ARG_TYPE_INT},
+{"key-size", 's', 0, N_("Key size (in bits)."), 0, ARG_TYPE_INT},
+{"sector-size", 'z', 0, N_("Device sector size."), 0, ARG_TYPE_INT},
+{"keyfile", 'd', 0, N_("Keyfile/disk path."), 0, ARG_TYPE_STRING},
+{"keyfile-offset", 'O', 0, N_("Keyfile offset (512 bit blocks)."), 0, 
ARG_TYPE_INT},
+{"keyfile-size", 'l', 0, N_("Keyfile data size (in bits)."), 0, 
ARG_TYPE_INT},
+{0, 0, 0, 0, 0, 0}
+  };
+
+struct grub_plainmount_args
+{
+  char *key_data, *cipher, *mode, *hash, *keyfile;
+  grub_size_t offset, size, key_size, sector_size, keyfile_offset, 
keyfile_size;
+  grub_disk_t disk;
+};
+typedef struct grub_plainmount_args *grub_plainmount_args_t;
+
+struct grub_plainmount_iterate_args
+{
+  char *uuid, *diskname;
+};
+
+
+/* Disk iterate callback */
+static int grub_plainmount_scan_real (const char *name, void *data)
+{
+  int ret = 0;
+  struct grub_plainmount_iterate_args *args = data;
+  grub_disk_t source = NULL, disk = NULL;
+  struct grub_partition *partition;
+  struct grub_gpt_partentry entry;
+  grub_gpt_part_guid_t *guid;
+

[PATCH 0/2] Support plain encryption mode.

2022-01-30 Thread Maxim Fomin
This patch adds support for plain encryption mode (plain dm-crypt) via new
module/command named 'plainmount'. This is an extension of previous patch
(member of crypto enhancement patch series) sent to grub-devel by John Lane.
The plain mode patch was fully reworked, the most important changes are:

1. The code is rewritten as a separate 'plainmount' module and command. This
change addresses several issues raised in previous patch discussion: complex
implementation and expanding too much cryptomount parameter list. Plainmount
module still depends on several functions in cryptodisk module just like LUKS,
LUKS2 and geli modules. I tried to minimize link to cryptodisk, but some link
is unavoidable.

2. Plainmount now uses GPT partition unique UUID to point to exact partition.
Plain mode does not have special header (as LUKS/LUKS2 does), so it does not
have any UUID. Absence of UUID was key issue in discussions of original plain
mode patch - without UUID any link like 'hdX,gptY' in grub.cfg can be broken
in multiboot setup because hdX can map to different disks each boot.

3. The patch brings support for keyfiles and using disk blocks as a keyfile.
This functionality was added in cryptodisk module (and thus was available for
plain mode) via other patch from 'crypto enhancement' patch series. Thus all
features from original patch series are available in plainmount module (except
detached header which is LUKS only feature and multiple password attempts which
are undefined in plain mode). The patch also brings support for large blocks
(1024/2048/4096 byte).

Some issues are still not resolved. Although the problem with UUID is solved,
no changes were made in grub-install or grub-mkconfig. Currently user must
manually edit grub.cfg to add plainmount command. Also, inserting plainmount
in standalone grub does not work because plainmount needs crypto arguments.
Theoretically they can be stored somewhere in efi core image - in this case
plain mode can be fully supported like LUKS. Currently user must manually edit
grub.cfg to use plainmount (or manually type in console). Still, this helps in
some use cases (including some complex setup scenarios). Also, except opening
plain mode encrypted disks plainmount command can decrypt LUKS volumes if
master key is known.

Maxim Fomin (2):
  plainmount: Support decryption of devices encrypted in plain mode.
  plainmount: Document new command syntax and options.

 docs/grub.texi  |  52 +++
 grub-core/Makefile.core.def |   5 +
 grub-core/disk/plainmount.c | 678 
 3 files changed, 735 insertions(+)
 create mode 100644 grub-core/disk/plainmount.c

--
2.35.1



___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH 2/2] plainmount: Document new command syntax and options.

2022-01-30 Thread Maxim Fomin
---
 docs/grub.texi | 52 ++
 1 file changed, 52 insertions(+)

diff --git a/docs/grub.texi b/docs/grub.texi
index a72d73ce3..6ce280389 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -4031,6 +4031,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
@@ -4323,6 +4324,9 @@ 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.
+
+Support for plain encryption mode (plain dm-crypt) is provided via separate
+plainmount command.
 @end deffn

 @node cutmem
@@ -4889,6 +4893,54 @@ to generate password hashes.  @xref{Security}.
 @end deffn


+@node plainmount
+@subsection plainmount
+
+@deffn Command plainmount device [@option{-h} hash] [@option{-c} cipher] 
[@option{-s} key size] [@option{-o} offset] [@option{-z} disk sector size] 
[@option{-d} keyfile] [@option{-O} keyfile offset] [@option{-l} keyfile size]
+
+Setup access to encrypted in plain mode device. Arguments @var{device} and
+@var{keyfile} can be real device names (like hd0,gpt1) or UUIDs. Note, only
+GPT partition unique UUIDs (different from type UUID) are supported. Unlike
+cryptomount syntax, plainmount accepts UUIDs with dash separators. Keyfile
+can be a regular file or some device block (segment). The length of the key
+from either keyfile or device block is limited by @option{-O} (offset of
+block from device start) and @option{-l} (key length) options and cannot be
+larger than cryptomount limit (currently 128 bytes). Attempts to use longer
+key (for example, specifying large file or device without option @option{-l}
+option) will result in error.
+
+If option @option{-d} is not given, then password is requested interactively
+(only one attempt is given) and options @option{-O} and @option {-l} are
+ignored. Provided password is hashed with hash specified by option @option{-h}.
+If keyfile or device block is provided, hash option is ignored.
+
+Options @option{-c}, @option{-h} and @option{-s} have default values
+aes-cbc-essiv:sha256, ripemd160 and 256 bits respectively. In general, any
+plainmount error (not related to failure to find specified disk and keyfile) is
+caused by mismatch between version of key size and cipher or hash.
+
+Option @option{-z} determines encrypted disk block size and can be 512 
(default)
+or 1024, 2048, 4096. Disk sector size in plain mode is set at encryption
+time. Attempt to decrypt already created device with diffferent sector size 
will
+result in error.
+
+Options @option{-o} and @option{-O} must be specified as 512 byte blocks,
+irrespective of whether underlying encrypted disk has 4096 byte sectors. These
+options are used only to specify the length of the offset and do not impact
+@option{-z} option.
+
+Options @option{-s} and @option{-l} must be specified in bits.
+
+Note, all these arguments (cipher, hash, key size, disk offset and disk sector
+size) must match those which were used to setup encrypted device in plain mode.
+If any of options does not match those used during initial encryption, 
plainmount
+will return garbage data and GRUB will report that filesystem of opened disk is
+unknown. Decrypted disks are named as (cryptoX) and have linear numeration with
+other decrypted devices opened by cryptodisk. If encrypted disk hosts some high
+level abstraction (like LVM2 or MDRAID) it will be created under separate name.
+@end deffn
+
+
 @node play
 @subsection play

--
2.35.1





___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel