All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Allow to add/change menu entry class defaults.
@ 2015-12-22 15:19 Robin Schneider
  2015-12-23  6:58 ` Andrei Borzenkov
  0 siblings, 1 reply; 8+ messages in thread
From: Robin Schneider @ 2015-12-22 15:19 UTC (permalink / raw)
  To: grub-devel

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Useful for changing the default access level for menu entries when using
GRUBs password protection feature.

Ansible role which makes use of this patch:
https://github.com/debops/ansible-grub/pull/7

Let me know what you think.

- -- 
Live long and prosper
Robin `ypid` Schneider
-----BEGIN PGP SIGNATURE-----

iQIcBAEBCgAGBQJWeWnqAAoJEIb9mAu/GkD4VG4P/2b7pozgh8MCGJs+5W5tB2Ix
JdNMnMi+txhVrSDUX2e0ILQkQk6+dVGAP5Lz3IyHtuJFogqdxYx+v5N/o31aAm6l
obSCzbgf5DhKxig00Me/8rT6tpGllULqIeKge7gduxNIy9qT5lsSqdD8qvtRf+Mv
Ak3KxOYH/smNVXBV2EuoqRHFE20GkDjUeW4ECgoB7k6kkVSBHikwo8cbzDSXnOqs
aJlx7vI/Ztpud8qYyJQbV6S+ezcHOeMViydLkd7UmPxJysULjUQJ5vYOsXQKiUW+
o97/xvXLzrMO3XXbJEub9utJYMqvsug1I9fzdPlJ+TmPVxGT9Cuw1RY3a/W60jXX
Vg8mu2DaZZeUP9U7BqrpAeKA0Rj/WEiYq++V11n7vx1MwvSRdXRtftGabKCTGSHq
a1IobHu3ojcbCj/q3V1c+N6SLGAWvzaSlcJurAKpNGGaxj628MvU9kYsqwImPxEW
mToRs0CEu7HS0+kQa3doSpArV1mC0tAb0MtEAQ/OZtfxMdGfgC1LSXwk6hWPi26x
4/bMeUr7eJzN9uSta5xUSW+7n4ooa7LN8sxY9DkQT3glWyUyZCPVwbXmI96nWZ8J
owMFvrU3tIV5llEpZ9sg1YUFKYkrpDffdRN5Qsnzm8gL72urPJ4B7S12c9rmut8p
1bU5G+0z+hM0/DBdVWTE
=bJj/
-----END PGP SIGNATURE-----

[-- Attachment #2: 0001-Allow-to-add-change-menu-entry-class-defaults.patch --]
[-- Type: text/x-patch, Size: 3813 bytes --]

From f769c045f246859224fbf0037a5617cdf4de35bb Mon Sep 17 00:00:00 2001
From: Robin Schneider <ypid@riseup.net>
Date: Tue, 22 Dec 2015 16:11:13 +0100
Subject: [PATCH] Allow to add/change menu entry class defaults.

Useful for changing the default access level for menu entries when using
GRUBs password protection feature.
Ansible role which makes use of this patch:
https://github.com/debops/ansible-grub/pull/7
---
 util/grub-mkconfig.in       | 12 +++++++++++-
 util/grub.d/10_hurd.in      |  2 +-
 util/grub.d/10_illumos.in   |  2 +-
 util/grub.d/10_kfreebsd.in  |  2 +-
 util/grub.d/10_linux.in     |  2 +-
 util/grub.d/20_linux_xen.in |  2 +-
 6 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
index 3183744..56a88e2 100644
--- a/util/grub-mkconfig.in
+++ b/util/grub-mkconfig.in
@@ -227,7 +227,17 @@ export GRUB_DEFAULT \
   GRUB_ENABLE_CRYPTODISK \
   GRUB_BADRAM \
   GRUB_OS_PROBER_SKIP_LIST \
-  GRUB_DISABLE_SUBMENU
+  GRUB_DISABLE_SUBMENU \
+  GRUB_LINUX_MENUENTRY_CLASS \
+  GRUB_LINUX_MENUENTRY_CLASS_ADDITIONAL \
+  GRUB_XEN_MENUENTRY_CLASS \
+  GRUB_XEN_MENUENTRY_CLASS_ADDITIONAL \
+  GRUB_HURD_MENUENTRY_CLASS \
+  GRUB_HURD_MENUENTRY_CLASS_ADDITIONAL \
+  GRUB_ILLUMOS_MENUENTRY_CLASS \
+  GRUB_ILLUMOS_MENUENTRY_CLASS_ADDITIONAL \
+  GRUB_KFREEBSD_MENUENTRY_CLASS \
+  GRUB_KFREEBSD_MENUENTRY_CLASS_ADDITIONAL
 
 if test "x${grub_cfg}" != "x"; then
   rm -f "${grub_cfg}.new"
diff --git a/util/grub.d/10_hurd.in b/util/grub.d/10_hurd.in
index 59a9a48..4b31d11 100644
--- a/util/grub.d/10_hurd.in
+++ b/util/grub.d/10_hurd.in
@@ -26,7 +26,7 @@ export TEXTDOMAINDIR="@localedir@"
 
 . "$pkgdatadir/grub-mkconfig_lib"
 
-CLASS="--class gnu --class os"
+CLASS=${GRUB_HURD_MENUENTRY_CLASS:-"--class gnu --class os ${GRUB_HURD_MENUENTRY_CLASS_ADDITIONAL:-}"}
 
 if [ "x${GRUB_DISTRIBUTOR}" = "x" ] ; then
   OS=GNU
diff --git a/util/grub.d/10_illumos.in b/util/grub.d/10_illumos.in
index a133e1b..0f62a24 100644
--- a/util/grub.d/10_illumos.in
+++ b/util/grub.d/10_illumos.in
@@ -25,7 +25,7 @@ datarootdir="@datarootdir@"
 export TEXTDOMAIN=@PACKAGE@
 export TEXTDOMAINDIR="@localedir@"
 
-CLASS="--class os"
+CLASS=${GRUB_ILLUMOS_MENUENTRY_CLASS:-"--class os ${GRUB_ILLUMOS_MENUENTRY_CLASS_ADDITIONAL:-}"}
 
 case "${GRUB_DISTRIBUTOR}" in
   *)
diff --git a/util/grub.d/10_kfreebsd.in b/util/grub.d/10_kfreebsd.in
index 9d8e8fd..773d151 100644
--- a/util/grub.d/10_kfreebsd.in
+++ b/util/grub.d/10_kfreebsd.in
@@ -25,7 +25,7 @@ datarootdir="@datarootdir@"
 export TEXTDOMAIN=@PACKAGE@
 export TEXTDOMAINDIR="@localedir@"
 
-CLASS="--class os"
+CLASS=${GRUB_KFREEBSD_MENUENTRY_CLASS:-"--class os ${GRUB_KFREEBSD_MENUENTRY_CLASS_ADDITIONAL:-}"}
 
 case "${GRUB_DISTRIBUTOR}" in
   Debian)
diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
index 859b608..18fb521 100644
--- a/util/grub.d/10_linux.in
+++ b/util/grub.d/10_linux.in
@@ -26,7 +26,7 @@ datarootdir="@datarootdir@"
 export TEXTDOMAIN=@PACKAGE@
 export TEXTDOMAINDIR="@localedir@"
 
-CLASS="--class gnu-linux --class gnu --class os"
+CLASS=${GRUB_LINUX_MENUENTRY_CLASS:-"--class gnu-linux --class gnu --class os ${GRUB_LINUX_MENUENTRY_CLASS_ADDITIONAL:-}"}
 
 if [ "x${GRUB_DISTRIBUTOR}" = "x" ] ; then
   OS=GNU/Linux
diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
index f532fb9..fcc8cf9 100644
--- a/util/grub.d/20_linux_xen.in
+++ b/util/grub.d/20_linux_xen.in
@@ -26,7 +26,7 @@ datarootdir="@datarootdir@"
 export TEXTDOMAIN=@PACKAGE@
 export TEXTDOMAINDIR="@localedir@"
 
-CLASS="--class gnu-linux --class gnu --class os --class xen"
+CLASS=${GRUB_XEN_MENUENTRY_CLASS:-"--class gnu-linux --class gnu --class os --class xen ${GRUB_XEN_MENUENTRY_CLASS_ADDITIONAL:-}"}
 
 if [ "x${GRUB_DISTRIBUTOR}" = "x" ] ; then
   OS=GNU/Linux
-- 
2.1.4


[-- Attachment #3: 0001-Allow-to-add-change-menu-entry-class-defaults.patch.sig --]
[-- Type: application/pgp-signature, Size: 543 bytes --]

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] Allow to add/change menu entry class defaults.
  2015-12-22 15:19 [PATCH] Allow to add/change menu entry class defaults Robin Schneider
@ 2015-12-23  6:58 ` Andrei Borzenkov
  2015-12-23 20:54   ` Robin Schneider
  0 siblings, 1 reply; 8+ messages in thread
From: Andrei Borzenkov @ 2015-12-23  6:58 UTC (permalink / raw)
  To: The development of GNU GRUB, ypid

On Tue, Dec 22, 2015 at 6:19 PM, Robin Schneider <ypid@riseup.net> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA512
>
> Useful for changing the default access level for menu entries when using
> GRUBs password protection feature.
>
> Ansible role which makes use of this patch:
> https://github.com/debops/ansible-grub/pull/7
>

That's overcomplicated. The first class found wins and you will not
define them unless you have them. So just having

class="$GRUB_..._CLASS --class ..."

is really enough.

> Let me know what you think.
>

I'm not sure I like turning grub-mkconfig into kitchen sink, but yes,
that probably makes sense. It allows easy vendor-specific icons
without need to patch grub every time.

> -CLASS="--class gnu-linux --class gnu --class os --class xen"

I guess "--class xen" should be in front as most specific (unrelated
to your patch).

I have half baked patch lying around that tries to use more sensible
icons in os-prober as well. May be it's time to revisit it.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Allow to add/change menu entry class defaults.
  2015-12-23  6:58 ` Andrei Borzenkov
@ 2015-12-23 20:54   ` Robin Schneider
  2015-12-24  8:21     ` Andrei Borzenkov
  0 siblings, 1 reply; 8+ messages in thread
