Skip to content

Commit

Permalink
enh: better error detection and logging in (account|group)Delete
Browse files Browse the repository at this point in the history
  • Loading branch information
@speed47
speed47 committed Jan 19, 2022
1 parent 9d371f9 commit 3331e15
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 35 deletions.
77 changes: 61 additions & 16 deletions bin/helper/osh-accountDelete
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ else {
}

# do the stuff
osh_info("Backing up home directory...");

if (!-d "/home/oldkeeper") {
mkdir "/home/oldkeeper";
}
Expand All @@ -147,8 +149,20 @@ mkdir $fulldir;
chown 0, 0, $fulldir;
chmod 0700, $fulldir;

move("/home/$account", "$fulldir/$account-home");
move("/home/allowkeeper/$account", "$fulldir/allowkeeper");
# from now on, as we're starting to move/remove/delete things, errors will be non fatal
# because we're trying to cleanup as much as we can, the idea is to log errors to the
# syslog, and then report to the user (and in the final formatted log) that we
# had issues and somebody might want to have a look
my $nbErrors = 0;

if (!move("/home/$account", "$fulldir/$account-home")) {
warn_syslog("Error while backing up to-be-deleted '/home/$account' to '$fulldir/$account-home': $!, continuing anyway...");
$nbErrors++;
}
if (!move("/home/allowkeeper/$account", "$fulldir/allowkeeper")) {
warn_syslog("Error while backing up to-be-deleted '/home/allowkeeper/$account' to '$fulldir/allowkeeper': $!, continuing anyway...");
$nbErrors++;
}

