Re: cryptodisk enabled returns to rescue prompt

Re: cryptodisk enabled returns to rescue prompt

[Andrei Borzenkov]

07.11.2015 07:13, westlake пишет:
> enabling GRUB_ENABLE_CRYPTODISK=y has crypt prompting only once on
> bootup, is it possible to have an option with grub-install or another
> option here with GRUB_EMABLE_CRYPTODISK so that the keypass prompts in a
> loop? (a wrong passphrase typed brings the user to a grub rescue shell
> and has to issue ctl-alt-delete which is imho not very presentable to

You need to just do

cryptomount -u xxxxxxxxxxx
normal

I am not convinced that being stuck in password entry loop is better. 
May be a command that retries to execute embedded config and enter 
normal may be useful.

> staff) -- I understand this is all in mbr bootcode so I suppose the best
> place to implement this would be when first generating the code in order
> to keep it small.
>
> it would be imho really great if this can be implemented

There was suggested patch that allowed multiple password entry attempts 
for LUKS. It was a part of patch series that implemented other things. 
May be it could be reconsidered if rebased to not depend on other changes.

Re: cryptodisk enabled returns to rescue prompt

[westlake]

actually the crypt would be internal inside the grub mbr that gets 
generated because even if i comment out cryptmount -u in grub.cfg and 
apply update-initramfs&&update-grub there's no effect(there still is a 
passphrase prompt), here the cryptsetup is taking effect prior the 
reading of grub.cfg..

it shouldn't also be hard to implement. afaik the lacking of 
documentation for using GRUB_ENABLE_CRYPTODISK='y' tells me this should 
be an area encouraging suggestion and feedback from those who are 
bothering to using it... It works but it can be improved.  Here my main 
concern is a "grub rescue" shouldn't be showing up right after the first 
failed attempt.

thanks


On 07/11/15 02:28 AM, Andrei Borzenkov wrote:
> 07.11.2015 07:13, westlake пишет:
>> enabling GRUB_ENABLE_CRYPTODISK=y has crypt prompting only once on
>> bootup, is it possible to have an option with grub-install or another
>> option here with GRUB_EMABLE_CRYPTODISK so that the keypass prompts in a
>> loop? (a wrong passphrase typed brings the user to a grub rescue shell
>> and has to issue ctl-alt-delete which is imho not very presentable to
>
> You need to just do
>
> cryptomount -u xxxxxxxxxxx
> normal
>
> I am not convinced that being stuck in password entry loop is better.
> May be a command that retries to execute embedded config and enter
> normal may be useful.
>
>> staff) -- I understand this is all in mbr bootcode so I suppose the best
>> place to implement this would be when first generating the code in order
>> to keep it small.
>>
>> it would be imho really great if this can be implemented
>
> There was suggested patch that allowed multiple password entry attempts
> for LUKS. It was a part of patch series that implemented other things.
> May be it could be reconsidered if rebased to not depend on other changes.
>

Re: cryptodisk enabled returns to rescue prompt

[Andrei Borzenkov]

07.11.2015 11:58, westlake пишет:
> actually the crypt would be internal inside the grub mbr that gets
> generated because even if i comment out cryptmount -u in grub.cfg and
> apply update-initramfs&&update-grub there's no effect(there still is a
> passphrase prompt), here the cryptsetup is taking effect prior the
> reading of grub.cfg..
>
> it shouldn't also be hard to implement.

Send patch for review.

Re: cryptodisk enabled returns to rescue prompt

[Jason Kushmaul]