From: Robin Schneider @ 2015-12-23 20:54 UTC (permalink / raw)
  To: grub-devel

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 23.12.2015 07:58, Andrei Borzenkov wrote:
> On Tue, Dec 22, 2015 at 6:19 PM, Robin Schneider <ypid@riseup.net> wrote:
>> Useful for changing the default access level for menu entries when using 
>> GRUBs password protection feature.
>> 
>> Ansible role which makes use of this patch: 
>> https://github.com/debops/ansible-grub/pull/7
>> 
> 
> That's overcomplicated. The first class found wins and you will not define
> them unless you have them. So just having
> 
> class="$GRUB_..._CLASS --class ..."
> 
> is really enough.

Thanks for the input. I agree that my first patch was probably a bit to
flexible. I attached a updated patch.

> 
>> Let me know what you think.
>> 
> 
> I'm not sure I like turning grub-mkconfig into kitchen sink, but yes, that
> probably makes sense. It allows easy vendor-specific icons without need to
> patch grub every time.
> 
>> -CLASS="--class gnu-linux --class gnu --class os --class xen"
> 
> I guess "--class xen" should be in front as most specific (unrelated to
> your patch).
> 
> I have half baked patch lying around that tries to use more sensible icons
> in os-prober as well. May be it's time to revisit it.

- -- 
Live long and prosper
Robin `ypid` Schneider
-----BEGIN PGP SIGNATURE-----

iQIcBAEBCgAGBQJWewoKAAoJEIb9mAu/GkD4tJoQAMtvZcUGrksI0VtCVOJvibrr
2/NN2B2191blBIgi9HffciLUDaI3zX1g3dWwhXh0PYLdMmxyfbR3ewK/bzHWN9gg
amCGsyPPbaWBpsE6c2jhLDqFX+on67DCCXiCS4QJX+u31ZwRbZhqhekAvZxvH00W
pju/2WEMQv4CA15lBvBYB/y6Sux1sNyl/4jdC5ZvtAgsNAj+Z1ADebSsi7G+ym48
N1gbjgFOR1p4ORfSKjhHvGAfJ/8JaNatphp1BMYJImozy/ktZC+vbr+7N/tc/LW2
XWxpi3drKM2x8kZerGA4vITVjTxWZyrkQtErRQPROE6r+NxoqUotzny7y2KSZtCB
3ncEn10sA360o5TLnY5WCqrN7XbLh73T/g7ePWCumy/IIKAY9oPuxzFI/vLElzDf
QBAhL/G7GYTRJX5nwRVbeMwwhcx+UrxllSSLswcLZCCpfO2XQXDSjlZiGVfvoRMS
oxmF/kQ5GD2X3Chs33gX4cPCE4NPf1xdg0mQEnpx8e3RcdaLR5kki8p8mi97ME+j
LyMJDUZsbnrz0ylljdieDJWHnHFLrRuj4Qksn0bWUTG/+1zSSX66rzGVzP9/pwKv
/3JkN2D7q3aTBcBts9BzeJgomi1iJtDsbd03ve1SiTX9iUH+6lylToxtuCT3T4jX
SF0yyzjjewj7idRFrhH6
=JF0U
-----END PGP SIGNATURE-----

[-- Attachment #2: 0001-Allow-to-add-change-menu-entry-class-defaults.patch --]
[-- Type: text/x-patch, Size: 3363 bytes --]

From c8a8c9901837f4d4217507cf82a754b99111e721 Mon Sep 17 00:00:00 2001
From: Robin Schneider <ypid@riseup.net>
Date: Wed, 23 Dec 2015 17:23:42 +0100
Subject: [PATCH] Allow to add/change menu entry class defaults.

Useful for changing the default access level for menu entries when using
GRUBs password protection feature.
Ansible role which makes use of this patch:
https://github.com/debops/ansible-grub/pull/7
---
 util/grub-mkconfig.in       | 7 ++++++-
 util/grub.d/10_hurd.in      | 2 +-
 util/grub.d/10_illumos.in   | 2 +-
 util/grub.d/10_kfreebsd.in  | 2 +-
 util/grub.d/10_linux.in     | 2 +-
 util/grub.d/20_linux_xen.in | 2 +-
 6 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
index 3183744..3f5468c 100644
--- a/util/grub-mkconfig.in
+++ b/util/grub-mkconfig.in
@@ -227,7 +227,12 @@ export GRUB_DEFAULT \
   GRUB_ENABLE_CRYPTODISK \
   GRUB_BADRAM \
   GRUB_OS_PROBER_SKIP_LIST \
-  GRUB_DISABLE_SUBMENU
+  GRUB_DISABLE_SUBMENU \
+  GRUB_LINUX_MENUENTRY_CLASS \
+  GRUB_XEN_MENUENTRY_CLASS \
+  GRUB_HURD_MENUENTRY_CLASS \
+  GRUB_ILLUMOS_MENUENTRY_CLASS \
+  GRUB_KFREEBSD_MENUENTRY_CLASS
 
 if test "x${grub_cfg}" != "x"; then
   rm -f "${grub_cfg}.new"
diff --git a/util/grub.d/10_hurd.in b/util/grub.d/10_hurd.in
index 59a9a48..9c3ba8e 100644
--- a/util/grub.d/10_hurd.in
+++ b/util/grub.d/10_hurd.in
@@ -26,7 +26,7 @@ export TEXTDOMAINDIR="@localedir@"
 
 . "$pkgdatadir/grub-mkconfig_lib"
 
-CLASS="--class gnu --class os"
+CLASS="${GRUB_HURD_MENUENTRY_CLASS} --class gnu --class os"
 
 if [ "x${GRUB_DISTRIBUTOR}" = "x" ] ; then
   OS=GNU
diff --git a/util/grub.d/10_illumos.in b/util/grub.d/10_illumos.in
index a133e1b..db85554 100644
--- a/util/grub.d/10_illumos.in
+++ b/util/grub.d/10_illumos.in
@@ -25,7 +25,7 @@ datarootdir="@datarootdir@"
 export TEXTDOMAIN=@PACKAGE@
 export TEXTDOMAINDIR="@localedir@"
 
-CLASS="--class os"
+CLASS="${GRUB_ILLUMOS_MENUENTRY_CLASS} --class os"
 
 case "${GRUB_DISTRIBUTOR}" in
   *)
diff --git a/util/grub.d/10_kfreebsd.in b/util/grub.d/10_kfreebsd.in
index 9d8e8fd..fb9f4b0 100644
--- a/util/grub.d/10_kfreebsd.in
+++ b/util/grub.d/10_kfreebsd.in
@@ -25,7 +25,7 @@ datarootdir="@datarootdir@"
 export TEXTDOMAIN=@PACKAGE@
 export TEXTDOMAINDIR="@localedir@"
 
-CLASS="--class os"
+CLASS="${GRUB_KFREEBSD_MENUENTRY_CLASS} --class os"
 
 case "${GRUB_DISTRIBUTOR}" in
   Debian)
diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
index 859b608..266158b 100644
--- a/util/grub.d/10_linux.in
+++ b/util/grub.d/10_linux.in
@@ -26,7 +26,7 @@ datarootdir="@datarootdir@"
 export TEXTDOMAIN=@PACKAGE@
 export TEXTDOMAINDIR="@localedir@"
 
-CLASS="--class gnu-linux --class gnu --class os"
+CLASS="${GRUB_LINUX_MENUENTRY_CLASS} --class gnu-linux --class gnu --class os"
 
 if [ "x${GRUB_DISTRIBUTOR}" = "x" ] ; then
   OS=GNU/Linux
diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
index f532fb9..80d0f9b 100644
--- a/util/grub.d/20_linux_xen.in
+++ b/util/grub.d/20_linux_xen.in
@@ -26,7 +26,7 @@ datarootdir="@datarootdir@"
 export TEXTDOMAIN=@PACKAGE@
 export TEXTDOMAINDIR="@localedir@"
 
-CLASS="--class gnu-linux --class gnu --class os --class xen"
+CLASS="${GRUB_XEN_MENUENTRY_CLASS} --class gnu-linux --class gnu --class os --class xen"
 
 if [ "x${GRUB_DISTRIBUTOR}" = "x" ] ; then
   OS=GNU/Linux
-- 
2.1.4


[-- Attachment #3: 0001-Allow-to-add-change-menu-entry-class-defaults.patch.sig --]
[-- Type: application/pgp-signature, Size: 543 bytes --]

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] Allow to add/change menu entry class defaults.
  2015-12-23 20:54   ` Robin Schneider
@ 2015-12-24  8:21     ` Andrei Borzenkov
  2015-12-26 21:17       ` Robin Schneider
  0 siblings, 1 reply; 8+ messages in thread
From: Andrei Borzenkov @ 2015-12-24  8:21 UTC (permalink / raw)
  To: The development of GNU GRUB

On Wed, Dec 23, 2015 at 11:54 PM, Robin Schneider <ypid@riseup.net> wrote:
> Thanks for the input. I agree that my first patch was probably a bit to
> flexible. I attached a updated patch.
>

I'm still unsure what problem it tries to solve and whether it solves
problem it intends to solve.

So you say

> Useful for changing the default access level for menu entries when using
> GRUBs password protection feature.

a) This does not change any "access level" whatever it means. It only
changes what icon is displayed for menu entry.

b) it is all or nothing. The first found icon is used so either all
menu entries are displayed with "need authentication" or none.

c) if it is all or nothing then the same can trivially be implemented
by replacing one set of icons ("unlocked") with another set of icons
("locked") during bootloader reconfiguration. This should be done by
tool you use to configure bootloader, grub-mkconfig has no knowledge
about access restrictions anyway.

So either it is trivially implemented without any need to change
grub-mkconfig or it does not solve the problem anyway.

But idea itself is actually interesting. Icon manager in grub could
select different icon if menu entry requires authentication. Or it
could display overlay (which is probably better). And it actually can
dynamically decide whether to display this overlay depending on
whether user is already authenticated.

How does it sound?

P.S. current situation with grub-mkconfig I do not like at all. It
became de-facto standard tool to configure GRUB by distributions but
it does not provide any sane way to differentiate between distribution
default vs. local admin configuration. And variables you propose sound
exactly like the type that will hit this confusion. We need to solve
this before.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Allow to add/change menu entry class defaults.
  2015-12-24  8:21     ` Andrei Borzenkov
