Re: [PATCH] broken ESC navigation if authentication is used

[Andrei Borzenkov <arvidjaar@gmail.com>]

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

08.11.2015 23:55, Vladimir 'φ-coder/phcoder' Serbinenko пишет:
> On 11.06.2015 05:55, Andrei Borzenkov wrote:
>> В Wed, 10 Jun 2015 21:35:51 +0200
>> "Vladimir 'phcoder' Serbinenko" <phcoder@gmail.com> пишет:
>>
>>> This patch may allow to escape to shell if menu was called from context
>>> without menu entries. This may happen inadvertently I.a. when using
>>> configfile. You need to add an additional parameter to indicate whether
>>> it's OK to break from menu
>>
>> Could you explain? Grub does
>>
>> grub_enter_normal
>>    grub_normal_execute
>>      grub_show_menu
>>    grub_cmdline_run
>>
>> if after processing config file there are no menu entries we do not
>> even call grub_show_menu. And even if we do, after return from it there
>> is mandatory authentication in grub_cmdline_run.
>>
> Imagine something like following:
> grub.cfg:
> # Use another config file
> configfile grub2.cfg
> grub2.cfg:
> superusers=root
> ....
> Then pressing escape would lead you to the parent context where there is
> no password protection.
> Question is whether this is a misconfiguration on grub.cfg side (i.a.
> should have been source, not configfile) or something to deal on code side.


OK what about attached patch? it moves authentication where it belongs - 
to return from nested configfile.

[-- Attachment #2: submenu-auth.diff --]
[-- Type: text/x-patch, Size: 3069 bytes --]

From: Andrei Borzenkov <arvidjaar@gmail.com>
Subject: [PATCH] normal: allow return from submenu without authentication

This change removes a superfluous authorization request,
which prevented the user from navigating backwards through
the menu tree using the ESC Key.

To preserve current semantic, require authentication when returning
from menu in nested configfile.

Based on patch suggested by Florian Kaiser <fkaiser@genua.de>
Also-By: Florian Kaiser <fkaiser@genua.de>

---
 grub-core/normal/main.c | 21 ++++++++++++++++++---
 grub-core/normal/menu.c | 34 ++--------------------------------
 include/grub/normal.h   |  2 +-
 3 files changed, 21 insertions(+), 36 deletions(-)

diff --git a/grub-core/normal/main.c b/grub-core/normal/main.c
index 78a70a8..dc675ae 100644
--- a/grub-core/normal/main.c
+++ b/grub-core/normal/main.c
@@ -287,9 +287,24 @@ grub_normal_execute (const char *config, int nested, int batch)
 	{
 
 	  grub_boot_time ("Entering menu");
-	  grub_show_menu (menu, nested, 0);
-	  if (nested)
-	    grub_normal_free_menu (menu);
+	  while (1)
+	    {
+	      grub_show_menu (menu, nested, 0);
+	      if (grub_normal_exit_level)
+		break;
+	      if (nested)
+		{
+		  if (grub_auth_check_authentication (NULL))
+		    {
+		      grub_print_error ();
+		      grub_errno = GRUB_ERR_NONE;
+		      grub_wait_after_message ();
+		      continue;
+		    }
+		  grub_normal_free_menu (menu);
+		  break;
+		}
+	    }
 	}
     }
 }
diff --git a/grub-core/normal/menu.c b/grub-core/normal/menu.c
index 719e2fb..d93c19e 100644
--- a/grub-core/normal/menu.c
+++ b/grub-core/normal/menu.c
@@ -852,8 +852,8 @@ static struct grub_menu_execute_callback execution_callback =
   .notify_failure = notify_execution_failure
 };
 
-static grub_err_t
-show_menu (grub_menu_t menu, int nested, int autobooted)
+void
+grub_show_menu (grub_menu_t menu, int nested, int autobooted)
 {
   while (1)
     {
@@ -879,34 +879,4 @@ show_menu (grub_menu_t menu, int nested, int autobooted)
       if (autobooted)
 	break;
     }
-
-  return GRUB_ERR_NONE;
-}
-
-grub_err_t
-grub_show_menu (grub_menu_t menu, int nested, int autoboot)
-{
-  grub_err_t err1, err2;
-
-  while (1)
-    {
-      err1 = show_menu (menu, nested, autoboot);
-      autoboot = 0;
-      grub_print_error ();
-
-      if (grub_normal_exit_level)
-	break;
-
-      err2 = grub_auth_check_authentication (NULL);
-      if (err2)
-	{
-	  grub_print_error ();
-	  grub_errno = GRUB_ERR_NONE;
-	  continue;
-	}
-
-      break;
-    }
-
-  return err1;
 }
diff --git a/include/grub/normal.h b/include/grub/normal.h
index 218cbab..ae2e0dd 100644
--- a/include/grub/normal.h
+++ b/include/grub/normal.h
@@ -112,7 +112,7 @@ void grub_print_message_indented (const char *msg, int margin_left,
 				  struct grub_term_output *term);
 void
 grub_menu_text_register_instances (int entry, grub_menu_t menu, int nested);
-grub_err_t
+void
 grub_show_menu (grub_menu_t menu, int nested, int autobooted);
 
 /* Defined in `handler.c'.  */
-- 
tg: (c899d9f..) u/submenu-without-authentication (depends on: master)