Fix to detect NetworkManager networking on Debian

ⓘ Prefer Netplan when Debian has Netplan YAML config, otherwise select the existing NetworkManager backend for Debian systems with saved NM connection profiles, with regression tests for backend selection.

https://github.com/webmin/webmin/issues/2559

.github/workflows/close-inactive.yml

@@ -0,0 +1,15 @@
+name: Close inactive
+
+on:
+  schedule:
+    - cron: "0 12 * * *"
+  workflow_dispatch:
+
+permissions:
+  contents: read
+  issues: write
+  pull-requests: write
+
+jobs:
+  close-inactive:
+    uses: webmin/webmin-ci-cd/.github/workflows/close-inactive.yml@main

.github/workflows/tests.yml

@@ -0,0 +1,19 @@
+name: Tests
+
+on:
+  pull_request:
+    branches:
+      - master
+  push:
+    branches:
+      - master
+
+jobs:
+  prove:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - name: Install Perl::Critic
+        run: sudo apt-get update && sudo apt-get install -y libperl-critic-perl
+      - name: prove -lr
+        run: prove -lr

.github/workflows/webmin.dev+webmin.yml

@@ -1,4 +1,4 @@
-name: "webmin.dev: webmin/webmin"
+name: Build
 
 on:
   push:
@@ -25,3 +25,5 @@ jobs:
       PRERELEASE_UPLOAD_SSH_DIR: ${{ secrets.PRERELEASE_UPLOAD_SSH_DIR }}
       DEV_SSH_PRV_KEY: ${{ secrets.DEV_SSH_PRV_KEY }}
       ALL_GPG_PH2: ${{ secrets.ALL_GPG_PH2 }}
+      CODE_REVIEW_API_KEY: ${{ secrets.CODE_REVIEW_API_KEY }}
+      CODE_REVIEW_SMTP_PASSWORD: ${{ secrets.CODE_REVIEW_SMTP_PASSWORD }}

.perlcriticrc

@@ -0,0 +1,6 @@
+severity = 5
+
+# Octal file permission literals (0700, 0640, etc.) are the standard Perl
+# idiom for chmod/mkdir/permission helpers throughout this codebase. The
+# policy flags chmod 0700 itself, so it is too coarse for our use.
+[-ValuesAndExpressions::ProhibitLeadingZeros]

CHANGELOG.md

@@ -1,5 +1,10 @@
 ## Changelog
 
+#### 2.641 (May 10, 2026)
+* Fixed a bug when editing monitors in the System and Server Status module
+* Fix Fail2Ban default jail options
+* Added support for trusted proxy IP addresses
+
 #### 2.640 (May 4, 2026)
 * Add new nftables module with profiles, saved tables, and chains/sets management
 * Add new Nginx module with look and feel matching the Apache module

acl/acl-lib.pl

@@ -11,7 +11,7 @@ Library for editing webmin users, passwords and access rights.
 
 =cut
 
-BEGIN { push(@INC, ".."); };
+BEGIN { push(@INC, ".."); };    ## no critic
 use strict;
 use warnings;
 no warnings 'redefine';
@@ -92,7 +92,6 @@ while(my $l = <$fh>) {
 		$user{'locale'} = $gconfig{"locale_$user[0]"};
 		$user{'dateformat'} = $gconfig{"dateformat_$user[0]"};
 		$user{'notabs'} = $gconfig{"notabs_$user[0]"};
-		$user{'rbacdeny'} = $gconfig{"rbacdeny_$user[0]"};
 		if ($gconfig{"theme_$user[0]"}) {
 			($user{'theme'}, $user{'overlay'}) =
 				split(/\s+/, $gconfig{"theme_$user[0]"});
@@ -353,7 +352,8 @@ each of which is a hash reference in the same format as their module.info files.
 sub list_module_infos
 {
 my @mods = grep { &check_os_support($_) } &get_all_module_infos();
-return sort { $a->{'desc'} cmp $b->{'desc'} } @mods;
+my @sorted = sort { $a->{'desc'} cmp $b->{'desc'} } @mods;
+return @sorted;
 }
 
 =head2 create_user(&details, [clone])
@@ -501,8 +501,6 @@ else {
 	$gconfig{"lang_".$user->{'name'}} = $user->{'lang'} if ($user->{'lang'});
 	delete($gconfig{"notabs_".$user->{'name'}});
 	$gconfig{"notabs_".$user->{'name'}} = $user->{'notabs'} if ($user->{'notabs'});
-	delete($gconfig{"rbacdeny_".$user->{'name'}});
-	$gconfig{"rbacdeny_".$user->{'name'}} = $user->{'rbacdeny'} if ($user->{'rbacdeny'});
 	delete($gconfig{"ownmods_".$user->{'name'}});
 	$gconfig{"ownmods_".$user->{'name'}} = join(" ", @{$user->{'ownmods'}})
 		if ($user->{'ownmods'} && @{$user->{'ownmods'}});
@@ -722,9 +720,6 @@ else {
 	delete($gconfig{"notabs_".$username});
 	$gconfig{"notabs_".$user->{'name'}} = $user->{'notabs'}
 		if ($user->{'notabs'});
-	delete($gconfig{"rbacdeny_".$username});
-	$gconfig{"rbacdeny_".$user->{'name'}} = $user->{'rbacdeny'}
-		if ($user->{'rbacdeny'});
 	delete($gconfig{"ownmods_".$username});
 	$gconfig{"ownmods_".$user->{'name'}} = join(" ", @{$user->{'ownmods'}})
 		if ($user->{'ownmods'} && @{$user->{'ownmods'}});
@@ -1337,12 +1332,12 @@ my ($miniserv) = @_;
 my $sfile = $miniserv->{'sessiondb'} ? $miniserv->{'sessiondb'} :
 	    $miniserv->{'pidfile'} =~ /^(.*)\/[^\/]+$/ ? "$1/sessiondb"
 						     : return;
-eval "use SDBM_File";
+eval { require SDBM_File; SDBM_File->import; 1 };
 dbmopen(%sessiondb, $sfile, 0700);
 eval { $sessiondb{'1111111111'} = 'foo bar' };
 if ($@) {
 	dbmclose(%sessiondb);
-	eval "use NDBM_File";
+	eval { require NDBM_File; NDBM_File->import; 1 };
 	dbmopen(%sessiondb, $sfile, 0700);
 	}
 else {
@@ -1428,10 +1423,10 @@ Creates a new session ID that's already logged in as the given user
 sub create_session_user
 {
 my ($miniserv, $username, $lifetime) = @_;
-return undef if (&is_readonly_mode());
+return if (&is_readonly_mode());
 &open_session_db($miniserv);
 my $sid = &generate_random_session_id();
-return undef if (!$sid);
+return if (!$sid);
 my $t = time();
 $sessiondb{$sid} = "$username $t 127.0.0.1".($lifetime ? " ".$lifetime : "");
 dbmclose(%sessiondb);
@@ -1699,7 +1694,7 @@ elsif (&has_command("ssleay")) {
 	return &has_command("ssleay");
 	}
 else {
-	return undef;
+	return;
 	}
 }
 
@@ -1815,6 +1810,7 @@ foreach my $g (&list_groups()) {
 		return $g;
 		}
 	}
+return;
 }
 
 =head2 check_password_restrictions(username, password)
@@ -1859,7 +1855,7 @@ if ($miniserv{'pass_oldblock'} && $user) {
 		last if ($c++ > $miniserv{'pass_oldblock'});
 		}
 	}
-return undef;
+return;
 }
 
 =head2 hash_session_id(sid)
@@ -1896,11 +1892,11 @@ my $use_md5 = &md5_perl_module();
 $use_md5 || &error("No Perl MD5 hashing module found!");
 
 # Add the password
-my $ctx = eval "new $use_md5";
+my $ctx = $use_md5->new;
 $ctx->add($passwd);
 
 # Add some more stuff from the hash of the password and salt
-my $ctx1 = eval "new $use_md5";
+my $ctx1 = $use_md5->new;
 $ctx1->add($passwd);
 $ctx1->add($passwd);
 my $final = $ctx1->digest();
@@ -1949,12 +1945,12 @@ Returns a Perl module for MD5 hashing, or undef if none.
 sub md5_perl_module
 {
 my $use_md5;
-eval "use MD5";
+eval { require MD5; MD5->import; 1 };
 if (!$@) {
         $use_md5 = "MD5";
         }
 else {
-        eval "use Digest::MD5";
+        eval { require Digest::MD5; Digest::MD5->import; 1 };
         if (!$@) {
                 $use_md5 = "Digest::MD5";
                 }
@@ -2111,16 +2107,16 @@ my ($str, $notablecheck) = @_;
 my ($proto, $user, $pass, $host, $prefix, $args) = &split_userdb_string($str);
 if ($proto eq "mysql" || $proto eq "postgresql") {
 	# Load DBI driver
-	eval 'use DBI;';
+	eval { require DBI; DBI->import; 1 };
 	return &text('sql_emod', 'DBI') if ($@);
 	if ($proto eq "mysql") {
-		eval 'use DBD::mysql;';
+		eval { require DBD::mysql; DBD::mysql->import; 1 };
 		return &text('sql_emod', 'DBD::mysql') if ($@);
 		my $drh = DBI->install_driver("mysql");
 		return $text{'sql_emysqldriver'} if (!$drh);
 		}
 	else {
-		eval 'use DBD::Pg;';
+		eval { require DBD::Pg; DBD::Pg->import; 1 };
 		return &text('sql_emod', 'DBD::Pg') if ($@);
 		my $drh = DBI->install_driver("Pg");
 		return $text{'sql_epostgresqldriver'} if (!$drh);
@@ -2152,11 +2148,11 @@ if ($proto eq "mysql" || $proto eq "postgresql") {
 			}
 		}
 	&disconnect_userdb($str, $dbh);
-	return undef;
+	return;
 	}
 elsif ($proto eq "ldap") {
 	# Load LDAP module
-	eval 'use Net::LDAP;';
+	eval { require Net::LDAP; Net::LDAP->import; 1 };
 	return &text('sql_emod', 'Net::LDAP') if ($@);
 
 	# Try to connect
@@ -2190,7 +2186,7 @@ elsif ($proto eq "ldap") {
 		$found || return &text('sql_eldapdn', $prefix);
 		}
 	&disconnect_userdb($str, $dbh);
-	return undef;
+	return;
 	}
 else {
 	return "Unknown user database type $proto";
@@ -2284,8 +2280,8 @@ if (!$miniserv) {
 	$miniserv = { };
 	&get_miniserv_config($miniserv);
 	}
-foreach $a (split(/\s+/, $miniserv->{'anonymous'})) {
-        if ($a =~ /^([^=]+)=(\S+)$/ && $2 eq $user) {
+foreach my $tok (split(/\s+/, $miniserv->{'anonymous'})) {
+        if ($tok =~ /^([^=]+)=(\S+)$/ && $2 eq $user) {
 		push(@rv, $1);
 		}
 	}
@@ -2299,7 +2295,7 @@ sub get_safe_acl
 my ($m) = @_;
 my $mdir = &module_root_directory($m);
 my %rv;
-&read_file_cached("$mdir/safeacl", \%rv) || return undef;
+&read_file_cached("$mdir/safeacl", \%rv) || return;
 return \%rv;
 }
 
@@ -2313,17 +2309,19 @@ sub generate_random_session_id
 my $sid;
 
 # Try /dev/urandom, but with a timeout
-$SIG{ALRM} = sub { close(RANDOM) };
+my $randomfh;
+$SIG{ALRM} = sub { close($randomfh) if ($randomfh) };
 alarm(5);
-if (open(RANDOM, "/dev/urandom")) {
+if (open($randomfh, "<", "/dev/urandom")) {
 	my $tmpsid;
-	if (read(RANDOM, $tmpsid, 16) == 16) {
+	if (read($randomfh, $tmpsid, 16) == 16) {
 		$sid = lc(unpack('h*',$tmpsid));
 		if ($sid !~ /^[0-9a-fA-F]{32}$/) {
 			$sid = 'bad';
 			}
 		}
-	close(RANDOM);
+	close($randomfh);
+	undef($randomfh);
 	}
 alarm(0);
 
@@ -2341,7 +2339,7 @@ return $sid eq 'bad' ? undef : $sid;
 # Generate an ID string that can be used for a password reset link
 sub generate_random_id
 {
-if (open(my $RANDOM, "</dev/urandom")) {
+if (open(my $RANDOM, "<", "/dev/urandom")) {
 	my $sid;
 	my $tmpsid;
 	if (read($RANDOM, $tmpsid, 16) == 16) {
@@ -2350,7 +2348,9 @@ if (open(my $RANDOM, "</dev/urandom")) {
 	close($RANDOM);
 	return $sid;
 	}
-return undef;
+# Explicit undef: callers consume this in hash-literal value position,
+# where bare 'return' would yield () and shift the surrounding pairing.
+return undef;    ## no critic (ProhibitExplicitReturnUndef)
 }
 
 # obsfucate_email(email)

acl/backup_config.pl

@@ -3,7 +3,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require 'acl-lib.pl';
+require 'acl-lib.pl';    ## no critic
 our ($config_directory, %gconfig);
 
 # backup_config_files()
@@ -43,7 +43,7 @@ return @rv;
 # Called before the files are actually read
 sub pre_backup
 {
-return undef;
+return;
 }
 
 # post_backup(&files)
@@ -52,7 +52,7 @@ sub post_backup
 {
 unlink("$config_directory/config.aclbackup");
 unlink("$config_directory/miniserv.conf.aclbackup");
-return undef;
+return;
 }
 
 # pre_restore(&files)
@@ -66,7 +66,7 @@ foreach my $u (&list_users(), &list_groups()) {
 		       glob("$config_directory/*/$u->{'name'}.acl"));
 		}
 	}
-return undef;
+return;
 }
 
 # post_restore(&files)
@@ -101,7 +101,7 @@ foreach my $k (keys %aclbackup) {
 &put_miniserv_config(\%miniserv);
 
 &restart_miniserv();
-return undef;
+return;
 }
 
 1;

acl/cert_form.cgi

@@ -5,11 +5,11 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access);
 &ui_print_header(undef, $text{'cert_title'}, "", undef, undef, undef, undef,
 		 undef, undef, "language=VBSCRIPT onload='postLoad()'");
-eval "use Net::SSLeay";
+eval { require Net::SSLeay; Net::SSLeay->import; 1 };
 
 print "<p>$text{'cert_msg'}<p>\n";
 if ($ENV{'SSL_USER'}) {

acl/cert_issue.cgi

@@ -5,7 +5,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access, $module_config_directory, $base_remote_user);
 &ReadParse();
 

acl/cert_output.cgi

@@ -5,7 +5,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access);
 
 &ReadParse();

acl/cgi_args.pl

@@ -27,5 +27,5 @@ elsif ($cgi eq 'edit_acl.cgi') {
 		}
 	return 'none';
 	}
-return undef;
+return;
 }

acl/convert.cgi

@@ -6,7 +6,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access, $config_directory);
 &ReadParse();
 &error_setup($text{'convert_err'});

acl/convert_form.cgi

@@ -6,7 +6,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access);
 $access{'sync'} && $access{'create'} || &error($text{'convert_ecannot'});
 &ui_print_header(undef, $text{'convert_title'}, "");

acl/defaultacl

@@ -19,7 +19,6 @@ sessions=1
 cats=1
 ips=1
 switch=1
-rbacenable=1
 logouttime=1
 times=1
 minsize=1

acl/delete_group.cgi

@@ -6,7 +6,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access, $base_remote_user);
 &ReadParse();
 &error_setup($text{'gdelete_err'});

acl/delete_groups.cgi

@@ -5,7 +5,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access, $base_remote_user);
 &ReadParse();
 &error_setup($text{'gdeletes_err'});

acl/delete_session.cgi

@@ -6,7 +6,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access, %sessiondb);
 &ReadParse();
 $access{'sessions'} || &error($text{'sessions_ecannot'});

acl/delete_user.cgi

@@ -6,7 +6,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access, $base_remote_user);
 &ReadParse();
 &error_setup($text{'delete_err'});