@ 2015-12-26 21:17       ` Robin Schneider
  2015-12-27 17:03         ` Andrei Borzenkov
  0 siblings, 1 reply; 8+ messages in thread
From: Robin Schneider @ 2015-12-26 21:17 UTC (permalink / raw)
  To: grub-devel

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 24.12.2015 09:21, Andrei Borzenkov wrote:
> On Wed, Dec 23, 2015 at 11:54 PM, Robin Schneider <ypid@riseup.net> wrote:
>> Thanks for the input. I agree that my first patch was probably a bit to
>> flexible. I attached a updated patch.
>>
> 
> I'm still unsure what problem it tries to solve and whether it solves
> problem it intends to solve.
> 
> So you say
> 
>> Useful for changing the default access level for menu entries when using
>> GRUBs password protection feature.
> 
> a) This does not change any "access level" whatever it means. It only
> changes what icon is displayed for menu entry.

> b) it is all or nothing. The first found icon is used so either all
> menu entries are displayed with "need authentication" or none.
> 
> c) if it is all or nothing then the same can trivially be implemented
> by replacing one set of icons ("unlocked") with another set of icons
> ("locked") during bootloader reconfiguration. This should be done by
> tool you use to configure bootloader, grub-mkconfig has no knowledge
> about access restrictions anyway.
> 
> So either it is trivially implemented without any need to change
> grub-mkconfig or it does not solve the problem anyway.
> 
> But idea itself is actually interesting. Icon manager in grub could
> select different icon if menu entry requires authentication. Or it
> could display overlay (which is probably better). And it actually can
> dynamically decide whether to display this overlay depending on
> whether user is already authenticated.
> 
> How does it sound?
> 
> P.S. current situation with grub-mkconfig I do not like at all. It
> became de-facto standard tool to configure GRUB by distributions but
> it does not provide any sane way to differentiate between distribution
> default vs. local admin configuration. And variables you propose sound
> exactly like the type that will hit this confusion. We need to solve
> this before.

I am sorry for the misunderstanding. I should have explained the indention
behind my patch a bit better then just linking to another patch which makes use
of the newly introduced variables by this patch.

My indented use case is to allow to add options like '--unrestricted' or
'--users "Jane"' to each menuentry generated by grub-mkconfig without altering
the scripts itself. Although the scripts end up under /etc in Debian, I did not
think that changing these files (and `dpkg-divert`ing them) would be such a good
idea. I have been searching for how to configure password protection and I did
only find "hacks" which either suggest to edit the grub config under /boot/grub
(which is totally outdated because the configurations is automatically generated
in Debian) or change the "CLASS" variable in the grub.d scripts [12]0_.*
scripts. So I did go with the later option. Patching these scripts with
configuration management was not ideal so I decided to propose this patch
upstream. With this patch in place it will be possible to configure this in
/etc/default/grub without touching the scripts.

This patch addresses the issue which is mentioned in this article (At least when
the assumption is true that all menuentry of the same distribution should get
the same restrictions/options):
https://help.ubuntu.com/community/Grub2/Passwords#Protecting_Menuentries
"There is currently no automated method of adding users or designating menu
items to be protected. The user must manually edit the GRUB 2 scripts."

Thanks for the second review. You are right, adding this to the CLASS variable
(and also the naming) was not ideal. I attached an updated patch which should
address these issues. This patch should also allow to overwrite icon files
because the variable is placed before the CLASS variable.

BTW: The efi menuentry has the class 'windows'. Is that correct? My patch
assumes that this menuentry is indented for UEFI applications.

- -- 
Live long and prosper
Robin `ypid` Schneider
-----BEGIN PGP SIGNATURE-----

iQIcBAEBCgAGBQJWfwPOAAoJEIb9mAu/GkD4XOMP/3iEyvNc4WrVk8y6ek+/e9DB
Zk4F3PmVE2lhbnvCcw0cxDH5XfjXwSw9qa6qKYodQtw89I/QcmMmpDuObqTZfdgn
fO4zYcBpm7uBNjH7YLakRtlXvxi1pMifSk9XyR7W4vnHx6b1NyxXkb5gjx4ux88X
xWyIe6zhdF/Ns+Pb9rxYr4ezsHfpZjLv9ZwmBvjwLfg8I3/jPB/r3WPg8rZPpNyN
HeHBIHTE/NeqEgkt3iEofuXQxPYOtA/FamUecwsIHciei33Q6meEsTULuiu1Oa+I
siTP7XnODrOkyvN9+RSdWQBUfk+PmXzf4L/qmVJzALqnk1U888gJjT+vxzINkBAR
RNAS1RLRTZAVe1rJlEGy6XlIdCLCU9ujm0HxT0xdpwceSVaK0DSRMK+QHPMNDspn
Xxjex9NIUgSvjVubNSDiW5hmmP5sJuUfRC5EG6++O0pU2X8sAimHxWRa5HhZqfjo
kuF4dGn918foGMNdN3RzNGcH3SUEsQyvQfZEFctJ63sFqHa+nsicaDxYcvqKl9VT
4Tm7PlbgWTDwUpIu1y8NOjyupcjSZtCrBGXwJbo/NMhwPQFHG/f26wP28ofrc2hD
hl8b7SC+aCd+CA9DFVOUL2ahXUexs8aSeefGC9gR8kQnskDX5AYXUtC7oN5PwBIC
s/RWiPzPnc1QR4L0nzuZ
=HTuO
-----END PGP SIGNATURE-----