[-- Attachment #1: Type: text/plain, Size: 3548 bytes --]

I'm sending this in a new thread using `git format-patch`

On Tue, Jun 25, 2019 at 12:48 AM Jason Kushmaul <jasonkushmaul@gmail.com>
wrote:

> Hello,
>
> I'm new to the list.  I've been working on this myself for personal
> reasons.  I think this should be viewed as an accessibility issue, not as a
> convenience.  Those that have motor impairments have a very difficult time
> booting their machines without a reboot, or rescue.
>
> Please see my patch attached which adds documentation, configuration, and
> implementation.  The patch is against master
> (4e7b5bb3be69633ed860cb74b0ef2c84a839523d) but I can change that if you
> like.
> I tested this in a virtualbox.
>
> If a more formal request is needed, I prepared this before finding this
> existing post:
>
> **************************************
> FEATURE
> **************************************
> Add LUKS full disk encryption passphrase retry config and logic, providing
> accessibility to people with motor impairments, Parkinson's, etc.
>
> **************************************
> JUSTIFICATION
> **************************************
> As of master (4e7b5bb3be69633ed860cb74b0ef2c84a839523d), I've found no
> other tickets mentioning this.
>
> When cryptodisk attempts to recover the key, it asks for the passphrase,
> just once.  You are required to reboot, or know how to recover grub
> yourself manually.
>
> Many people enjoy the confidence of encrypting their full disk, including
> /boot. However, for those who may be plagued with motor impairment, shaking
> of hands, twitches in the fingers as they type, one would have severe
> barriers to enjoying that same level of security due to bneing required to
> type a passphrase once, and getting it right without having to reboot again.
>
> I know there is a concern for security.  This configuration would default
> to 1 attempt as it is today, and those who chose, may choose any amount
> they like up to 256.  Defaulting to 1 will maintain exactly the same
> behavior for users upgrading.
>
> **************************************
> STEPS TO REPRODUCE
> **************************************
> Steps:
> * Setup
> * Observation
>
> Setup:
> * Encrypt the full disk using luks so that the /boot is contained in luks
> disk.
> * Use a passphrase 32 characters long with an equal distribution of
> [0-9a-zA-Z] and specials.
> * Boot and wait for passphrase prompt.
>
> Observation:
> Enter the incorrect password and hit enter.  You are not asked to retry,
> or allowed to configure it before install  of grub on the full disk crypto
> setup.  You must then type the full blown steps to ask again, or simply
> CTL-ALT-DEL and wait 45 more seconds...
>
> With the patches, one can configure with a "-t" and a number of retry
> attempts.  They will see the same prompt, see a notification about key
> recovery in progress, and if incorrect, another message stating such, but
> then be prompted again on failure.
>
> **************************************
> SUMMARY
> **************************************
> Those with motor impairments have a barrier preventing them from enjoying
>  LUKS full disk encryption with strong passphrases.  Causing them a need to
> reboot until correct.
>
> This is easy to reproduce, but a little more difficult to realize how
> people with impaired motor function would struggle.
>
> The changes in this patch offer a configurable way to increase the number
> of attempts from 1, to any number <= 256, but maintains the default
> behavior as all users expect, which is just 1 attempt.
>
>
>
>

[-- Attachment #2: Type: text/html, Size: 4060 bytes --]

Re: cryptodisk enabled returns to rescue prompt

[Jason Kushmaul]

[-- Attachment #1.1: Type: text/plain, Size: 3252 bytes --]

Hello,

I'm new to the list.  I've been working on this myself for personal
reasons.  I think this should be viewed as an accessibility issue, not as a
convenience.  Those that have motor impairments have a very difficult time
booting their machines without a reboot, or rescue.

Please see my patch attached which adds documentation, configuration, and
implementation.  The patch is against master
(4e7b5bb3be69633ed860cb74b0ef2c84a839523d) but I can change that if you
like.
I tested this in a virtualbox.

If a more formal request is needed, I prepared this before finding this
existing post:

**************************************
FEATURE
**************************************
Add LUKS full disk encryption passphrase retry config and logic, providing
accessibility to people with motor impairments, Parkinson's, etc.

**************************************
JUSTIFICATION
**************************************
As of master (4e7b5bb3be69633ed860cb74b0ef2c84a839523d), I've found no
other tickets mentioning this.

When cryptodisk attempts to recover the key, it asks for the passphrase,
just once.  You are required to reboot, or know how to recover grub
yourself manually.

Many people enjoy the confidence of encrypting their full disk, including
/boot. However, for those who may be plagued with motor impairment, shaking
of hands, twitches in the fingers as they type, one would have severe
barriers to enjoying that same level of security due to bneing required to
type a passphrase once, and getting it right without having to reboot again.

I know there is a concern for security.  This configuration would default
to 1 attempt as it is today, and those who chose, may choose any amount
they like up to 256.  Defaulting to 1 will maintain exactly the same
behavior for users upgrading.

**************************************
STEPS TO REPRODUCE
**************************************
Steps:
* Setup
* Observation

Setup:
* Encrypt the full disk using luks so that the /boot is contained in luks
disk.
* Use a passphrase 32 characters long with an equal distribution of
[0-9a-zA-Z] and specials.
* Boot and wait for passphrase prompt.

Observation:
Enter the incorrect password and hit enter.  You are not asked to retry, or
allowed to configure it before install  of grub on the full disk crypto
setup.  You must then type the full blown steps to ask again, or simply
CTL-ALT-DEL and wait 45 more seconds...

With the patches, one can configure with a "-t" and a number of retry
attempts.  They will see the same prompt, see a notification about key
recovery in progress, and if incorrect, another message stating such, but
then be prompted again on failure.

**************************************
SUMMARY
**************************************
Those with motor impairments have a barrier preventing them from enjoying
 LUKS full disk encryption with strong passphrases.  Causing them a need to
reboot until correct.

This is easy to reproduce, but a little more difficult to realize how
people with impaired motor function would struggle.

The changes in this patch offer a configurable way to increase the number
of attempts from 1, to any number <= 256, but maintains the default
behavior as all users expect, which is just 1 attempt.

[-- Attachment #1.2: Type: text/html, Size: 3628 bytes --]

[-- Attachment #2: master_luks_key_recover_with_attempts.patch --]
[-- Type: text/x-patch, Size: 11642 bytes --]

diff --git a/docs/grub.texi b/docs/grub.texi
index 308b25074..00df49fa1 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -1508,6 +1508,11 @@ check for encrypted disks and generate additional commands needed to access
 them during boot.  Note that in this case unattended boot is not possible
 because GRUB will wait for passphrase to unlock encrypted container.
 
+@item GRUB_CRYPTODISK_ATTEMPTS
+If set, @command{grub-install} will allow the provided number of attempts
+on key recovery.  The default if not present, or outside of [1,256] is 1.
+Currently only supported for LUKS.
+
 @item GRUB_INIT_TUNE
 Play a tune on the speaker when GRUB starts.  This is particularly useful
 for users unable to see the screen.  The value of this option is passed
@@ -4194,12 +4199,13 @@ Alias for @code{hashsum --hash crc32 arg @dots{}}. See command @command{hashsum}
 @node cryptomount
 @subsection cryptomount
 
-@deffn Command cryptomount device|@option{-u} uuid|@option{-a}|@option{-b}
+@deffn Command cryptomount [@option{-t} tries] device|@option{-u} uuid|@option{-a}|@option{-b}
 Setup access to encrypted device. If necessary, passphrase
 is requested interactively. Option @var{device} configures specific grub device
 (@pxref{Naming convention}); option @option{-u} @var{uuid} configures device
 with specified @var{uuid}; option @option{-a} configures all detected encrypted
-devices; option @option{-b} configures all geli containers that have boot flag set.
+devices; option @option{-b} configures all geli containers that have boot flag set;
+option @option{-t}, luks only, configures passphrase retry attempts, defaulting to 1.
 
 GRUB suports devices encrypted using LUKS and geli. Note that necessary modules (@var{luks} and @var{geli}) have to be loaded manually before this command can
 be used.
diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
index 5037768fc..3b90e550e 100644
--- a/grub-core/disk/cryptodisk.c
+++ b/grub-core/disk/cryptodisk.c
@@ -41,6 +41,7 @@ static const struct grub_arg_option options[] =
     /* TRANSLATORS: It's still restricted to cryptodisks only.  */
     {"all", 'a', 0, N_("Mount all."), 0, 0},
     {"boot", 'b', 0, N_("Mount all volumes with `boot' flag set."), 0, 0},
+    {"tries", 't', 0, N_("Max passphrase attempts."), 0, GRUB_KEY_ARG},
     {0, 0, 0, 0, 0, 0}
   };
 
@@ -807,6 +808,7 @@ grub_util_cryptodisk_get_uuid (grub_disk_t disk)
 
 #endif
 
+unsigned max_attempts;
 static int check_boot, have_it;
 static char *search_uuid;
 
@@ -820,7 +822,7 @@ cryptodisk_close (grub_cryptodisk_t dev)
 }
 
 static grub_err_t
-grub_cryptodisk_scan_device_real (const char *name, grub_disk_t source)
+grub_cryptodisk_scan_device_real(const char *name, grub_disk_t source)
 {
   grub_err_t err;
   grub_cryptodisk_t dev;
@@ -933,7 +935,23 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
 
   if (argc < 1 && !state[1].set && !state[2].set)
     return grub_error (GRUB_ERR_BAD_ARGUMENT, "device name required");
-
+  
+  /* Default to original behavior of 1 attempt */
+  max_attempts = 1;
+  if (state[3].set)
+  {
+      const char *max_attempts_str = state[3].arg;
+      if (max_attempts_str)
+      {
+          char *end = NULL;
+          long  tmpl = grub_strtol (max_attempts_str, &end, 10);
+          if (tmpl > 0 && tmpl <= 256) 
+          {
+              max_attempts = (unsigned) tmpl;
+          }
+      }
+  }
+  
   have_it = 0;
   if (state[0].set)
     {
@@ -1141,7 +1159,7 @@ GRUB_MOD_INIT (cryptodisk)
 {
   grub_disk_dev_register (&grub_cryptodisk_dev);
   cmd = grub_register_extcmd ("cryptomount", grub_cmd_cryptomount, 0,
-			      N_("SOURCE|-u UUID|-a|-b"),
+                             N_("[-t] SOURCE|-u UUID|-a|-b"),
 			      N_("Mount a crypto device."), options);
   grub_procfs_register ("luks_script", &luks_script);
 }
diff --git a/grub-core/disk/luks.c b/grub-core/disk/luks.c
index 86c50c612..02b999f07 100644
--- a/grub-core/disk/luks.c
+++ b/grub-core/disk/luks.c
@@ -308,10 +308,10 @@ configure_ciphers (grub_disk_t disk, const char *check_uuid,
 }
 
 static grub_err_t
-luks_recover_key (grub_disk_t source,
-		  grub_cryptodisk_t dev)
+luks_recover_key_attempt (grub_disk_t source,
+                          grub_cryptodisk_t dev,
+                          struct grub_luks_phdr header)
 {
-  struct grub_luks_phdr header;
   grub_size_t keysize;
   grub_uint8_t *split_key = NULL;
   char passphrase[MAX_PASSPHRASE] = "";
@@ -322,10 +322,6 @@ luks_recover_key (grub_disk_t source,
   grub_size_t max_stripes = 1;
   char *tmp;
 
-  err = grub_disk_read (source, 0, 0, sizeof (header), &header);
-  if (err)
-    return err;
-
   grub_puts_ (N_("Attempting to decrypt master key..."));
   keysize = grub_be_to_cpu32 (header.keyBytes);
   if (keysize > GRUB_CRYPTODISK_MAX_KEYLEN)
@@ -467,6 +463,31 @@ luks_recover_key (grub_disk_t source,
   return GRUB_ACCESS_DENIED;
 }
 
+
+extern unsigned max_attempts;
+static grub_err_t
+luks_recover_key (grub_disk_t source,
+                  grub_cryptodisk_t dev)
+{
+    grub_err_t err;
+    struct grub_luks_phdr header;
+    err = grub_disk_read (source, 0, 0, sizeof (header), &header);
+    if (err)
+        return err;
+    
+    if (!max_attempts || max_attempts > 256)
+      max_attempts = 1;
+    err = GRUB_ERR_FILE_NOT_FOUND;
+    for (unsigned attempt = 0; attempt < max_attempts; attempt++)
+    {
+        grub_errno = GRUB_ERR_NONE;
+        err = luks_recover_key_attempt(source, dev, header);
+        if (err != GRUB_ERR_ACCESS_DENIED)
+            break;
+    }
+    return err;
+}
+
 struct grub_cryptodisk_dev luks_crypto = {
   .scan = configure_ciphers,
   .recover_key = luks_recover_key
diff --git a/grub-core/osdep/aros/config.c b/grub-core/osdep/aros/config.c
index c82d0ea8e..683823233 100644
--- a/grub-core/osdep/aros/config.c
+++ b/grub-core/osdep/aros/config.c
@@ -73,6 +73,22 @@ grub_util_load_config (struct grub_util_config *cfg)
   v = getenv ("GRUB_ENABLE_CRYPTODISK");
   if (v && v[0] == 'y' && v[1] == '\0')
     cfg->is_cryptodisk_enabled = 1;
+  
+  cfg->cryptodisk_attempts = 1;
+  v = getenv("GRUB_CRYPTODISK_ATTEMPTS");
+  if (v)
+    {
+      char *end = NULL;
+      long attempts = grub_strtol(v, &end, 10);
+      if (attempts > 0 && attempts <= 256)
+        {
+          cfg->cryptodisk_attempts = (unsigned) attempts;
+        }
+      else
+        {
+          grub_util_warn (_("grub_util_load_config: GRUB_CRYPTODISK_ATTEMPTS must be postive and less than 256"));
+        }
+    }
 
   v = getenv ("GRUB_DISTRIBUTOR");
   if (v)
diff --git a/grub-core/osdep/unix/config.c b/grub-core/osdep/unix/config.c
index 65effa9f3..3cf0f21c6 100644
--- a/grub-core/osdep/unix/config.c
+++ b/grub-core/osdep/unix/config.c
@@ -78,6 +78,22 @@ grub_util_load_config (struct grub_util_config *cfg)
   if (v && v[0] == 'y' && v[1] == '\0')
     cfg->is_cryptodisk_enabled = 1;
 
+  cfg->cryptodisk_attempts = 1;
+  v = getenv("GRUB_CRYPTODISK_ATTEMPTS");
+  if (v)
+  {
+      char *end = NULL;
+      long attempts = grub_strtol(v, &end, 10);
+      if (attempts > 0 && attempts <= 256)
+      {
+          cfg->cryptodisk_attempts = (unsigned) attempts;
+      }
+      else
+      {
+          grub_util_warn (_("grub_util_load_config: GRUB_CRYPTODISK_ATTEMPTS must be postive and less than 256"));
+      }
+  }
+
   v = getenv ("GRUB_DISTRIBUTOR");
   if (v)
     cfg->grub_distributor = xstrdup (v);
@@ -105,8 +121,8 @@ grub_util_load_config (struct grub_util_config *cfg)
       *ptr++ = *iptr;
     }
 
-  strcpy (ptr, "'; printf \"GRUB_ENABLE_CRYPTODISK=%s\\nGRUB_DISTRIBUTOR=%s\\n\" "
-	  "\"$GRUB_ENABLE_CRYPTODISK\" \"$GRUB_DISTRIBUTOR\"");
+  strcpy (ptr, "'; printf \"GRUB_CRYPTODISK_ATTEMPTS=%s\\nGRUB_ENABLE_CRYPTODISK=%s\\nGRUB_DISTRIBUTOR=%s\\n\" "
+  "\"$GRUB_CRYPTODISK_ATTEMPTS\" \"$GRUB_ENABLE_CRYPTODISK\" \"$GRUB_DISTRIBUTOR\"");
 
   argv[2] = script;
   argv[3] = '\0';
diff --git a/grub-core/osdep/windows/config.c b/grub-core/osdep/windows/config.c
index 928ab1a49..2e3808f22 100644
--- a/grub-core/osdep/windows/config.c
+++ b/grub-core/osdep/windows/config.c
@@ -41,6 +41,22 @@ grub_util_load_config (struct grub_util_config *cfg)
   if (v && v[0] == 'y' && v[1] == '\0')
     cfg->is_cryptodisk_enabled = 1;
 
+  cfg->cryptodisk_attempts = 1;
+  v = getenv("GRUB_CRYPTODISK_ATTEMPTS");
+  if (v)
+  {
+      char *end = NULL;
+      long attempts = grub_strtol(v, &end, 10);
+      if (attempts > 0 && attempts <= 256)
+      {
+          cfg->cryptodisk_attempts = (unsigned) attempts;
+      }
+      else
+      {
+          grub_util_warn (_("grub_util_load_config: GRUB_CRYPTODISK_ATTEMPTS must be postive and less than 256"));
+      }
+  }
+
   v = getenv ("GRUB_DISTRIBUTOR");
   if (v)
     cfg->grub_distributor = xstrdup (v);
diff --git a/include/grub/emu/config.h b/include/grub/emu/config.h
index 875d5896c..fd4cb9e33 100644
--- a/include/grub/emu/config.h
+++ b/include/grub/emu/config.h
@@ -36,6 +36,7 @@ grub_util_get_localedir (void);
 struct grub_util_config
 {
   int is_cryptodisk_enabled;
+  unsigned cryptodisk_attempts;
   char *grub_distributor;
 };
 
diff --git a/util/config.c b/util/config.c
index ebcdd8f5e..9d460712c 100644
--- a/util/config.c
+++ b/util/config.c
@@ -32,6 +32,20 @@ grub_util_parse_config (FILE *f, struct grub_util_config *cfg, int simple)
     {
       const char *ptr;
       for (ptr = buffer; *ptr && grub_isspace (*ptr); ptr++);
+        if (grub_strncmp (ptr, "GRUB_CRYPTODISK_ATTEMPTS=",
+            sizeof ("GRUB_CRYPTODISK_ATTEMPTS=") - 1) == 0)
+          {
+            ptr += sizeof ("GRUB_CRYPTODISK_ATTEMPTS=") - 1;
+            char *end = NULL;
+            long tmpl = strtol(ptr, &end, 10);
+            if (!tmpl || tmpl > 256) {
+                grub_util_warn (_("grub_util_parse_config: GRUB_CRYPTODISK_ATTEMPTS was out of range=%ld, defaulting to 1"), tmpl);
+                cfg->cryptodisk_attempts = 1;
+            }
+            cfg->cryptodisk_attempts = tmpl;
+            continue;
+          }
+
       if (grub_strncmp (ptr, "GRUB_ENABLE_CRYPTODISK=",
 			sizeof ("GRUB_ENABLE_CRYPTODISK=") - 1) == 0)
 	{
diff --git a/util/grub-install.c b/util/grub-install.c
index 8a55ad4b8..081fcc446 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -509,7 +509,7 @@ have_bootdev (enum grub_install_plat pl)
 }
 
 static void
-probe_cryptodisk_uuid (grub_disk_t disk)
+probe_cryptodisk_uuid (grub_disk_t disk, unsigned attempts)
 {
   grub_disk_memberlist_t list = NULL, tmp;
 
@@ -520,7 +520,7 @@ probe_cryptodisk_uuid (grub_disk_t disk)
     }
   while (list)
     {
-      probe_cryptodisk_uuid (list->disk);
+      probe_cryptodisk_uuid (list->disk, attempts);
       tmp = list->next;
       free (list);
       list = tmp;
@@ -532,8 +532,8 @@ probe_cryptodisk_uuid (grub_disk_t disk)
 	load_cfg_f = grub_util_fopen (load_cfg, "wb");
       have_load_cfg = 1;
 
-      fprintf (load_cfg_f, "cryptomount -u %s\n",
-	      uuid);
+      fprintf (load_cfg_f, "cryptomount -u %s -t %u\n",
+               uuid, attempts);
     }
 }
 
@@ -1531,7 +1531,7 @@ main (int argc, char *argv[])
       if (config.is_cryptodisk_enabled)
 	{
 	  if (grub_dev->disk)
-	    probe_cryptodisk_uuid (grub_dev->disk);
+	    probe_cryptodisk_uuid (grub_dev->disk, config.cryptodisk_attempts);
 
 	  for (curdrive = grub_drives + 1; *curdrive; curdrive++)
 	    {
@@ -1539,7 +1539,7 @@ main (int argc, char *argv[])
 	      if (!dev)
 		continue;
 	      if (dev->disk)
-		probe_cryptodisk_uuid (dev->disk);
+		probe_cryptodisk_uuid (dev->disk, config.cryptodisk_attempts);
 	      grub_device_close (dev);
 	    }
 	}