acl/delete_users.cgi

@@ -5,7 +5,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access, $base_remote_user);
 &ReadParse();
 &error_setup($in{'joingroup'} ? $text{'udeletes_jerr'} : $text{'udeletes_err'});

acl/edit_acl.cgi

@@ -6,7 +6,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access, $base_remote_user, %gconfig);
 &ReadParse();
 $access{'acl'} || &error($text{'acl_emod'});
@@ -48,15 +48,6 @@ else {
 	}
 print &ui_table_start(&text('acl_options', $minfo{'desc'}), "width=100%", 4);
 
-if ($in{'mod'} && $in{'user'} && &supports_rbac($in{'mod'}) &&
-    !$gconfig{'rbacdeny_'.$who}) {
-	# Show RBAC option
-	print &ui_table_row($text{'acl_rbac'},
-		&ui_radio("rbac", $maccess{'rbac'} ? 1 : 0,
-			[ [ 1, $text{'acl_rbacyes'} ],
-			  [ 0, $text{'no'} ] ]), 3);
-	}
-
 # Load custom ACL library
 my $mdir = &module_root_directory($in{'mod'});
 if (-r "$mdir/acl_security.pl") {

acl/edit_group.cgi

@@ -6,7 +6,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access, $config_directory);
 &ReadParse();
 $access{'groups'} || &error($text{'gedit_ecannot'});

acl/edit_pass.cgi

@@ -5,7 +5,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access);
 $access{'pass'} || &error($text{'pass_ecannot'});
 &ui_print_header(undef, $text{'pass_title'}, "");

acl/edit_rbac.cgi

@@ -1,32 +0,0 @@
-#!/usr/local/bin/perl
-# Show RBAC status
-
-use strict;
-use warnings;
-no warnings 'redefine';
-no warnings 'uninitialized';
-require './acl-lib.pl';
-our (%in, %text, %gconfig, %access, $module_name, $module_root_directory);
-$access{'rbacenable'} || &error($text{'rbac_ecannot'});
-&ui_print_header(undef, $text{'rbac_title'}, "");
-
-print "$text{'rbac_desc'}<p>\n";
-if ($gconfig{'os_type'} ne 'solaris') {
-	print &text('rbac_esolaris', $gconfig{'real_os_type'}),"<p>\n";
-	}
-elsif (!&supports_rbac()) {
-	if (&foreign_available("cpan")) {
-		print &text('rbac_eperl', "<tt>Authen::SolarisRBAC</tt>",
-			    "../cpan/download.cgi?source=0&local=$module_root_directory/Authen-SolarisRBAC-0.1.tar.gz&mode=2&return=/$module_name/&returndesc=".&urlize($text{'index_return'})),"<p>\n";
-		}
-	else {
-		print &text('rbac_ecpan', "<tt>Authen::SolarisRBAC</tt>"),
-		      "<p>\n";
-		}
-	}
-else {
-	print "$text{'rbac_ok'}<p>\n";
-	}
-
-&ui_print_footer("", $text{'index_return'});
-

acl/edit_sql.cgi

@@ -5,7 +5,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access);
 $access{'sql'} || &error($text{'sql_ecannot'});
 &ui_print_header(undef, $text{'sql_title'}, "");

acl/edit_sync.cgi

@@ -6,7 +6,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access);
 $access{'sync'} && $access{'create'} && $access{'delete'} ||
 	&error($text{'sync_ecannot'});

acl/edit_unix.cgi

@@ -7,7 +7,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access);
 $access{'unix'} && $access{'create'} && $access{'delete'} ||
 	&error($text{'unix_ecannot'});

acl/edit_user.cgi

@@ -6,7 +6,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %gconfig, %access, $config_directory, $base_remote_user, $remote_user);
 &foreign_require("webmin", "webmin-lib.pl");
 
@@ -42,6 +42,11 @@ else {
 				      : $text{'edit_title2'}, "");
 	}
 my $me = &get_user($base_remote_user);
