Fix to use direct ACL checks in GRUB 2 module

https://github.com/webmin/webmin/pull/2743#discussion_r3319948219

grub2/acl_security.pl

@@ -14,18 +14,18 @@ sub acl_security_form
 my ($o) = @_;
 # View permissions
 print &ui_table_row($text{'acl_view'},
-		    &ui_yesno_radio("view", &grub2_check_acl("view", $o)), 3);
+		    &ui_yesno_radio("view", $o->{'view'}), 3);
 print &ui_table_hr();
 # Change permissions modify GRUB configuration without granting install rights.
 foreach my $a (qw(edit security apply runtime)) {
 	print &ui_table_row($text{'acl_'.$a},
-			    &ui_yesno_radio($a, &grub2_check_acl($a, $o)), 3);
+			    &ui_yesno_radio($a, $o->{$a}), 3);
 	}
 print &ui_table_hr();
 # Admin permissions expose direct file editing, backup, and boot-loader install.
 foreach my $a (qw(manual install backup)) {
 	print &ui_table_row($text{'acl_'.$a},
-			    &ui_yesno_radio($a, &grub2_check_acl($a, $o)), 3);
+			    &ui_yesno_radio($a, $o->{$a}), 3);
 	}
 }
 
@@ -34,7 +34,7 @@ foreach my $a (qw(manual install backup)) {
 sub acl_security_save
 {
 my ($o) = @_;
-foreach my $a (&grub2_acl_keys()) {
+foreach my $a (qw(view edit security apply runtime manual install backup)) {
 	# Missing checkbox/radio values fall back to denied.
 	$o->{$a} = $in{$a} || 0;
 	}

grub2/custom_action.cgi

@@ -9,7 +9,8 @@ our (%in, %text);
 
 &ReadParse();
 &error_setup($text{'custom_err'});
-&grub2_assert_acl('manual');
+my %access = &get_module_acl();
+&error("$text{'eacl_np'} $text{'eacl_pmanual'}") if (!$access{'manual'});
 
 # Per-row up/down links post an index and direction directly.
 if (defined($in{'idx'}) || defined($in{'dir'})) {

grub2/edit_custom.cgi

@@ -9,7 +9,8 @@ our (%in, %text);
 
 &ReadParse();
 &error_setup($text{'custom_err'});
-&grub2_assert_acl('manual');
+my %access = &get_module_acl();
+&error("$text{'eacl_np'} $text{'eacl_pmanual'}") if (!$access{'manual'});
 
 my $is_new = !defined($in{'idx'}) || $in{'idx'} eq '';
 my ($title, $id, $body) =

grub2/edit_defaults.cgi

@@ -9,7 +9,8 @@ our (%text);
 
 &ReadParse();
 &error_setup($text{'defaults_err'});
-&grub2_assert_acl('edit');
+my %access = &get_module_acl();
+&error("$text{'eacl_np'} $text{'eacl_pedit'}") if (!$access{'edit'});
 
 my $parsed = &read_grub_defaults();
 my $values = $parsed->{'values'};

grub2/edit_install.cgi

@@ -9,7 +9,8 @@ our (%text);
 
 &ReadParse();
 &error_setup($text{'install_err'});
-&grub2_assert_acl('install');
+my %access = &get_module_acl();
+&error("$text{'eacl_np'} $text{'eacl_pinstall'}") if (!$access{'install'});
 
 my $cmd = &grub2_command('install_cmd');
 my $platform = &grub2_default_platform_target();

grub2/edit_manual.cgi

@@ -9,7 +9,8 @@ our (%in, %text);
 
 &ReadParse();
 &error_setup($text{'manual_err'});
-&grub2_assert_acl('manual');
+my %access = &get_module_acl();
+&error("$text{'eacl_np'} $text{'eacl_pmanual'}") if (!$access{'manual'});
 
 # The manual editor is restricted to discovered GRUB-related files only.
 my @files = &grub2_manual_files();

grub2/edit_security.cgi

@@ -9,7 +9,8 @@ our (%text);
 
 &ReadParse();
 &error_setup($text{'security_err'});
-&grub2_assert_acl('security');
+my %access = &get_module_acl();
+&error("$text{'eacl_np'} $text{'eacl_psecurity'}") if (!$access{'security'});
 
 my $state = &grub2_read_security_config();
 

grub2/edit_theme.cgi

@@ -9,7 +9,8 @@ our (%text);
 
 &ReadParse();
 &error_setup($text{'theme_err'});
-&grub2_assert_acl('edit');
+my %access = &get_module_acl();
+&error("$text{'eacl_np'} $text{'eacl_pedit'}") if (!$access{'edit'});
 
 my $parsed = &read_grub_defaults();
 my $values = $parsed->{'values'};

grub2/generate.cgi

@@ -9,7 +9,8 @@ our (%in, %text);
 
 &ReadParse();
 &error_setup($text{'generate_err'});
-&grub2_assert_acl('apply');
+my %access = &get_module_acl();
+&error("$text{'eacl_np'} $text{'eacl_papply'}") if (!$access{'apply'});
 
 my $return_url = $in{'redir'} || "index.cgi";
 my ($current_step, $failed_printed, $failure_output_shown, $command_output) =

grub2/grub2-lib.pl

@@ -21,51 +21,6 @@ $grub2_config_change_flag = $module_var_directory."/config-flag";
 $grub2_generate_time_flag = $module_var_directory."/generate-flag";
 &load_grub2_defaults();
 
-# grub2_acl_keys()
-# Returns the supported GRUB 2 ACL capabilities.
-sub grub2_acl_keys
-{
-return qw(view edit security apply runtime manual install backup);
-}
-
-# grub2_effective_acl([&raw-acl])
-# Returns normalized ACL settings for a supplied ACL hash or current user.
-sub grub2_effective_acl
-{
-my ($rawacl) = @_;
-my %raw = $rawacl ? %$rawacl : &get_module_acl();
-return map { $_ => $raw{$_} ? 1 : 0 } &grub2_acl_keys();
-}
-
-# grub2_check_acl(action, [&raw-acl])
-# Returns true when an effective ACL permits the requested action.
-sub grub2_check_acl
-{
-my ($action, $rawacl) = @_;
-my %acl = &grub2_effective_acl($rawacl);
-return $acl{$action} ? 1 : 0;
-}
-
-# grub2_assert_acl(action)
-# Fails if the current Webmin user cannot perform an action.
-sub grub2_assert_acl
-{
-my ($action) = @_;
-&grub2_check_acl($action) ||
-	&error("$text{'eacl_np'} $text{'eacl_p'.$action}");
-}
-
-# grub2_can_enter_module(&acl)
-# Returns true if a user has at least one useful module capability.
-sub grub2_can_enter_module
-{
-my ($acl) = @_;
-foreach my $a (&grub2_acl_keys()) {
-	return 1 if ($acl->{$a});
-	}
-return 0;
-}
-
 # grub2_mark_regenerate_needed()
 # Updates the flag indicating that grub.cfg needs to be regenerated.
 sub grub2_mark_regenerate_needed

grub2/index.cgi

@@ -5,13 +5,13 @@ use strict;
 use warnings;
 require './grub2-lib.pl';    ## no critic
 
-our (%in, %text);
+our (%in, %text, %access);
 our $module_name;
 our $grub2_formno = 0;
 
 &ReadParse();
 &error_setup($text{'acl_ecannot'});
-my %access = &grub2_effective_acl();
+%access = &get_module_acl();
 &error("$text{'eacl_np'} $text{'eacl_pview'}")
 	if (!&can_use_index(\%access));
 
@@ -361,7 +361,7 @@ if ($direct_file) {
 else {
 	$html = &ui_tag('tt', &html_escape($file));
 	}
-if (&grub2_check_acl('manual') && &grub2_manual_file($file)) {
+if ($access{'manual'} && &grub2_manual_file($file)) {
 	# The manual editor repeats its allowlist check on entry.
 	$html = &ui_tag('a', $html, {
 		'href' => "edit_manual.cgi?file=".&urlize($file),

grub2/install.cgi

@@ -9,7 +9,8 @@ our (%in, %text);
 
 &ReadParse();
 &error_setup($text{'install_err'});
-&grub2_assert_acl('install');
+my %access = &get_module_acl();
+&error("$text{'eacl_np'} $text{'eacl_pinstall'}") if (!$access{'install'});
 
 # Trim text fields before validation so shell arguments are deterministic.
 foreach my $field (qw(target efi_dir platform directory boot_directory

grub2/reboot_once.cgi

@@ -9,7 +9,8 @@ our (%in, %text);
 
 &ReadParse();
 &error_setup($text{'runtime_err'});
-&grub2_assert_acl('runtime');
+my %access = &get_module_acl();
+&error("$text{'eacl_np'} $text{'eacl_pruntime'}") if (!$access{'runtime'});
 
 # Runtime commands operate on the parsed generated menu index shown on index.cgi.
 my $entry = &grub2_entry_by_index($in{'idx'});

grub2/save_custom.cgi

@@ -9,7 +9,8 @@ our (%in, %text);
 
 &ReadParse();
 &error_setup($text{'custom_err'});
-&grub2_assert_acl('manual');
+my %access = &get_module_acl();
+&error("$text{'eacl_np'} $text{'eacl_pmanual'}") if (!$access{'manual'});
 
 # A missing index means create; a present one must address a parsed entry.
 my $idx = defined($in{'idx'}) && $in{'idx'} ne '' ? $in{'idx'} : undef;

grub2/save_defaults.cgi

@@ -9,7 +9,8 @@ our (%in, %text);
 
 &ReadParse();
 &error_setup($text{'defaults_err'});
-&grub2_assert_acl('edit');
+my %access = &get_module_acl();
+&error("$text{'eacl_np'} $text{'eacl_pedit'}") if (!$access{'edit'});
 
 # Capture both the current defaults file and generated entries for validation.
 my $current = &read_grub_defaults();

grub2/save_manual.cgi

@@ -9,7 +9,8 @@ our (%in, %text);
 
 &ReadParseMime();
 &error_setup($text{'manual_err'});
-&grub2_assert_acl('manual');
+my %access = &get_module_acl();
+&error("$text{'eacl_np'} $text{'eacl_pmanual'}") if (!$access{'manual'});
 
 # Re-check the allowlist on save; the form value is not trusted.
 my $file = $in{'file'} || '';

grub2/save_security.cgi

@@ -9,7 +9,8 @@ our (%in, %text);
 
 &ReadParse();
 &error_setup($text{'security_err'});
-&grub2_assert_acl('security');
+my %access = &get_module_acl();
+&error("$text{'eacl_np'} $text{'eacl_psecurity'}") if (!$access{'security'});
 
 # The enable flag is deliberately boolean; all other fields normalize below.
 defined($in{'enabled'}) && $in{'enabled'} =~ /^[01]\z/ ||

grub2/save_theme.cgi

@@ -9,7 +9,8 @@ our (%in, %text);
 
 &ReadParse();
 &error_setup($text{'theme_err'});
-&grub2_assert_acl('edit');
+my %access = &get_module_acl();
+&error("$text{'eacl_np'} $text{'eacl_pedit'}") if (!$access{'edit'});
 
 my $parsed = &read_grub_defaults();
 my $current_values = $parsed->{'values'};

grub2/set_default.cgi

@@ -9,7 +9,8 @@ our (%in, %text);
 
 &ReadParse();
 &error_setup($text{'runtime_err'});
-&grub2_assert_acl('runtime');
+my %access = &get_module_acl();
+&error("$text{'eacl_np'} $text{'eacl_pruntime'}") if (!$access{'runtime'});
 
 # Runtime commands operate on the parsed generated menu index shown on index.cgi.
 my $entry = &grub2_entry_by_index($in{'idx'});

grub2/status.cgi

@@ -5,12 +5,11 @@ use strict;
 use warnings;
 require './grub2-lib.pl';    ## no critic
 
-our (%text);
+our (%text, %access);
 
 &error_setup($text{'acl_ecannot'});
-&grub2_assert_acl('view');
-
-my %access = &grub2_effective_acl();
+%access = &get_module_acl();
+&error("$text{'eacl_np'} $text{'eacl_pview'}") if (!$access{'view'});
 &ui_print_header(undef, $text{'status_title'}, "");
 
 # Missing-install output mirrors index.cgi but keeps this page read-only.
@@ -194,7 +193,7 @@ return &ui_tag('tt', &html_escape($raw)).' '.$text{'index_not_readable'};
 sub manual_path_link
 {
 my ($path, $html) = @_;
-return $html if (!&grub2_check_acl('manual') || !&grub2_manual_file($path));
+return $html if (!$access{'manual'} || !&grub2_manual_file($path));
 # Link only allowlisted paths; generated grub.cfg remains informational.
 return &ui_tag('a', $html, {
 	'href' => "edit_manual.cgi?file=".&urlize($path),

grub2/t/run-tests.t

@@ -101,14 +101,6 @@ $config{'shell_cmd'} = '/bin/sh';
 mkdir $config{'grub_dir'} or die "mkdir grub.d: $!";
 mkdir $bls_dir or die "mkdir bls_dir: $!";
 
-{
-    my %acl = grub2_effective_acl({ view => 1, edit => 0 });
-    ok($acl{'view'}, 'view ACL is enabled');
-    ok(!$acl{'edit'}, 'edit ACL is disabled');
-    ok(!grub2_can_enter_module({ map { $_ => 0 } grub2_acl_keys() }),
-       'empty ACL cannot enter module');
-}
-
 ok(!grub2_needs_regenerate(), 'fresh module does not need regeneration');
 grub2_mark_regenerate_needed();
 ok(grub2_needs_regenerate(), 'config change flag requires regeneration');
@@ -191,7 +183,12 @@ like($acl_source,
      'ACL view permission is rendered as a standalone row');
 unlike($acl_source, qr/acl_section_view/,
        'ACL view permission does not use a one-row section heading');
-like($status_source, qr/grub2_assert_acl\('view'\)/,
+like($acl_source, qr/&ui_yesno_radio\("view", \$o->\{'view'\}\)/,
+     'ACL editor uses the supplied view ACL value directly');
+unlike($acl_source, qr/grub2_(?:check|effective)_acl/,
+       'ACL editor does not normalize supplied ACL values');
+like($status_source,
+     qr/%access = &get_module_acl\(\).*?\$access\{'view'\}/s,
      'status page enforces view ACL directly');
 like($status_source, qr/index_boot_mode.*boot_mode_cell/s,
      'status page displays detected boot mode');
@@ -242,7 +239,7 @@ like($status_source,
      qr/sub path_cell\b.*manual_path_link\(\$path, &ui_tag\('tt'/s,
      'status page renders paths with tt tags');
 like($status_source,
-     qr/sub manual_path_link\b.*grub2_check_acl\('manual'\).*grub2_manual_file\(\$path\).*edit_manual\.cgi\?file=/s,
+     qr/sub manual_path_link\b.*\$access\{'manual'\}.*grub2_manual_file\(\$path\).*edit_manual\.cgi\?file=/s,
      'status page links allowlisted paths to the manual editor when permitted');
 like($status_source, qr/sub status_table_row\b.*hlink\(\$label, \$help\)/s,
      'status page labels use contextual help links');
@@ -324,7 +321,7 @@ like($index_source,
      qr/sub entry_source_detail_line\b.*index_col_file.*index_col_generator.*ui_tag\('a'.*edit_manual\.cgi\?file=/ms,
      'entry details label generator scripts and link direct entry files to the manual editor');
 like($index_source,
-     qr/sub entry_source_detail_line\b.*else \{\s*\$html = &ui_tag\('tt', &html_escape\(\$file\)\).*?grub2_check_acl\('manual'\).*?edit_manual\.cgi\?file=/s,
+     qr/sub entry_source_detail_line\b.*else \{\s*\$html = &ui_tag\('tt', &html_escape\(\$file\)\).*?\$access\{'manual'\}.*?edit_manual\.cgi\?file=/s,
      'entry details link editable generator scripts to the manual editor');
 like($index_source,
      qr/sub entry_source_detail_line\b.*entry_file_display_name.*'title'\s*=>\s*\$file.*edit_manual\.cgi\?file=/s,