Merge pull request #85 from ovh/guestfix

fix: guests: get rid of ghost guest accesses in corner cases
This commit is contained in:
Stéphane Lesimple 2020-12-10 12:23:25 +01:00 committed by GitHub
commit 896721aad6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 70 additions and 1 deletions

View file

@ -163,7 +163,44 @@ sub act {
if ($type eq 'member') {
# in that case, we also need to handle the symlink
if ($action eq 'add' && OVH::Bastion::is_group_guest(group => $shortGroup, account => $account)) {
# if the user is a guest, must remove all his guest accesses first
$fnret = OVH::Bastion::get_acl_way(way => 'groupguest', group => $shortGroup, account => $account);
if ($fnret && $fnret->value) {
osh_warn("This account was previously a guest of this group, with the following accesses:");
my @acl = @{$fnret->value};
OVH::Bastion::print_acls(acls => [{type => 'group-guest', group => $shortGroup, acl => \@acl}]);
osh_info("\nCleaning these guest accesses before granting membership...");
# foreach guest access, delete
foreach my $access (@acl) {
my $machine = $access->{'ip'};
$machine .= ':' . $access->{'port'} if defined $access->{'port'};
$machine = $access->{'user'} . '@' . $machine if defined $access->{'user'};
$fnret = OVH::Bastion::Plugin::groupSetRole::act(
account => $account,
group => $shortGroup,
action => 'del',
type => 'guest',
user => $access->{'user'},
userAny => (defined $access->{'user'} ? 0 : 1),
port => $access->{'port'},
portAny => (defined $access->{'port'} ? 0 : 1),
host => $access->{'ip'},
self => $self,
);
if (!$fnret) {
osh_warn("Failed removing guest access to $machine, proceeding anyway...");
warn_syslog("Failed removing guest access to $machine in group $shortGroup for $account, before granting this account full membership on behalf of $self: "
. $fnret->msg);
}
}
}
}
# then, for add and del, we need to handle the symlink
@command = qw{ sudo -n -u allowkeeper -- /usr/bin/env perl -T };
push @command, $OVH::Bastion::BASEPATH . '/bin/helper/osh-groupAddSymlinkToAccount';
push @command, '--group', $group; # must be first params, forced in sudoers.d
@ -204,6 +241,11 @@ sub act {
}
}
# If the account is already a member, can't add/del them as guest
if (OVH::Bastion::is_group_member(group => $shortGroup, account => $account)) {
return R('ERR_MEMBER_CANNOT_BE_GUEST', msg => "Can't $action $account as a guest of group $shortGroup, they're already a member!");
}
# Add/Del user access to user@host:port with group key
@command = qw{ sudo -n -u allowkeeper -- /usr/bin/env perl -T };
push @command, $OVH::Bastion::BASEPATH . '/bin/helper/osh-accountAddGroupServer';

View file

@ -81,6 +81,8 @@ testsuite_groups()
# now create g1
grant groupCreate
run groupCreate a2_fail_to_create_g1_with_a1_as_owner_because_not_granted $a2 --osh groupCreate --group $group1 --algo rsa --size 2048 --owner $account1
retvalshouldbe 106
contain "you to be specifically granted"
@ -127,6 +129,8 @@ EOS
#g3_pubkey=$(get_json | $jq .value.public_key.line)
#g3_fp=$( get_json | $jq .value.public_key.fingerprint)
revoke groupCreate
success groupInfo a0_info_on_g3_after_create $a0 --osh groupInfo --group $group3
json .error_code OK .command groupInfo .value.group $group3
json --arg want "$account0" '.value.owners|sort == ($want|split(" ")|sort)' true
@ -527,11 +531,32 @@ EOS
retvalshouldbe 107
json .error_code KO_ACCESS_DENIED
# try to add a3 as a guest of g1, should not work because already a member
plgfail groupAddGuestAccess a1_add_a3_guest_of_g1_fail_already_member $a1 --osh groupAddGuestAccess --group $group1 --account $account3 --user g2 --host 127.0.0.2 --port 22
json .command groupAddGuestAccess .error_code ERR_MEMBER_CANNOT_BE_GUEST
# now remove membership of a3
success groupDelMember a2_del_a3_as_g1_member $a2 --osh groupDelMember --group $group1 --account $account3
json .command groupDelMember .error_code OK .value null
# add a guest access to a3...
success groupAddGuestAccess a1_add_a3_guest_of_g1 $a1 --osh groupAddGuestAccess --group $group1 --account $account3 --user g2 --host 127.0.0.2 --port 22
json .command groupAddGuestAccess .error_code OK
# ... then add it as member again: it should remove the guest access we've added just above...
success groupAddMember a1_add_a3_member_of_g1 $a1 --osh groupAddMember --group $group1 --account $account3
contain "Cleaning these guest accesses"
json .command groupAddMember .error_code OK
# ... then remove its membership
success groupDelMember a2_del_a3_as_g1_member_2 $a2 --osh groupDelMember --group $group1 --account $account3
json .command groupDelMember .error_code OK .value null
# ... and verify there's no ghost guest access remaining
success groupListGuestAccesses a2_list_a3_guest_access_g1_empty $a2 --osh groupListGuestAccesses --group $group1 --account $account3
json .command groupListGuestAccesses .error_code OK_EMPTY .value null
# new state: g1[a1(ow,gk,acl,member) a2(gk) acl(g1@127.0.0.1:22,g2@127.0.0.2:22)] g3[a0,a2,a3(ow,gk,acl,member)]
# check that a3 can no longer access the ips
@ -580,6 +605,8 @@ EOS
success groupAddServer a3_add_server_to_g3 $a3 --osh groupAddServer --group $group3 --host 10.20.0.0/17 --port-any --user-any
grant accountAddPersonalAccess
run accountAddPersonalAccess a0_add_personal_access_to_a3_works_slash $a0 --osh accountAddPersonalAccess --account $account3 --host 77.66.55.0/24
json .command accountAddPersonalAccess .error_code OK .value null