+my %uaccess = &get_module_acl($in{'user'} || "", "", 1);
+if (!$in{'user'} && $uaccess{'rpc'} == 2) {
+	# Don't offer the confusing 'root' or 'admin' RPC option by default
+	$uaccess{'rpc'} = 0;
+	}
 
 # Give up if readonly
 if ($user{'readonly'} && !$in{'readwrite'}) {
@@ -210,7 +215,10 @@ if ($access{'lang'}) {
 
 if ($access{'locale'}) {
 	# Current locale
-	eval "use DateTime; use DateTime::Locale; use DateTime::TimeZone;";
+	eval { require DateTime; DateTime->import;
+	       require DateTime::Locale; DateTime::Locale->import;
+	       require DateTime::TimeZone; DateTime::TimeZone->import;
+	       1 };
 	if (!$@ && $] > 5.011) {
 		my $locales = &list_locales();
 		my %localesrev = reverse %{$locales};
@@ -280,7 +288,7 @@ if ($showui) {
 # Start of security options section
 my $showsecurity = $access{'logouttime'} || $access{'ips'} ||
 		   $access{'minsize'} ||
-		   &supports_rbac() && $access{'mode'} == 0 || $access{'times'};
+		   $access{'times'};
 if ($showsecurity) {
 	print &ui_hidden_table_start($text{'edit_security'}, "width=100%", 2,
 				     "security", 0, [ "width=30%" ]);
@@ -321,14 +329,6 @@ if ($access{'ips'}) {
 		    4, 30));
 	}
 
-if (&supports_rbac() && $access{'mode'} == 0) {
-	# Deny access to modules not managed by RBAC?
-	print &ui_table_row($text{'edit_rbacdeny'},
-		&ui_radio("rbacdeny", $user{'rbacdeny'} ? 1 : 0,
-			  [ [ 0, $text{'edit_rbacdeny0'} ],
-			    [ 1, $text{'edit_rbacdeny1'} ] ]));
-	}
-
 if ($access{'times'}) {
 	# Show allowed days of the week
 	my %days = map { $_, 1 } split(/,/, $user{'days'} || '');
@@ -371,6 +371,16 @@ elsif ($miniserv{'twofactor_provider'}) {
 		&ui_submit($text{'edit_twofactoradd'}, "twofactor"));
 	}
 
+# Can accept RPC calls?
+if ($access{'acl'} && !$safe) {
+	print &ui_table_row(&hlink($text{'acl_rpc'}, 'rpc'),
+		&ui_radio("rpc", int($uaccess{'rpc'}),
+			  [ [ 1, $text{'acl_rpc1'} ],
+			    $uaccess{'rpc'} == 2 ? ( [ 2, $text{'acl_rpc2'} ] ) : ( ),
+			    [ 3, $text{'acl_rpc3'} ],
+			    [ 0, $text{'acl_rpc0'} ] ]));
+	}
+
 print &ui_hidden_table_end("security");
 
 # Work out which modules can be selected
@@ -450,11 +460,9 @@ print &ui_hidden_table_end("mods");
 
 # Add global ACL section, but only if not set from the group
 my $groupglobal = $memg && -r "$config_directory/$memg->{'name'}.acl";
-if ($access{'acl'} && !$groupglobal && $in{'user'} && !$safe) {
+if ($access{'acl'} && !$groupglobal && !$safe) {
 	print &ui_hidden_table_start($text{'edit_global'}, "width=100%", 2,
 				     "global", 0, [ "width=30%" ]);
-	my %uaccess;
-	%uaccess = &get_module_acl($in{'user'}, "", 1);
 	print &ui_hidden("acl_security_form", 1);
 	&foreign_require("", "acl_security.pl");
 	&foreign_call("", "acl_security_form", \%uaccess);

acl/forgot_form.cgi

@@ -5,7 +5,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text);
 &foreign_require("webmin");
 &error_setup($text{'forgot_err'});

acl/forgot_send.cgi

@@ -5,7 +5,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %gconfig);
 &foreign_require("webmin");
 &error_setup($text{'forgot_err'});

acl/help/rpc.html

@@ -0,0 +1,14 @@
+<header>Can accept RPC calls?</header>
+
+This option determines if the user can make use of Webmin's poweful RPC calls
+feature, which is designed to allow this system to be controlled by a script
+or other Webmin server via HTTP. Because the RPC feature does <b>not</b>
+respect any module grants or fine-grained access controls, this option should
+only be enabled for fully trusted <tt>root</tt>-equivalent users. <p>
+
+In addition, if the <b>RPC calls only</b> option is selected, the user will not
+be able to use the Webmin UI. This is useful for creating a separate login with
+a longer password for RPC purposes only, as two-factor authentication cannot be
+used when making RPC calls. <p>
+
+<footer>

acl/index.cgi

@@ -6,7 +6,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %gconfig, %access, $base_remote_user);
 &ReadParse();
 &ui_print_header(undef, $text{'index_title'}, "", undef, 1, 1);
@@ -206,11 +206,6 @@ if (uc($ENV{'HTTPS'}) eq "ON" && $miniserv{'ca'}) {
 push(@icons, "images/twofactor.gif");
 push(@links, "twofactor_form.cgi");
 push(@titles, $text{'index_twofactor'});
-if ($access{'rbacenable'} && $gconfig{'os_type'} eq 'solaris') {
-	push(@icons, "images/rbac.gif");
-	push(@links, "edit_rbac.cgi");
-	push(@titles, $text{'index_rbac'});
-	}
 if ($access{'pass'}) {
 	push(@icons, "images/pass.gif");
 	push(@links, "edit_pass.cgi");

acl/lang/en

@@ -23,7 +23,6 @@ index_modgroups=Modules from group $1
 index_sync=Configure Unix User Synchronization
 index_unix=Configure Unix User Authentication
 index_sessions=View Login Sessions
-index_rbac=Setup RBAC
 index_delete=Delete Selected
 index_joingroup=Add To Group:
 index_eulist=Failed to list users : $1
@@ -95,9 +94,6 @@ edit_switch=Switch to User
 edit_forgot=Send Password Reset Link
 edit_return=Webmin user
 edit_return2=Webmin group
-edit_rbacdeny=RBAC access mode
-edit_rbacdeny0=RBAC only controls selected module ACLs
-edit_rbacdeny1=RBAC controls all modules and ACLs
 edit_global=Permissions for all modules
 edit_templock=Temporarily locked
 edit_temppass=Force change at next login
@@ -185,8 +181,6 @@ acl_title3=For group $1 in $2
 acl_options=$1 access control options
 acl_config=Can edit module configuration?
 acl_reset=Reset To Full Access
-acl_rbac=Get access control settings from RBAC?
-acl_rbacyes=Yes (overrides settings below)
 
 acl_uall=All users
 acl_uthis=This user
@@ -382,13 +376,6 @@ hide_clone=(Clone $1)
 switch_euser=You are not allowed to switch to this user
 switch_eold=Existing session not found!
 
-rbac_title=Setup RBAC
-rbac_desc=Webmin's RBAC integration provides a way for user module and ACL permissions to be determined from an RBAC (Role Based Access Control) database, rather than Webmin's own configuration files. Once RBAC support is enabled, any user for whom the <b>RBAC controls all modules and ACLs</b> option is selected will have his capabilities determined by RBAC rather than Webmin's own access control settings.
-rbac_esolaris=RBAC is only supported on Solaris at the moment, and so cannot be used on this $1 system.
-rbac_eperl=The Perl module $1 needed for RBAC integration is not installed. <a href='$2'>Click here</a> to have it installed now.
-rbac_ecpan=You do not have access to Webmin's Perl Modules page in order to install the necessary $1 module for RBAC integration.
-rbac_ok=RBAC integration is available on this system, and can be enabled on a per-user basis on the Edit Webmin User page.
-
 udeletes_err=Failed to delete users
 udeletes_jerr=Failed to add users to group
 udeletes_enone=None selected

acl/list_sessions.cgi

@@ -6,7 +6,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access, %sessiondb);
 $access{'sessions'} || &error($text{'sessions_ecannot'});
 &ui_print_header(undef, $text{'sessions_title'}, "");

acl/log_parser.pl

@@ -16,8 +16,9 @@ my ($user, $script, $action, $type, $object, $p) = @_;
 my $g = $type eq 'group' ? "_g" : "";
 if ($action eq 'modify') {
 	if ($p->{'old'} ne $p->{'name'}) {
-		return &text('log_rename'.$g, "<tt>$p->{'old'}</tt>",
-					      "<tt>$p->{'name'}</tt>");
+		return &text('log_rename'.$g,
+			     "<tt>".&html_escape($p->{'old'})."</tt>",
+			     "<tt>".&html_escape($p->{'name'})."</tt>");
 		}
 	else {
 		return &text('log_modify'.$g,
@@ -26,7 +27,8 @@ if ($action eq 'modify') {
 	}
 elsif ($action eq 'create') {
 	if ($p->{'clone'}) {
-		return &text('log_clone'.$g, "<tt>$p->{'clone'}</tt>",
+		return &text('log_clone'.$g,
+			     "<tt>".&html_escape($p->{'clone'})."</tt>",
 			     "<tt>".&html_escape($object)."</tt>");
 		}
 	else {
@@ -36,21 +38,23 @@ elsif ($action eq 'create') {
 	}
 elsif ($action eq 'delete') {
 	if ($type eq "users" || $type eq "groups") {
-		return &text('log_delete_'.$type, $object);
+		return &text('log_delete_'.$type, &html_escape($object));
 		}
 	else {
-		return &text('log_delete'.$g, "<tt>$object</tt>");
+		return &text('log_delete'.$g,
+			     "<tt>".&html_escape($object)."</tt>");
 		}
 	}
 elsif ($action eq 'joingroup') {
-	return &text('log_joingroup', $object, $p->{'group'});
+	return &text('log_joingroup', &html_escape($object),
+		     &html_escape($p->{'group'}));
 	}
 elsif ($action eq 'acl') {
-	return &text('log_acl', "<tt>$object</tt>",
+	return &text('log_acl', "<tt>".&html_escape($object)."</tt>",
 		     "<i>".&html_escape($p->{'moddesc'})."</i>");
 	}
 elsif ($action eq 'reset') {
-	return &text('log_reset', "<tt>$object</tt>",
+	return &text('log_reset', "<tt>".&html_escape($object)."</tt>",
 		     "<i>".&html_escape($p->{'moddesc'})."</i>");
 	}
 elsif ($action eq 'cert') {
@@ -60,7 +64,9 @@ elsif ($action eq 'switch') {
 	return &text('log_switch', "<tt>".&html_escape($object)."</tt>");
 	}
 elsif ($action eq 'twofactor') {
-	return &text('log_twofactor', $object, $p->{'provider'}, $p->{'id'});
+	return &text('log_twofactor', &html_escape($object),
+		     &html_escape($p->{'provider'}),
+		     &html_escape($p->{'id'}));
 	}
 elsif ($action eq 'forgot') {
 	return &text('log_forgot_'.$type, &html_escape($p->{'user'}),

acl/makedn.cgi

@@ -5,7 +5,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access);
 $access{'pass'} || &error($text{'sql_ecannot'});
 &ReadParse();

acl/maketables.cgi

@@ -5,7 +5,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access);
 $access{'pass'} || &error($text{'sql_ecannot'});
 &ReadParse();

acl/postinstall.pl

@@ -3,7 +3,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require 'acl-lib.pl';
+require 'acl-lib.pl';    ## no critic
 our ($config_directory);
 
 # Rename the .acl files for any groups to .gacl files

acl/save_acl.cgi

@@ -6,7 +6,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access, $base_remote_user, %gconfig,
      $config_directory);
 &ReadParse();
@@ -54,13 +54,8 @@ else {
 	if (defined($in{'noconfig'})) {
 		$maccess{'noconfig'} = $in{'noconfig'};
 		}
-	if ($in{'rbac'}) {
-		# RBAC overrides everything
-		$maccess{'rbac'} = 1;
-		}
-	elsif (-r "../$in{'_acl_mod'}/acl_security.pl") {
+	if (-r "../$in{'_acl_mod'}/acl_security.pl") {
 		# Use user inputs
-		$maccess{'rbac'} = 0 if (defined($in{'rbac'}));
 		&foreign_require($in{'_acl_mod'}, "acl_security.pl");
 		&foreign_call($in{'_acl_mod'}, "acl_security_save",
 			      \%maccess, \%in);

acl/save_group.cgi

@@ -6,7 +6,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access, $config_directory);
 &ReadParse();
 

acl/save_pass.cgi

@@ -5,7 +5,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access);
 $access{'pass'} || &error($text{'pass_ecannot'});
 &error_setup($text{'pass_err'});

acl/save_sql.cgi

@@ -5,7 +5,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access);
 $access{'pass'} || &error($text{'sql_ecannot'});
 &ReadParse();

acl/save_sync.cgi

@@ -6,7 +6,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access, $module_config_directory);
 &ReadParse();
 $access{'sync'} && $access{'create'} && $access{'delete'} ||

acl/save_twofactor.cgi

@@ -5,7 +5,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access, $base_remote_user);
 &foreign_require("webmin");
 &error_setup($text{'twofactor_err'});

acl/save_unix.cgi

@@ -6,7 +6,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access);
 &ReadParse();
 &error_setup($text{'unix_err'});
@@ -70,7 +70,7 @@ else {
 	}
 if ($in{'sudo'}) {
 	&has_command("sudo") || &error(&text('unix_esudo', "<tt>sudo</tt>"));
-	eval "use IO::Pty";
+	eval { require IO::Pty; IO::Pty->import; 1 };
 	$@ && &error(&text('unix_esudomod', "<tt>IO::Pty</tt>"));
 	}
 $miniserv{'sudo'} = $in{'sudo'};

acl/save_user.cgi

@@ -6,7 +6,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access, $config_directory, $base_remote_user);
 &foreign_require("webmin", "webmin-lib.pl");
 &ReadParse();
@@ -105,11 +105,6 @@ foreach my $u (@ulist) {
 # Find the current group
 my $oldgroup = $in{'old'} ? &get_users_group($in{'old'}) : undef;
 
-if (&supports_rbac()) {
-	# Save RBAC mode
-	$user{'rbacdeny'} = $in{'rbacdeny'};
-	}
-
 my $newgroup;
 if (defined($in{'group'})) {
 	# Check if group is allowed
@@ -374,11 +369,13 @@ else {
 	}
 
 my $aclfile = "$config_directory/$in{'name'}.acl";
-if ($in{'old'} && $in{'acl_security_form'} && !$newgroup && !$in{'safe'}) {
-	# Update user's global ACL
+if ($in{'acl_security_form'} && !$newgroup && !$in{'safe'}) {
+	# Update user's global ACL, and merge in RPC setting which has
+	# been moved out of this form
 	&foreign_require("", "acl_security.pl");
 	my %uaccess;
 	&foreign_call("", "acl_security_save", \%uaccess, \%in);
+	$uaccess{'rpc'} = $in{'rpc'};
 	&lock_file($aclfile);
 	&save_module_acl(\%uaccess, $in{'name'}, "", 1);
 	&set_ownership_permissions(undef, undef, 0640, $aclfile);

acl/schema.cgi

@@ -5,7 +5,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access);
 $access{'pass'} || &error($text{'sql_ecannot'});
 

acl/switch.cgi

@@ -5,7 +5,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access, %sessiondb);
 &ReadParse();
 &can_edit_user($in{'user'}) && $access{'switch'} ||

acl/t/perlcritic.t

@@ -0,0 +1,65 @@
+#!/usr/bin/perl
+use strict;
+use warnings;
+use Test::More;
+
+BEGIN {
+    eval { require Perl::Critic; 1 }
+        or plan skip_all => 'Perl::Critic not installed';
+}
+
+use File::Find;
+
+sub script_dir
+{
+    my $path = $0;
+    if ($path =~ m{^/}) {
+        $path =~ s{/[^/]+$}{};
+        return $path;
+    }
+    my $cwd = `pwd`;
+    chomp($cwd);
+    if ($path =~ m{/}) {
+        $path =~ s{/[^/]+$}{};
+        return $cwd.'/'.$path;
+    }
+    return $cwd;
+}
+
+my $bindir = script_dir();
+my $module_dir = "$bindir/..";
+chdir($module_dir) or die "chdir: $!";
+
+my @files;
+find(
+    sub {
+        return if -d;
+        # Skip symlinks: shared libs (e.g. md5-lib.pl -> ../useradmin/md5-lib.pl)
+        # belong to the module that owns the underlying file.
+        return if -l;
+        return unless /\.(pl|cgi)\z/;
+        # *.info.pl is the Polish-locale translation of *.info, not Perl code.
+        return if /\.info\.pl\z/;
+        push(@files, $File::Find::name);
+    },
+    '.'
+);
+
+@files = sort @files;
+if (!@files) {
+    plan skip_all => 'no perl files to check';
+}
+
+my $critic = Perl::Critic->new(
+    -profile => "$bindir/../../.perlcriticrc",
+);
+
+foreach my $file (@files) {
+    my @violations = $critic->critique($file);
+    is(scalar @violations, 0, "$file perlcritic");
+    if (@violations) {
+        diag join("", @violations);
+    }
+}
+
+done_testing();

acl/test_twofactor.cgi

@@ -5,7 +5,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %access, $base_remote_user);
 &foreign_require("webmin");
 &error_setup($text{'twofactor_terr'});

acl/twofactor.pl

@@ -1,6 +1,10 @@
 #!/usr/local/bin/perl
 # Validate the OTP for some user
 
+use strict;
+use warnings;
+no warnings 'once';
+our $module_name;
 $main::no_acl_check = 1;
 $main::no_referers_check = 1;
 $ENV{'WEBMIN_CONFIG'} = "/etc/webmin";
@@ -8,17 +12,19 @@ $ENV{'WEBMIN_VAR'} = "/var/webmin";
 if ($0 =~ /^(.*\/)[^\/]+$/) {
         chdir($1);
         }
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 $module_name eq 'acl' || die "Command must be run with full path";
 
 # Check command-line args
 @ARGV == 5 || die "Usage: $0 user provider id token api-key";
-($user, $provider, $id, $token, $apikey) = @ARGV;
+my ($user, $provider, $id, $token, $apikey) = @ARGV;
 
 # Call the provider validation function
 &foreign_require("webmin");
-$func = "webmin::validate_twofactor_".$provider;
-$err = &$func($id, $token, $apikey);
+my $method = "validate_twofactor_".$provider;
+my $code = webmin->can($method)
+	or die "Unknown twofactor provider: $provider\n";
+my $err = $code->($id, $token, $apikey);
 if ($err) {
 	$err =~ s/\r|\n/ /g;
 	print $err,"\n";

acl/twofactor_form.cgi

@@ -5,7 +5,7 @@ use strict;
 use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
-require './acl-lib.pl';
+require './acl-lib.pl';    ## no critic
 our (%in, %text, %config, %access, $base_remote_user);
 &foreign_require("webmin");
 &error_setup($text{'twofactor_err'});

acl_security.pl

@@ -16,7 +16,7 @@ print &ui_table_row($text{'acl_root'},
 # Other dirs to allow
 print &ui_table_row($text{'acl_otherdirs'},
 	&ui_textarea("otherdirs", join("\n", split(/\t+/, $o->{'otherdirs'})),
-		     5, 40), 3);
+		     3, 40), 3);
 
 # Can see dot files?
 print &ui_table_row($text{'acl_nodot'},
@@ -77,18 +77,6 @@ print &ui_table_row($text{'acl_gedit'},
 
 print &ui_table_hr();
 
-# Can submit feedback?
-print &ui_table_row($text{'acl_feedback'},
-	&ui_radio("feedback", int($o->{'feedback'}),
-	  	  [ map { [ $_, $text{'acl_feedback'.$_} ] } (2,3,1,0) ]));
-
-# Can accept RPC calls?
-print &ui_table_row($text{'acl_rpc'},
-	&ui_radio("rpc", int($o->{'rpc'}),
-		  [ [ 1, $text{'acl_rpc1'} ],
-		    $o->{'rpc'} == 2 ? ( [ 2, $text{'acl_rpc2'} ] ) : ( ),
-		    [ 0, $text{'acl_rpc0'} ] ]));
-
 # Get new permissions?
 print &ui_table_row($text{'acl_negative'},
 	&ui_radio("negative", int($o->{'negative'}),
@@ -110,27 +98,26 @@ print &ui_table_row($text{'acl_webminsearch'},
 # Parse the form for global security options
 sub acl_security_save
 {
-$_[0]->{'root'} = $in{'root_def'} ? undef : $in{'root'};
-$_[0]->{'otherdirs'} = join("\t", split(/\r?\n/, $in{'otherdirs'}));
-$_[0]->{'nodot'} = $in{'nodot'};
+my ($o) = @_;
+$o->{'root'} = $in{'root_def'} ? undef : $in{'root'};
+$o->{'otherdirs'} = join("\t", split(/\r?\n/, $in{'otherdirs'}));
+$o->{'nodot'} = $in{'nodot'};
 
-$_[0]->{'uedit_mode'} = $in{'uedit_mode'};
-$_[0]->{'uedit'} = $in{'uedit_mode'} == 2 ? $in{'uedit_can'} :
+$o->{'uedit_mode'} = $in{'uedit_mode'};
+$o->{'uedit'} = $in{'uedit_mode'} == 2 ? $in{'uedit_can'} :
 		   $in{'uedit_mode'} == 3 ? $in{'uedit_cannot'} :
 		   $in{'uedit_mode'} == 4 ? $in{'uedit_uid'} :
 		   $in{'uedit_mode'} == 5 ? getgrnam($in{'uedit_group'}) : "";
-$_[0]->{'uedit2'} = $in{'uedit_mode'} == 4 ? $in{'uedit_uid2'} : undef;
+$o->{'uedit2'} = $in{'uedit_mode'} == 4 ? $in{'uedit_uid2'} : undef;
 
-$_[0]->{'gedit_mode'} = $in{'gedit_mode'};
-$_[0]->{'gedit'} = $in{'gedit_mode'} == 2 ? $in{'gedit_can'} :
+$o->{'gedit_mode'} = $in{'gedit_mode'};
+$o->{'gedit'} = $in{'gedit_mode'} == 2 ? $in{'gedit_can'} :
 		   $in{'gedit_mode'} == 3 ? $in{'gedit_cannot'} :
 		   $in{'gedit_mode'} == 4 ? $in{'gedit_gid'} : "";
-$_[0]->{'gedit2'} = $in{'gedit_mode'} == 4 ? $in{'gedit_gid2'} : undef;
-$_[0]->{'feedback'} = $in{'feedback'};
-$_[0]->{'rpc'} = $in{'rpc'};
-$_[0]->{'negative'} = $in{'negative'};
-$_[0]->{'readonly'} = $in{'readonly'};
-$_[0]->{'fileunix'} = $in{'fileunix_def'} ? undef : $in{'fileunix'};
-$_[0]->{'webminsearch'} = $in{'webminsearch'};
+$o->{'gedit2'} = $in{'gedit_mode'} == 4 ? $in{'gedit_gid2'} : undef;
+$o->{'negative'} = $in{'negative'};
+$o->{'readonly'} = $in{'readonly'};
+$o->{'fileunix'} = $in{'fileunix_def'} ? undef : $in{'fileunix'};
+$o->{'webminsearch'} = $in{'webminsearch'};
 }
 

apache/apache-lib.pl

@@ -436,6 +436,13 @@ foreach $v (@virt) {
 return \@get_config_cache;
 }
 
+# flush_config_cache()
+# Delete all in-memory config caches
+sub flush_config_cache
+{
+undef(@get_config_cache);
+}
+
 # get_config_file(filename, [&seen-files])
 # Returns a list of config hash refs from some file
 sub get_config_file
@@ -788,6 +795,428 @@ unlink($file);
 &delete_webfile_link($file);
 }
 
+# can_manage_vhost_files()
+# Returns 1 if this system uses Debian-style available/enabled site dirs
+sub can_manage_vhost_files
+{
+return 0 if ($gconfig{'os_type'} ne 'debian-linux');
+my $avail = &vhost_available_dir();
+my $enabled = &vhost_enabled_dir();
+return $avail && -d $avail && $enabled && -d $enabled &&
+       &simplify_path(&resolve_links($avail)) ne
+       &simplify_path(&resolve_links($enabled));
+}
+
+# vhost_available_dir()
+# Returns the configured directory of available Apache virtual host files
+sub vhost_available_dir
+{
+return $config{'virt_file'} ? &server_root($config{'virt_file'}) : undef;
+}
+
+# vhost_enabled_dir()
+# Returns the configured directory of enabled Apache virtual host symlinks
+sub vhost_enabled_dir
+{
+return $config{'link_dir'} ? &server_root($config{'link_dir'}) : undef;
+}
+
+# get_vhost_available_files()
+# Returns real config files from the directory used for new virtual hosts
+sub get_vhost_available_files
+{
+my @rv;
+return @rv if (!&can_manage_vhost_files());
+my $avail = &vhost_available_dir();
+opendir(AVAIL, $avail) || return @rv;
+foreach my $f (sort { lc($a) cmp lc($b) } readdir(AVAIL)) {
+	next if ($f eq "." || $f eq "..");
+	my $file = $avail."/".$f;
+	my $rfile = &simplify_path(&resolve_links($file));
+	next if (!$rfile || !-f $rfile || !-r $rfile);
+	push(@rv, $rfile);
+	}
+closedir(AVAIL);
+return &unique(@rv);
+}
+
+# find_virtuals_in_file(file)
+# Returns VirtualHost blocks parsed from one config file
+sub find_virtuals_in_file
+{
+my ($file) = @_;
+my $rfile = &simplify_path(&resolve_links($file));
+$rfile ||= $file;
+return ( ) if (!-r $rfile);
+my @conf = &get_config_file($rfile);
+return grep { $_->{'file'} eq $rfile }
+       &find_directive_struct("VirtualHost", \@conf);
+}
+
+# is_default_vhost(&virt)
+# Returns 1 if a VirtualHost looks like a default/catch-all host
+sub is_default_vhost
+{
+my ($virt) = @_;
+return 1 if (!$virt);
+return 1 if ($virt->{'value'} =~ /_default_/i);
+return 1 if (!&find_directive("ServerName", $virt->{'members'}));
+return 0;
+}
+
+# can_manage_vhost_file(file)
+# Returns 1 if all virtual hosts in a file are manageable by this user
+sub can_manage_vhost_file
+{
+my ($file) = @_;
+my $rfile = &simplify_path(&resolve_links($file));
+$rfile ||= $file;
+return 0 if (!$rfile || !-f $rfile || !-r $rfile);
+my @virts = &find_virtuals_in_file($rfile);
+return 0 if (!@virts);
+foreach my $virt (@virts) {
+	return 0 if (&is_default_vhost($virt));
+	return 0 if (!&can_edit_virt($virt));
+	}
+return 1;
+}
+
+# can_manage_vhost_state_file(file)
+# Returns 1 if a virtual host file can have its enabled state managed here
+sub can_manage_vhost_state_file
+{
+my ($file) = @_;
+my $rfile = &simplify_path(&resolve_links($file));
+$rfile ||= $file;
+return 0 if (!$rfile || !-f $rfile);
+my %available = map { $_, 1 } &get_vhost_available_files();
+return 0 if (!$available{$rfile});
+return &can_manage_vhost_file($rfile);
+}
+
+# get_virtual_list_rows(&config)
+# Returns row hashes for the virtual-host list, preserving sites-available order
+sub get_virtual_list_rows
+{
+my ($conf) = @_;
+my @active = grep { &can_edit_virt($_) }
+	     &find_directive_struct("VirtualHost", $conf);
+if (&can_manage_vhost_files()) {
+	my @rows;
+	my %active_by_file;
+	foreach my $v (@active) {
+		my $file = &simplify_path(&resolve_links($v->{'file'}));
+		$file ||= $v->{'file'};
+		push(@{$active_by_file{$file}}, $v);
+		}
+	my %done_virt;
+	foreach my $file (&get_vhost_available_files()) {
+		my @filevirts = @{$active_by_file{$file} || [ ]};
+		my $active = @filevirts ? 1 : 0;
+		if (!@filevirts) {
+			@filevirts = grep { &can_edit_virt($_) &&
+					     !&is_default_vhost($_) }
+				     &find_virtuals_in_file($file);
+			}
+		foreach my $v (@filevirts) {
+			push(@rows, { 'virt' => $v,
+				      'active' => $active,
+				      'file' => $file });
+			$done_virt{$v}++;
+			}
+		}
+	foreach my $v (@active) {
+		next if ($done_virt{$v});
+		push(@rows, { 'virt' => $v,
+			      'active' => 1,
+			      'file' => $v->{'file'} });
+		}
+	return @rows;
+	}
+return map { { 'virt' => $_, 'active' => 1, 'file' => $_->{'file'} } }
+       @active;
+}
+
+# vhost_file_link(file)
+# Returns the enabled symlink path for a virtual host file
+sub vhost_file_link
+{
+my ($file) = @_;
+return undef if (!&can_manage_vhost_files());
+my $rfile = &simplify_path(&resolve_links($file));
+$rfile ||= $file;
+my $avail = &vhost_available_dir();
+my $short;
+if (opendir(AVAIL, $avail)) {
+	foreach my $f (sort { lc($a) cmp lc($b) } readdir(AVAIL)) {
+		next if ($f eq "." || $f eq "..");
+		my $afile = $avail."/".$f;
+		my $rafile = &simplify_path(&resolve_links($afile));
+		if ($rafile && $rafile eq $rfile) {
+			$short = $f;
+			last;
+			}
+		}
+	closedir(AVAIL);
+	}
+$short ||= $rfile;
+$short =~ s/^.*\///;
+return &vhost_enabled_dir()."/".$short;
+}
+
+# vhost_file_links(file)
+# Returns enabled symlinks for a virtual host file
+sub vhost_file_links
+{
+my ($file) = @_;
+my @rv;
+return @rv if (!&can_manage_vhost_files());
+my $rfile = &simplify_path(&resolve_links($file));
+$rfile ||= $file;
+my $enabled = &vhost_enabled_dir();
+opendir(LINKDIR, $enabled) || return @rv;
+foreach my $f (readdir(LINKDIR)) {
+	next if ($f eq "." || $f eq "..");
+	my $link = $enabled."/".$f;
+	next if (!-l $link);
+	my $rlink = &simplify_path(&resolve_links($link));
+	if ($rlink && $rlink eq $rfile) {
+		push(@rv, $link);
+		}
+	}
+closedir(LINKDIR);
+return @rv;
+}
+
+# vhost_file_enabled(file)
+# Returns 1 if a virtual host file has an enabled symlink
+sub vhost_file_enabled
+{
+my ($file) = @_;
+return scalar(&vhost_file_links($file)) ? 1 : 0;
+}
+
+# enable_vhost_file(file)
+# Enables a virtual host file and rolls back if apache configtest fails
+sub enable_vhost_file
+{
+my ($file) = @_;
+my $rfile = &simplify_path(&resolve_links($file));
+$rfile ||= $file;
+return $text{'enable_efile'} if (!&can_manage_vhost_state_file($rfile));
+my $verr = &virtualmin_vhost_file_state_error($rfile, "enable");
+return $verr if ($verr);
+my $link = &vhost_file_link($rfile);
+$link || return $text{'enable_elinkdir'};
+return undef if (&vhost_file_enabled($rfile));
+if (-e $link || -l $link) {
+	return &text('enable_elinkexists', "<tt>".&html_escape($link)."</tt>");
+	}
+&symlink_logged($rfile, $link) ||
+	return &text('enable_elink', "<tt>".&html_escape($link)."</tt>",
+		     "<tt>".&html_escape($!)."</tt>");
+my $err = &test_config();
+if ($err) {
+	&unlink_logged($link);
+	return &text('enable_etest', "<tt>".&html_escape($err)."</tt>");
+	}
+&flush_config_cache();
+&update_last_config_change();
+return undef;
+}
+
+# disable_vhost_file(file)
+# Disables a virtual host file and rolls back if apache configtest fails
+sub disable_vhost_file
+{
+my ($file) = @_;
+my $rfile = &simplify_path(&resolve_links($file));
+$rfile ||= $file;
+return $text{'enable_efile'} if (!&can_manage_vhost_state_file($rfile));
+my $verr = &virtualmin_vhost_file_state_error($rfile, "disable");
+return $verr if ($verr);
+my @links = &vhost_file_links($file);
+return undef if (!@links);
+my @restore = map { [ $_, readlink($_) ] } @links;
+my @removed;
+foreach my $link (@links) {
+	if (!&unlink_logged($link)) {
+		foreach my $r (@removed) {
+			&symlink_logged($r->[1], $r->[0])
+				if (defined($r->[1]) && !-e $r->[0] && !-l $r->[0]);
+			}
+		return &text('enable_eunlink',
+			     "<tt>".&html_escape($link)."</tt>",
+			     "<tt>".&html_escape($!)."</tt>");
+		}
+	my ($restore) = grep { $_->[0] eq $link } @restore;
+	push(@removed, $restore) if ($restore);
+	}
+my $err = &test_config();
+if ($err) {
+	foreach my $r (@restore) {
+		&symlink_logged($r->[1], $r->[0])
+			if (defined($r->[1]) && !-e $r->[0] && !-l $r->[0]);
+		}
+	return &text('enable_etest', "<tt>".&html_escape($err)."</tt>");
+	}
+&flush_config_cache();
+&update_last_config_change();
+return undef;
+}
+
+# virtualmin_available()
+# Returns 1 if Virtualmin is installed and supported on this system
+sub virtualmin_available
+{
+return $main::apache_virtualmin_available
+	if (defined($main::apache_virtualmin_available));
+$main::apache_virtualmin_available = &foreign_check("virtual-server");
+return $main::apache_virtualmin_available;
+}
+
+# virtualmin_domain_by_name(name)
+# Returns a Virtualmin domain object by domain name, if one exists
+sub virtualmin_domain_by_name
+{
+my ($name) = @_;
+return undef if (!&virtualmin_available());
+return $main::apache_virtualmin_domain_by_name_cache{$name}
+	if (exists($main::apache_virtualmin_domain_by_name_cache{$name}));
+&foreign_require("virtual-server");
+my $d = &virtual_server::get_domain_by("dom", $name);
+$main::apache_virtualmin_domain_by_name_cache{$name} = $d;
+return $d;
+}
+
+# virtual_names(&virt)
+# Returns all hostnames from ServerName and ServerAlias directives
+sub virtual_names
+{
+my ($virt) = @_;
+my @rv;
+my $sn = &find_directive("ServerName", $virt->{'members'});
+push(@rv, $sn) if ($sn);
+foreach my $sa (&find_directive_struct("ServerAlias", $virt->{'members'})) {
+	push(@rv, @{$sa->{'words'} || [ ]});
+	if (!@{$sa->{'words'} || [ ]} && $sa->{'value'}) {
+		push(@rv, $sa->{'value'});
+		}
+	}
+return grep { $_ && $_ ne "*" } &unique(@rv);
+}
+
+# virtualmin_domain_for_vhost_file(file)
+# Returns the Virtualmin domain object for a virtual host file, if any
+sub virtualmin_domain_for_vhost_file
+{
+my ($file) = @_;
+return undef if (!&virtualmin_available());
+my $rfile = &simplify_path(&resolve_links($file));
+$rfile ||= $file;
+return $main::apache_virtualmin_domain_for_file_cache{$rfile}
+	if (exists($main::apache_virtualmin_domain_for_file_cache{$rfile}));
+foreach my $virt (&find_virtuals_in_file($file)) {
+	next if (!&can_edit_virt($virt));
+	foreach my $name (&virtual_names($virt)) {
+		my $d = &virtualmin_domain_by_name($name);
+		if (!$d && $name =~ /^www\.(\S+)/i) {
+			$d = &virtualmin_domain_by_name($1);
+			}
+		if ($d) {
+			$main::apache_virtualmin_domain_for_file_cache{$rfile} = $d;
+			return $d;
+			}
+		}
+	}
+$main::apache_virtualmin_domain_for_file_cache{$rfile} = undef;
+return undef;
+}
+
+# vhost_file_state(file)
+# Returns the effective enabled state for a virtual host file
+sub vhost_file_state
+{
+my ($file) = @_;
+my $d = &virtualmin_domain_for_vhost_file($file);
+if ($d) {
+	return { 'enabled' => $d->{'disabled'} ? 0 : 1,
+		 'source' => 'virtualmin',
+		 'domain' => $d };
+	}
+return { 'enabled' => &vhost_file_enabled($file) ? 1 : 0,
+	 'source' => 'apache' };
+}
+
+# vhost_file_toggle_action(file)
+# Returns the action needed to toggle a virtual host file's effective state
+sub vhost_file_toggle_action
+{
+my ($file) = @_;
+return &vhost_file_state($file)->{'enabled'} ? "disable" : "enable";
+}
+
+# virtualmin_domain_state_link(&domain, enabled?)
+# Returns a link to the Virtualmin state change form for some domain
+sub virtualmin_domain_state_link
+{
+my ($d, $enabled) = @_;
+my $page = $enabled ? "disable_domain.cgi" : "enable_domain.cgi";
+my $label = $enabled ? $text{'enable_virtualmin_disable_label'} :
+		       $text{'enable_virtualmin_enable_label'};
+my $url = "../virtual-server/".$page."?dom=".&urlize($d->{'id'});
+return &ui_link(&quote_escape($url), "\"".$label."\"");
+}
+
+# virtualmin_vhost_file_state_error(file, action)
+# Returns an error if a Virtualmin-owned site is being enabled or disabled here
+sub virtualmin_vhost_file_state_error
+{
+my ($file, $action) = @_;
+return undef if ($action ne "enable" && $action ne "disable");
+my $state_info = &vhost_file_state($file);
+return undef if ($state_info->{'source'} ne "virtualmin");
+my $d = $state_info->{'domain'};
+return undef if (!$d);
+my $state = lc($state_info->{'enabled'} ? $text{'index_enabled'} :
+					  $text{'index_disabled'});
+my $dom = "<tt>".&html_escape($d->{'dom'})."</tt>";
+my $link = &virtualmin_domain_state_link($d, $state_info->{'enabled'});
+return $state_info->{'enabled'} ?
+	&text('enable_evirtualmin_disable', $dom, $state, $link) :
+	&text('enable_evirtualmin_enable', $dom, $state, $link);
+}
+
+# delete_virtuals_from_file(file, &virtualhosts...)
+# Deletes VirtualHost blocks from one file and removes the file if empty
+sub delete_virtuals_from_file
+{
+my ($file, @virts) = @_;
+return 0 if (!@virts);
+my $lref = &read_file_lines($file);
+foreach my $virt (sort { $b->{'line'} <=> $a->{'line'} } @virts) {
+	my $len = $virt->{'eline'} - $virt->{'line'} + 1;
+	splice(@$lref, $virt->{'line'}, $len);
+	}
+my $empty = 1;
+foreach my $line (@$lref) {
+	if ($line =~ /\S/) {
+		$empty = 0;
+		last;
+		}
+	}
+&flush_file_lines($file);
+if ($empty) {
+	foreach my $link (&vhost_file_links($file)) {
+		&unlink_logged($link);
+		}
+	&unlink_logged($file);
+	}
+&flush_config_cache();
+&update_last_config_change();
+return scalar(@virts);
+}
+
 # renumber(&config, line, file, offset)
 # Recursively changes the line number of all directives from some file 
 # beyond the given line.
@@ -1544,9 +1973,10 @@ return undef;
 # if necessary.
 sub before_changing
 {
+my @extra = grep { $_ } @_;
 if ($config{'test_always'} || $access{'test_always'}) {
 	local $conf = &get_config();
-	local @files = &unique(map { $_->{'file'} } @$conf);
+	local @files = &unique((map { $_->{'file'} } @$conf), @extra);
 	local $/ = undef;
 	local $f;
 	foreach $f (@files) {
@@ -1969,10 +2399,11 @@ return @rv;
 sub create_webfile_link
 {
 local ($file) = @_;
-if ($config{'link_dir'}) {
+my $linkdir = &vhost_enabled_dir();
+if ($linkdir) {
 	local $short = $file;
 	$short =~ s/^.*\///;
-	local $linksrc = "$config{'link_dir'}/$short";
+	local $linksrc = "$linkdir/$short";
 	&lock_file($linksrc);
 	symlink($file, $linksrc);
 	&unlock_file($linksrc);
@@ -1985,16 +2416,16 @@ if ($config{'link_dir'}) {
 sub delete_webfile_link
 {
 local ($file) = @_;
-if ($config{'link_dir'}) {
-	local $short = $file;
-	$short =~ s/^.*\///;
-	opendir(LINKDIR, $config{'link_dir'});
+$file = &simplify_path(&resolve_links($file));
+my $linkdir = &vhost_enabled_dir();
+if ($linkdir && opendir(LINKDIR, $linkdir)) {
 	foreach my $f (readdir(LINKDIR)) {
-		if ($f ne "." && $f ne ".." &&
-		    (&simplify_path(
-		       &resolve_links($config{'link_dir'}."/".$f)) eq $file ||
-		     $short eq $f)) {
-			&unlink_logged($config{'link_dir'}."/".$f);
+		if ($f ne "." && $f ne "..") {
+			my $link = $linkdir."/".$f;
+			next if (!-l $link);
+			if (&simplify_path(&resolve_links($link)) eq $file) {
+				&unlink_logged($link);
+				}
 			}
 		}
 	closedir(LINKDIR);

apache/delete_vservs.cgi

@@ -3,31 +3,107 @@
 
 require './apache-lib.pl';
 &ReadParse();
-&error_setup($text{'delete_err'});
+@d = split(/\0/, $in{'d'});
+$file_action = $in{'toggle'} ? "toggle" : undef;
+&error_setup($file_action ? $text{'enable_err'} : $text{'delete_err'});
 $access{'vaddr'} || &error($text{'delete_ecannot'});
 $conf = &get_config();
-@d = split(/\0/, $in{'d'});
+$can_vhost_files = &can_manage_vhost_files();
 @d || &error($text{'delete_enone'});
 
+if ($file_action) {
+	&can_manage_vhost_files() || &error($text{'enable_elinkdir'});
+	foreach $d (@d) {
+		if ($d =~ /^file\t([^\t]+)/) {
+			$file = $1;
+			}
+		elsif ($d !~ /^file\t/) {
+			($vmembers, $vconf) = &get_virtual_config($d);
+			next if (!$vconf || !&can_edit_virt($vconf));
+			$file = $vconf->{'file'};
+			}
+		else {
+			next;
+			}
+		$rfile = $file ? &simplify_path(&resolve_links($file)) : undef;
+		$files{$rfile}++ if ($rfile && -f $rfile &&
+				      &can_manage_vhost_state_file($rfile));
+		}
+	@files = keys %files;
+	@files || &error($text{'enable_enone'});
+	foreach $file (@files) {
+		$action = &vhost_file_toggle_action($file);
+		$err = &virtualmin_vhost_file_state_error($file, $action);
+		$err && &error($err);
+		$file_actions{$file} = $action;
+		}
+	foreach $file (@files) {
+		$err = $file_actions{$file} eq "enable" ?
+			&enable_vhost_file($file) :
+			&disable_vhost_file($file);
+		$err && &error($err);
+		}
+	&webmin_log($file_action, "vhostfile", scalar(@files));
+	&redirect("");
+	exit;
+	}
+if (!$in{'delete'}) {
+	&error($text{'delete_eaction'});
+	}
+
 # Get them all
 foreach $d (@d) {
+	if ($d =~ /^file\t([^\t]+)\t(\d+)$/) {
+		push(@{$file_lines{$1}}, $2);
+		next;
+		}
+	elsif ($d =~ /^file\t/) {
+		next;
+		}
 	($vmembers, $vconf) = &get_virtual_config($d);
+	$vconf || &error($text{'delete_egone'});
 	&can_edit_virt($vconf) || &error(&text('delete_ecannot2',
 					       &virtual_name($vconf)));
+	$can_vhost_files && &is_default_vhost($vconf) &&
+		&error($text{'delete_edefault'});
 	push(@virts, $vconf);
 	}
+if (%file_lines) {
+	foreach $file (keys %file_lines) {
+		$rfile = &simplify_path(&resolve_links($file));
+		next if (!$rfile || !-f $rfile ||
+			 !&can_manage_vhost_state_file($rfile));
+		@fvirts = &find_virtuals_in_file($rfile);
+		foreach $line (@{$file_lines{$file}}) {
+			($vconf) = grep { $_->{'line'} == $line } @fvirts;
+			$vconf || &error($text{'delete_egone'});
+			&can_edit_virt($vconf) ||
+				&error(&text('delete_ecannot2',
+					     &virtual_name($vconf)));
+			&is_default_vhost($vconf) &&
+				&error($text{'delete_edefault'});
+			push(@{$file_virts{$rfile}}, $vconf);
+			}
+		}
+	}
+@virts || %file_virts || &error($text{'delete_enone'});
 
 # Delete their structures
-&before_changing();
+&before_changing(keys %file_virts);
 foreach $vconf (@virts) {
 	&lock_file($vconf->{'file'});
 	&save_directive_struct($vconf, undef, $conf, $conf);
 	&delete_file_if_empty($vconf->{'file'});
 	}
+foreach $file (keys %file_virts) {
+	&lock_file($file);
+	$deleted += &delete_virtuals_from_file($file, @{$file_virts{$file}});
+	&unlock_file($file);
+	}
 &flush_file_lines();
 &unlock_all_files();
 &update_last_config_change();
 &after_changing();
-&webmin_log("virts", "delete", scalar(@virts));
+$deleted += scalar(@virts);
+&webmin_log("virts", "delete", $deleted);
 &redirect("");
-

apache/index.cgi

@@ -102,6 +102,10 @@ if (&can_edit_virt()) {
 	push(@vproxy, undef);
 	$sn ||= &get_system_hostname();
 	push(@vurl, $defport ? "http://$sn:$defport/" : "http://$sn/");
+	push(@vfile, undef);
+	push(@vstatus, "");
+	push(@vsel, undef);
+	push(@vfilemanage, 0);
 	$showing_default++;
 	}
 
@@ -128,16 +132,23 @@ elsif ($httpd_modules{'core'} >= 1.2) {
 	$ba = &find_directive("ServerName", $conf);
 	$nv{&to_ipaddress($ba ? $ba : &get_system_hostname())}++;
 	}
-@virt = grep { &can_edit_virt($_) } @virt;
+$can_vhost_files = &can_manage_vhost_files();
+@vrows = &get_virtual_list_rows($conf);
 if ($config{'show_order'} == 1) {
 	# sort by server name
-	@virt = sort { &server_name_sort($a) cmp &server_name_sort($b) } @virt;
+	@vrows = sort { &server_name_sort($a->{'virt'}) cmp
+			&server_name_sort($b->{'virt'}) } @vrows;
 	}
 elsif ($config{'show_order'} == 2) {
 	# sort by IP address
-	@virt = sort { &server_ip_sort($a) cmp &server_ip_sort($b) } @virt;
+	@vrows = sort { &server_ip_sort($a->{'virt'}) cmp
+			&server_ip_sort($b->{'virt'}) } @vrows;
 	}
-foreach $v (@virt) {
+@virt = map { $_->{'virt'} } grep { $_->{'active'} } @vrows;
+%available_vhost_file = map { $_, 1 } &get_vhost_available_files()
+	if ($can_vhost_files);
+foreach $r (@vrows) {
+	$v = $r->{'virt'};
 	$vm = $v->{'members'};
 	if ($v->{'words'}->[0] =~ /^\[(\S+)\]:(\d+)$/) {
 		# IPv6 address and port
@@ -163,7 +174,7 @@ foreach $v (@virt) {
 	$idx = &indexof($v, @$conf);
 	push(@vidx, $idx);
 	push(@vname, $text{'index_virt'});
-	push(@vlink, "virt_index.cgi?virt=$idx");
+	push(@vlink, $r->{'active'} ? "virt_index.cgi?virt=$idx" : undef);
 	$sname = &find_directive("ServerName", $vm);
 	local $daddr = $addr eq "_default_" ||
                        ($addr eq "*" && $httpd_modules{'core'} < 1.2);
@@ -225,10 +236,34 @@ foreach $v (@virt) {
 		}
 	$sp = undef if ($sp == 80 && $prot eq "http" ||
 			$sp == 443 && $prot eq "https");
-	push(@vurl, $sp ? "$prot://$sn:$sp/" : "$prot://$sn/");
+	push(@vurl, $r->{'active'} ?
+		    ($sp ? "$prot://$sn:$sp/" : "$prot://$sn/") : undef);
+	local $rfile = $r->{'file'} ? &simplify_path(&resolve_links($r->{'file'}))
+				    : undef;
+	push(@vfile, $rfile);
+	local $status = "";
+	if ($can_vhost_files && $rfile && $available_vhost_file{$rfile}) {
+		local $enabled = &vhost_file_state($rfile)->{'enabled'};
+		$status = $enabled ? $text{'index_enabled'} :
+				     $text{'index_disabled'};
+		}
+	push(@vstatus, $status);
+	local $file_manage = $can_vhost_files && $rfile &&
+			     $available_vhost_file{$rfile} &&
+			     &can_manage_vhost_state_file($rfile);
+	push(@vfilemanage, $file_manage ? 1 : 0);
+	local $sel;
+	if ($r->{'active'} && (!$can_vhost_files || !&is_default_vhost($v))) {
+		$sel = $idx;
+		}
+	elsif (!$r->{'active'} && $can_vhost_files && $rfile &&
+	       $available_vhost_file{$rfile} && $file_manage) {
+		$sel = "file\t".$rfile."\t".$v->{'line'};
+		}
+	push(@vsel, $sel);
 	}
 
-if (@vlink == 1 && !$access{'global'} && $access{'virts'} ne "*" &&
+if (@vlink == 1 && $vlink[0] && !$access{'global'} && $access{'virts'} ne "*" &&
     !$access{'create'} && $access{'noconfig'}) {
 	# Can only manage one vhost, so go direct to it
 	&redirect($vlink[0]);
@@ -297,7 +332,9 @@ if ($access{'global'}) {
 # work out select links
 print &ui_tabs_start_tab("mode", "list");
 #print $text{'index_desclist'},"<p>\n";
-$showdel = $access{'vaddr'} && ($vidx[0] || $vidx[1]);
+$showdel = $access{'vaddr'} &&
+	   grep { defined($_) && $_ ne "" } @vsel;
+$showtoggle = $can_vhost_files && grep { $_ } @vfilemanage;
 @links = ( );
 if ($showdel) {
 	push(@links, &select_all_link("d"),
@@ -326,8 +363,10 @@ if ($config{'max_servers'} && @vname > $config{'max_servers'}) {
 	}
 elsif ($config{'show_list'} && scalar(@vname)) {
 	# as list for people with lots of servers
+	$list_form = "vhosts_form";
 	if ($showdel) {
-		print &ui_form_start("delete_vservs.cgi", "post");
+		print &ui_form_start("delete_vservs.cgi", "post", undef,
+				     "id='$list_form'");
 		}
 	print &ui_links_row(\@links);
 	print &ui_columns_start([
@@ -337,19 +376,23 @@ elsif ($config{'show_list'} && scalar(@vname)) {
 		$text{'index_port'},
 		$text{'index_name'},
 		$text{'index_root'},
+		$can_vhost_files ? ( $text{'index_status'} ) : ( ),
 		$text{'index_url'} ], 100);
 	for($i=0; $i<@vname; $i++) {
 		local @cols;
-		push(@cols, &ui_link($vlink[$i], $vname[$i]) );
+		push(@cols, $vlink[$i] ? &ui_link($vlink[$i], $vname[$i]) :
+					  $vname[$i] );
 		push(@cols, &html_escape($vaddr[$i]));
 		push(@cols, &html_escape($vport[$i]));
 		push(@cols, $vserv[$i] || $text{'index_auto'});
 		push(@cols, &html_escape($vproxy[$i]) ||
 			    &html_escape($vroot[$i]));
-		push(@cols, &ui_link($vurl[$i], $text{'index_view'}) );
-		if ($showdel && $vidx[$i]) {
+		push(@cols, $vstatus[$i]) if ($can_vhost_files);
+		push(@cols, $vurl[$i] ? &ui_link($vurl[$i], $text{'index_view'}) :
+					"" );
+		if ($showdel && defined($vsel[$i]) && $vsel[$i] ne "") {
 			print &ui_checked_columns_row(\@cols, undef,
-						      "d", $vidx[$i]);
+						      "d", $vsel[$i]);
 			}
 		elsif ($showdel) {
 			print &ui_columns_row([ "", @cols ]);
@@ -361,13 +404,23 @@ elsif ($config{'show_list'} && scalar(@vname)) {
 	print &ui_columns_end();
 	print &ui_links_row(\@links);
 	if ($showdel) {
-		print &ui_form_end([ [ "delete", $text{'index_delete'} ] ]);
+		if ($showtoggle) {
+			print &ui_form_end_side_by_side($list_form,
+				[ [ "delete", $text{'index_delete'} ] ],
+				[ [ "toggle", $text{'index_toggle'}, undef,
+				    undef, "form=\"$list_form\"" ] ]);
+			}
+		else {
+			print &ui_form_end([ [ "delete", $text{'index_delete'} ] ]);
+			}
 		}
 	}
 else {
 	# as icons for niceness
+	$list_form = "vhosts_form";
 	if ($showdel) {
-		print &ui_form_start("delete_vservs.cgi", "post");
+		print &ui_form_start("delete_vservs.cgi", "post", undef,
+				     "id='$list_form'");
 		}
 	print &ui_links_row(\@links);
 	print "<table width=100% cellpadding=5>\n";
@@ -376,8 +429,9 @@ else {
 		print '<div class="row icons-row inline-row">';
 		&generate_icon("images/virt.gif", $vname[$i], $vlink[$i],
 			       undef, undef, undef,
-			       $vidx[$i] && $access{'vaddr'} ?
-					&ui_checkbox("d", $vidx[$i]) : "");
+			       defined($vsel[$i]) && $vsel[$i] ne "" &&
+			       $access{'vaddr'} ?
+					&ui_checkbox("d", $vsel[$i]) : "");
 		print "</div>\n";
 		print "</td> <td valign=top>\n";
 		print "$vdesc[$i]<br>\n";
@@ -397,12 +451,24 @@ else {
 			print "<b>$text{'index_root'}</b> ",
 			      &html_escape($vroot[$i]),"</td> </tr>\n";
 			}
+		if ($can_vhost_files && $vstatus[$i]) {
+			print "<tr><td colspan=2><b>$text{'index_status'}</b> ",
+			      $vstatus[$i],"</td></tr>\n";
+			}
 		print "</table></td> </tr>\n";
 		}
 	print "</table>\n";
 	print &ui_links_row(\@links);
 	if ($showdel) {
-		print &ui_form_end([ [ "delete", $text{'index_delete'} ] ]);
+		if ($showtoggle) {
+			print &ui_form_end_side_by_side($list_form,
+				[ [ "delete", $text{'index_delete'} ] ],
+				[ [ "toggle", $text{'index_toggle'}, undef,
+				    undef, "form=\"$list_form\"" ] ]);
+			}
+		else {
+			print &ui_form_end([ [ "delete", $text{'index_delete'} ] ]);
+			}
 		}
 	}
 print &ui_tabs_end_tab();
@@ -492,4 +558,3 @@ return $addr eq '_default_' || $addr eq '*' ? undef :
        $addr =~ /^\[(\S+)\]$/ && &check_ip6address($1) ? $1 :
 			         &to_ipaddress($addr);
 }
-

apache/lang/en

@@ -34,6 +34,9 @@ index_listen=Listen on address (if needed)
 index_port=Port
 index_name=Server Name
 index_root=Document Root
+index_status=State
+index_enabled=Enabled
+index_disabled=Disabled
 index_url=URL
 index_view=Open..
 index_adddir=Allow access to this directory
@@ -57,6 +60,7 @@ index_fmode1=Virtual servers file $1
 index_fmode1d=New file under virtual servers directory $1
 index_fmode2=Selected file..
 index_delete=Delete Selected Servers
+index_toggle=Toggle State
 
 cvirt_ecannot=You are not allowed to create a virtual server
 cvirt_err=Failed to create virtual server
@@ -1032,6 +1036,7 @@ log_stop=Stopped webserver
 log_apply=Applied changes
 log_manual=Manually edited configuration file $1
 log_virts_delete=Deleted $1 virtual servers
+log_toggle_vhostfile=Toggled state of $1 virtual host files
 
 search_title=Find Servers
 search_notfound=No matching virtual servers found
@@ -1148,6 +1153,22 @@ delete_err=Failed to delete virtual servers
 delete_enone=None selected
 delete_ecannot=You are not allowed to delete servers
 delete_ecannot2=You are not allowed to edit the server $1
+delete_eaction=No action was selected
+delete_egone=The selected virtual server no longer exists
+delete_edefault=The default virtual server cannot be deleted
+
+enable_err=Failed to change virtual host file state
+enable_enone=No manageable virtual host files were selected
+enable_efile=Virtual host file does not exist or cannot be managed
+enable_elinkdir=No enabled virtual host links directory is configured
+enable_elink=Failed to create symbolic link $1 : $2
+enable_eunlink=Failed to remove symbolic link $1 : $2
+enable_elinkexists=The symbolic link $1 already exists
+enable_etest=Apache configuration test failed after changing the virtual host file state : $1
+enable_evirtualmin_disable=This Apache virtual host is managed by Virtualmin virtual server $1, which is currently $2. Site disabling should be done in Virtualmin using $3.
+enable_evirtualmin_enable=This Apache virtual host is managed by Virtualmin virtual server $1, which is currently $2. Site enabling should be done in Virtualmin using $3.
+enable_virtualmin_disable_label=Disable and Delete ⇾ Disable Virtual Server
+enable_virtualmin_enable_label=Disable and Delete ⇾ Enable Virtual Server
 
 syslog_desc=Apache error log
 

apache/t/vhost-files.t

@@ -0,0 +1,409 @@
+#!/usr/bin/perl
+# Tests for Debian-style Apache sites-available/sites-enabled handling.
+
+use strict;
+use warnings;
+use Test::More;
+use File::Basename qw(dirname);
+use File::Path qw(make_path);
+use File::Spec;
+use File::Temp qw(tempdir);
+use Cwd qw(abs_path);
+
+my $root = abs_path(File::Spec->catdir(dirname(__FILE__), '..', '..'));
+my $tmp = abs_path(tempdir(CLEANUP => 1));
+my $webmin_config = File::Spec->catdir($tmp, 'webmin-config');
+my $webmin_var = File::Spec->catdir($tmp, 'webmin-var');
+my $apache_root = File::Spec->catdir($tmp, 'apache2');
+my $available = File::Spec->catdir($apache_root, 'sites-available');
+my $enabled = File::Spec->catdir($apache_root, 'sites-enabled');
+my $apache_conf = File::Spec->catfile($apache_root, 'apache2.conf');
+
+make_path($webmin_config, $webmin_var, "$webmin_config/apache",
+	  "$webmin_var/apache", $apache_root, $available, $enabled);
+
+sub write_text
+{
+my ($file, $text) = @_;
+open(my $fh, '>', $file) || die "Failed to write $file: $!";
+print $fh $text;
+close($fh) || die "Failed to close $file: $!";
+}
+
+sub read_text
+{
+my ($file) = @_;
+open(my $fh, '<', $file) || die "Failed to read $file: $!";
+local $/ = undef;
+my $text = <$fh>;
+close($fh) || die "Failed to close $file: $!";
+return $text;
+}
+
+sub vhost_conf
+{
+my ($name, $rootdir) = @_;
+my $name_line = defined($name) ? "    ServerName $name\n" : "";
+return "<VirtualHost *:80>\n".
+       $name_line.
+       "    DocumentRoot $rootdir\n".
+       "</VirtualHost>\n";
+}
+
+my $default = File::Spec->catfile($available, '000-default.conf');
+my $alpha = File::Spec->catfile($available, 'alpha.conf');
+my $beta = File::Spec->catfile($available, 'beta.conf');
+my $charlie = File::Spec->catfile($available, 'charlie.conf');
+
+write_text($default, vhost_conf(undef, '/srv/default'));
+write_text($alpha, vhost_conf('alpha.example', '/srv/alpha'));
+write_text($beta, vhost_conf('beta.example', '/srv/beta'));
+write_text($charlie, vhost_conf('charlie.example', '/srv/charlie'));
+write_text($apache_conf,
+	"ServerRoot \"$apache_root\"\n".
+	"Listen 80\n".
+	"IncludeOptional $enabled/*.conf\n");
+
+symlink($default, File::Spec->catfile($enabled, '000-default.conf')) ||
+	die "Failed to symlink default: $!";
+symlink($alpha, File::Spec->catfile($enabled, 'alpha.conf')) ||
+	die "Failed to symlink alpha: $!";
+symlink($charlie, File::Spec->catfile($enabled, 'charlie.conf')) ||
+	die "Failed to symlink charlie: $!";
+
+write_text(File::Spec->catfile($webmin_config, 'config'),
+	"os_type=debian-linux\n".
+	"os_version=12\n".
+	"real_os_type=Debian Linux\n".
+	"real_os_version=12\n");
+write_text(File::Spec->catfile($webmin_config, 'miniserv.conf'),
+	"root=$root\n");
+write_text(File::Spec->catfile($webmin_config, 'apache', 'config'),
+	"httpd_dir=$apache_root\n".
+	"httpd_path=/bin/true\n".
+	"httpd_conf=$apache_conf\n".
+	"apachectl_path=/bin/true\n".
+	"httpd_version=2.4.57\n".
+	"test_apachectl=0\n".
+	"test_config=1\n".
+	"virt_file=$available\n".
+	"link_dir=$enabled\n");
+
+$ENV{'WEBMIN_CONFIG'} = $webmin_config;
+$ENV{'WEBMIN_VAR'} = $webmin_var;
+$ENV{'FOREIGN_MODULE_NAME'} = 'apache';
+$ENV{'FOREIGN_ROOT_DIRECTORY'} = $root;
+$ENV{'REMOTE_USER'} = 'root';
+
+unshift(@INC, $root);
+require File::Spec->catfile($root, 'apache', 'apache-lib.pl');
+
+{
+	no warnings 'once';
+	$main::text{'enable_elinkdir'} = 'No enabled virtual host links directory is configured';
+	$main::text{'enable_efile'} = 'Virtual host file does not exist or cannot be managed';
+	$main::text{'enable_elink'} = 'Failed to create symbolic link $1 : $2';
+	$main::text{'enable_eunlink'} = 'Failed to remove symbolic link $1 : $2';
+	$main::text{'enable_elinkexists'} = 'The symbolic link $1 already exists';
+	$main::text{'enable_etest'} = 'Apache configuration test failed after changing the virtual host file state : $1';
+	$main::text{'enable_evirtualmin_disable'} = 'This Apache virtual host is managed by Virtualmin virtual server $1, which is currently $2. Site disabling should be done in Virtualmin using $3.';
+	$main::text{'enable_evirtualmin_enable'} = 'This Apache virtual host is managed by Virtualmin virtual server $1, which is currently $2. Site enabling should be done in Virtualmin using $3.';
+	$main::text{'enable_virtualmin_disable_label'} = 'Disable and Delete &#x21fe; Disable Virtual Server';
+	$main::text{'enable_virtualmin_enable_label'} = 'Disable and Delete &#x21fe; Enable Virtual Server';
+	$main::text{'index_enabled'} = 'Enabled';
+	$main::text{'index_disabled'} = 'Disabled';
+	$main::text{'eafter'} = 'Apache configuration test failed : $1';
+}
+
+sub apache_config
+{
+main::flush_config_cache();
+my $conf = main::get_config();
+ok($conf, 'test apache config can be parsed');
+return $conf;
+}
+
+sub row_names
+{
+return [ map {
+	scalar(main::find_directive('ServerName', $_->{'virt'}->{'members'})) || ''
+	} @_ ];
+}
+
+sub row_states
+{
+return [ map { $_->{'active'} ? 'enabled' : 'disabled' } @_ ];
+}
+
+subtest 'sites-available files are manageable and ordered' => sub {
+	ok(main::can_manage_vhost_files(),
+	   'sites-available/enabled dirs are manageable');
+	is_deeply(
+		[ main::get_vhost_available_files() ],
+		[ $default, $alpha, $beta, $charlie ],
+		'available files are listed in stable filename order',
+	);
+
+	my @rows = main::get_virtual_list_rows(apache_config());
+	is_deeply(row_names(@rows),
+		  [ '', 'alpha.example', 'beta.example', 'charlie.example' ],
+		  'disabled rows stay in sites-available order');
+	is_deeply(row_states(@rows),
+		  [ 'enabled', 'enabled', 'disabled', 'enabled' ],
+		  'row active state follows sites-enabled symlinks');
+	ok(!main::can_manage_vhost_file($default),
+	   'default virtual host file is not file-state manageable');
+};
+
+subtest 'disable removes only the enabled symlink' => sub {
+	no warnings 'once';
+	unlink($main::last_config_change_flag);
+	unlink($main::last_restart_time_flag);
+	main::restart_last_restart_time();
+	my $old = time() - 10;
+	utime($old, $old, $main::last_restart_time_flag);
+
+	{
+		no warnings 'redefine';
+		local *main::test_config = sub { return undef; };
+		is(main::disable_vhost_file($alpha), undef, 'disable succeeds');
+	}
+	ok(main::needs_config_restart(),
+	   'disable marks config as needing apply');
+
+	ok(-f $alpha, 'disable leaves the sites-available file in place');
+	ok(!-e File::Spec->catfile($enabled, 'alpha.conf'),
+	   'disable removes the sites-enabled symlink');
+
+	my @rows = main::get_virtual_list_rows(apache_config());
+	is_deeply(row_names(@rows),
+		  [ '', 'alpha.example', 'beta.example', 'charlie.example' ],
+		  'disabled row remains in the same list position');
+	is_deeply(row_states(@rows),
+		  [ 'enabled', 'disabled', 'disabled', 'enabled' ],
+		  'disabled row status is updated');
+};
+
+subtest 'enable creates a symlink without touching the source file' => sub {
+	no warnings 'once';
+	unlink($main::last_config_change_flag);
+	unlink($main::last_restart_time_flag);
+	main::restart_last_restart_time();
+	my $old = time() - 10;
+	utime($old, $old, $main::last_restart_time_flag);
+
+	{
+		no warnings 'redefine';
+		local *main::test_config = sub { return undef; };
+		is(main::enable_vhost_file($beta), undef, 'enable succeeds');
+	}
+	ok(main::needs_config_restart(),
+	   'enable marks config as needing apply');
+
+	my $link = File::Spec->catfile($enabled, 'beta.conf');
+	ok(-f $beta, 'enable leaves the sites-available file in place');
+	ok(-l $link, 'enable creates the sites-enabled symlink');
+	is(readlink($link), $beta, 'enabled symlink points to the available file');
+	ok(main::vhost_file_enabled($beta), 'vhost_file_enabled sees the symlink');
+};
+
+subtest 'same-name symlink to another target is not disabled' => sub {
+	my $otherdir = File::Spec->catdir($tmp, 'other-sites');
+	my $other = File::Spec->catfile($otherdir, 'charlie.conf');
+	my $link = File::Spec->catfile($enabled, 'charlie.conf');
+	make_path($otherdir);
+	write_text($other, vhost_conf('other.example', '/srv/other'));
+	unlink($link) || die "Failed to remove charlie link: $!";
+	symlink($other, $link) || die "Failed to symlink other charlie: $!";
+
+	ok(!main::vhost_file_enabled($charlie),
+	   'same-name symlink to another file is not considered enabled');
+	{
+		no warnings 'redefine';
+		local *main::test_config = sub { return undef; };
+		is(main::disable_vhost_file($charlie), undef, 'disable is a no-op');
+	}
+	ok(-l $link, 'same-name symlink to another target is preserved');
+	is(readlink($link), $other, 'preserved symlink target is unchanged');
+};
+
+subtest 'disabled default virtual hosts stay hidden' => sub {
+	my $disabled_default = File::Spec->catfile($available,
+						  'zz-disabled-default.conf');
+	write_text($disabled_default, vhost_conf(undef, '/srv/disabled-default'));
+
+	my @rows = main::get_virtual_list_rows(apache_config());
+	ok(!(grep { $_->{'file'} eq $disabled_default } @rows),
+	   'disabled catch-all virtual host file is not listed as a normal vhost');
+};
+
+subtest 'legacy webfile link helpers resolve relative link_dir' => sub {
+	my $relative = File::Spec->catfile($available, 'relative.conf');
+	my $link = File::Spec->catfile($enabled, 'relative.conf');
+	write_text($relative, vhost_conf('relative.example', '/srv/relative'));
+	unlink($link);
+
+	{
+		no warnings 'once';
+		local $main::config{'link_dir'} = 'sites-enabled';
+		main::create_webfile_link($relative);
+		ok(-l $link, 'relative link_dir creates link under ServerRoot');
+		is(readlink($link), $relative,
+		   'created relative link_dir symlink points to the vhost file');
+		main::delete_webfile_link($relative);
+		ok(!-e $link && !-l $link,
+		   'relative link_dir delete removes the enabled symlink');
+	}
+};
+
+subtest 'file-level actions require access to every virtual host in the file' => sub {
+	my $mixed = File::Spec->catfile($available, 'mixed.conf');
+	write_text($mixed,
+		vhost_conf('alpha.example', '/srv/mixed-alpha').
+		vhost_conf('hidden.example', '/srv/mixed-hidden'));
+
+	{
+		no warnings 'once';
+		local $main::access{'virts'} = 'alpha.example:80';
+		ok(!main::can_manage_vhost_file($mixed),
+		   'mixed-access file cannot be managed by a restricted user');
+	}
+	ok(main::can_manage_vhost_file($mixed),
+	   'shared file can be managed when all contained vhosts are allowed');
+};
+
+subtest 'state helpers enforce allowed files and ACLs directly' => sub {
+	my $outside = File::Spec->catfile($tmp, 'outside.conf');
+	write_text($outside, vhost_conf('outside.example', '/srv/outside'));
+	is(main::enable_vhost_file($outside),
+	   'Virtual host file does not exist or cannot be managed',
+	   'enable rejects files outside sites-available');
+
+	my $mixed = File::Spec->catfile($available, 'state-mixed.conf');
+	write_text($mixed,
+		vhost_conf('alpha.example', '/srv/state-alpha').
+		vhost_conf('hidden.example', '/srv/state-hidden'));
+	{
+		no warnings 'once';
+		local $main::access{'virts'} = 'alpha.example:80';
+		is(main::enable_vhost_file($mixed),
+		   'Virtual host file does not exist or cannot be managed',
+		   'enable rejects mixed-access files without relying on caller validation');
+	}
+};
+
+subtest 'change rollback covers extra disabled vhost files' => sub {
+	my $rollback = File::Spec->catfile($available, 'rollback.conf');
+	my $original = vhost_conf('rollback.example', '/srv/rollback');
+	write_text($rollback, $original);
+	my @virts = main::find_virtuals_in_file($rollback);
+	is(scalar(@virts), 1, 'rollback fixture has one vhost');
+
+	{
+		no warnings qw(redefine once);
+		local %main::before_changing;
+		local $main::config{'test_always'} = 1;
+		local *main::test_config = sub { return 'bad config'; };
+		local *main::error = sub { die $_[0]; };
+		main::before_changing($rollback);
+		is(main::delete_virtuals_from_file($rollback, @virts), 1,
+		   'disabled vhost file deletion removes the vhost');
+		ok(!-e $rollback, 'empty disabled vhost file is deleted');
+		like(eval { main::after_changing(); 1 } ? '' : $@,
+		     qr/bad config/, 'failed post-change test reports an error');
+	}
+	ok(-f $rollback, 'rollback recreates the disabled vhost file');
+	is(read_text($rollback), $original,
+	   'rollback restores the disabled vhost file contents');
+};
+
+subtest 'apache configtest failure rolls back link changes' => sub {
+	my $delta = File::Spec->catfile($available, 'delta.conf');
+	my $delta_link = File::Spec->catfile($enabled, 'delta.conf');
+	write_text($delta, vhost_conf('delta.example', '/srv/delta'));
+
+	{
+		no warnings 'redefine';
+		local *main::test_config = sub { return 'bad config'; };
+		like(main::enable_vhost_file($delta), qr/bad config/,
+		     'failed enable reports apache configtest output');
+	}
+	ok(!-e $delta_link, 'failed enable removes the new symlink');
+
+	symlink($delta, $delta_link) || die "Failed to symlink delta: $!";
+	{
+		no warnings 'redefine';
+		local *main::test_config = sub { return 'bad config'; };
+		like(main::disable_vhost_file($delta), qr/bad config/,
+		     'failed disable reports apache configtest output');
+	}
+	ok(-l $delta_link, 'failed disable restores the removed symlink');
+	is(readlink($delta_link), $delta, 'restored symlink target is unchanged');
+};
+
+subtest 'Virtualmin-managed virtual host files cannot be toggled directly' => sub {
+	my $enabled_domain = File::Spec->catfile($available, 'vm-enabled.conf');
+	my $disabled_domain = File::Spec->catfile($available, 'vm-disabled.conf');
+	write_text($enabled_domain,
+		vhost_conf('www.vm-enabled.example', '/srv/vm-enabled'));
+	write_text($disabled_domain,
+		vhost_conf('vm-disabled.example', '/srv/vm-disabled'));
+
+	{
+		no warnings qw(redefine once);
+		local %main::apache_virtualmin_domain_for_file_cache;
+		local %main::apache_virtualmin_domain_by_name_cache;
+		local *main::virtualmin_available = sub { return 1; };
+		local *main::virtualmin_domain_by_name = sub {
+			my ($name) = @_;
+			return $name eq 'vm-enabled.example' ?
+				{ 'dom' => $name, 'id' => '12345',
+				  'disabled' => '' } :
+			       $name eq 'vm-disabled.example' ?
+				{ 'dom' => $name, 'id' => '67890',
+				  'disabled' => 'web' } :
+				undef;
+			};
+
+		my $disable_err =
+			main::virtualmin_vhost_file_state_error($enabled_domain,
+								'disable');
+		my $enabled_state = main::vhost_file_state($enabled_domain);
+		is($enabled_state->{'source'}, 'virtualmin',
+		   'Virtualmin is the effective state source for managed files');
+		ok($enabled_state->{'enabled'},
+		   'Virtualmin enabled domain is reported as enabled');
+		is(main::vhost_file_toggle_action($enabled_domain), 'disable',
+		   'toggle action follows the Virtualmin enabled state');
+		like($disable_err, qr/currently enabled/,
+		     'Virtualmin state is included for enabled domains');
+		like($disable_err, qr/Disable Virtual Server/,
+		     'disabling directs users to Virtualmin disable action');
+		like($disable_err,
+		     qr{virtual-server/disable_domain\.cgi\?dom=12345},
+		     'disabling links to the Virtualmin disable form');
+
+		my $enable_err =
+			main::virtualmin_vhost_file_state_error($disabled_domain,
+								'enable');
+		my $disabled_state = main::vhost_file_state($disabled_domain);
+		is($disabled_state->{'source'}, 'virtualmin',
+		   'Virtualmin remains the state source for disabled domains');
+		ok(!$disabled_state->{'enabled'},
+		   'Virtualmin disabled domain is reported as disabled');
+		is(main::vhost_file_toggle_action($disabled_domain), 'enable',
+		   'toggle action follows the Virtualmin disabled state');
+		like($enable_err, qr/currently disabled/,
+		     'Virtualmin state is included for disabled domains');
+		like($enable_err, qr/Enable Virtual Server/,
+		     'enabling directs users to Virtualmin enable action');
+		like($enable_err,
+		     qr{virtual-server/enable_domain\.cgi\?dom=67890},
+		     'enabling links to the Virtualmin enable form');
+
+		is(main::virtualmin_vhost_file_state_error($alpha, 'disable'),
+		   undef, 'non-Virtualmin virtual host files can still be toggled');
+	}
+};
+
+done_testing();

at/macos-lib.pl

@@ -35,21 +35,24 @@ return @rv;
 # create_atjob(user, time, commands, directory, send-email)
 sub create_atjob
 {
-my @tm = localtime($_[1]);
+my ($user, $tm, $cmds, $dir, $email) = @_;
+my @tm = localtime($tm);
 my $date = sprintf "%2.2d:%2.2d %d.%d.%d",
 		$tm[2], $tm[1], $tm[3], $tm[4]+1, $tm[5]+1900;
-my $mailflag = $_[4] ? "-m" : "";
+my $mailflag = $email ? "-m" : "";
 no strict "subs";
-&open_execute_command(AT, "su \"$_[0]\" -c \"cd $_[3] ; at $mailflag $date\" >/dev/null 2>&1", 0); 
-print AT $_[2];
+my $fullcmd = &command_as_user($user, 0, "cd $dir ; at $mailflag $date");
+&open_execute_command(AT, "$fullcmd >/dev/null 2>&1", 0); 
+print AT $cmds;
 close(AT);
 use strict "subs";
-&additional_log('exec', undef, "su \"$_[0]\" -c \"cd $_[3] ; at $mailflag $date\"");
+&additional_log('exec', undef, $fullcmd);
 }
 
 # delete_atjob(id)
 sub delete_atjob
 {
-&system_logged("atrm \"$_[0]\" >/dev/null 2>&1");
+my ($id) = @_;
+&system_logged("atrm ".quotemeta($id)." >/dev/null 2>&1");
 }
 

at/openserver-lib.pl

@@ -5,13 +5,13 @@ no warnings "redefine";
 
 sub list_atjobs
 {
-local @rv;
+my @rv;
 opendir(DIR, $config{'at_dir'}) || return ();
 while($f = readdir(DIR)) {
-	local $p = "$config{'at_dir'}/$f";
+	my $p = "$config{'at_dir'}/$f";
 	if ($f =~ /^(\d+)\.a(\S+)$/) {
-		local @st = stat($p);
-		local $job = { 'id' => $f,
+		my @st = stat($p);
+		my $job = { 'id' => $f,
 			       'date' => $1,
 			       'user' => scalar(getpwuid($st[4])),
 			       'created' => $st[9] };
@@ -32,17 +32,20 @@ return @rv;
 # create_atjob(user, time, commands, directory)
 sub create_atjob
 {
-local @tm = localtime($_[1]);
-local $date = strftime "%H:%M %b %d", @tm;
-&open_execute_command(AT, "su \"$_[0]\" -c \"cd $_[3] ; at $date\"", 0); 
-print AT $_[2];
+my ($user, $tm, $cmds, $dir) = @_;
+my @tm = localtime($tm);
+my $date = strftime "%H:%M %b %d", @tm;
+my $fullcmd = &command_as_user($user, 0, "cd $dir ; at $date");
+&open_execute_command(AT, $fullcmd, 0); 
+print AT $cmds;
 close(AT);
-&additional_log('exec', undef, "su \"$_[0]\" -c \"cd $_[3] ; at $date\"");
+&additional_log('exec', undef, $fullcmd);
 }
 
 # delete_atjob(id)
 sub delete_atjob
 {
-&system_logged("at -r \"$_[0]\"");
+my ($id) = @_;
+&system_logged("at -r ".quotemeta($id));
 }
 

bacula-backup/bacula-backup-lib.pl

@@ -58,6 +58,23 @@ $testcmd->finish();
 return $dbh;
 }
 
+# bacula_catalog_table_exists(&dbh, table)
+# Returns 1 if the Bacula catalog contains a table, 0 if not
+sub bacula_catalog_table_exists
+{
+local ($dbh, $table) = @_;
+$table =~ /^\w+\z/ || die "Illegal catalog table name";
+local $dbh->{'PrintError'} = 0;
+local $dbh->{'RaiseError'} = 0;
+my $cmd = eval { $dbh->prepare("SELECT 1 FROM $table WHERE 1 = 0") };
+my $ok;
+if ($cmd) {
+	$ok = eval { $cmd->execute() };
+	$cmd->finish();
+	}
+return $ok ? 1 : 0;
+}
+
 # read_config_file(file)
 # Parses a bacula config file
 sub read_config_file
@@ -1678,4 +1695,3 @@ return ( \%tags, $run );
 }
 
 1;
-

bacula-backup/list.cgi

@@ -78,6 +78,12 @@ $cmd->finish();
 
 @rv = &unique(@rv);
 
+my $filename_table = &bacula_catalog_table_exists($dbh, "Filename");
+my $filename_col = $filename_table ? "Filename.Name" : "File.Filename";
+my $filename_from = $filename_table ? ", Filename" : "";
+my $filename_join = $filename_table ?
+	"AND File.FilenameId  = Filename.FilenameId" : "";
+
 # Build the nodes structure for folders
 foreach $f (@rv) {
 	$f =~ /([^\/]+)\/\Z/;
@@ -92,16 +98,16 @@ foreach $f (@rv) {
 if ($in{'volume'}) {
 	# Files in directory, that are on this volume
 	$cmd = $dbh->prepare("
-		SELECT Filename.Name
-		FROM File, Filename, Job, JobMedia, Media
-		WHERE File.FilenameId  = Filename.FilenameId
-		  AND File.JobId       = Job.JobId
+		SELECT $filename_col
+		FROM File$filename_from, Job, JobMedia, Media
+		WHERE File.JobId       = Job.JobId
 		  AND Job.JobId        = JobMedia.JobId
 		  AND JobMedia.MediaId = Media.MediaId
 		  AND File.PathId      = ?
 		  AND Media.VolumeName = ?
+		  $filename_join
 		  $jobsql
-		ORDER BY Filename.Name
+		ORDER BY $filename_col
 	");
 
 	$cmd->execute($pid, $in{'volume'}) || die "db error: ".$dbh->errstr;
@@ -109,13 +115,13 @@ if ($in{'volume'}) {
 else {
 	# Files in directory
 	$cmd = $dbh->prepare("
-		SELECT Filename.Name
-		FROM Job, File, Filename
+		SELECT $filename_col
+		FROM Job, File$filename_from
 		WHERE Job.JobId       = File.JobId
-		  AND File.FilenameId = Filename.FilenameId
 		  AND File.PathId     = ?
+		  $filename_join
 		  $jobsql
-		ORDER BY Filename.Name
+		ORDER BY $filename_col
 	");
 
 	$cmd->execute($pid) || die "db error: ".$dbh->errstr;

bind8/acl_security.pl

@@ -3,7 +3,7 @@ use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
 
-require 'bind8-lib.pl';
+require 'bind8-lib.pl';    ## no critic
 # Globals from bind8-lib.pl
 our (%config, %text, %in);
 

bind8/backup_config.pl

@@ -45,21 +45,21 @@ return map { &make_chroot($_) } &unique(@rv);
 # Called before the files are actually read
 sub pre_backup
 {
-return undef;
+return;
 }
 
 # post_backup(&files)
 # Called after the files are actually read
 sub post_backup
 {
-return undef;
+return;
 }
 
 # pre_restore(&files)
 # Called before the files are restored from a backup
 sub pre_restore
 {
-return undef;
+return;
 }
 
 # post_restore(&files)
@@ -71,7 +71,7 @@ my $pidfile = &get_pid_file();
 if (&check_pid_file(&make_chroot($pidfile, 1))) {
 	return &restart_bind();
 	}
-return undef;
+return;
 }
 
 1;

bind8/bind8-lib.pl

@@ -12,16 +12,31 @@ use WebminCore;
 our (%text, %config, %gconfig, $module_name, $module_var_directory, $module_config_file, $module_config_directory);
 
 my $dnssec_tools_minver = 1.13;
-my $have_dnssec_tools = eval "require Net::DNS::SEC::Tools::dnssectools;";
+my $have_dnssec_tools = eval {
+	require Net::DNS::SEC::Tools::dnssectools;
+	1;
+	};
 my %freeze_zone_count;
 
 if ($have_dnssec_tools) {
-	eval "use Net::DNS::SEC::Tools::dnssectools;
-	      use Net::DNS::SEC::Tools::rollmgr;
-	      use Net::DNS::SEC::Tools::rollrec;
-	      use Net::DNS::SEC::Tools::keyrec;
-	      use Net::DNS::RR::DS;
-	      use Net::DNS;";
+	# All companion modules must load cleanly. A partial install would
+	# otherwise leave unqualified calls like rollmgr_sendcmd / rollrec_*
+	# undefined, causing runtime death deep inside dnssec helpers.
+	$have_dnssec_tools = eval {
+		require Net::DNS::SEC::Tools::dnssectools;
+		Net::DNS::SEC::Tools::dnssectools->import;
+		require Net::DNS::SEC::Tools::rollmgr;
+		Net::DNS::SEC::Tools::rollmgr->import;
+		require Net::DNS::SEC::Tools::rollrec;
+		Net::DNS::SEC::Tools::rollrec->import;
+		require Net::DNS::SEC::Tools::keyrec;
+		Net::DNS::SEC::Tools::keyrec->import;
+		require Net::DNS::RR::DS;
+		Net::DNS::RR::DS->import;
+		require Net::DNS;
+		Net::DNS->import;
+		1;
+		};
 	}
 
 &init_config();
@@ -78,7 +93,7 @@ if ($gconfig{'os_type'} =~ /-linux$/ &&
 		# Version: 9.14.2 deprecated the use of -r option
 		# in favor of using /dev/random [bugs:#5370]. So no
 		# entropy generation is needed.
-		return undef;
+		return;
 		}
 	}
 # No random flag, and entropy is needed
@@ -97,13 +112,13 @@ sub have_dnssec_tools_support
 		# dnssectools_rollrec
 		# dnssectools_keydir
 		# dnssectools_rollmgr_pidfile
-		return undef if (!$config{'dnssectools_conf'} ||
+		return if (!$config{'dnssectools_conf'} ||
 				 !$config{'dnssectools_rollrec'} ||
 				 !$config{'dnssectools_keydir'} ||
 				 !$config{'dnssectools_rollmgr_pidfile'});
 		return 1;
 	}
-	return undef;
+	return;
 }
 
 # get_bind_version()
@@ -116,7 +131,7 @@ if (&has_command($config{'named_path'})) {
 		return $2;
 		}
 	}
-return undef;
+return;
 }
 
 our @get_config_cache;
@@ -330,7 +345,7 @@ else {
 	while(1) {
 		$t = $_[0]->[++$i];
 		if ($t eq "{" || $t eq ";" || $t eq "}") { last; }
-		elsif (!defined($t)) { ${$_[2]} = $i; return undef; }
+		elsif (!defined($t)) { ${$_[2]} = $i; return; }
 		else { push(@vals, $t); }
 		}
 	$str{'values'} = \@vals;
@@ -342,7 +357,7 @@ else {
 		$str{'type'} = 1;
 		$j = 0;
 		while($_[0]->[$i] ne "}") {
-			if (!defined($_[0]->[$i])) { ${$_[2]} = $i; return undef; }
+			if (!defined($_[0]->[$i])) { ${$_[2]} = $i; return; }
 			my $substr = &parse_struct(
 				$_[0], $_[1], \$i, $j++, $_[4]);
 			if ($substr) {
@@ -388,7 +403,7 @@ sub find_value
 {
 my @v = &find($_[0], $_[1]);
 if (!@v) {
-	return undef;
+	return wantarray ? () : undef;
 	}
 elsif (wantarray) {
 	return map { &extract_value($_) } @v;
@@ -2075,9 +2090,9 @@ foreach my $v (&find("view", $conf)) {
         push(@zones, &find("zone", $v->{'members'}));
         }
 my ($z) = grep { lc($_->{'value'}) eq lc($name) } @zones;
-return undef if (!$z);
+return if (!$z);
 my $file = &find("file", $z->{'members'});
-return undef if (!$file);
+return if (!$file);
 my $filename = &absolute_path($file->{'values'}->[0]);
 $filename = &make_chroot($filename) if ($chroot);
 return $filename;
@@ -2256,7 +2271,7 @@ else {
 		}
 	}
 &refresh_nscd();
-return undef;
+return;
 }
 
 # before_editing(&zone)
@@ -2330,7 +2345,7 @@ elsif ($ex || $out =~ /failed|not found|error/i) {
 	return &text('restart_endc', "<tt>".&html_escape($out)."</tt>");
 	}
 &refresh_nscd();
-return undef;
+return;
 }
 
 # start_bind()
@@ -2375,7 +2390,7 @@ my $rv = $?;
 if ($rv || $out =~ /chroot.*not available/i) {
 	return &text('start_error', $out ? "<tt>$out</tt>" : "Unknown error");
 	}
-return undef;
+return;
 }
 
 # stop_bind()
@@ -2398,7 +2413,7 @@ else {
 		return $text{'stop_epid'};
 		}
 	}
-return undef;
+return;
 }
 
 # is_bind_running()
@@ -2437,7 +2452,7 @@ foreach my $c (@$vconf) {
 		return $c->{'index'};
 		}
 	}
-return undef;
+return;
 }
 
 # create_zone(&zone, &conf, [view-idx])
@@ -2641,7 +2656,7 @@ foreach my $z (@zones) {
 		return $z;
 		}
 	}
-return undef;
+return;
 }
 
 # get_zone_name_or_error(index|name, [viewindex|"any"])
@@ -2870,7 +2885,7 @@ if ($config{'tmpl_dnssec'} && &supports_dnssec()) {
 if ($secerr) {
 	return &text('mcreate_ednssec', $secerr);
 	}
-return undef;
+return;
 }
 
 # automatic_filename(domain, is-reverse, base, [viewname])
@@ -3305,7 +3320,7 @@ if ($view eq '' && @views || $view ne '' && @views > 1) {
                         [ map { [ $_->{'index'}, $_->{'value'} ] }
                             grep { $_->{'index'} ne $view } @views ]));
 	}
-return undef;
+return;
 }
 
 # download_root_zone(file)
@@ -3344,7 +3359,7 @@ if ($temp) {
 				     quotemeta($rootfile)." </dev/null");
 	return &text('boot_egzip2', "<tt>".&html_escape($out)."</tt>") if ($?);
 	}
-return undef;
+return;
 }
 
 # restart_links([&zone-name])
@@ -3458,13 +3473,14 @@ $fn || return "Could not work out keys directory!";
 my $dom = $z->{'members'} ? $z->{'values'}->[0] : $z->{'name'};
 
 # Remove all keys for the same zone
-opendir(ZONEDIR, $fn);
-foreach my $f (readdir(ZONEDIR)) {
+opendir(my $zonedir, $fn)
+	|| return "Failed to open keys directory $fn : $!";
+foreach my $f (readdir($zonedir)) {
 	if ($f =~ /^K\Q$dom\E\.\+(\d+)\+(\d+)\.(key|private)$/) {
 		&unlink_file("$fn/$f");
 		}
 	}
-closedir(ZONEDIR);
+closedir($zonedir);
 
 # Fork a background job to do lots of IO, to generate entropy
 my $pid;
@@ -3570,7 +3586,7 @@ foreach my $key (@keys) {
 	}
 &bump_soa_record($chrootfn, \@recs);
 
-return undef;
+return;
 }
 
 # resign_dnssec_key(&zone|&zone-name)
@@ -3643,7 +3659,7 @@ $newzonekey || return "Could not find new DNSSEC zone key";
 my $err = &sign_dnssec_zone($z);
 return "Re-signing failed : $err" if ($err);
 
-return undef;
+return;
 }
 
 # delete_dnssec_key(&zone|&zone-name, [save-key])
@@ -3753,7 +3769,7 @@ foreach my $r (@signedrecs) {
 	}
 &create_multiple_records($fn, \@addrecs);
 &unlink_file($signed);
-return undef;
+return;
 }
 
 # check_if_dnssec_tools_managed(&domain)
@@ -3801,7 +3817,7 @@ if (&check_if_dnssec_tools_managed($dom)) {
 	my $err = &dt_resign_zone($dom, $zonefile, $krfile, 0);
 	&unlock_file(&make_chroot($zonefile));
 	&error($err) if ($err);
-	return undef;
+	return;
 	}
 
 my $keyrec = &get_dnskey_record($z, $recs);
@@ -3820,8 +3836,17 @@ my ($z, $saved) = @_;
 my $dir = &get_keys_dir($z);
 my $dom = $z->{'members'} ? $z->{'values'}->[0] : $z->{'name'};
 my %keymap;
-opendir(ZONEDIR, $dir);
-foreach my $f (readdir(ZONEDIR)) {
+my $zonedir;
+if (!opendir($zonedir, $dir)) {
+	# A missing keys directory is the normal state before any DNSSEC
+	# keys have been generated for this zone; an unreadable one is a
+	# real error, but we can't return a string here because several
+	# list-context callers would treat it as a key hashref. Fall back
+	# to "no keys" and leave a breadcrumb in the error log.
+	warn "get_dnssec_key: opendir $dir failed: $!\n" if (-e $dir);
+	return wantarray ? () : undef;
+	}
+foreach my $f (readdir($zonedir)) {
 	if ($f =~ /^K\Q$dom\E\.\+(\d+)\+(\d+)\.key(\.saved)?$/) {
 		# Found the public key file .. read it
 		next if ($3 && !$saved);
@@ -3867,7 +3892,7 @@ foreach my $f (readdir(ZONEDIR)) {
 		while($rv->{'privatetext'} =~ s/^;.*\r?\n//) { }
 		}
 	}
-closedir(ZONEDIR);
+closedir($zonedir);
 
 # Sort to put KSK first
 my @rv = values %keymap;
@@ -4155,7 +4180,7 @@ sub dt_sign_zone
 
 	&dt_rollerd_restart();
 	&restart_bind();
-	return undef;
+	return;
 }
 
 # dt_resign_zone(zone-name, zonefile, krfile, threshold)
@@ -4212,7 +4237,7 @@ sub dt_resign_zone
 
 	&restart_zone($d);
 
-	return undef;
+	return;
 }
 
 # dt_zskroll_zone(zone-name)
@@ -4220,12 +4245,13 @@ sub dt_resign_zone
 sub dt_zskroll_zone
 {
 	my ($d) = @_;
-	no strict "subs";
+	# Constants exported by Net::DNS::SEC::Tools::rollmgr,
+	# which is only loaded when dnssec-tools is installed.
+	no strict "subs";    ## no critic (ProhibitNoStrict)
 	if (!rollmgr_sendcmd(CHANNEL_WAIT,ROLLCMD_ROLLZSK,$d)) {
 		return $text{'dt_zone_erollctl'};
 	}
-	use strict "subs";
-	return undef;
+	return;
 }
 
 # dt_kskroll_zone(zone-name)
@@ -4233,12 +4259,11 @@ sub dt_zskroll_zone
 sub dt_kskroll_zone
 {
 	my ($d) = @_;
-	no strict "subs";
+	no strict "subs";    ## no critic (ProhibitNoStrict)
 	if (!rollmgr_sendcmd(CHANNEL_WAIT,ROLLCMD_ROLLKSK,$d)) {
 		return $text{'dt_zone_erollctl'};
 	}
-	use strict "subs";
-	return undef;
+	return;
 }
 
 # dt_notify_parentzone(zone-name)
@@ -4246,12 +4271,11 @@ sub dt_kskroll_zone
 sub dt_notify_parentzone
 {
 	my ($d) = @_;
-	no strict "subs";
+	no strict "subs";    ## no critic (ProhibitNoStrict)
 	if (!rollmgr_sendcmd(CHANNEL_WAIT,ROLLCMD_DSPUB,$d)) {
 		return $text{'dt_zone_erollctl'};
 	}
-	use strict "subs";
-	return undef;
+	return;
 }
 
 # dt_rollerd_restart()
@@ -4270,7 +4294,7 @@ sub dt_rollerd_restart
 	$r = $config{"dnssectools_rollrec"};
 	$cmd = "$rollerd -rrfile ".quotemeta($r);
 	&execute_command($cmd);
-	return undef;
+	return;
 }
 
 # dt_genkrf()
@@ -4329,7 +4353,7 @@ sub dt_genkrf
 	$out = &backquote_logged("$cmd 2>&1");
 
 	return $out if ($?);
-	return undef;
+	return;
 }
 
 
@@ -4400,7 +4424,7 @@ sub dt_delete_dnssec_state
 		&unlink_file($z_dir."/dsset-".$dom.".");
 	}
 
-	return undef;
+	return;
 }
 
 # get_ds_record(&zone|&zone-name)
@@ -4424,7 +4448,7 @@ else {
 if (&has_command("dnssec-dsfromkey")) {
 	# Generate with a command
 	my $out = &backquote_command("dnssec-dsfromkey -f ".quotemeta(&make_chroot(&absolute_path($zonefile)))." ".quotemeta($dom)." 2>/dev/null");
-	return undef if ($?);
+	return if ($?);
 	$out =~ s/\r|\n//g;
 	return $out;
 	}
@@ -4445,9 +4469,9 @@ my $conf = &get_config();
 my $options = &find("options", $conf);
 my $mems = $options ? $options->{'members'} : [ ];
 my $en = &find_value("dnssec-enable", $mems);
-return undef if (!$en || $en !~ /yes/i);
+return if (!$en || $en !~ /yes/i);
 my $tkeys = &find("trusted-keys", $conf);
-return undef if (!$tkeys || !@{$tkeys->{'members'}});
+return if (!$tkeys || !@{$tkeys->{'members'}});
 return &text('trusted_warning',
 	     &get_webprefix().'/bind8/conf_trusted.cgi')."<p>\n".
        &ui_form_start(&get_webprefix().'/bind8/fix_trusted.cgi')."\n".

bind8/cgi_args.pl

@@ -62,5 +62,5 @@ elsif ($cgi eq 'view_text.cgi' || $cgi eq 'edit_soptions.cgi') {
 	return $z ? 'zone='.$z->{'zone'}.
 		    ($z->{'view'} ? '&view='.$z->{'viewindex'} : '') : 'none';
 	}
-return undef;
+return;
 }

bind8/check_zone.cgi

@@ -5,7 +5,7 @@ use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 # Globals from bind8-lib.pl
 our (%access, %text, %in);
 

bind8/close.cgi

@@ -7,7 +7,7 @@ no warnings 'uninitialized';
 # Globals
 our (%in);
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 &ReadParse();
 my @heiropen = &get_heiropen();
 @heiropen = grep { $_ ne $in{'what'} } @heiropen;

bind8/conf_acls.cgi

@@ -8,7 +8,7 @@ no warnings 'uninitialized';
 # Globals
 our (%text, %access);
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 $access{'defaults'} || &error($text{'acls_ecannot'});
 &ui_print_header(undef, $text{'acls_title'}, "",
 		 undef, undef, undef, undef, &restart_links());

bind8/conf_controls.cgi

@@ -7,7 +7,7 @@ no warnings 'uninitialized';
 # Globals
 our (%text, %access); 
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 $access{'defaults'} || &error($text{'controls_ecannot'});
 &ui_print_header(undef, $text{'controls_title'}, "",
 		 undef, undef, undef, undef, &restart_links());

bind8/conf_dnssec.cgi

@@ -7,7 +7,7 @@ no warnings 'uninitialized';
 # Globals
 our (%text, %access, %config);
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 &ReadParse();
 $access{'defaults'} || &error($text{'dnssec_ecannot'});
 &ui_print_header(undef, $text{'dnssec_title'}, "",

bind8/conf_dnssectools.cgi

@@ -7,7 +7,7 @@ no warnings 'uninitialized';
 # Globals
 our (%access, %text, %config);
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 
 &ReadParse();
 $access{'defaults'} || &error($text{'dt_conf_ecannot'});

bind8/conf_files.cgi

@@ -8,7 +8,7 @@ no warnings 'uninitialized';
 # Globals
 our (%access, %text);
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 $access{'defaults'} || &error($text{'files_ecannot'});
 &ui_print_header(undef, $text{'files_title'}, "",
 		 undef, undef, undef, undef, &restart_links());

bind8/conf_forwarding.cgi

@@ -8,7 +8,7 @@ no warnings 'uninitialized';
 # Globals
 our (%access, %text);
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 $access{'defaults'} || &error($text{'forwarding_ecannot'});
 &ui_print_header(undef, $text{'forwarding_title'}, "",
 		 undef, undef, undef, undef, &restart_links());

bind8/conf_keys.cgi

@@ -6,7 +6,7 @@ use warnings;
 no warnings 'redefine';
 no warnings 'uninitialized';
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 # Globals
 our (%access, %text);
 

bind8/conf_logging.cgi

@@ -8,7 +8,7 @@ no warnings 'uninitialized';
 our (%access, %text, %in);
 our (@syslog_levels, @severities, @cat_list);
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 $access{'defaults'} || &error($text{'logging_ecannot'});
 &ui_print_header(undef, $text{'logging_title'}, "",
 		 undef, undef, undef, undef, &restart_links());

bind8/conf_manual.cgi

@@ -7,7 +7,7 @@ no warnings 'uninitialized';
 # Globals
 our (%access, %text, %in);
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 $access{'defaults'} || &error($text{'manual_ecannot'});
 &ReadParse();
 &ui_print_header(undef, $text{'manual_title'}, "",

bind8/conf_misc.cgi

@@ -8,7 +8,7 @@ no warnings 'uninitialized';
 # Globals
 our (%access, %text);
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 $access{'defaults'} || &error($text{'misc_ecannot'});
 &ui_print_header(undef, $text{'misc_title'}, "",
 		 undef, undef, undef, undef, &restart_links());

bind8/conf_ncheck.cgi

@@ -7,7 +7,7 @@ no warnings 'uninitialized';
 # Globals
 our (%access, %text, %config);
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 &ReadParse();
 $access{'defaults'} || &error($text{'ncheck_ecannot'});
 

bind8/conf_net.cgi

@@ -7,7 +7,7 @@ no warnings 'uninitialized';
 # Globals
 our (%access, %text); 
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 $access{'defaults'} || &error($text{'net_ecannot'});
 &ui_print_header(undef, $text{'net_title'}, "",
 		 undef, undef, undef, undef, &restart_links());

bind8/conf_rndc.cgi

@@ -9,7 +9,7 @@ no warnings 'uninitialized';
 our (%access, %text, %config);
 our $module_name;
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 $access{'defaults'} || &error($text{'rndc_ecannot'});
 &ui_print_header(undef, $text{'rndc_title'}, "",
 		 undef, undef, undef, undef, &restart_links());

bind8/conf_servers.cgi

@@ -8,7 +8,7 @@ no warnings 'uninitialized';
 # Globals
 our (%access, %text);
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 $access{'defaults'} || &error($text{'servers_ecannot'});
 &ui_print_header(undef, $text{'servers_title'}, "",
 		 undef, undef, undef, undef, &restart_links());

bind8/conf_trusted.cgi

@@ -8,7 +8,7 @@ no warnings 'uninitialized';
 our (%access, %text, $bind_version);
 our $dnssec_dlv_zone;
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 &ReadParse();
 $access{'defaults'} || &error($text{'trusted_ecannot'});
 &supports_dnssec_client() || &error($text{'trusted_esupport'});

bind8/conf_zonedef.cgi

@@ -8,7 +8,7 @@ no warnings 'uninitialized';
 # Globals
 our (%access, %text, %config);
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 $access{'defaults'} || &error($text{'zonedef_ecannot'});
 &ui_print_header(undef, $text{'zonedef_title'}, "",
 		 undef, undef, undef, undef, &restart_links());
@@ -92,7 +92,7 @@ if (&supports_dnssec()) {
 
 	# Default algorithm
 	print &ui_table_row($text{'zonedef_alg'},
-		&ui_select("alg", $config{'tmpl_dnssecalg'} || "RSASHA1",
+		&ui_select("alg", $config{'tmpl_dnssecalg'} || "RSASHA256",
 			   [ &list_dnssec_algorithms() ]), 3);
 
 	# Default size

bind8/convert_master.cgi

@@ -8,7 +8,7 @@ no warnings 'uninitialized';
 # Globals
 our (%access, %text, %in, %config);
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 &ReadParse();
 &error_setup($text{'convert_err'});
 

bind8/convert_slave.cgi

@@ -8,7 +8,7 @@ no warnings 'uninitialized';
 # Globals
 our (%access, %text, %in);
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 &ReadParse();
 &error_setup($text{'convert_err'});
 

bind8/cpan_modules.pl

@@ -1,4 +1,6 @@
-require 'bind8-lib.pl';
+use strict;
+use warnings;
+require 'bind8-lib.pl';    ## no critic
 
 sub cpan_recommended
 {

bind8/create_delegation.cgi

@@ -8,7 +8,7 @@ no warnings 'uninitialized';
 # Globals
 our (%access, %text, %in, %config);
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 &ReadParse();
 &error_setup($text{'dcreate_err'});
 $access{'delegation'} || &error($text{'dcreate_ecannot'});

bind8/create_forward.cgi

@@ -8,7 +8,7 @@ no warnings 'uninitialized';
 # Globals
 our (%access, %text, %in, %config);
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 &ReadParse();
 &error_setup($text{'fcreate_err'});
 $access{'forward'} || &error($text{'fcreate_ecannot'});

bind8/create_hint.cgi

@@ -9,7 +9,7 @@ no warnings 'uninitialized';
 our (%access, %text, %in);
 our $module_root_directory;
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 &ReadParse();
 &error_setup($text{'hcreate_err'});
 $access{'master'} || &error($text{'hcreate_ecannot'});

bind8/create_master.cgi

@@ -8,7 +8,7 @@ no warnings 'uninitialized';
 # Globals
 our (%access, %text, %in, %config);
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 &ReadParse();
 &error_setup($text{'mcreate_err'});
 $access{'master'} || &error($text{'mcreate_ecannot'});

bind8/create_slave.cgi

@@ -11,7 +11,7 @@ no warnings 'uninitialized';
 # Globals
 our (%access, %text, %in, %config);
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 &ReadParse();
 &error_setup($in{'type'} ? $text{'screate_err1'} : $text{'screate_err2'});
 $access{'slave'} || &error($in{'type'} ? $text{'screate_ecannot1'}

bind8/create_view.cgi

@@ -7,7 +7,7 @@ no warnings 'uninitialized';
 # Globals
 our (%access, %text, %in);
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 &error_setup($text{'vcreate_err'});
 &ReadParse();
 my $add_to_file = &add_to_file();

bind8/delegation_form.cgi

@@ -8,7 +8,7 @@ no warnings 'uninitialized';
 # Globals
 our (%access, %text, %config);
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 $access{'delegation'} || &error($text{'dcreate_ecannot'});
 $access{'ro'} && &error($text{'master_ero'});
 &ui_print_header(undef, $text{'dcreate_title'}, "",

bind8/delete_recs.cgi

@@ -7,7 +7,7 @@ no warnings 'uninitialized';
 # Globals
 our (%access, %text, %in, %config);
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 &ReadParse();
 &error_setup($text{'drecs_err'});
 my $zone = &get_zone_name_or_error($in{'zone'}, $in{'view'});

bind8/delete_view.cgi

@@ -8,7 +8,7 @@ no warnings 'uninitialized';
 # Globals
 our (%access, %text, %in); 
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 &ReadParse();
 my $parent = &get_config_parent();
 my $conf = $parent->{'members'};

bind8/delete_zone.cgi

@@ -11,7 +11,7 @@ our (%access, %text, %in, %config);
 # Globals from records-lib.pl
 our $ipv6revzone;
 
-require './bind8-lib.pl';
+require './bind8-lib.pl';    ## no critic
 &ReadParse();
 
 my $zone = &get_zone_name_or_error($in{'zone'}, $in{'view'});