ovh/the-bastion
1
1
Fork 0
mirror of https://github.com/ovh/the-bastion.git synced 2024-09-20 06:55:58 +08:00
Code Issues Packages Projects Releases Wiki Activity

enh: add lock for group ACL change to avoid race conditions

Browse source
This commit is contained in:
Stéphane Lesimple 2024-08-30 13:19:11 +00:00 committed by Stéphane Lesimple
parent 85c448d60c
commit 733e67ef1d
7 changed files with 82 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
bin/helper/osh-accountCreate
View file

@ -69,7 +69,9 @@ if (not grep { $type eq $_ } qw{ normal realm }) {
# take a lock here, do it before checking for account existence,
# because another parallel creation of the same account might be
# occuring, in which case we'd still hit a race condition
$fnret = OVH::Bastion::Helper::get_lock_fh();
# additionally, the lock type "passwd" is used by all helpers
# that may modify /etc/passwd or /etc/group.
$fnret = OVH::Bastion::Helper::get_lock_fh(category => "passwd");
$fnret or HEXIT($fnret);
my $lock_fh = $fnret->value;
$fnret = OVH::Bastion::Helper::acquire_lock($lock_fh);

9
bin/helper/osh-accountDelete
View file

@ -238,6 +238,15 @@ if (-d $fulldir) {
}
osh_info("Backup done");
# take a lock here, as we're going to remove system accounts and groups.
# additionally, the lock type "passwd" is used by all helpers
# that may modify /etc/passwd or /etc/group.
$fnret = OVH::Bastion::Helper::get_lock_fh(category => "passwd");
$fnret or HEXIT($fnret);
my $lock_fh = $fnret->value;
$fnret = OVH::Bastion::Helper::acquire_lock($lock_fh);
$fnret or HEXIT($fnret);
osh_info "Removing '$account' group membership from 'keyreader' user";
$fnret = OVH::Bastion::sys_delmemberfromgroup(user => "keyreader", group => $account);
if (!$fnret) {

7
bin/helper/osh-groupAddServer
View file

@ -78,6 +78,13 @@ else {
#<RIGHTSCHECK
# take a lock here, to block other group ACL modifying commands for this group until we're done.
$fnret = OVH::Bastion::Helper::get_lock_fh(category => "groupacl", basepath => "/home/$group");
$fnret or HEXIT($fnret);
my $lock_fh = $fnret->value;
$fnret = OVH::Bastion::Helper::acquire_lock($lock_fh);
$fnret or HEXIT($fnret);
#>CODE
my $machine = $ip;
$port and $machine .= ":$port";

6
bin/helper/osh-groupCreate
View file

@ -96,8 +96,10 @@ my $shortGroup = $fnret->value->{'shortGroup'};
# take a lock here, do it before checking for group existence,
# because another parallel creation of the same group might be
# occuring, in which case we'd still hit a race condition
$fnret = OVH::Bastion::Helper::get_lock_fh();
# occuring, in which case we'd still hit a race condition.
# additionally, the lock type "passwd" is used by all helpers
# that may modify /etc/passwd or /etc/group.
$fnret = OVH::Bastion::Helper::get_lock_fh(category => "passwd");
$fnret or HEXIT($fnret);
my $lock_fh = $fnret->value;
$fnret = OVH::Bastion::Helper::acquire_lock($lock_fh);

9
bin/helper/osh-groupDelete
View file

@ -162,6 +162,15 @@ if (-d $fulldir) {
}
osh_info("Backup done");
# take a lock here, as we're going to remove system accounts and groups.
# additionally, the lock type "passwd" is used by all helpers
# that may modify /etc/passwd or /etc/group.
$fnret = OVH::Bastion::Helper::get_lock_fh(category => "passwd");
$fnret or HEXIT($fnret);
my $lock_fh = $fnret->value;
$fnret = OVH::Bastion::Helper::acquire_lock($lock_fh);
$fnret or HEXIT($fnret);
# remove dead symlinks in users homes
my $dh;
if (opendir($dh, "/home/allowkeeper")) {

7
bin/helper/osh-groupSetServers
View file

@ -77,6 +77,13 @@ if (!$data || ref $data ne 'ARRAY') {
HEXIT('ERR_INVALID_ARGUMENT', msg => "Invalid JSON import format sent by the plugin");
}
# take a lock here, to block other group ACL modifying commands for this group until we're done.
$fnret = OVH::Bastion::Helper::get_lock_fh(category => "groupacl", basepath => "/home/$group");
$fnret or HEXIT($fnret);
my $lock_fh = $fnret->value;
$fnret = OVH::Bastion::Helper::acquire_lock($lock_fh);
$fnret or HEXIT($fnret);
$fnret = OVH::Bastion::access_modify(
way => 'group',
action => 'clear',

61
lib/perl/OVH/Bastion/Helper.pm
View file

@ -75,27 +75,52 @@ if (not defined $self) {
}
sub get_lock_fh {
my $fh;
my $lockdir = "/tmp/bastion.lock";
my $lockfile = "$lockdir/lock";
my %params = @_;
my $category = $params{'category'};
# to avoid symlink attacks, we first create a subdir only accessible by root
unlink $lockdir; # will silently fail if doesn't exist or is not a file
mkdir $lockdir; # will silently fail if we lost the race
chown 0, 0, $lockdir;
chmod 0700, $lockdir;
return R('ERR_MISSING_PARAMETER', msg => "Missing category in get_lock_fh") if !$category;
# now, check if we do have a directory, or if we lost the race
if (!-d $lockdir) {
warn_syslog("Couldn't create $lockdir: are we being raced against?");
return R('ERR_CANNOT_LOCK', msg => "Couldn't create lock file, please retry");
my ($lockdir, $lockfile, $lockdircreate);
if ($category eq 'passwd') {
$lockdir = "/tmp/bastion.lock.passwd";
$lockfile = "$lockdir/lock";
$lockdircreate = 1;
}
# here, $lockdir is guaranteed to be a directory, check its perms
my @perms = stat($lockdir);
if ($perms[4] != 0 || $perms[5] != 0 || S_IMODE($perms[2]) != oct(700)) {
warn_syslog("The $lockdir directory has invalid perms: are we being raced against? mode="
. sprintf("%04o", S_IMODE($perms[2])));
return R('ERR_CANNOT_LOCK', msg => "Couldn't create lock file, please retry");
elsif ($category eq 'groupacl') {
$lockdir = $params{'basepath'};
$lockdircreate = 0; # must already exist
if (!$lockdir || !-d $lockdir || !-w $lockdir) {
return R('ERR_INVALID_PARAMETER',
msg => "Missing or invalid basepath '" . ($lockdir // '<u>') . "' in get_lock_fh");
}
# we use the .db suffix because it's already excluded from the cluster sync:
$lockfile = "$lockdir/lock.db";
}
else {
return R('ERR_INVALID_PARAMETER', msg => "Unknown category '$category' in get_lock_fh");
}
my $fh;
if ($lockdircreate) {
# to avoid symlink attacks, we first create a subdir only accessible by root
unlink $lockdir; # will silently fail if doesn't exist or is not a file
mkdir $lockdir; # will silently fail if we lost the race
chown 0, 0, $lockdir;
chmod 0700, $lockdir;
# now, check if we do have a directory, or if we lost the race
if (!-d $lockdir) {
warn_syslog("Couldn't create $lockdir: are we being raced against?");
return R('ERR_CANNOT_LOCK', msg => "Couldn't create lock file, please retry");
}
# here, $lockdir is guaranteed to be a directory, check its perms
my @perms = stat($lockdir);
if ($perms[4] != $< || $perms[5] != $( || S_IMODE($perms[2]) != oct(700)) {
warn_syslog("The $lockdir directory has invalid perms: are we being raced against? mode="
. sprintf("%04o", S_IMODE($perms[2])));
return R('ERR_CANNOT_LOCK', msg => "Couldn't create lock file, please retry");
}
}
# here, $lockdir is guaranteed to be owned only by us. but rogue files