diff --git a/bin/plugin/open/groupList b/bin/plugin/open/groupList index 3099b55..63501af 100755 --- a/bin/plugin/open/groupList +++ b/bin/plugin/open/groupList @@ -36,7 +36,7 @@ EOF my $fnret; -$fnret = OVH::Bastion::get_group_list(groupType => "key"); +$fnret = OVH::Bastion::get_group_list(); $fnret or osh_exit $fnret; my $includere = OVH::Bastion::build_re_from_wildcards(wildcards => \@includes, implicit_contains => 1)->value; diff --git a/bin/plugin/open/info b/bin/plugin/open/info index 523456f..0074efe 100755 --- a/bin/plugin/open/info +++ b/bin/plugin/open/info @@ -118,9 +118,9 @@ osh_info sprintf("\nI am %s, aka %s", colored(Sys::Hostname::hostname(), 'green' $ret{'hostname'} = Sys::Hostname::hostname(); $ret{'bastion_name'} = $config->{'bastionName'}; -$fnret = OVH::Bastion::get_account_list(); +$fnret = OVH::Bastion::get_account_list(cache => 1); my $nbaccounts = $fnret ? keys %{$fnret->value} : '?'; -$fnret = OVH::Bastion::get_group_list(groupType => 'key'); +$fnret = OVH::Bastion::get_group_list(cache => 1); my $nbgroups = $fnret ? keys %{$fnret->value} : '?'; osh_info "I have " . colored($nbaccounts, 'green') . " registered accounts and " . colored($nbgroups, 'green') . " groups"; $ret{'registered_accounts'} = $nbaccounts; diff --git a/bin/plugin/restricted/accountInfo b/bin/plugin/restricted/accountInfo index 4922cc5..a2d963c 100755 --- a/bin/plugin/restricted/accountInfo +++ b/bin/plugin/restricted/accountInfo @@ -68,18 +68,18 @@ $ret{'allowed_commands'} = \@granted; my $result_hash = {}; if ($listGroups) { - $fnret = OVH::Bastion::get_group_list(groupType => "key"); + $fnret = OVH::Bastion::get_group_list(cache => 1); $fnret or osh_exit $fnret; osh_info "\nThis account is part of the following groups:"; foreach my $name (sort keys %{$fnret->value}) { my @flags; - push @flags, 'owner' if OVH::Bastion::is_group_owner(group => $name, account => $account); - push @flags, 'gatekeeper' if OVH::Bastion::is_group_gatekeeper(group => $name, account => $account); - push @flags, 'aclkeeper' if OVH::Bastion::is_group_aclkeeper(group => $name, account => $account); - push @flags, 'member' if OVH::Bastion::is_group_member(group => $name, account => $account); - push @flags, 'guest' if OVH::Bastion::is_group_guest(group => $name, account => $account); + push @flags, 'owner' if OVH::Bastion::is_group_owner(group => $name, account => $account, cache => 1); + push @flags, 'gatekeeper' if OVH::Bastion::is_group_gatekeeper(group => $name, account => $account, cache => 1); + push @flags, 'aclkeeper' if OVH::Bastion::is_group_aclkeeper(group => $name, account => $account, cache => 1); + push @flags, 'member' if OVH::Bastion::is_group_member(group => $name, account => $account, cache => 1); + push @flags, 'guest' if OVH::Bastion::is_group_guest(group => $name, account => $account, cache => 1); if (@flags) { my $line = sprintf "%18s", $name; $line .= sprintf " %14s", colored(grep({ $_ eq 'owner' } @flags) ? 'Owner' : '-', 'red'); diff --git a/lib/perl/OVH/Bastion.pm b/lib/perl/OVH/Bastion.pm index 3961af3..3800798 100644 --- a/lib/perl/OVH/Bastion.pm +++ b/lib/perl/OVH/Bastion.pm @@ -141,7 +141,7 @@ my %_autoload_files = ( qw{ enable_mocking is_mocking set_mock_data mock_get_account_entry mock_get_account_accesses mock_get_account_personal_accesses mock_get_account_legacy_accesses mock_get_group_accesses mock_get_account_guest_accesses } ], os => [ - qw{ sysinfo is_linux is_debian is_redhat is_bsd is_freebsd is_openbsd is_netbsd has_acls sys_useradd sys_groupadd sys_userdel sys_groupdel sys_addmembertogroup sys_delmemberfromgroup sys_changepassword sys_neutralizepassword sys_setpasswordpolicy sys_getpasswordinfo sys_getsudoersfolder sys_setfacl is_in_path } + qw{ sysinfo is_linux is_debian is_redhat is_bsd is_freebsd is_openbsd is_netbsd has_acls sys_useradd sys_groupadd sys_userdel sys_groupdel sys_addmembertogroup sys_delmemberfromgroup sys_changepassword sys_neutralizepassword sys_setpasswordpolicy sys_getpasswordinfo sys_getsudoersfolder sys_setfacl is_in_path sys_getent_pw sys_getent_gr } ], password => [qw{ get_hashes_from_password get_hashes_list is_valid_hash }], ssh => [ diff --git a/lib/perl/OVH/Bastion/allowdeny.inc b/lib/perl/OVH/Bastion/allowdeny.inc index 283a23e..4c02ab4 100644 --- a/lib/perl/OVH/Bastion/allowdeny.inc +++ b/lib/perl/OVH/Bastion/allowdeny.inc @@ -340,9 +340,8 @@ sub ip2host { return R('OK', value => $host); } -# Return an array containing the groups for which user is a member of -my %_cache_get_user_groups; - +# returns a list of the bastion groups (more precisely, the system group names +# mapped to these groups) a user is member of. sub get_user_groups { my %params = @_; my $user = $params{'user'} || $params{'account'}; @@ -353,19 +352,16 @@ sub get_user_groups { return R('ERR_MISSING_PARAMETER', msg => "Missing parameter 'account'"); } - if (not %_cache_get_user_groups) { - - # build cache, it'll be faster than even one exec `id -nG` anyway - setgrent(); - while (my ($name, $passwd, $gid, $members) = getgrent()) { - foreach my $member (split / /, $members) { - push @{$_cache_get_user_groups{$member}}, $name; - } + # try to use sys_getent_gr()'s cache if available and allowed by caller. + # we loop through all the system groups to find the ones having user + # as a meember + my @groups; + foreach my $gr (@{OVH::Bastion::sys_getent_gr(cache => $cache)->value}) { + if (grep { $user eq $_ } @{$gr->{'members'} || []}) { + push @groups, $gr->{'name'}; } - setgrent(); } - my @groups = @{$_cache_get_user_groups{$user} || []}; my @availableGroups; foreach my $group (@groups) { if ($group =~ /^key.+-(gatekeeper|aclkeeper|owner)$/) { diff --git a/lib/perl/OVH/Bastion/allowkeeper.inc b/lib/perl/OVH/Bastion/allowkeeper.inc index 00106bf..3a8e673 100644 --- a/lib/perl/OVH/Bastion/allowkeeper.inc +++ b/lib/perl/OVH/Bastion/allowkeeper.inc @@ -6,61 +6,64 @@ use common::sense; use Time::Piece; # $t->strftime -# used to speedup extensive calls to is_user_in_group() and is_group_existing(), -# if our caller allows us: -my %_cache_getgrnam; - -# Check if user belongs to a specific group +# Check if a system user belongs to a specific system group sub is_user_in_group { my %params = @_; my $group = $params{'group'}; my $user = $params{'user'} || OVH::Bastion::get_user_from_env()->value; - my $cache = $params{'cache'}; # if true, allow cache use from potential previous calls + my $cache = $params{'cache'}; # if true, allow cache use of sys_getent_gr() # mandatory keys if (!$user || !$group) { return R('ERR_MISSING_PARAMETER', msg => "Missing parameter 'user' or 'group'"); } - # if we're not allowed to use cache, call getgrnam() in all cases. - # if we're allowed to use it; only call getgrnam() if we have no cache for this group - if (!$cache || (!$_cache_getgrnam{$group} || ref $_cache_getgrnam{$group} ne 'ARRAY')) { - $_cache_getgrnam{$group} = [(getgrnam($group))]; + # try to use sys_getent_gr()'s cache if available and allowed by caller. + # we loop through all the system groups to find the proper one, then + # check whether $user appears in the member list + foreach my $gr (@{OVH::Bastion::sys_getent_gr(cache => $cache)->value}) { + next if $gr->{'name'} ne $group; + if (grep { $user eq $_ } @{$gr->{'members'} || []}) { + return R('OK', value => {account => $user}); + } + else { + return R('KO_NOT_IN_GROUP', msg => "Account $user doesn't belong to the group $group"); + } } - my ($groupname, $passwd, $gid, $members) = @{$_cache_getgrnam{$group}}; - my @membersList = split / /, $members; - - if (grep { $user eq $_ } @membersList) { - return R('OK', value => {account => $user}); - } - return R('KO_NOT_IN_GROUP', msg => "Account $user doesn't belong to the group $group"); + return R('KO_GROUP_NOT_FOUND', msg => "The group $group doesn't exist"); } -# does this group exist ? -# uses %_cache_getgrnam defined above +# does this system group exist ? if it happens to be mapped to a bastion group, +# also return the corresponding "shortGroup" (with the "key" prefix removed) sub is_group_existing { - my %params = @_; - my $group = $params{'group'}; - my $cache = $params{'cache'}; # if true, allow cache use from potential previous calls + my %params = @_; + my $group = $params{'group'}; + my $cache = $params{'cache'}; # if true, allow cache use from potential previous calls + my $user_friendly_error = $params{'user_friendly_error'}; if (!$group) { return R('ERR_MISSING_PARAMETER', msg => "Missing parameter 'group'"); } - # if we're not allowed to use cache, call getgrnam() in all cases. - # if we're allowed to use it; only call getgrnam() if we have no cache for this group - if (!$cache || (!$_cache_getgrnam{$group} || ref $_cache_getgrnam{$group} ne 'ARRAY')) { - $_cache_getgrnam{$group} = [(getgrnam($group))]; - } - my ($groupname, $passwd, $gid, $members) = @{$_cache_getgrnam{$group}}; - if ($groupname) { + # try to use sys_getent_gr()'s cache if available and allowed by caller. + # we loop through all the system groups to find the proper one + foreach my $gr (@{OVH::Bastion::sys_getent_gr(cache => $cache)->value}) { + next if $gr->{'name'} ne $group; my (undef, $shortGroup) = $group =~ m{^(key)?(.+)}; - return R('OK', value => {group => $group, shortGroup => $shortGroup, gid => $gid, keyhome => "/home/keykeeper/$group", members => [split(/ /, $members)]}); + return R( + 'OK', + value => { + group => $group, + shortGroup => $shortGroup, + gid => $gr->{'gid'}, + keyhome => "/home/keykeeper/$group", + members => $gr->{'members'}, + } + ); } - if ($params{'user_friendly_error'}) { - - # build a user-compatible error message, as it can make its way through osh_exit() + # build a user-compatible error message if asked to, as it can make its way through osh_exit() + if ($user_friendly_error) { $group =~ s/^key//; return R('KO_GROUP_NOT_FOUND', msg => "The bastion group '$group' doesn't exist.\nYou may use groupList --all to see all existing groups."); } @@ -244,42 +247,52 @@ sub is_account_valid { return R('ERR_IMPOSSIBLE_CASE'); } -# check that this account is present on the bastion -# it also returns untainted data, including splitting by realm where applicable -my %_cache_getpwnam; - sub is_account_existing { my %params = @_; my $account = $params{'account'}; - my $checkBastionShell = $params{'checkBastionShell'}; # check if this account is a bastion user - my $cache = $params{'cache'}; # allow cache use to speedup commands that don't modify accounts but may call us extensively, i.e. groupList + my $checkBastionShell = $params{'checkBastionShell'}; # check if this account is a bastion user + my $cache = $params{'cache'}; # allow cache use if (!$account) { return R('ERR_MISSING_PARAMETER', msg => "Missing parameter 'account'"); } - # if we're not allowed to use cache, call getpwnam() in all cases. - # if we're allowed to use it; only call getpwnam() if we have no cache for this account - if (!$cache || (!$_cache_getpwnam{$account} || ref $_cache_getpwnam{$account} ne 'ARRAY')) { - $_cache_getpwnam{$account} = [(getpwnam($account))]; - } - my ($name, $passwd, $uid, $gid, $quota, $comment, $gcos, $dir, $shell, $expire) = @{$_cache_getpwnam{$account}}; + my %entry; if (OVH::Bastion::is_mocking()) { - ($name, $passwd, $uid, $gid, $gcos, $dir, $shell) = OVH::Bastion::mock_get_account_entry(account => $account); + my @fields = OVH::Bastion::mock_get_account_entry(account => $account); + %entry = ( + name => $fields[0], + passwd => $fields[1], + uid => $fields[2], + gid => $fields[3], + gcos => $fields[4], + dir => $fields[5], + shell => $fields[6], + ); + } + else { + # try to use sys_getent_pw()'s cache if available and allowed by caller. + # we loop through all the system accounts to find the proper one + foreach my $pw (@{OVH::Bastion::sys_getent_pw(cache => $cache)->value}) { + next if $pw->{'name'} ne $account; + %entry = %$pw; + last; + } } - if ($name) { - my ($newname) = $name =~ m{([a-zA-Z0-9._-]+)}; - return R('ERR_SECURITY_VIOLATION', msg => "Forbidden characters in account name") if ($newname ne $name); - $name = $newname; # untaint - if ($checkBastionShell && $shell ne $OVH::Bastion::BASEPATH . "/bin/shell/osh.pl") { + if (%entry) { + my ($newname) = $entry{'name'} =~ m{([a-zA-Z0-9._-]+)}; + return R('ERR_SECURITY_VIOLATION', msg => "Forbidden characters in account name") if ($newname ne $entry{'name'}); + $entry{'name'} = $newname; # untaint + + if ($checkBastionShell && $entry{'shell'} ne $OVH::Bastion::BASEPATH . "/bin/shell/osh.pl") { return R('KO_NOT_FOUND', msg => "Account '$account' doesn't exist"); # msg is the same as below, voluntarily } - my ($newdir) = $dir =~ m{([/a-zA-Z0-9._-]+)}; # untaint - return R('ERR_SECURITY_VIOLATION', msg => "Forbidden characters in account home directory") if ($newdir ne $dir); - $dir = $newdir; # untaint - return R('OK', value => {uid => $uid, gid => $gid, dir => $dir, account => $name}); + my ($newdir) = $entry{'dir'} =~ m{([/a-zA-Z0-9._-]+)}; # untaint + return R('ERR_SECURITY_VIOLATION', msg => "Forbidden characters in account home directory") if ($newdir ne $entry{'dir'}); + $entry{'dir'} = $newdir; # untaint + return R('OK', value => {uid => $entry{'uid'}, gid => $entry{'gid'}, dir => $entry{'dir'}, account => $entry{'name'}}); } return R('KO_NOT_FOUND', msg => "Account '$account' doesn't exist"); } @@ -722,97 +735,114 @@ sub add_user_to_group { return R('OK'); } -my %_cache_get_group_list; - +# return the list of the bastion groups (i.e. not the system group list) sub get_group_list { - my %params = @_; - my $groupType = $params{'groupType'}; - my $cache = $params{'cache'}; # if true, allow cache use + my %params = @_; + my $cache = $params{'cache'}; # if true, allow cache use + state $cached_response; - if ($cache and $_cache_get_group_list{$groupType}) { - return $_cache_get_group_list{$groupType}; + # if we've been called before and can use the cache, just return it + if ($cache and $cached_response) { + return $cached_response; } my %groups; - setgrent(); - while (my @nextgroup = getgrent()) { - my ($name, $passwd, $gid, $members) = @nextgroup; - # also be nice and fill this cache for our sibling funcs - $_cache_getgrnam{$name} = \@nextgroup; - if ( $groupType eq 'key' - and $name =~ /^key/ - and $name !~ /-(?:owner|gatekeeper|aclkeeper)$/ - and not grep { $name eq $_ } qw{ keykeeper keyreader }) + # sys_getent_gr() might have been called before, and has its own cache, + # so also try to use its cache if allowed and available. + # we loop through all the system groups and only retain those starting + # with "key", and not finishing in -owner, -gatekeeper or -aclkeeper. + # we also exclude special builtin groups (keykeeper and keyreader) + foreach my $gr (@{OVH::Bastion::sys_getent_gr(cache => $cache)->value}) { + if ( $gr->{'name'} =~ /^key/ + && $gr->{'name'} !~ /-(?:owner|gatekeeper|aclkeeper)$/ + && !grep { $gr->{'name'} eq $_ } qw{ keykeeper keyreader }) { - $name =~ s/^key//; - $groups{$name} = {gid => $gid, members => [split(/ /, $members)]} if ($name ne ''); + $gr->{'name'} =~ s/^key//; + $groups{$gr->{'name'}} = {gid => $gr->{'gid'}, members => $gr->{'members'}} if ($gr->{'name'} ne ''); } } - $_cache_get_group_list{$groupType} = R('OK', value => \%groups); - return $_cache_get_group_list{$groupType}; + $cached_response = R('OK', value => \%groups); + return $cached_response; } +# return the list of bastion accounts (i.e. not the system user list) sub get_account_list { my %params = @_; my $accounts = $params{'accounts'} || []; my $cache = $params{'cache'}; # if true, allow cache use state $cached_response; - if ($cache and $cached_response) { + # if we've been called before and can use the cache, just return it + # don't do it if we're asked to only return a subset of all the accounts + if ($cache && $cached_response && !@$accounts) { return $cached_response; } my %users; + # sys_getent_pw() might have been called before, and has its own cache, + # so also try to use its cache if allowed and available. + # we loop through all the accounts known to the OS + foreach my $pw (@{OVH::Bastion::sys_getent_pw(cache => $cache)->value}) { + + # if $accounts has been specified, only consider those + next if (@$accounts && !grep { $pw->{'name'} eq $_ } @$accounts); + + # skip invalid accounts + next if not OVH::Bastion::is_bastion_account_valid_and_existing(account => $pw->{'name'}); + + # add proper accounts, only include a subset of the fields we got + $users{$pw->{'name'}} = {name => $pw->{'name'}, gid => $pw->{'gid'}, home => $pw->{'dir'}, shell => $pw->{'shell'}, uid => $pw->{'uid'}}; + } + if (@$accounts) { - foreach (@$accounts) { - my ($name, $passwd, $uid, $gid, $quota, $comment, $gcos, $dir, $shell, $expire) = getpwnam($_); - next unless OVH::Bastion::is_bastion_account_valid_and_existing(account => $name); - $users{$name} = {name => $name, uid => $uid, gid => $gid, home => $dir, shell => $shell}; - } + return R('OK', value => \%users); } else { - setpwent(); - while (my @nextaccount = getpwent()) { - my ($name, $passwd, $uid, $gid, $quota, $comment, $gcos, $dir, $shell, $expire) = @nextaccount; - - # also be nice and fill this cache for our sibling funcs - $_cache_getpwnam{$name} = \@nextaccount; - next unless OVH::Bastion::is_bastion_account_valid_and_existing(account => $name); - $users{$name} = {name => $name, uid => $uid, gid => $gid, home => $dir, shell => $shell}; - } + $cached_response = R('OK', value => \%users); + return $cached_response; } - $cached_response = R('OK', value => \%users); - return $cached_response; } sub get_realm_list { my %params = @_; my $realms = $params{'realms'} || []; + my $cache = $params{'cache'}; # if true, allow cache use + state $cached_response; + + # if we've been called before and can use the cache, just return it + # don't do it if we're asked to only return a subset of all the accounts + if ($cache && $cached_response && !@$realms) { + return $cached_response; + } my %users; - setpwent(); + # sys_getent_pw() might have been called before, and has its own cache, + # so also try to use its cache if allowed and available. + # we loop through all the accounts known to the OS + foreach my $pw (@{OVH::Bastion::sys_getent_pw(cache => $cache)->value}) { + + # if $realms has been specified, only consider those + next if (@$realms && !grep { $pw->{'name'} eq "realm_$_" } @$realms); + + # skip invalid realms + next if not OVH::Bastion::is_bastion_account_valid_and_existing(account => $pw->{'name'}, accountType => "realm"); + + # add proper realms + my $name = $pw->{'name'}; + $name =~ s{^realm_}{}; + $users{$name} = {name => $name}; + } if (@$realms) { - foreach (@$realms) { - my ($name, $passwd, $uid, $gid, $quota, $comment, $gcos, $dir, $shell, $expire) = getpwnam("realm_" . $_); - next unless OVH::Bastion::is_bastion_account_valid_and_existing(account => $name, accountType => "realm"); - $name =~ s{^realm_}{}; - $users{$name} = {name => $name}; - } + return R('OK', value => \%users); } else { - setpwent(); - while (my ($name, $passwd, $uid, $gid, $quota, $comment, $gcos, $dir, $shell, $expire) = getpwent()) { - next unless OVH::Bastion::is_bastion_account_valid_and_existing(account => $name, accountType => "realm"); - $name =~ s{^realm_}{}; - $users{$name} = {name => $name}; - } + $cached_response = R('OK', value => \%users); + return $cached_response; } - - return R('OK', value => \%users); } # check if account is a bastion admin (gives access to adminXyz commands) diff --git a/lib/perl/OVH/Bastion/os.inc b/lib/perl/OVH/Bastion/os.inc index dd7fcdc..a460076 100644 --- a/lib/perl/OVH/Bastion/os.inc +++ b/lib/perl/OVH/Bastion/os.inc @@ -595,4 +595,84 @@ sub is_in_path { return R('KO', msg => "$binary was not found in PATH"); } +# as setpwent/endpwent can have side effects to any getpwent used between +# them, possibly in other parts of the program, encapsulate everything here +# to avoid side effects +sub sys_getent_pw { + my %params = @_; + my $cache = $params{'cache'}; + state $cached_response; + + if ($cache && $cached_response) { + return $cached_response; + } + + my @all; + + # end/set: for some reason, if we don't end() before set(), there seem + # to be a cache somewhere, and we get entries that have been + # deleted (milli)seconds before. This behaviour is undocumented, as set() + # is supposed to reset the pointer, and a no mention of a cache is done + # anywhere. Luckily, Prepending the set() with an end() seems to + # reliably fix the issue on all tested OSes. + endpwent(); + setpwent(); + while (my @line = getpwent()) { + push @all, + { + name => $line[0], + passwd => $line[1], + uid => $line[2], + gid => $line[3], + quota => $line[4], + comment => $line[5], + gcos => $line[6], + dir => $line[7], + shell => $line[8], + expire => $line[9], + }; + } + endpwent(); + + $cached_response = R('OK', value => \@all); + return $cached_response; +} + +# as setgrent/endgrent can have side effects to any getgrent used between +# them, possibly in other parts of the program, encapsulate everything here +# to avoid side effects +sub sys_getent_gr { + my %params = @_; + my $cache = $params{'cache'}; + state $cached_response; + + if ($cache && $cached_response) { + return $cached_response; + } + + my @all; + + # end/set: for some reason, if we don't end() before set(), there seem + # to be a cache somewhere, and we get entries that have been + # deleted (milli)seconds before. This behaviour is undocumented, as set() + # is supposed to reset the pointer, and a no mention of a cache is done + # anywhere. Luckily, pPrepending the set() with an end() seems to + # reliably fix the issue on all tested OSes. + endgrent(); + setgrent(); + while (my @line = getgrent()) { + push @all, + { + name => $line[0], + passwd => $line[1], + gid => $line[2], + members => [split(/ /, $line[3])], + }; + } + endgrent(); + + $cached_response = R('OK', value => \@all); + return $cached_response; +} + 1;