From b7860c27a8ab5072440e3a87df5c61ad5f7443b8 Mon Sep 17 00:00:00 2001 From: Tim Mullin Date: Mon, 5 Feb 2024 15:46:27 +0000 Subject: [PATCH] Convert ssh blocker to an autofix Case RE-112: For sshd, the default value for PermitRootLogin changes from "yes" to "no" when upgrading from CentOS 7 to a CentOS 8 variant. We used to block if PermitRootLogin was not explicitly set; but, instead it is more desirable to simply emit a warning that we are going to set it to "yes" and go ahead and set it so the behavior stays the same post upgrade. I've altered the SSH blocker module to emit the warning and not block. I've also moved the SSH blocker execution to happen earlier so the warning is more visible. And, I've added a component that will provide the autofix for the sshd configuration file. Changelog: Set PermitRootLogin for sshd to 'yes' if not explicitly set in the configuration file so that the behavior does not change after the upgrade. --- elevate-cpanel | 69 ++++++++++++++++++++++++++++++----- lib/Elevate/Blockers.pm | 2 +- lib/Elevate/Blockers/SSH.pm | 18 ++++----- lib/Elevate/Components/SSH.pm | 51 ++++++++++++++++++++++++++ script/elevate-cpanel.PL | 2 + t/blocker-SSH.t | 53 ++++++++++----------------- t/components-SSH.t | 62 +++++++++++++++++++++++++++++++ 7 files changed, 203 insertions(+), 54 deletions(-) create mode 100644 lib/Elevate/Components/SSH.pm create mode 100644 t/components-SSH.t diff --git a/elevate-cpanel b/elevate-cpanel index 93c3df92..4d319ce0 100755 --- a/elevate-cpanel +++ b/elevate-cpanel @@ -50,6 +50,7 @@ BEGIN { # Suppress load of all of these at earliest point. $INC{'Elevate/Components/RpmDB.pm'} = 'script/elevate-cpanel.PL.static'; $INC{'Elevate/Components/RmMod.pm'} = 'script/elevate-cpanel.PL.static'; $INC{'Elevate/Components/WPToolkit.pm'} = 'script/elevate-cpanel.PL.static'; + $INC{'Elevate/Components/SSH.pm'} = 'script/elevate-cpanel.PL.static'; $INC{'Elevate/Fetch.pm'} = 'script/elevate-cpanel.PL.static'; $INC{'Elevate/Logger.pm'} = 'script/elevate-cpanel.PL.static'; $INC{'Elevate/Motd.pm'} = 'script/elevate-cpanel.PL.static'; @@ -227,6 +228,7 @@ BEGIN { # Suppress load of all of these at earliest point. IsContainer ElevateScript + SSH DiskSpace WHM @@ -235,7 +237,6 @@ BEGIN { # Suppress load of all of these at earliest point. Databases Repositories - SSH JetBackup NICs EA4 @@ -1895,23 +1896,23 @@ EOS sub check ($self) { - return $self->_blocker_invalid_ssh_config; - } - - sub _blocker_invalid_ssh_config ($self) { - return $self->has_blocker(q[Issue with sshd configuration]) unless $self->_sshd_setup(); - return 0; + return $self->_check_ssh_config(); } - sub _sshd_setup ($self) { + sub _check_ssh_config ($self) { my $sshd_config = q[/etc/ssh/sshd_config]; my $setup = eval { File::Slurper::read_binary($sshd_config) } // ''; + if ( my $exception = $@ ) { + ERROR("The system could not read the sshd config file ($sshd_config): $exception"); + return $self->has_blocker(qq[Unable to read the sshd config file: $sshd_config]); + } if ( $setup !~ m{^\s*PermitRootLogin\b}m ) { - ERROR( <<~"EOS" ); + WARN( <<~"EOS" ); OpenSSH configuration file does not explicitly state the option PermitRootLogin in sshd_config file, which will default in RHEL8 to "prohibit-password". - Please set the 'PermitRootLogin' value in $sshd_config before upgrading. + We will set the 'PermitRootLogin' value in $sshd_config to 'yes' before upgrading. + EOS return 0; @@ -4085,6 +4086,52 @@ EOS } # --- END lib/Elevate/Components/WPToolkit.pm +{ # --- BEGIN lib/Elevate/Components/SSH.pm + + package Elevate::Components::SSH; + + use cPstrict; + + use Elevate::Constants (); + + use Cwd (); + use File::Slurper (); + + # use Log::Log4perl qw(:easy); + INIT { Log::Log4perl->import(qw{:easy}); } + + # use Elevate::Components::Base(); + our @ISA; + BEGIN { push @ISA, qw(Elevate::Components::Base); } + + sub pre_leapp ($self) { + + my $sshd_config = q[/etc/ssh/sshd_config]; + + my $setup = File::Slurper::read_binary($sshd_config) // ''; + + return if ( $setup =~ m{^\s*PermitRootLogin\b}m ); + + if ( $setup !~ m{\n$} && length $setup ) { + $setup .= "\n"; + } + + $setup .= "PermitRootLogin yes\n"; + + File::Slurper::write_binary( $sshd_config, $setup ); + + return; + } + + sub post_leapp ($self) { + + return; + } + + 1; + +} # --- END lib/Elevate/Components/SSH.pm + { # --- BEGIN lib/Elevate/Fetch.pm package Elevate::Fetch; @@ -5223,6 +5270,7 @@ use Elevate::Components::Repositories (); use Elevate::Components::RpmDB (); use Elevate::Components::RmMod (); use Elevate::Components::WPToolkit (); +use Elevate::Components::SSH (); use Elevate::Fetch (); use Elevate::Logger (); @@ -5923,6 +5971,7 @@ sub run_stage_2 ($self) { Elevate::Motd->setup(); + $self->run_component_once( 'SSH' => 'pre_leapp' ); $self->run_component_once( 'KernelCare' => 'pre_leapp' ); $self->run_component_once( 'Grub2' => 'pre_leapp' ); diff --git a/lib/Elevate/Blockers.pm b/lib/Elevate/Blockers.pm index f7d5b875..52ecfebc 100644 --- a/lib/Elevate/Blockers.pm +++ b/lib/Elevate/Blockers.pm @@ -51,6 +51,7 @@ our @BLOCKERS = qw{ IsContainer ElevateScript + SSH DiskSpace WHM @@ -59,7 +60,6 @@ our @BLOCKERS = qw{ Databases Repositories - SSH JetBackup NICs EA4 diff --git a/lib/Elevate/Blockers/SSH.pm b/lib/Elevate/Blockers/SSH.pm index 0f3676f6..945c656c 100644 --- a/lib/Elevate/Blockers/SSH.pm +++ b/lib/Elevate/Blockers/SSH.pm @@ -22,23 +22,23 @@ use Log::Log4perl qw(:easy); sub check ($self) { - return $self->_blocker_invalid_ssh_config; + return $self->_check_ssh_config(); } -sub _blocker_invalid_ssh_config ($self) { - return $self->has_blocker(q[Issue with sshd configuration]) unless $self->_sshd_setup(); - return 0; -} - -sub _sshd_setup ($self) { +sub _check_ssh_config ($self) { my $sshd_config = q[/etc/ssh/sshd_config]; my $setup = eval { File::Slurper::read_binary($sshd_config) } // ''; + if ( my $exception = $@ ) { + ERROR("The system could not read the sshd config file ($sshd_config): $exception"); + return $self->has_blocker(qq[Unable to read the sshd config file: $sshd_config]); + } if ( $setup !~ m{^\s*PermitRootLogin\b}m ) { - ERROR( <<~"EOS" ); + WARN( <<~"EOS" ); OpenSSH configuration file does not explicitly state the option PermitRootLogin in sshd_config file, which will default in RHEL8 to "prohibit-password". - Please set the 'PermitRootLogin' value in $sshd_config before upgrading. + We will set the 'PermitRootLogin' value in $sshd_config to 'yes' before upgrading. + EOS return 0; diff --git a/lib/Elevate/Components/SSH.pm b/lib/Elevate/Components/SSH.pm new file mode 100644 index 00000000..5556eb4f --- /dev/null +++ b/lib/Elevate/Components/SSH.pm @@ -0,0 +1,51 @@ +package Elevate::Components::SSH; + +=encoding utf-8 + +=head1 NAME + +Elevate::Components::SSH + +Ensure that the sshd config file has 'PermitRootLogin' set to 'yes' +if it is not set. + +=cut + +use cPstrict; + +use Elevate::Constants (); + +use Cwd (); +use File::Slurper (); +use Log::Log4perl qw(:easy); + +use parent qw{Elevate::Components::Base}; + +sub pre_leapp ($self) { + + my $sshd_config = q[/etc/ssh/sshd_config]; + + my $setup = File::Slurper::read_binary($sshd_config) // ''; + + # PermitRootLogin is explicitly set, no need for changes + return if ( $setup =~ m{^\s*PermitRootLogin\b}m ); + + # Add ending newline if file does not end with newline + if ( $setup !~ m{\n$} && length $setup ) { + $setup .= "\n"; + } + + $setup .= "PermitRootLogin yes\n"; + + File::Slurper::write_binary( $sshd_config, $setup ); + + return; +} + +sub post_leapp ($self) { + + # Nothing to do + return; +} + +1; diff --git a/script/elevate-cpanel.PL b/script/elevate-cpanel.PL index 0bcc31c5..cfec6849 100755 --- a/script/elevate-cpanel.PL +++ b/script/elevate-cpanel.PL @@ -279,6 +279,7 @@ use Elevate::Components::Repositories (); use Elevate::Components::RpmDB (); use Elevate::Components::RmMod (); use Elevate::Components::WPToolkit (); +use Elevate::Components::SSH (); use Elevate::Fetch (); use Elevate::Logger (); @@ -979,6 +980,7 @@ sub run_stage_2 ($self) { Elevate::Motd->setup(); + $self->run_component_once( 'SSH' => 'pre_leapp' ); $self->run_component_once( 'KernelCare' => 'pre_leapp' ); $self->run_component_once( 'Grub2' => 'pre_leapp' ); diff --git a/t/blocker-SSH.t b/t/blocker-SSH.t index df5ee93b..f4169a7c 100644 --- a/t/blocker-SSH.t +++ b/t/blocker-SSH.t @@ -24,69 +24,54 @@ my $cpev = cpev->new; my $ssh = $cpev->get_blocker('SSH'); { - note "checking _sshd_setup"; + note "checking _check_ssh_config"; my $mock_sshd_cfg = Test::MockFile->file(q[/etc/ssh/sshd_config]); my $sshd_error_message = <<~'EOS'; OpenSSH configuration file does not explicitly state the option PermitRootLogin in sshd_config file, which will default in RHEL8 to "prohibit-password". - Please set the 'PermitRootLogin' value in /etc/ssh/sshd_config before upgrading. + We will set the 'PermitRootLogin' value in /etc/ssh/sshd_config to 'yes' before upgrading. + EOS - is $ssh->_sshd_setup() => 0, "sshd_config does not exist"; - message_seen( 'ERROR', $sshd_error_message ); + my $blocker = $ssh->_check_ssh_config(); + is ref $blocker, "cpev::Blocker", "sshd_config does not exist"; + message_seen( 'ERROR', qr/The system could not read the sshd config file/ ); + message_seen( 'WARN', qr/Elevation Blocker detected/ ); $mock_sshd_cfg->contents(''); - is $ssh->_sshd_setup() => 0, "sshd_config with empty content"; - message_seen( 'ERROR', $sshd_error_message ); + is $ssh->_check_ssh_config() => 0, "sshd_config with empty content"; + message_seen( 'WARN', $sshd_error_message ); $mock_sshd_cfg->contents( <<~EOS ); Fruit=cherry Veggy=carrot EOS - is $ssh->_sshd_setup() => 0, "sshd_config without PermitRootLogin option"; - message_seen( 'ERROR', $sshd_error_message ); + is $ssh->_check_ssh_config() => 0, "sshd_config without PermitRootLogin option"; + message_seen( 'WARN', $sshd_error_message ); $mock_sshd_cfg->contents( <<~EOS ); Key=value PermitRootLogin=yes EOS - is $ssh->_sshd_setup() => 1, "sshd_config with PermitRootLogin=yes - multilines"; + is $ssh->_check_ssh_config() => 1, "sshd_config with PermitRootLogin=yes - multilines"; $mock_sshd_cfg->contents(q[PermitRootLogin=no]); - is $ssh->_sshd_setup() => 1, "sshd_config with PermitRootLogin=no"; + is $ssh->_check_ssh_config() => 1, "sshd_config with PermitRootLogin=no"; $mock_sshd_cfg->contents(q[PermitRootLogin no]); - is $ssh->_sshd_setup() => 1, "sshd_config with PermitRootLogin=no"; + is $ssh->_check_ssh_config() => 1, "sshd_config with PermitRootLogin=no"; $mock_sshd_cfg->contents(q[PermitRootLogin = no]); - is $ssh->_sshd_setup() => 1, "sshd_config with PermitRootLogin = no"; + is $ssh->_check_ssh_config() => 1, "sshd_config with PermitRootLogin = no"; $mock_sshd_cfg->contents(q[#PermitRootLogin=no]); - is $ssh->_sshd_setup() => 0, "sshd_config with commented PermitRootLogin=no"; - message_seen( 'ERROR', $sshd_error_message ); + is $ssh->_check_ssh_config() => 0, "sshd_config with commented PermitRootLogin=no"; + message_seen( 'WARN', $sshd_error_message ); $mock_sshd_cfg->contents(q[#PermitRootLogin=yes]); - is $ssh->_sshd_setup() => 0, "sshd_config with commented PermitRootLogin=yes"; - message_seen( 'ERROR', $sshd_error_message ); -} - -{ - note "sshd setup check"; - - $ssh_mock->redefine( '_sshd_setup' => 0 ); - is( - $ssh->_blocker_invalid_ssh_config(), - { - id => q[Elevate::Blockers::SSH::_blocker_invalid_ssh_config], - msg => 'Issue with sshd configuration', - }, - q{Block if sshd is not explicitly configured.} - ); - - $ssh_mock->redefine( '_sshd_setup' => 1 ); - is( $ssh->_blocker_invalid_ssh_config, 0, "no blocker if _sshd_setup is ok" ); - $ssh_mock->unmock('_sshd_setup'); + is $ssh->_check_ssh_config() => 0, "sshd_config with commented PermitRootLogin=yes"; + message_seen( 'WARN', $sshd_error_message ); } done_testing(); diff --git a/t/components-SSH.t b/t/components-SSH.t new file mode 100644 index 00000000..214c4b98 --- /dev/null +++ b/t/components-SSH.t @@ -0,0 +1,62 @@ +#!/usr/local/cpanel/3rdparty/bin/perl + +package test::cpev::components; + +use FindBin; + +use Test2::V0; +use Test2::Tools::Explain; +use Test2::Plugin::NoWarnings; +use Test2::Tools::Exception; + +use Test::MockFile 0.032; +use Test::MockModule qw/strict/; + +use lib $FindBin::Bin . "/lib"; +use Test::Elevate; + +use cPstrict; + +my $ssh = bless {}, 'Elevate::Components::SSH'; + +{ + note "checking pre_leapp"; + + my $mock_sshd_cfg = Test::MockFile->file(q[/etc/ssh/sshd_config]); + + $mock_sshd_cfg->contents(''); + $ssh->pre_leapp(); + is $mock_sshd_cfg->contents(), "PermitRootLogin yes\n", 'Added PermitRootLogin to empty config'; + + my $pre_contents = "PasswordAuthentication no\nUseDNS no\nDenyGroups cpaneldemo\n"; + $mock_sshd_cfg->contents($pre_contents); + $ssh->pre_leapp(); + is $mock_sshd_cfg->contents(), $pre_contents . "PermitRootLogin yes\n", 'Added PermitRootLogin when ommited, trailing newline'; + + $pre_contents = "PasswordAuthentication no\nUseDNS no\nDenyGroups cpaneldemo"; + $mock_sshd_cfg->contents($pre_contents); + $ssh->pre_leapp(); + is $mock_sshd_cfg->contents(), $pre_contents . "\nPermitRootLogin yes\n", 'Added PermitRootLogin when ommited, no trailing newline'; + + $pre_contents = "PasswordAuthentication no\nPermitRootLogin yes\nUseDNS no\nDenyGroups cpaneldemo"; + $mock_sshd_cfg->contents($pre_contents); + $ssh->pre_leapp(); + is $mock_sshd_cfg->contents(), $pre_contents, 'Contents unchanged when PermitRootLogin present and active'; + + $pre_contents = "PermitRootLogin no\nPasswordAuthentication no\nUseDNS no\nDenyGroups cpaneldemo\n"; + $mock_sshd_cfg->contents($pre_contents); + $ssh->pre_leapp(); + is $mock_sshd_cfg->contents(), $pre_contents, 'Contents unchanged when PermitRootLogin present and active'; + + $pre_contents = "PasswordAuthentication no\n#PermitRootLogin yes\nUseDNS no\nDenyGroups cpaneldemo"; + $mock_sshd_cfg->contents($pre_contents); + $ssh->pre_leapp(); + is $mock_sshd_cfg->contents(), $pre_contents . "\nPermitRootLogin yes\n", 'Contents updated when PermitRootLogin present but commented out'; + + $pre_contents = "# PermitRootLogin no\nPasswordAuthentication no\nUseDNS no\nDenyGroups cpaneldemo\n"; + $mock_sshd_cfg->contents($pre_contents); + $ssh->pre_leapp(); + is $mock_sshd_cfg->contents(), $pre_contents . "PermitRootLogin yes\n", 'Contents updated when PermitRootLogin present but commented out'; +} + +done_testing();