# remove +a or tar won't be able to rm files, don't check if it succeeded if we're on a system without chattr
$fnret = OVH::Bastion::execute(cmd => ['find', "$fulldir/$account-home", '-maxdepth', '1', '-name', "*.log", '-exec', 'chattr', '-a', '{}', ';']);
Expand All @@ -157,6 +171,7 @@ $fnret = OVH::Bastion::execute(cmd => ['find', "$fulldir/$account-home", '-maxde
$fnret = OVH::Bastion::execute(cmd => [$OVH::Bastion::BASEPATH . '/bin/sudogen/generate-sudoers.sh', 'delete', 'account', $account], must_succeed => 1, noisy_stdout => 1);
if (!$fnret) {
warn_syslog("Error during account deletion of '$account', couldn't delete sudoers file: " . $fnret->msg);
$nbErrors++;
}

# add a text file with all the groups the user was a member of
Expand All @@ -179,43 +194,73 @@ push @tarcmd, $fulldir . '.tar.gz';
push @tarcmd, '--acls' if OVH::Bastion::has_acls();
push @tarcmd, '--one-file-system', '-p', '--remove-files', $fulldir;

osh_info("Backing up home directory...");
$fnret = OVH::Bastion::execute(cmd => \@tarcmd, must_succeed => 1);
if (!$fnret) {
osh_warn("Couldn't tar the backup homedir of this account (" . $fnret->msg . "), proceeding anyway.");
chmod 0000, $fulldir;
warn_syslog("Couldn't tar the backup homedir of this account (" . $fnret->msg . "), proceeding anyway.");
my $i = 0;
foreach (@{$fnret->value->{'stderr'} || []}) {
warn_syslog("tar: $_");
if (++$i >= 10) {
warn_syslog("more tar errors, suppressing");
last;
}
}
$nbErrors++;
}
else {
chmod 0000, $fulldir . '.tar.gz';
unlink($fulldir);

if (-e "$fulldir.tar.gz") {
chmod 0000, "$fulldir.tar.gz";
}

# if the folder still exists, tar failed in some way, warn the admins
if (-d $fulldir) {
chmod 0000, $fulldir;
warn_syslog("While archiving the account '$account', $fulldir still exists, manual cleanup might be needed");
$nbErrors++;
}
osh_info("Backup done");

osh_info "Removing '$account' group membership from 'keyreader' user";
$fnret = OVH::Bastion::sys_delmemberfromgroup(user => "keyreader", group => $account);
$fnret or HEXIT($fnret);
if (!$fnret) {
warn_syslog("Couldn't delete '$account' group membership from 'keyreader' user, proceeding anyway");
$nbErrors++;
}
osh_info "Deleting system user '$account'...";
$fnret = OVH::Bastion::sys_userdel(user => $account);
$fnret or HEXIT($fnret);
if (!$fnret) {
warn_syslog("Couldn't delete system user '$account', proceeding anyway");
$nbErrors++;
}

# some systems don't delete the primary group with userdel (suse at least)
$fnret = OVH::Bastion::execute(cmd => ['getent', 'group', $account]);
if ($fnret && $fnret->value->{'sysret'} == 0) {
$fnret = OVH::Bastion::is_group_existing(group => $account);
if ($fnret) {
osh_info "Deleting account main group '$account'...";
$fnret = OVH::Bastion::sys_groupdel(group => $account);
$fnret or HEXIT($fnret);
if (!$fnret) {
warn_syslog("Couldn't delete account main group '$account', proceeding anyway");
$nbErrors++;
}
}

if (defined $ttygroup) {
osh_info "Deleting group $ttygroup...";
$fnret = OVH::Bastion::sys_groupdel(group => $ttygroup);
$fnret or HEXIT($fnret);
if (!$fnret) {
warn_syslog("Couldn't delete group '$ttygroup', proceeding anyway");
$nbErrors++;
}
}

if ($nbErrors) {
osh_warn("Encountered $nbErrors error(s) while deleting account, " . "please refer to the system log for more details (uniqid: " . $ENV{'UNIQID'} . ")");
}

OVH::Bastion::syslogFormatted(
severity => 'info',
type => 'account',
fields => [['action', 'delete'], ['account', $account]]
fields => [['action', 'delete'], ['account', $account], ['errors', $nbErrors]]
);

HEXIT('OK', value => {account => $account, ttygroup => $ttygroup, operation => 'deleted'});
HEXIT('OK', value => {account => $account, ttygroup => $ttygroup, operation => 'deleted', errors => $nbErrors + 0}, msg => "Account $account has been deleted");
81 changes: 62 additions & 19 deletions bin/helper/osh-groupDelete
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,10 @@ if (not -e "/home/$group/allowed.ip" or not -e "/home/keykeeper/$group") {
HEXIT('ERR_INVALID_GROUP', msg => "Sorry, but $shortGroup doesn't seem to be a legit bastion group");
}

if (not -d "/home/oldkeeper") {
# do the stuff
osh_info("Backing up group directory...");

if (!-d "/home/oldkeeper") {
mkdir "/home/oldkeeper";
}
chown 0, 0, "/home/oldkeeper";
Expand All @@ -93,15 +96,27 @@ my $suffix = 'at-' . time() . '.by-' . $self;

my $fulldir = "/home/oldkeeper/groups/$group.$suffix";
if (-e $fulldir) {
exitError("Errr... $fulldir exists?!");
HEXIT('ERR_BACKUP_DIR_COLLISION', msg => "This shouldn't happen, $fulldir already exists!");
}

mkdir $fulldir;
chown 0, 0, $fulldir;
chmod 0700, $fulldir;

move("/home/$group", "$fulldir/$group-home");
move("/home/keykeeper/$group", "$fulldir/$group-keykeeper");
# from now on, as we're starting to move/remove/delete things, errors will be non fatal
# because we're trying to cleanup as much as we can, the idea is to log errors to the
# syslog, and then report to the user (and in the final formatted log) that we
# had issues and somebody might want to have a look
my $nbErrors = 0;

if (!move("/home/$group", "$fulldir/$group-home")) {
warn_syslog("Error while backing up to-be-deleted '/home/$group' to '$fulldir/$group-home': $!, continuing anyway...");
$nbErrors++;
}
if (!move("/home/keykeeper/$group", "$fulldir/$group-keykeeper")) {
warn_syslog("Error while backing up to-be-deleted '/home/keykeeper/$group' to '$fulldir/$group-keykeeper': $!, continuing anyway...");
$nbErrors++;
}

# now tar.gz the directory, this is important because inside we'll keep the
# old GID of the group, and we don't want GID-orphaned files on our filesystem, it's
Expand All @@ -110,16 +125,33 @@ my @tarcmd = qw{ tar czf };
push @tarcmd, $fulldir . '.tar.gz';
push @tarcmd, '--acls' if OVH::Bastion::has_acls();
push @tarcmd, '--one-file-system', '-p', '--remove-files', $fulldir;

$fnret = OVH::Bastion::execute(cmd => \@tarcmd, must_succeed => 1);
if (!$fnret) {
osh_warn("Couldn't tar the backup homedir of this group (" . $fnret->msg . "), proceeding anyway.");
chmod 0000, $fulldir;
warn_syslog("Couldn't tar the backup homedir of this group (" . $fnret->msg . "), proceeding anyway.");
my $i = 0;
foreach (@{$fnret->value->{'stderr'} || []}) {
warn_syslog("tar: $_");
if (++$i >= 10) {
warn_syslog("more tar errors, suppressing");
last;
}
}
$nbErrors++;
}
else {
chmod 0000, $fulldir . '.tar.gz';
unlink($fulldir);

if (-e "$fulldir.tar.gz") {
chmod 0000, "$fulldir.tar.gz";
}

# if the folder still exists, tar failed in some way, warn the admins
if (-d $fulldir) {
chmod 0000, $fulldir;
warn_syslog("While archiving the group '$group', $fulldir still exists, manual cleanup might be needed");
$nbErrors++;
}
osh_info("Backup done");

# remove dead symlinks in users homes
my $dh;
if (opendir($dh, "/home/allowkeeper")) {
Expand All @@ -131,14 +163,18 @@ if (opendir($dh, "/home/allowkeeper")) {
foreach my $file ("$dir/allowed.ip.$shortGroup", "$dir/allowed.partial.$shortGroup") {
if (-e $file || -l $file) {
osh_info "Removing $file...";
unlink($file);
if (!unlink($file)) {
warn_syslog("Couldn't remove symlink '$file': $!");
$nbErrors++;
}
}
}
}
close($dh);
}
else {
osh_warn("Couldn't open /home/allowkeeper ?!");
warn_syslog("Couldn't open /home/allowkeeper to cleanup symlinks: $!");
$nbErrors++;
}

# trying to remove main and gatekeeper and owner groups
Expand All @@ -152,15 +188,19 @@ foreach my $todelete ("$group-owner", "$group-aclkeeper", "$group-gatekeeper", $
foreach my $member (@$members) {
osh_info "... removing $member from group $todelete";
$fnret = OVH::Bastion::sys_delmemberfromgroup(user => $member, group => $todelete, noisy_stderr => 1);
$fnret->err eq 'OK'
or HEXIT('ERR_DELUSER_FAILED', msg => "Error while attempting to remove member $member from group $todelete (" . $fnret->msg . ")");
if ($fnret->err ne 'OK') {
warn_syslog("Error while attempting to remove member $member from group $todelete (" . $fnret->msg . "), proceeding anyway");
$nbErrors++;
}
}
}

if ($todelete eq $group) {
osh_info "Deleting main user of group $todelete...", $fnret = OVH::Bastion::sys_userdel(user => $todelete, noisy_stderr => 1);
$fnret->err eq 'OK'
or HEXIT('ERR_DELUSER_FAILED', msg => "Error while attempting to delete main user of group $todelete (" . $fnret->msg . ")");
if ($fnret->err ne 'OK') {
warn_syslog("Error while attempting to delete main user of group $todelete (" . $fnret->msg . "), proceeding anyway");
$nbErrors++;
}
}

# some OSes delete the main group of user if it has the same name
Expand All @@ -170,8 +210,10 @@ foreach my $todelete ("$group-owner", "$group-aclkeeper", "$group-gatekeeper", $
if ($fnret) {
osh_info "Deleting group $todelete...";
$fnret = OVH::Bastion::sys_groupdel(group => $todelete, noisy_stderr => 1);
$fnret
or HEXIT('ERR_DELGROUP_FAILED', msg => "Error while attempting to delete group $todelete (" . $fnret->msg . ")");
if ($fnret->err ne 'OK') {
warn_syslog("Error while attempting to delete group $todelete (" . $fnret->msg . "), proceeding anyway");
$nbErrors++;
}
}
}
else {
Expand All @@ -183,12 +225,13 @@ foreach my $todelete ("$group-owner", "$group-aclkeeper", "$group-gatekeeper", $
$fnret = OVH::Bastion::execute(cmd => [$OVH::Bastion::BASEPATH . '/bin/sudogen/generate-sudoers.sh', 'delete', 'group', $group], must_succeed => 1, noisy_stdout => 1);
if (!$fnret) {
warn_syslog("Error during group deletion of '$group', couldn't delete sudoers file: " . $fnret->msg);
$nbErrors++;
}

OVH::Bastion::syslogFormatted(
severity => 'info',
type => 'group',
fields => [['action', 'delete'], ['group', $shortGroup],]
fields => [['action', 'delete'], ['group', $shortGroup], ['errors', $nbErrors]]
);

HEXIT('OK');
HEXIT('OK', value => {group => $group, operation => 'deleted', errors => $nbErrors + 0}, msg => "Group $group has been deleted");

0 comments on commit 3331e15

Please sign in to comment.