[-- Attachment #2: 0001-Added-GRUB_-_MENUENTRY_OPTIONS-variables-to-configur.patch --]
[-- Type: text/x-patch, Size: 16048 bytes --]

From 058e95249c7ab92e8fecfc2f3938600abd58eecd Mon Sep 17 00:00:00 2001
From: Robin Schneider <ypid@riseup.net>
Date: Sat, 26 Dec 2015 21:53:30 +0100
Subject: [PATCH] Added GRUB_*_MENUENTRY_OPTIONS variables to configure menu
 entries.

Useful for making each menuentry '--unrestricted' when using GRUBs
password protection feature.

Ansible role which makes use of this patch:
https://github.com/debops/ansible-grub/pull/7

My indented use case is to allow to add options like '--unrestricted' or
'--users "Jane"' to each menuentry generated by grub-mkconfig without
altering the scripts itself. Although the scripts end up under /etc in
Debian, I did not think that changing these files (and `dpkg-divert`ing
them) would be such a good idea. I have been searching for how to
configure password protection and I did only find "hacks" which either
suggest to edit the grub config under /boot/grub (which is totally
outdated because the configurations is automatically generated in
Debian) or change the "CLASS" variable in the grub.d scripts [12]0_.*
scripts.  So I did go with the later option. Patching these scripts with
configuration management was not ideal so I decided to propose this
patch upstream. With this patch in place it will be possible to
configure this in /etc/default/grub without touching the scripts.

This patch addresses the issue which is mentioned in this article (At
least when the assumption is true that all menuentry of the same
distribution should get the same restrictions/options):
https://help.ubuntu.com/community/Grub2/Passwords#Protecting_Menuentries
"There is currently no automated method of adding users or designating
menu items to be protected. The user must manually edit the GRUB 2
scripts."
---
 util/grub-mkconfig.in       | 13 ++++++++++++-
 util/grub.d/10_hurd.in      |  4 ++--
 util/grub.d/10_illumos.in   |  2 +-
 util/grub.d/10_kfreebsd.in  |  4 ++--
 util/grub.d/10_linux.in     |  4 ++--
 util/grub.d/10_netbsd.in    |  4 ++--
 util/grub.d/10_windows.in   |  2 +-
 util/grub.d/10_xnu.in       |  2 +-
 util/grub.d/20_linux_xen.in |  4 ++--
 util/grub.d/30_os-prober.in | 14 +++++++-------
 10 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
index 3183744..d7fdd78 100644
--- a/util/grub-mkconfig.in
+++ b/util/grub-mkconfig.in
@@ -227,7 +227,18 @@ export GRUB_DEFAULT \
   GRUB_ENABLE_CRYPTODISK \
   GRUB_BADRAM \
   GRUB_OS_PROBER_SKIP_LIST \
-  GRUB_DISABLE_SUBMENU
+  GRUB_DISABLE_SUBMENU \
+  GRUB_HURD_MENUENTRY_OPTIONS \
+  GRUB_ILLUMOS_MENUENTRY_OPTIONS \
+  GRUB_KFREEBSD_MENUENTRY_OPTIONS \
+  GRUB_LINUX_MENUENTRY_OPTIONS \
+  GRUB_NETBSD_MENUENTRY_OPTIONS \
+  GRUB_WINDOWS_MENUENTRY_OPTIONS \
+  GRUB_XNU_MENUENTRY_OPTIONS \
+  GRUB_XEN_MENUENTRY_OPTIONS \
+  GRUB_MINIX_MENUENTRY_OPTIONS \
+  GRUB_OSX_MENUENTRY_OPTIONS \
+  GRUB_EFI_MENUENTRY_OPTIONS
 
 if test "x${grub_cfg}" != "x"; then
   rm -f "${grub_cfg}.new"
diff --git a/util/grub.d/10_hurd.in b/util/grub.d/10_hurd.in
index 59a9a48..c07fbfd 100644
--- a/util/grub.d/10_hurd.in
+++ b/util/grub.d/10_hurd.in
@@ -100,11 +100,11 @@ hurd_entry () {
 	  grub_warn "$(gettext_printf "Please don't use old title \`%s' for GRUB_DEFAULT, use \`%s' (for versions before 2.00) or \`%s' (for 2.00 or later)" "$GRUB_ACTUAL_DEFAULT" "$replacement_title" "gnuhurd-advanced-$boot_device_id>'gnuhurd-$kernel-$type-$(grub_get_device_id "${GRUB_DEVICE_BOOT}")'")"
       fi
       sed "s/^/$submenu_indentation/" << EOF
-menuentry '$(echo "$title" | grub_quote)' ${CLASS} \$menuentry_id_option 'gnuhurd-$kernel-$type-$(grub_get_device_id "${GRUB_DEVICE_BOOT}")' {
+menuentry '$(echo "$title" | grub_quote)' ${GRUB_HURD_MENUENTRY_OPTIONS} ${CLASS} \$menuentry_id_option 'gnuhurd-$kernel-$type-$(grub_get_device_id "${GRUB_DEVICE_BOOT}")' {
 EOF
   else
       sed "s/^/$submenu_indentation/" << EOF
-menuentry '$(echo "$OS" | grub_quote)' ${CLASS} \$menuentry_id_option 'gnuhurd-simple-$(grub_get_device_id "${GRUB_DEVICE_BOOT}")' {
+menuentry '$(echo "$OS" | grub_quote)' ${GRUB_HURD_MENUENTRY_OPTIONS} ${CLASS} \$menuentry_id_option 'gnuhurd-simple-$(grub_get_device_id "${GRUB_DEVICE_BOOT}")' {
 EOF
   fi
 
diff --git a/util/grub.d/10_illumos.in b/util/grub.d/10_illumos.in
index a133e1b..76a5043 100644
--- a/util/grub.d/10_illumos.in
+++ b/util/grub.d/10_illumos.in
@@ -34,7 +34,7 @@ case "${GRUB_DISTRIBUTOR}" in
   ;;
 esac
 
-echo "menuentry '$(echo "$OS" | grub_quote)' ${CLASS} \$menuentry_id_option 'illumos-$(grub_get_device_id "${GRUB_DEVICE_BOOT}")' {"
+echo "menuentry '$(echo "$OS" | grub_quote)' ${GRUB_ILLUMOS_MENUENTRY_OPTIONS} ${CLASS} \$menuentry_id_option 'illumos-$(grub_get_device_id "${GRUB_DEVICE_BOOT}")' {"
 save_default_entry | grub_add_tab
 prepare_grub_to_access_device "${GRUB_DEVICE_BOOT}" | grub_add_tab
 message="$(gettext_printf "Loading kernel of Illumos ...")"
diff --git a/util/grub.d/10_kfreebsd.in b/util/grub.d/10_kfreebsd.in
index 9d8e8fd..79005ed 100644
--- a/util/grub.d/10_kfreebsd.in
+++ b/util/grub.d/10_kfreebsd.in
@@ -86,9 +86,9 @@ kfreebsd_entry ()
 	  title_correction_code="${title_correction_code}if [ \"x\$default\" = '$quoted' ]; then default='$(echo "$replacement_title" | grub_quote)'; fi;"
 	  grub_warn "$(gettext_printf "Please don't use old title \`%s' for GRUB_DEFAULT, use \`%s' (for versions before 2.00) or \`%s' (for 2.00 or later)" "$GRUB_ACTUAL_DEFAULT" "$replacement_title" "kfreebsd-advanced-$boot_device_id>kfreebsd-$version-$type-$boot_device_id")"
       fi
-      echo "menuentry '$(echo "$title" | grub_quote)' ${CLASS} \$menuentry_id_option 'kfreebsd-$version-$type-$boot_device_id' {" | sed "s/^/$submenu_indentation/"
+      echo "menuentry '$(echo "$title" | grub_quote)' ${GRUB_KFREEBSD_MENUENTRY_OPTIONS} ${CLASS} \$menuentry_id_option 'kfreebsd-$version-$type-$boot_device_id' {" | sed "s/^/$submenu_indentation/"
   else
-      echo "menuentry '$(echo "$OS" | grub_quote)' ${CLASS} \$menuentry_id_option 'kfreebsd-simple-$boot_device_id' {" | sed "s/^/$submenu_indentation/"
+      echo "menuentry '$(echo "$OS" | grub_quote)' ${GRUB_KFREEBSD_MENUENTRY_OPTIONS} ${CLASS} \$menuentry_id_option 'kfreebsd-simple-$boot_device_id' {" | sed "s/^/$submenu_indentation/"
   fi
   if [ x$type != xrecovery ] ; then
       save_default_entry | grub_add_tab | sed "s/^/$submenu_indentation/"
diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
index 859b608..3fa8fb0 100644
--- a/util/grub.d/10_linux.in
+++ b/util/grub.d/10_linux.in
@@ -90,9 +90,9 @@ linux_entry ()
 	  title_correction_code="${title_correction_code}if [ \"x\$default\" = '$quoted' ]; then default='$(echo "$replacement_title" | grub_quote)'; fi;"
 	  grub_warn "$(gettext_printf "Please don't use old title \`%s' for GRUB_DEFAULT, use \`%s' (for versions before 2.00) or \`%s' (for 2.00 or later)" "$GRUB_ACTUAL_DEFAULT" "$replacement_title" "gnulinux-advanced-$boot_device_id>gnulinux-$version-$type-$boot_device_id")"
       fi
-      echo "menuentry '$(echo "$title" | grub_quote)' ${CLASS} \$menuentry_id_option 'gnulinux-$version-$type-$boot_device_id' {" | sed "s/^/$submenu_indentation/"
+      echo "menuentry '$(echo "$title" | grub_quote)' ${GRUB_LINUX_MENUENTRY_OPTIONS} ${CLASS} \$menuentry_id_option 'gnulinux-$version-$type-$boot_device_id' {" | sed "s/^/$submenu_indentation/"
   else
-      echo "menuentry '$(echo "$os" | grub_quote)' ${CLASS} \$menuentry_id_option 'gnulinux-simple-$boot_device_id' {" | sed "s/^/$submenu_indentation/"
+      echo "menuentry '$(echo "$os" | grub_quote)' ${GRUB_LINUX_MENUENTRY_OPTIONS} ${CLASS} \$menuentry_id_option 'gnulinux-simple-$boot_device_id' {" | sed "s/^/$submenu_indentation/"
   fi      
   if [ x$type != xrecovery ] ; then
       save_default_entry | grub_add_tab
diff --git a/util/grub.d/10_netbsd.in b/util/grub.d/10_netbsd.in
index 874f599..ed93123 100644
--- a/util/grub.d/10_netbsd.in
+++ b/util/grub.d/10_netbsd.in
@@ -113,9 +113,9 @@ netbsd_entry ()
 	  grub_warn "$(gettext_printf "Please don't use old title \`%s' for GRUB_DEFAULT, use \`%s' (for versions before 2.00) or \`%s' (for 2.00 or later)" "$GRUB_ACTUAL_DEFAULT" "$replacement_title" "netbsd-advanced-$boot_device_id>netbsd-${loader}-$kernel-$type-$boot_device_id")"
       fi
 
-      echo "menuentry '$(echo "$title" | grub_quote)' \$menuentry_id_option 'netbsd-${loader}-$kernel-$type-$boot_device_id' {"  | sed "s/^/$submenu_indentation/"
+      echo "menuentry '$(echo "$title" | grub_quote)' ${GRUB_NETBSD_MENUENTRY_OPTIONS} \$menuentry_id_option 'netbsd-${loader}-$kernel-$type-$boot_device_id' {"  | sed "s/^/$submenu_indentation/"
   else
-      echo "menuentry '$(echo "$OS" | grub_quote)' \$menuentry_id_option 'netbsd-${loader}-simple-$boot_device_id' {"  | sed "s/^/$submenu_indentation/"
+      echo "menuentry '$(echo "$OS" | grub_quote)' ${GRUB_NETBSD_MENUENTRY_OPTIONS} \$menuentry_id_option 'netbsd-${loader}-simple-$boot_device_id' {"  | sed "s/^/$submenu_indentation/"
   fi
 
   printf "%s\n" "${prepare_boot_cache}" | sed "s/^/$submenu_indentation/"
diff --git a/util/grub.d/10_windows.in b/util/grub.d/10_windows.in
index 554c561..d766bd3 100644
--- a/util/grub.d/10_windows.in
+++ b/util/grub.d/10_windows.in
@@ -84,7 +84,7 @@ for drv in $drives ; do
 
   gettext_printf "Found %s on %s (%s)\n" "$OS" "$drv" "$dev" >&2
   cat << EOF
-menuentry '$(echo "$OS" | grub_quote)' \$menuentry_id_option '$osid-$(grub_get_device_id "${dev}")' {
+menuentry '$(echo "$OS" | grub_quote)' ${GRUB_WINDOWS_MENUENTRY_OPTIONS} \$menuentry_id_option '$osid-$(grub_get_device_id "${dev}")' {
 EOF
 
   save_default_entry | sed -e 's,^,$grub_tab,'
diff --git a/util/grub.d/10_xnu.in b/util/grub.d/10_xnu.in
index 51ee2f4..c122933 100644
--- a/util/grub.d/10_xnu.in
+++ b/util/grub.d/10_xnu.in
@@ -37,7 +37,7 @@ osx_entry() {
     # TRANSLATORS: it refers on the OS residing on device %s
     onstr="$(gettext_printf "(on %s)" "${GRUB_DEVICE}")"
         cat << EOF
-menuentry '$(echo "Darwin/Mac OS X $bitstr $onstr" | grub_quote)' --class osx --class darwin --class os \$menuentry_id_option 'osprober-xnu-$2-$(grub_get_device_id "${GRUB_DEVICE}")'  {
+menuentry '$(echo "Darwin/Mac OS X $bitstr $onstr" | grub_quote)' ${GRUB_XNU_MENUENTRY_OPTIONS} --class osx --class darwin --class os \$menuentry_id_option 'osprober-xnu-$2-$(grub_get_device_id "${GRUB_DEVICE}")'  {
 EOF
 	save_default_entry | grub_add_tab
 	prepare_grub_to_access_device ${GRUB_DEVICE} | grub_add_tab
diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
index f532fb9..7b78ff1 100644
--- a/util/grub.d/20_linux_xen.in
+++ b/util/grub.d/20_linux_xen.in
@@ -98,10 +98,10 @@ linux_entry ()
          title_correction_code="${title_correction_code}if [ \"x\$default\" = '$quoted' ]; then default='$(echo "$replacement_title" | grub_quote)'; fi;"
          grub_warn "$(gettext_printf "Please don't use old title \`%s' for GRUB_DEFAULT, use \`%s' (for versions before 2.00) or \`%s' (for 2.00 or later)" "$GRUB_ACTUAL_DEFAULT" "$replacement_title" "gnulinux-advanced-$boot_device_id>gnulinux-$version-$type-$boot_device_id")"
       fi
-      echo "menuentry '$(echo "$title" | grub_quote)' ${CLASS} \$menuentry_id_option 'xen-gnulinux-$version-$type-$boot_device_id' {" | sed "s/^/$submenu_indentation/"
+      echo "menuentry '$(echo "$title" | grub_quote)' ${GRUB_XEN_MENUENTRY_OPTIONS} ${CLASS} \$menuentry_id_option 'xen-gnulinux-$version-$type-$boot_device_id' {" | sed "s/^/$submenu_indentation/"
   else
       title="$(gettext_printf "%s, with Xen hypervisor" "${os}")"
-      echo "menuentry '$(echo "$title" | grub_quote)' ${CLASS} \$menuentry_id_option 'xen-gnulinux-simple-$boot_device_id' {" | sed "s/^/$submenu_indentation/"
+      echo "menuentry '$(echo "$title" | grub_quote)' ${GRUB_XEN_MENUENTRY_OPTIONS} ${CLASS} \$menuentry_id_option 'xen-gnulinux-simple-$boot_device_id' {" | sed "s/^/$submenu_indentation/"
   fi
   if [ x$type != xrecovery ] ; then
       save_default_entry | grub_add_tab | sed "s/^/$submenu_indentation/"
diff --git a/util/grub.d/30_os-prober.in b/util/grub.d/30_os-prober.in
index 5fc4f0c..e65e2f1 100644
--- a/util/grub.d/30_os-prober.in
+++ b/util/grub.d/30_os-prober.in
@@ -52,7 +52,7 @@ osx_entry() {
     # TRANSLATORS: it refers on the OS residing on device %s
     onstr="$(gettext_printf "(on %s)" "${DEVICE}")"
         cat << EOF
-menuentry '$(echo "${LONGNAME} $bitstr $onstr" | grub_quote)' --class osx --class darwin --class os \$menuentry_id_option 'osprober-xnu-$2-$(grub_get_device_id "${DEVICE}")'  {
+menuentry '$(echo "${LONGNAME} $bitstr $onstr" | grub_quote)' ${GRUB_OSX_MENUENTRY_OPTIONS} --class osx --class darwin --class os \$menuentry_id_option 'osprober-xnu-$2-$(grub_get_device_id "${DEVICE}")'  {
 EOF
 	save_default_entry | grub_add_tab
 	prepare_grub_to_access_device ${DEVICE} | grub_add_tab
@@ -142,7 +142,7 @@ for OS in ${OSPROBED} ; do
 
 	  onstr="$(gettext_printf "(on %s)" "${DEVICE}")"
       cat << EOF
-menuentry '$(echo "${LONGNAME} $onstr" | grub_quote)' --class windows --class os \$menuentry_id_option 'osprober-chain-$(grub_get_device_id "${DEVICE}")' {
+menuentry '$(echo "${LONGNAME} $onstr" | grub_quote)' ${GRUB_WINDOWS_MENUENTRY_OPTIONS} --class windows --class os \$menuentry_id_option 'osprober-chain-$(grub_get_device_id "${DEVICE}")' {
 EOF
       save_default_entry | grub_add_tab
       prepare_grub_to_access_device ${DEVICE} | grub_add_tab
@@ -174,7 +174,7 @@ EOF
 	DEVICE=${DEVICE%@*}
 	onstr="$(gettext_printf "(on %s)" "${DEVICE}")"
       cat << EOF
-menuentry '$(echo "${LONGNAME} $onstr" | grub_quote)' --class windows --class os \$menuentry_id_option 'osprober-efi-$(grub_get_device_id "${DEVICE}")' {
+menuentry '$(echo "${LONGNAME} $onstr" | grub_quote)' ${GRUB_EFI_MENUENTRY_OPTIONS} --class windows --class os \$menuentry_id_option 'osprober-efi-$(grub_get_device_id "${DEVICE}")' {
 EOF
       save_default_entry | sed -e "s/^/\t/"
       prepare_grub_to_access_device ${DEVICE} | sed -e "s/^/\t/"
@@ -230,7 +230,7 @@ EOF
 
 	if [ "x$is_top_level" = xtrue ] && [ "x${GRUB_DISABLE_SUBMENU}" != xy ]; then
             cat << EOF
-menuentry '$(echo "$OS $onstr" | grub_quote)' --class gnu-linux --class gnu --class os \$menuentry_id_option 'osprober-gnulinux-simple-$boot_device_id' {
+menuentry '$(echo "$OS $onstr" | grub_quote)' ${GRUB_LINUX_MENUENTRY_OPTIONS} --class gnu-linux --class gnu --class os \$menuentry_id_option 'osprober-gnulinux-simple-$boot_device_id' {
 EOF
 	    save_default_entry | grub_add_tab
 	    printf '%s\n' "${prepare_boot_cache}"
@@ -250,7 +250,7 @@ EOF
 	fi
 	title="${LLABEL} $onstr"
         cat << EOF
-	menuentry '$(echo "$title" | grub_quote)' --class gnu-linux --class gnu --class os \$menuentry_id_option 'osprober-gnulinux-$LKERNEL-${recovery_params}-$boot_device_id' {
+	menuentry '$(echo "$title" | grub_quote)' ${GRUB_LINUX_MENUENTRY_OPTIONS} --class gnu-linux --class gnu --class os \$menuentry_id_option 'osprober-gnulinux-$LKERNEL-${recovery_params}-$boot_device_id' {
 EOF
 	save_default_entry | sed -e "s/^/$grub_tab$grub_tab/"
 	printf '%s\n' "${prepare_boot_cache}" | grub_add_tab
@@ -287,7 +287,7 @@ EOF
     hurd)
       onstr="$(gettext_printf "(on %s)" "${DEVICE}")"
       cat << EOF
-menuentry '$(echo "${LONGNAME} $onstr" | grub_quote)' --class hurd --class gnu --class os \$menuentry_id_option 'osprober-gnuhurd-/boot/gnumach.gz-false-$(grub_get_device_id "${DEVICE}")' {
+menuentry '$(echo "${LONGNAME} $onstr" | grub_quote)' ${GRUB_HURD_MENUENTRY_OPTIONS} --class hurd --class gnu --class os \$menuentry_id_option 'osprober-gnuhurd-/boot/gnumach.gz-false-$(grub_get_device_id "${DEVICE}")' {
 EOF
       save_default_entry | grub_add_tab
       prepare_grub_to_access_device ${DEVICE} | grub_add_tab
@@ -312,7 +312,7 @@ EOF
     ;;
     minix)
 	  cat << EOF
-menuentry "${LONGNAME} (on ${DEVICE}, Multiboot)" {
+menuentry "${LONGNAME} (on ${DEVICE}, Multiboot)" ${GRUB_MINIX_MENUENTRY_OPTIONS} {
 EOF
          save_default_entry | sed -e "s/^/\t/"
          prepare_grub_to_access_device ${DEVICE} | sed -e "s/^/\t/"
-- 
2.1.4


[-- Attachment #3: 0001-Added-GRUB_-_MENUENTRY_OPTIONS-variables-to-configur.patch.sig --]
[-- Type: application/pgp-signature, Size: 543 bytes --]

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] Allow to add/change menu entry class defaults.
  2015-12-26 21:17       ` Robin Schneider
@ 2015-12-27 17:03         ` Andrei Borzenkov
  2015-12-27 20:44           ` Robin Schneider
  0 siblings, 1 reply; 8+ messages in thread
From: Andrei Borzenkov @ 2015-12-27 17:03 UTC (permalink / raw)
  To: grub-devel

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

27.12.2015 00:17, Robin Schneider пишет:
> I am sorry for the misunderstanding. I should have explained the indention
> behind my patch a bit better then just linking to another patch which makes use
> of the newly introduced variables by this patch.
> 
> My indented use case is to allow to add options like '--unrestricted' or
> '--users "Jane"' to each menuentry generated by grub-mkconfig without altering
> the scripts itself.

Oh, no, sorry. CLASS is for adding --class option and --class option is
for defining icon used to represent menu entry. Please do not misuse it
for something else.

I try to understand possible use cases.

Please get a look at
https://lists.gnu.org/archive/html/grub-devel/2015-05/msg00170.html
thread. SUSE has actually implemented my suggestion. This gives us "all
menu entries unrestricted" case.

Do you really have situation where you need separate category of users
that won't have access to CLI but will be the *only* users allowed to
select non-default menu entry? Moreover, do you really need to allow
different users to boot different categories of menu entries?

...

> 
> BTW: The efi menuentry has the class 'windows'. Is that correct? My patch
> assumes that this menuentry is indented for UEFI applications.
> 

Well, so far upstream os-prober only detects Windows on EFI. But yes,
SUSE includes additional script.

See https://lists.gnu.org/archive/html/grub-devel/2015-12/msg00103.html
- does it address your concern?


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] Allow to add/change menu entry class defaults.
  2015-12-27 17:03         ` Andrei Borzenkov
@ 2015-12-27 20:44           ` Robin Schneider
  2016-01-13 12:12             ` Robin Schneider
  0 siblings, 1 reply; 8+ messages in thread
From: Robin Schneider @ 2015-12-27 20:44 UTC (permalink / raw)
  To: grub-devel

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 27.12.2015 18:03, Andrei Borzenkov wrote:
> 27.12.2015 00:17, Robin Schneider пишет:
>> I am sorry for the misunderstanding. I should have explained the
>> indention behind my patch a bit better then just linking to another patch
>> which makes use of the newly introduced variables by this patch.
>> 
>> My indented use case is to allow to add options like '--unrestricted' or 
>> '--users "Jane"' to each menuentry generated by grub-mkconfig without
>> altering the scripts itself.
> 
> Oh, no, sorry. CLASS is for adding --class option and --class option is for
> defining icon used to represent menu entry. Please do not misuse it for
> something else.

Sorry for that.

> I try to understand possible use cases.
> 
> Please get a look at 
> https://lists.gnu.org/archive/html/grub-devel/2015-05/msg00170.html thread.
> SUSE has actually implemented my suggestion. This gives us "all menu
> entries unrestricted" case.

The patch suggested would not allow to overwrite icons via the --class option.
Otherwise it looks very similar to mine :)

> 
> Do you really have situation where you need separate category of users that
> won't have access to CLI but will be the *only* users allowed to select
> non-default menu entry? Moreover, do you really need to allow different
> users to boot different categories of menu entries?

I personally don’t need either right now. The "all menu entries unrestricted"
thing is enough for me. But allowing to specify a user instead of
- --unrestricted to all menu entries should not make this patch more complected
so I still would like to allow it. I attached an updated patch :)

Although I don’t need either of those features, I still think that they can be
useful. For example, you want to use --unrestricted for the default boot
entry, but boot images like memtest+ (as packaged by Debian [1]) only for
authenticated user(s). Another example would be when users put DBAN into the
boot menu :) (Sure, memtest+ and DBAN are not included in upstream grub.d, but
it should emphasize the point that it can make sense to restrict based on type
of bootable image/system).

Another reason for restricting based on type might be if you have installed a
distribution/OS (which is not the default entry), lets say windows, which the
administrator thinks could be used to manipulate the GRUB or other
configuration on the system when booted thus restricting it with a separate
user (--users).

[1]: https://packages.debian.org/jessie/memtest86+

You can chose if you want to apply my updated/simplified patch, my previous
patch allowing restricts based on type or the patch from Michael Chang (or
none of the above :) ).

>> BTW: The efi menuentry has the class 'windows'. Is that correct? My
>> patch assumes that this menuentry is indented for UEFI applications.
>> 
> 
> Well, so far upstream os-prober only detects Windows on EFI. But yes, SUSE
> includes additional script.
> 
> See https://lists.gnu.org/archive/html/grub-devel/2015-12/msg00103.html -
> does it address your concern?

Yes. Looks good.

- -- 
Live long and prosper
Robin `ypid` Schneider
-----BEGIN PGP SIGNATURE-----

iQIcBAEBCgAGBQJWgE2fAAoJEIb9mAu/GkD4DroP+wS3d8pVE+fZJy3VdHmZdU9r
AwTv5Q97tnRM1HqwtGMCxr04btJsUWSpX9BOkrNGYzODJtHPaP20ZPQMEKQmg7sE
5GJoGtA0KVhuvkeuuLaE03vNQciLn9RGPWNJDK4GiB886zjThuVhUTxvVABMkMtG
ox66//9O3DmGCyMrBah+Up1K0o9FncoYomQuL3UwD8TDjzoIDODMQKhRffP3hles
WKxmswK+johunOV8cme1GuropJ14Yb1bNxkiCA/JzSqo3cnuB6WZ58SvoAE9nZMN
YAeDQ5GhI2uMV6wCCI62zKTe65eut8EhlIB///ZwY5Rd/NiODfIbodzMDTaNOqy/
Rxd1dIb2natuIkbvhJXPuy9VSNkLrR6c0cJI3AY4lDCNFe4Z6ck9aS9PgVKJ+QBZ
7O8Tl8gmL1K8MJ+RLeEODOwG1GNtdXe+q0KLmA8Vm6qoI+Xquqogcdq2rhDS6Xha
u6K72fTfG7WYY4F0ol2Go7jzkYEojOJ9ezTFRK+lJi88PMFYhUTXLkVL311cWkho
9SRSPQoLVz0Tp+pt7g4L+YGQKyi54fe4+T966JT9JlStn1TjIkbfcqJdGiOabw0K
mCWujseeNEYTJmLEMSMihljjlUOi3sbuKUCPWpyLClGiXxGq3j8aO+NmDhwEtJ3U
bdERiXtbRktkDoCxojGJ
=FW7J
-----END PGP SIGNATURE-----

[-- Attachment #2: 0001-Added-GRUB_MENU_ENTRY_OPTIONS-variable-to-add-option.patch --]
[-- Type: text/x-patch, Size: 15595 bytes --]

From 4ca4265ebf23d07405d3f4218d3ed6d858b4e1c5 Mon Sep 17 00:00:00 2001
From: Robin Schneider <ypid@riseup.net>
Date: Sat, 26 Dec 2015 21:53:30 +0100
Subject: [PATCH] Added GRUB_MENU_ENTRY_OPTIONS variable to add options to menu
 entries.

Useful for making each menuentry '--unrestricted' when using GRUBs
password protection feature.

Ansible role which makes use of this patch:
https://github.com/debops/ansible-grub/pull/7

My indented use case is to allow to add options like '--unrestricted' or
'--users "Jane"' to each menuentry generated by grub-mkconfig without
altering the scripts itself. Although the scripts end up under /etc in
Debian, I did not think that changing these files (and `dpkg-divert`ing
them) would be such a good idea. I have been searching for how to
configure password protection and I did only find "hacks" which either
suggest to edit the grub config under /boot/grub (which is totally
outdated because the configurations is automatically generated in
Debian) or change the "CLASS" variable in the grub.d scripts [12]0_.*
scripts.  So I did go with the later option. Patching these scripts with
configuration management was not ideal so I decided to propose this
patch upstream. With this patch in place it will be possible to
configure this in /etc/default/grub without touching the scripts.

This patch addresses the issue which is mentioned in this article (At
least when the assumption is true that all menuentry of the same
distribution should get the same restrictions/options):
https://help.ubuntu.com/community/Grub2/Passwords#Protecting_Menuentries
"There is currently no automated method of adding users or designating
menu items to be protected. The user must manually edit the GRUB 2
scripts."
---
 util/grub-mkconfig.in       |  3 ++-
 util/grub.d/10_hurd.in      |  4 ++--
 util/grub.d/10_illumos.in   |  2 +-
 util/grub.d/10_kfreebsd.in  |  4 ++--
 util/grub.d/10_linux.in     |  4 ++--
 util/grub.d/10_netbsd.in    |  4 ++--
 util/grub.d/10_windows.in   |  2 +-
 util/grub.d/10_xnu.in       |  2 +-
 util/grub.d/20_linux_xen.in |  4 ++--
 util/grub.d/30_os-prober.in | 14 +++++++-------
 10 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
index 3183744..8c6747b 100644
--- a/util/grub-mkconfig.in
+++ b/util/grub-mkconfig.in
@@ -227,7 +227,8 @@ export GRUB_DEFAULT \
   GRUB_ENABLE_CRYPTODISK \
   GRUB_BADRAM \
   GRUB_OS_PROBER_SKIP_LIST \
-  GRUB_DISABLE_SUBMENU
+  GRUB_DISABLE_SUBMENU \
+  GRUB_MENU_ENTRY_OPTIONS
 
 if test "x${grub_cfg}" != "x"; then
   rm -f "${grub_cfg}.new"
diff --git a/util/grub.d/10_hurd.in b/util/grub.d/10_hurd.in
index 59a9a48..8dadaaa 100644
--- a/util/grub.d/10_hurd.in
+++ b/util/grub.d/10_hurd.in
@@ -100,11 +100,11 @@ hurd_entry () {
 	  grub_warn "$(gettext_printf "Please don't use old title \`%s' for GRUB_DEFAULT, use \`%s' (for versions before 2.00) or \`%s' (for 2.00 or later)" "$GRUB_ACTUAL_DEFAULT" "$replacement_title" "gnuhurd-advanced-$boot_device_id>'gnuhurd-$kernel-$type-$(grub_get_device_id "${GRUB_DEVICE_BOOT}")'")"
       fi
       sed "s/^/$submenu_indentation/" << EOF
-menuentry '$(echo "$title" | grub_quote)' ${CLASS} \$menuentry_id_option 'gnuhurd-$kernel-$type-$(grub_get_device_id "${GRUB_DEVICE_BOOT}")' {
+menuentry '$(echo "$title" | grub_quote)' ${GRUB_MENU_ENTRY_OPTIONS} ${CLASS} \$menuentry_id_option 'gnuhurd-$kernel-$type-$(grub_get_device_id "${GRUB_DEVICE_BOOT}")' {
 EOF
   else
       sed "s/^/$submenu_indentation/" << EOF
-menuentry '$(echo "$OS" | grub_quote)' ${CLASS} \$menuentry_id_option 'gnuhurd-simple-$(grub_get_device_id "${GRUB_DEVICE_BOOT}")' {
+menuentry '$(echo "$OS" | grub_quote)' ${GRUB_MENU_ENTRY_OPTIONS} ${CLASS} \$menuentry_id_option 'gnuhurd-simple-$(grub_get_device_id "${GRUB_DEVICE_BOOT}")' {
 EOF
   fi
 
diff --git a/util/grub.d/10_illumos.in b/util/grub.d/10_illumos.in
index a133e1b..799cf26 100644
--- a/util/grub.d/10_illumos.in
+++ b/util/grub.d/10_illumos.in
@@ -34,7 +34,7 @@ case "${GRUB_DISTRIBUTOR}" in
   ;;
 esac
 
-echo "menuentry '$(echo "$OS" | grub_quote)' ${CLASS} \$menuentry_id_option 'illumos-$(grub_get_device_id "${GRUB_DEVICE_BOOT}")' {"
+echo "menuentry '$(echo "$OS" | grub_quote)' ${GRUB_MENU_ENTRY_OPTIONS} ${CLASS} \$menuentry_id_option 'illumos-$(grub_get_device_id "${GRUB_DEVICE_BOOT}")' {"
 save_default_entry | grub_add_tab
 prepare_grub_to_access_device "${GRUB_DEVICE_BOOT}" | grub_add_tab
 message="$(gettext_printf "Loading kernel of Illumos ...")"
diff --git a/util/grub.d/10_kfreebsd.in b/util/grub.d/10_kfreebsd.in
index 9d8e8fd..ba45601 100644
--- a/util/grub.d/10_kfreebsd.in
+++ b/util/grub.d/10_kfreebsd.in
@@ -86,9 +86,9 @@ kfreebsd_entry ()
 	  title_correction_code="${title_correction_code}if [ \"x\$default\" = '$quoted' ]; then default='$(echo "$replacement_title" | grub_quote)'; fi;"
 	  grub_warn "$(gettext_printf "Please don't use old title \`%s' for GRUB_DEFAULT, use \`%s' (for versions before 2.00) or \`%s' (for 2.00 or later)" "$GRUB_ACTUAL_DEFAULT" "$replacement_title" "kfreebsd-advanced-$boot_device_id>kfreebsd-$version-$type-$boot_device_id")"
       fi
-      echo "menuentry '$(echo "$title" | grub_quote)' ${CLASS} \$menuentry_id_option 'kfreebsd-$version-$type-$boot_device_id' {" | sed "s/^/$submenu_indentation/"
+      echo "menuentry '$(echo "$title" | grub_quote)' ${GRUB_MENU_ENTRY_OPTIONS} ${CLASS} \$menuentry_id_option 'kfreebsd-$version-$type-$boot_device_id' {" | sed "s/^/$submenu_indentation/"
   else
-      echo "menuentry '$(echo "$OS" | grub_quote)' ${CLASS} \$menuentry_id_option 'kfreebsd-simple-$boot_device_id' {" | sed "s/^/$submenu_indentation/"
+      echo "menuentry '$(echo "$OS" | grub_quote)' ${GRUB_MENU_ENTRY_OPTIONS} ${CLASS} \$menuentry_id_option 'kfreebsd-simple-$boot_device_id' {" | sed "s/^/$submenu_indentation/"
   fi
   if [ x$type != xrecovery ] ; then
       save_default_entry | grub_add_tab | sed "s/^/$submenu_indentation/"
diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
index 859b608..535eeda 100644
--- a/util/grub.d/10_linux.in
+++ b/util/grub.d/10_linux.in
@@ -90,9 +90,9 @@ linux_entry ()
 	  title_correction_code="${title_correction_code}if [ \"x\$default\" = '$quoted' ]; then default='$(echo "$replacement_title" | grub_quote)'; fi;"
 	  grub_warn "$(gettext_printf "Please don't use old title \`%s' for GRUB_DEFAULT, use \`%s' (for versions before 2.00) or \`%s' (for 2.00 or later)" "$GRUB_ACTUAL_DEFAULT" "$replacement_title" "gnulinux-advanced-$boot_device_id>gnulinux-$version-$type-$boot_device_id")"
       fi
-      echo "menuentry '$(echo "$title" | grub_quote)' ${CLASS} \$menuentry_id_option 'gnulinux-$version-$type-$boot_device_id' {" | sed "s/^/$submenu_indentation/"
+      echo "menuentry '$(echo "$title" | grub_quote)' ${GRUB_MENU_ENTRY_OPTIONS} ${CLASS} \$menuentry_id_option 'gnulinux-$version-$type-$boot_device_id' {" | sed "s/^/$submenu_indentation/"
   else
-      echo "menuentry '$(echo "$os" | grub_quote)' ${CLASS} \$menuentry_id_option 'gnulinux-simple-$boot_device_id' {" | sed "s/^/$submenu_indentation/"
+      echo "menuentry '$(echo "$os" | grub_quote)' ${GRUB_MENU_ENTRY_OPTIONS} ${CLASS} \$menuentry_id_option 'gnulinux-simple-$boot_device_id' {" | sed "s/^/$submenu_indentation/"
   fi      
   if [ x$type != xrecovery ] ; then
       save_default_entry | grub_add_tab
diff --git a/util/grub.d/10_netbsd.in b/util/grub.d/10_netbsd.in
index 874f599..8587456 100644
--- a/util/grub.d/10_netbsd.in
+++ b/util/grub.d/10_netbsd.in
@@ -113,9 +113,9 @@ netbsd_entry ()
 	  grub_warn "$(gettext_printf "Please don't use old title \`%s' for GRUB_DEFAULT, use \`%s' (for versions before 2.00) or \`%s' (for 2.00 or later)" "$GRUB_ACTUAL_DEFAULT" "$replacement_title" "netbsd-advanced-$boot_device_id>netbsd-${loader}-$kernel-$type-$boot_device_id")"
       fi
 
-      echo "menuentry '$(echo "$title" | grub_quote)' \$menuentry_id_option 'netbsd-${loader}-$kernel-$type-$boot_device_id' {"  | sed "s/^/$submenu_indentation/"
+      echo "menuentry '$(echo "$title" | grub_quote)' ${GRUB_MENU_ENTRY_OPTIONS} \$menuentry_id_option 'netbsd-${loader}-$kernel-$type-$boot_device_id' {"  | sed "s/^/$submenu_indentation/"
   else
-      echo "menuentry '$(echo "$OS" | grub_quote)' \$menuentry_id_option 'netbsd-${loader}-simple-$boot_device_id' {"  | sed "s/^/$submenu_indentation/"
+      echo "menuentry '$(echo "$OS" | grub_quote)' ${GRUB_MENU_ENTRY_OPTIONS} \$menuentry_id_option 'netbsd-${loader}-simple-$boot_device_id' {"  | sed "s/^/$submenu_indentation/"
   fi
 
   printf "%s\n" "${prepare_boot_cache}" | sed "s/^/$submenu_indentation/"
diff --git a/util/grub.d/10_windows.in b/util/grub.d/10_windows.in
index 554c561..3f120f1 100644
--- a/util/grub.d/10_windows.in
+++ b/util/grub.d/10_windows.in
@@ -84,7 +84,7 @@ for drv in $drives ; do
 
   gettext_printf "Found %s on %s (%s)\n" "$OS" "$drv" "$dev" >&2
   cat << EOF
-menuentry '$(echo "$OS" | grub_quote)' \$menuentry_id_option '$osid-$(grub_get_device_id "${dev}")' {
+menuentry '$(echo "$OS" | grub_quote)' ${GRUB_MENU_ENTRY_OPTIONS} \$menuentry_id_option '$osid-$(grub_get_device_id "${dev}")' {
 EOF
 
   save_default_entry | sed -e 's,^,$grub_tab,'
diff --git a/util/grub.d/10_xnu.in b/util/grub.d/10_xnu.in
index 51ee2f4..1d2c45a 100644
--- a/util/grub.d/10_xnu.in
+++ b/util/grub.d/10_xnu.in
@@ -37,7 +37,7 @@ osx_entry() {
     # TRANSLATORS: it refers on the OS residing on device %s
     onstr="$(gettext_printf "(on %s)" "${GRUB_DEVICE}")"
         cat << EOF
-menuentry '$(echo "Darwin/Mac OS X $bitstr $onstr" | grub_quote)' --class osx --class darwin --class os \$menuentry_id_option 'osprober-xnu-$2-$(grub_get_device_id "${GRUB_DEVICE}")'  {
+menuentry '$(echo "Darwin/Mac OS X $bitstr $onstr" | grub_quote)' ${GRUB_MENU_ENTRY_OPTIONS} --class osx --class darwin --class os \$menuentry_id_option 'osprober-xnu-$2-$(grub_get_device_id "${GRUB_DEVICE}")'  {
 EOF
 	save_default_entry | grub_add_tab
 	prepare_grub_to_access_device ${GRUB_DEVICE} | grub_add_tab
diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
index f532fb9..c655179 100644
--- a/util/grub.d/20_linux_xen.in
+++ b/util/grub.d/20_linux_xen.in
@@ -98,10 +98,10 @@ linux_entry ()
          title_correction_code="${title_correction_code}if [ \"x\$default\" = '$quoted' ]; then default='$(echo "$replacement_title" | grub_quote)'; fi;"
          grub_warn "$(gettext_printf "Please don't use old title \`%s' for GRUB_DEFAULT, use \`%s' (for versions before 2.00) or \`%s' (for 2.00 or later)" "$GRUB_ACTUAL_DEFAULT" "$replacement_title" "gnulinux-advanced-$boot_device_id>gnulinux-$version-$type-$boot_device_id")"
       fi
-      echo "menuentry '$(echo "$title" | grub_quote)' ${CLASS} \$menuentry_id_option 'xen-gnulinux-$version-$type-$boot_device_id' {" | sed "s/^/$submenu_indentation/"
+      echo "menuentry '$(echo "$title" | grub_quote)' ${GRUB_MENU_ENTRY_OPTIONS} ${CLASS} \$menuentry_id_option 'xen-gnulinux-$version-$type-$boot_device_id' {" | sed "s/^/$submenu_indentation/"
   else
       title="$(gettext_printf "%s, with Xen hypervisor" "${os}")"
-      echo "menuentry '$(echo "$title" | grub_quote)' ${CLASS} \$menuentry_id_option 'xen-gnulinux-simple-$boot_device_id' {" | sed "s/^/$submenu_indentation/"
+      echo "menuentry '$(echo "$title" | grub_quote)' ${GRUB_MENU_ENTRY_OPTIONS} ${CLASS} \$menuentry_id_option 'xen-gnulinux-simple-$boot_device_id' {" | sed "s/^/$submenu_indentation/"
   fi
   if [ x$type != xrecovery ] ; then
       save_default_entry | grub_add_tab | sed "s/^/$submenu_indentation/"
diff --git a/util/grub.d/30_os-prober.in b/util/grub.d/30_os-prober.in
index 5fc4f0c..8be9c69 100644
--- a/util/grub.d/30_os-prober.in
+++ b/util/grub.d/30_os-prober.in
@@ -52,7 +52,7 @@ osx_entry() {
     # TRANSLATORS: it refers on the OS residing on device %s
     onstr="$(gettext_printf "(on %s)" "${DEVICE}")"
         cat << EOF
-menuentry '$(echo "${LONGNAME} $bitstr $onstr" | grub_quote)' --class osx --class darwin --class os \$menuentry_id_option 'osprober-xnu-$2-$(grub_get_device_id "${DEVICE}")'  {
+menuentry '$(echo "${LONGNAME} $bitstr $onstr" | grub_quote)' ${GRUB_MENU_ENTRY_OPTIONS} --class osx --class darwin --class os \$menuentry_id_option 'osprober-xnu-$2-$(grub_get_device_id "${DEVICE}")'  {
 EOF
 	save_default_entry | grub_add_tab
 	prepare_grub_to_access_device ${DEVICE} | grub_add_tab
@@ -142,7 +142,7 @@ for OS in ${OSPROBED} ; do
 
 	  onstr="$(gettext_printf "(on %s)" "${DEVICE}")"
       cat << EOF
-menuentry '$(echo "${LONGNAME} $onstr" | grub_quote)' --class windows --class os \$menuentry_id_option 'osprober-chain-$(grub_get_device_id "${DEVICE}")' {
+menuentry '$(echo "${LONGNAME} $onstr" | grub_quote)' ${GRUB_MENU_ENTRY_OPTIONS} --class windows --class os \$menuentry_id_option 'osprober-chain-$(grub_get_device_id "${DEVICE}")' {
 EOF
       save_default_entry | grub_add_tab
       prepare_grub_to_access_device ${DEVICE} | grub_add_tab
@@ -174,7 +174,7 @@ EOF
 	DEVICE=${DEVICE%@*}
 	onstr="$(gettext_printf "(on %s)" "${DEVICE}")"
       cat << EOF
-menuentry '$(echo "${LONGNAME} $onstr" | grub_quote)' --class windows --class os \$menuentry_id_option 'osprober-efi-$(grub_get_device_id "${DEVICE}")' {
+menuentry '$(echo "${LONGNAME} $onstr" | grub_quote)' ${GRUB_MENU_ENTRY_OPTIONS} --class windows --class os \$menuentry_id_option 'osprober-efi-$(grub_get_device_id "${DEVICE}")' {
 EOF
       save_default_entry | sed -e "s/^/\t/"
       prepare_grub_to_access_device ${DEVICE} | sed -e "s/^/\t/"
@@ -230,7 +230,7 @@ EOF
 
 	if [ "x$is_top_level" = xtrue ] && [ "x${GRUB_DISABLE_SUBMENU}" != xy ]; then
             cat << EOF
-menuentry '$(echo "$OS $onstr" | grub_quote)' --class gnu-linux --class gnu --class os \$menuentry_id_option 'osprober-gnulinux-simple-$boot_device_id' {
+menuentry '$(echo "$OS $onstr" | grub_quote)' ${GRUB_MENU_ENTRY_OPTIONS} --class gnu-linux --class gnu --class os \$menuentry_id_option 'osprober-gnulinux-simple-$boot_device_id' {
 EOF
 	    save_default_entry | grub_add_tab
 	    printf '%s\n' "${prepare_boot_cache}"
@@ -250,7 +250,7 @@ EOF
 	fi
 	title="${LLABEL} $onstr"
         cat << EOF
-	menuentry '$(echo "$title" | grub_quote)' --class gnu-linux --class gnu --class os \$menuentry_id_option 'osprober-gnulinux-$LKERNEL-${recovery_params}-$boot_device_id' {
+	menuentry '$(echo "$title" | grub_quote)' ${GRUB_MENU_ENTRY_OPTIONS} --class gnu-linux --class gnu --class os \$menuentry_id_option 'osprober-gnulinux-$LKERNEL-${recovery_params}-$boot_device_id' {
 EOF
 	save_default_entry | sed -e "s/^/$grub_tab$grub_tab/"
 	printf '%s\n' "${prepare_boot_cache}" | grub_add_tab
@@ -287,7 +287,7 @@ EOF
     hurd)
       onstr="$(gettext_printf "(on %s)" "${DEVICE}")"
       cat << EOF
-menuentry '$(echo "${LONGNAME} $onstr" | grub_quote)' --class hurd --class gnu --class os \$menuentry_id_option 'osprober-gnuhurd-/boot/gnumach.gz-false-$(grub_get_device_id "${DEVICE}")' {
+menuentry '$(echo "${LONGNAME} $onstr" | grub_quote)' ${GRUB_MENU_ENTRY_OPTIONS} --class hurd --class gnu --class os \$menuentry_id_option 'osprober-gnuhurd-/boot/gnumach.gz-false-$(grub_get_device_id "${DEVICE}")' {
 EOF
       save_default_entry | grub_add_tab
       prepare_grub_to_access_device ${DEVICE} | grub_add_tab
@@ -312,7 +312,7 @@ EOF
     ;;
     minix)
 	  cat << EOF
-menuentry "${LONGNAME} (on ${DEVICE}, Multiboot)" {
+menuentry "${LONGNAME} (on ${DEVICE}, Multiboot)" ${GRUB_MENU_ENTRY_OPTIONS} {
 EOF
          save_default_entry | sed -e "s/^/\t/"
          prepare_grub_to_access_device ${DEVICE} | sed -e "s/^/\t/"
-- 
2.1.4


[-- Attachment #3: 0001-Added-GRUB_MENU_ENTRY_OPTIONS-variable-to-add-option.patch.sig --]
[-- Type: application/pgp-signature, Size: 543 bytes --]

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] Allow to add/change menu entry class defaults.
  2015-12-27 20:44           ` Robin Schneider
@ 2016-01-13 12:12             ` Robin Schneider
  0 siblings, 0 replies; 8+ messages in thread
From: Robin Schneider @ 2016-01-13 12:12 UTC (permalink / raw)
  To: grub-devel

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 12/27/2015 09:44 PM, Robin Schneider wrote:
> On 27.12.2015 18:03, Andrei Borzenkov wrote:
>> 27.12.2015 00:17, Robin Schneider ?????:
>>> I am sorry for the misunderstanding. I should have explained the 
>>> indention behind my patch a bit better then just linking to another
>>> patch which makes use of the newly introduced variables by this patch.
>>> 
>>> My indented use case is to allow to add options like '--unrestricted'
>>> or '--users "Jane"' to each menuentry generated by grub-mkconfig
>>> without altering the scripts itself.
> 
>> Oh, no, sorry. CLASS is for adding --class option and --class option is
>> for defining icon used to represent menu entry. Please do not misuse it
>> for something else.
> 
> Sorry for that.
> 
>> I try to understand possible use cases.
> 
>> Please get a look at 
>> https://lists.gnu.org/archive/html/grub-devel/2015-05/msg00170.html
>> thread. SUSE has actually implemented my suggestion. This gives us "all
>> menu entries unrestricted" case.
> 
> The patch suggested would not allow to overwrite icons via the --class
> option. Otherwise it looks very similar to mine :)
> 
> 
>> Do you really have situation where you need separate category of users
>> that won't have access to CLI but will be the *only* users allowed to
>> select non-default menu entry? Moreover, do you really need to allow
>> different users to boot different categories of menu entries?
> 
> I personally don’t need either right now. The "all menu entries
> unrestricted" thing is enough for me. But allowing to specify a user
> instead of --unrestricted to all menu entries should not make this patch
> more complected so I still would like to allow it. I attached an updated
> patch :)
> 
> Although I don’t need either of those features, I still think that they can
> be useful. For example, you want to use --unrestricted for the default
> boot entry, but boot images like memtest+ (as packaged by Debian [1]) only
> for authenticated user(s). Another example would be when users put DBAN
> into the boot menu :) (Sure, memtest+ and DBAN are not included in upstream
> grub.d, but it should emphasize the point that it can make sense to
> restrict based on type of bootable image/system).
> 
> Another reason for restricting based on type might be if you have installed
> a distribution/OS (which is not the default entry), lets say windows, which
> the administrator thinks could be used to manipulate the GRUB or other 
> configuration on the system when booted thus restricting it with a
> separate user (--users).
> 
> [1]: https://packages.debian.org/jessie/memtest86+
> 
> You can chose if you want to apply my updated/simplified patch, my
> previous patch allowing restricts based on type or the patch from Michael
> Chang (or none of the above :) ).

Any updates?

> 
>>> BTW: The efi menuentry has the class 'windows'. Is that correct? My 
>>> patch assumes that this menuentry is indented for UEFI applications.
>>> 
> 
>> Well, so far upstream os-prober only detects Windows on EFI. But yes,
>> SUSE includes additional script.
> 
>> See https://lists.gnu.org/archive/html/grub-devel/2015-12/msg00103.html
>> - does it address your concern?
> 
> Yes. Looks good.

- -- 
Live long and prosper
Robin `ypid` Schneider
-----BEGIN PGP SIGNATURE-----

iQIcBAEBCgAGBQJWlj84AAoJEIb9mAu/GkD4FTsQAIdpf5d7Sd1c2aP+ONmeeS99
5/uWy0xAAGE6Uck1ht2BN+NneX5o7AEAFC1ttGe7bOXJho7vz8A3/RQ2XN5tjjj1
JXzuo5s85C9b5B/AMIW4y/H276Px5sMVNKv42tKuVQBGXU7vheyfQt27LpvpuzEq
jqJrgPLbgGQBgyiImUTkWiQkF6fzxNlhXu96eymrqLlzR/XcEEox5cKqCgvjcm/V
KfpUzyHtnkBSsFcjXLA+JjFc+IB53oMJouThhggUXPB4M/UQ8vJu7TC8ai2Cbvpg
TRNIDTZ7GdSG/LcHYndNrcGhrhEnlbFTmoR86PJ7XtcfLhs4b8/mvbZlT178qK/i
tBB0i8y+yD6YWeeB/A9sfzpcZPo9L90Ug5ipZuMQHwcLsJo+MHVCHGRHsrskSova
6nve+iBYNLy0dEwxBTHpu5PK6hsjfS1kGzupzR9hSa1lbYFvQc+Gy4b8L7DS9AjC
Cjb42b73BwI6gibMaq7W7wMk0v1R9ycOxO1/0qZ6+SiVfGtv/xG85qf6vOClxX4f
kVCT+5kyunWuShCvFMgcI2jgajlg+ak1iyjm0bo4PGEd5kdMckXJddp4MKwc0SIH
cvPTVQ1e429TT8w6KteDI58WiJJKb38l8nTomHku2bDV2Vspu2qdNo7UT47+yyGw
tsF29ubKGH6uDSJbzog5
=yyKo
-----END PGP SIGNATURE-----


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-01-13 12:12 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-22 15:19 [PATCH] Allow to add/change menu entry class defaults Robin Schneider
2015-12-23  6:58 ` Andrei Borzenkov
2015-12-23 20:54   ` Robin Schneider
2015-12-24  8:21     ` Andrei Borzenkov
2015-12-26 21:17       ` Robin Schneider
2015-12-27 17:03         ` Andrei Borzenkov
2015-12-27 20:44           ` Robin Schneider
2016-01-13 12:12             ` Robin Schneider

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.