diff --git a/bin/helper/osh-accountAddGroupServer b/bin/helper/osh-accountAddGroupServer index a244090..9d9f24b 100755 --- a/bin/helper/osh-accountAddGroupServer +++ b/bin/helper/osh-accountAddGroupServer @@ -31,7 +31,7 @@ if (not defined $self) { # Fetch command options my $fnret; my ($result, @optwarns); -my ($account, $group, $ip, $user, $port, $action, $ttl, $forceKey); +my ($account, $group, $ip, $user, $port, $action, $ttl, $comment, $forceKey); eval { local $SIG{__WARN__} = sub { push @optwarns, shift }; $result = GetOptions( @@ -42,6 +42,7 @@ eval { "port=i" => sub { $port //= $_[1] }, "action=s" => sub { $action //= $_[1] }, "ttl=i" => sub { $ttl //= $_[1] }, + "comment=s" => sub { $comment //= $_[1] }, "force-key=s" => sub { $forceKey //= $_[1] }, ); }; @@ -78,6 +79,7 @@ $fnret = OVH::Bastion::access_modify( ip => $ip, port => $port, ttl => $ttl, + comment => $comment, forceKey => $forceKey ); HEXIT($fnret); diff --git a/bin/helper/osh-accountModifyPersonalAccess b/bin/helper/osh-accountModifyPersonalAccess index 8492c6e..da6faa7 100755 --- a/bin/helper/osh-accountModifyPersonalAccess +++ b/bin/helper/osh-accountModifyPersonalAccess @@ -91,6 +91,10 @@ if (not grep { $action eq $_ } qw{ add del }) { #CODE +my $machine = $ip; +$port and $machine .= ":$port"; +$user and $machine = $user . '@' . $machine; + # access_modify validates all its parameters, don't do it ourselves here for clarity $fnret = OVH::Bastion::access_modify( way => 'personal', @@ -103,4 +107,18 @@ $fnret = OVH::Bastion::access_modify( forceKey => $forceKey, comment => $comment, ); +if ($fnret->err eq 'OK') { + my $ttlmsg = $ttl ? ' (expires in ' . OVH::Bastion::duration2human(seconds => $ttl)->value->{'human'} . ')' : ''; + HEXIT( + 'OK', + value => {action => $action, account => $account, ip => $ip, user => $user, port => $port, ttl => $ttl, comment => $comment}, + msg => $action eq 'add' ? "Access to $machine was added to account $account$ttlmsg" : "Access to $machine was removed from account $account$ttlmsg" + ); +} +elsif ($fnret->err eq 'OK_NO_CHANGE') { + HEXIT('OK_NO_CHANGE', + msg => $action eq 'add' + ? "Access to $machine was already granted to account $account, nothing done" + : "Access to $machine was not granted to account $account, nothing done"); +} HEXIT($fnret); diff --git a/bin/plugin/group-gatekeeper/groupAddGuestAccess b/bin/plugin/group-gatekeeper/groupAddGuestAccess index 4a10340..483f6d7 100755 --- a/bin/plugin/group-gatekeeper/groupAddGuestAccess +++ b/bin/plugin/group-gatekeeper/groupAddGuestAccess @@ -20,6 +20,7 @@ my $remainingOptions = OVH::Bastion::Plugin::begin( "scpup" => \my $scpUp, "scpdown" => \my $scpDown, "ttl=s" => \my $ttl, + "comment=s" => \my $comment, }, helptext => <<'EOF', Add a specific group server access to an account @@ -27,7 +28,7 @@ Add a specific group server access to an account Usage: --osh SCRIPT_NAME --group GROUP --account ACCOUNT [OPTIONS] --group GROUP group to add guest access to - --account ACCOUNT name of the other bastion account to add access to, he'll be given access to the GROUP key + --account ACCOUNT name of the other bastion account to add access to, they'll be given access to the GROUP key --host HOST|IP add access to this HOST (which must belong to the GROUP) --user USER allow connecting to HOST only with remote login USER --user-any allow connecting to HOST with any remote login @@ -35,7 +36,9 @@ Usage: --osh SCRIPT_NAME --group GROUP --account ACCOUNT [OPTIONS] --port-any allow connecting to HOST with any remote port --scpup allow SCP upload, you--bastion-->server (omit --user in this case) --scpdown allow SCP download, you<--bastion--server (omit --user in this case) - --ttl SECONDS|DURATION Specify a number of seconds after which the access will automatically expire + --ttl SECONDS|DURATION specify a number of seconds after which the access will automatically expire + --comment '"ANY TEXT'" add a comment alongside this access. + If omitted, we'll use the closest preexisting group access' comment as seen in groupListServers This command adds, to an existing bastion account, access to the egress keys of a group, but only to accessing one or several given servers, instead of all the servers of this group. @@ -83,6 +86,7 @@ my $fnret = OVH::Bastion::Plugin::groupSetRole::act( portAny => $portAny, host => ($ip || $host), ttl => $ttl, + comment => $comment, sudo => 0, silentoverride => 0, self => $self, diff --git a/lib/perl/OVH/Bastion/Plugin/groupSetRole.pm b/lib/perl/OVH/Bastion/Plugin/groupSetRole.pm index 3181186..6d2b384 100644 --- a/lib/perl/OVH/Bastion/Plugin/groupSetRole.pm +++ b/lib/perl/OVH/Bastion/Plugin/groupSetRole.pm @@ -33,7 +33,7 @@ sub preconditions { if ($type eq 'guest' && !$sudo) { # guest access need (user||user-any), host and (port||port-any) - # in sudo mode, these are not used, because the helper doesn't handle the guest access add by itself, the do() func of this package does + # in sudo mode, these are not used, because the helper doesn't handle the guest access add by itself, the act() func of this package does if (!($user xor $userAny)) { return R('ERR_MISSING_PARAMETER', msg => "Require exactly one argument of user or user-any"); } @@ -144,8 +144,8 @@ sub act { # get returned untainted value my %values = %{$fnret->value()}; - my ($group, $shortGroup, $account, $type, $realm, $remoteaccount, $sysaccount) = @values{qw{ group shortGroup account type realm remoteaccount sysaccount }}; - my ($action, $self, $user, $host, $port, $ttl) = @params{qw{ action self user host port ttl }}; + my ($group, $shortGroup, $account, $type, $realm, $remoteaccount, $sysaccount) = @values{qw{ group shortGroup account type realm remoteaccount sysaccount }}; + my ($action, $self, $user, $host, $port, $ttl, $comment) = @params{qw{ action self user host port ttl comment }}; undef $user if $params{'userAny'}; undef $port if $params{'portAny'}; @@ -168,7 +168,7 @@ sub act { if ($action eq 'add' && OVH::Bastion::is_group_guest(group => $shortGroup, account => $account, sudo => $params{'sudo'})) { - # if the user is a guest, must remove all his guest accesses first + # if the user is a guest, must remove all their guest accesses first $fnret = OVH::Bastion::get_acl_way(way => 'groupguest', group => $shortGroup, account => $account); if ($fnret && $fnret->value && @{$fnret->value}) { osh_warn("This account was previously a guest of this group, with the following accesses:"); @@ -242,6 +242,9 @@ sub act { msg => "The group $shortGroup doesn't have access to $machine, so you can't add a guest group access " . "to it (first add it to the group if applicable, with groupAddServer)"); } + + # if no comment was specified for this guest access, reuse the one from the matching group ACL entry + $comment ||= $fnret->value->{'comment'}; } # If the account is already a member, can't add/del them as guest @@ -259,6 +262,7 @@ sub act { push @command, '--user', $user if $user; push @command, '--port', $port if $port; push @command, '--ttl', $ttl if $ttl; + push @command, '--comment', $comment if $comment; $fnret = OVH::Bastion::helper(cmd => \@command); $fnret or return $fnret; @@ -350,6 +354,7 @@ sub act { ['host', $host], ['port', $port], ['ttl', $ttl], + ['comment', $comment || ''], ] ); } diff --git a/lib/perl/OVH/Bastion/allowdeny.inc b/lib/perl/OVH/Bastion/allowdeny.inc index b57a9fe..5f13143 100644 --- a/lib/perl/OVH/Bastion/allowdeny.inc +++ b/lib/perl/OVH/Bastion/allowdeny.inc @@ -98,7 +98,7 @@ sub is_access_way_granted { "checking way $way/$account/$group with ignorePort=$ignorePort ignoreUser=$ignoreUser exactIpMatch=$exactIpMatch exactPortMatch=$exactPortMatch exactUserMatch=$exactUserMatch" ); - my ($bestMatch, $bestMatchSize, $forceKey); + my ($bestMatch, $bestMatchSize, $bestMatchComment, $forceKey); foreach my $entry (@acl) { my $allowedIp = $entry->{'ip'}; # can be a prefix my $allowedUser = $entry->{'user'}; # can be undef (if any-user) @@ -200,10 +200,11 @@ sub is_access_way_granted { next if ($allowedIp ne $wantedIp); # here, we got a perfect match - $forceKey = $localForceKey; - $bestMatch = $allowedIp; - $bestMatchSize = undef; # not needed - last; # perfect match, don't search further + $forceKey = $localForceKey; + $bestMatch = $allowedIp; + $bestMatchComment = $entry->{'userComment'}; + $bestMatchSize = undef; # not needed + last; # perfect match, don't search further } # check IP in not-exactIpMatch case. if it contains / then it's a prefix @@ -215,9 +216,10 @@ sub is_access_way_granted { if ($ipCheck && $ipCheck->match($wantedIp)) { osh_debug("... we got a slash match !"); if (not defined $bestMatchSize or $ipCheck->size() < $bestMatchSize) { - $forceKey = $localForceKey; - $bestMatch = $allowedIp; - $bestMatchSize = $ipCheck->size(); + $forceKey = $localForceKey; + $bestMatch = $allowedIp; + $bestMatchComment = $entry->{'userComment'}; + $bestMatchSize = $ipCheck->size(); $bestMatchSize == 1 and last; # we won't get better than this } } @@ -226,16 +228,17 @@ sub is_access_way_granted { # it's a single ip, so a stupid strcmp() does the trick if ($allowedIp eq $wantedIp) { osh_debug("... we got a singleip match !"); - $forceKey = $localForceKey; - $bestMatch = $allowedIp; - $bestMatchSize = 1; + $forceKey = $localForceKey; + $bestMatch = $allowedIp; + $bestMatchComment = $entry->{'userComment'}; + $bestMatchSize = 1; last; } } } if (defined $bestMatch) { - return R('OK', value => {match => $bestMatch, size => $bestMatchSize, forceKey => $forceKey}); + return R('OK', value => {match => $bestMatch, size => $bestMatchSize, forceKey => $forceKey, comment => $bestMatchComment}); } return R('KO_ACCESS_DENIED'); } diff --git a/lib/perl/OVH/Bastion/allowkeeper.inc b/lib/perl/OVH/Bastion/allowkeeper.inc index 58205bf..d98fade 100644 --- a/lib/perl/OVH/Bastion/allowkeeper.inc +++ b/lib/perl/OVH/Bastion/allowkeeper.inc @@ -255,14 +255,14 @@ sub access_modify { my $ip = $params{'ip'}; # can be a single ip or prefix my $port = $params{'port'}; # if undef, means a port-wildcard access - my $ttl = $params{'ttl'}; + my $ttl = $params{'ttl'}; + my $comment = $params{'comment'}; my $way = $params{'way'}; # group, groupguest, personal my $group = $params{'group'}; # only for way=group or way=groupguest my $account = $params{'account'}; # only for way=personal my $forceKey = $params{'forceKey'}; - my $comment = $params{'comment'}; my $fnret; @@ -354,7 +354,7 @@ sub access_modify { } } - # check if the caller has the right to make the change he's asking + # check if the caller has the right to make the change they're asking # ... 1. either $> is allowkeeper and $ENV{'SUDO_USER'} is the requesting account # ... 2. or $> is $grouptomodify and $ENV{'SUDO_USER'} is the requesting account @@ -406,8 +406,6 @@ sub access_modify { ); osh_debug("... result is $fnret"); - #return $fnret if $fnret->is_err; - if ($action eq 'add' and $fnret) { return R('OK_NO_CHANGE', msg => "The requested access to add was already granted"); } @@ -415,8 +413,6 @@ sub access_modify { return R('OK_NO_CHANGE', msg => "The requested access to delete was not found, no change made"); } - # TODO for groupguest case, also check that the group has the right - # ok, now do the change, first define this sub my $_access_modify_file = sub { diff --git a/tests/functional/tests.d/340-selfaccesses.sh b/tests/functional/tests.d/340-selfaccesses.sh index df820a0..c0bea9c 100644 --- a/tests/functional/tests.d/340-selfaccesses.sh +++ b/tests/functional/tests.d/340-selfaccesses.sh @@ -115,16 +115,14 @@ testsuite_selfaccesses() success selfAddPersonalAccess mustwork $a0 -osh selfAddPersonalAccess -h 127.0.0.2 -u $shellaccount -p 22 --kbd-interactive nocontain "already" - contain "Access to $shellaccount@127.0.0.2:22 successfully added" - json .command selfAddPersonalAccess .error_code OK .value null + json .command selfAddPersonalAccess .error_code OK .value.ip 127.0.0.2 .value.user $shellaccount .value.port 22 success selfAddPersonalAccess dupe $a0 -osh selfAddPersonalAccess -h 127.0.0.2 -u $shellaccount -p 22 --kbd-interactive contain "already" json .command selfAddPersonalAccess .error_code OK_NO_CHANGE .value null success selfAddPersonalAccess withttl $a0 -osh selfAddPersonalAccess -h 127.0.0.4 -u $shellaccount -p 22 --force --ttl 0d0h0m3s - contain "Access to $shellaccount@127.0.0.4:22 successfully added (expires in 00:00:0" - json .command selfAddPersonalAccess .error_code OK .value null + json .command selfAddPersonalAccess .error_code OK .value.ip 127.0.0.4 .value.user $shellaccount .value.port 22 .value.ttl 3 run ssh a1atlo2_login8 $a0 127.0.0.2 -- id retvalshouldbe 107 @@ -360,8 +358,8 @@ testsuite_selfaccesses() #sudo usermod -a -G osh-selfDelPersonalAccess $account1 success selfDelPersonalAccess mustwork $a0 -osh selfDelPersonalAccess -h 127.0.0.2 -u $shellaccount -p 22 - contain "Access to $shellaccount@127.0.0.2:22 successfully removed" - json .command selfDelPersonalAccess .error_code OK .value null + contain "Access to $shellaccount@127.0.0.2:22" + json .command selfDelPersonalAccess .error_code OK .value.ip 127.0.0.2 .value.user $shellaccount .value.port 22 run ssh shellaccountatlo2_mustfail $a1 $shellaccount@127.0.0.2 -- echo $randomstr retvalshouldbe 107 @@ -371,8 +369,7 @@ testsuite_selfaccesses() success selfAddPersonalAccess mustwork $a0 -osh selfAddPersonalAccess -h 127.0.0.2 -u $shellaccount -p 226 nocontain "already" - contain "Access to $shellaccount@127.0.0.2:226 successfully added" - json .command selfAddPersonalAccess .error_code OK .value null + json .command selfAddPersonalAccess .error_code OK .value.ip 127.0.0.2 .value.user $shellaccount .value.port 226 # shouldn't work @@ -391,8 +388,8 @@ testsuite_selfaccesses() contain "$randomstr" success selfDelPersonalAccess mustwork $a0 -osh selfDelPersonalAccess -h 127.0.0.2 -u $shellaccount -p 226 - contain "Access to $shellaccount@127.0.0.2:226 successfully removed" - json .command selfDelPersonalAccess .error_code OK .value null + contain "Access to $shellaccount@127.0.0.2:226" + json .command selfDelPersonalAccess .error_code OK .value.ip 127.0.0.2 .value.user $shellaccount .value.port 226 run ssh shellaccountatlo2_mustfailnow $a0 $shellaccount@127.0.0.2 -p 226 -- echo $randomstr retvalshouldbe 107 @@ -408,8 +405,7 @@ testsuite_selfaccesses() success selfAddPersonalAccess nousernoport $a0 -osh selfAddPersonalAccess -h 127.0.0.4 --force nocontain "already" contain "Forcing add as asked" - contain "Access to 127.0.0.4 successfully added" - json .command selfAddPersonalAccess .error_code OK .value null + json .command selfAddPersonalAccess .error_code OK .value.ip 127.0.0.4 .value.port null .value.user null run ssh rootport22 $a0 root@127.0.0.4 -- echo $randomstr retvalshouldbe 255 @@ -441,8 +437,8 @@ testsuite_selfaccesses() nocontain "$randomstr" success selfDelPersonalAccess nousernoport $a0 -osh selfDelPersonalAccess -h 127.0.0.4 - contain "Access to 127.0.0.4 successfully removed" - json .command selfDelPersonalAccess .error_code OK .value null + contain "Access to 127.0.0.4 " + json .command selfDelPersonalAccess .error_code OK .value.ip 127.0.0.4 .value.port null .value.user null success selfDelPersonalAccess nousernoport_dupe $a0 -osh selfDelPersonalAccess -h 127.0.0.4 nocontain "no longer has a personal access" diff --git a/tests/functional/tests.d/350-groups.sh b/tests/functional/tests.d/350-groups.sh index 8035166..5dcaaf9 100644 --- a/tests/functional/tests.d/350-groups.sh +++ b/tests/functional/tests.d/350-groups.sh @@ -663,14 +663,14 @@ EOS 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 + run accountAddPersonalAccess a0_add_personal_access_to_a3_works_slash_1 $a0 --osh accountAddPersonalAccess --account $account3 --host 77.66.55.0/24 + json .command accountAddPersonalAccess .error_code OK .value.ip 77.66.55.0/24 .value.port null .value.user null run accountAddPersonalAccess a0_add_personal_access_to_a3_fail_badslash $a0 --osh accountAddPersonalAccess --account $account3 --host 77.66.55.0/23 json .command null .error_code KO_INVALID_IP .value null - run accountAddPersonalAccess a0_add_personal_access_to_a3_works_slash $a0 --osh accountAddPersonalAccess --account $account3 --host 1.2.3.4/32 - json .command accountAddPersonalAccess .error_code OK .value null + run accountAddPersonalAccess a0_add_personal_access_to_a3_works_slash_2 $a0 --osh accountAddPersonalAccess --account $account3 --host 1.2.3.4/32 + json .command accountAddPersonalAccess .error_code OK .value.ip 1.2.3.4 .value.port null .value.user null success accountAddPersonalAccess a0_add_personal_access_to_a3_works $a0 --osh accountAddPersonalAccess --account $account3 --host 77.66.55.4 @@ -889,7 +889,7 @@ EOS # group1: a1(owner,aclkeeper,gatekeeper,member) a2() servers() success groupAddServer firstadd_ok $a1 --osh groupAddServer --group $group1 --host 127.0.0.10 --port-any --user-any --force contain "was added to group" - json .command groupAddServer .error_code OK .value.group $group1 .value.host 127.0.0.10 .value.port null .value.user null + json .command groupAddServer .error_code OK .value.group $group1 .value.ip 127.0.0.10 .value.port null .value.user null # group1: a1(owner,aclkeeper,gatekeeper,member) a2() servers(127.0.0.10) success groupAddServer firstadd_dup $a1 --osh groupAddServer --group $group1 --host 127.0.0.10 --port-any --user-any --force @@ -898,13 +898,13 @@ EOS # group1: a1(owner,aclkeeper,gatekeeper,member) a2() servers(127.0.0.10) success groupAddServer secondadd $a1 --osh groupAddServer --group $group1 --host 127.0.0.11 --port-any --user-any --force contain "was added to group" - json .command groupAddServer .error_code OK .value.group $group1 .value.host 127.0.0.11 .value.port null .value.user null + json .command groupAddServer .error_code OK .value.group $group1 .value.ip 127.0.0.11 .value.port null .value.user null # group1: a1(owner,aclkeeper,gatekeeper,member) a2() servers(127.0.0.10,127.0.0.11) success groupAddServer thirdaddttl $a1 --osh groupAddServer --group $group1 --host 127.0.0.12 --port-any --user-any --force --ttl 0w19s0d contain "was added to group" contain "expires in 00:00:" - json .command groupAddServer .error_code OK .value.group $group1 .value.host 127.0.0.12 .value.port null .value.user null .value.ttl 19 + json .command groupAddServer .error_code OK .value.group $group1 .value.ip 127.0.0.12 .value.port null .value.user null .value.ttl 19 # group1: a1(owner,aclkeeper,gatekeeper,member) a2() servers(127.0.0.10,127.0.0.11,127.0.0.12-TTL) success groupListServers list $a1 --osh groupListServers --group $group1 @@ -1011,13 +1011,13 @@ EOS # group1: a1(owner,aclkeeper,gatekeeper,member) a2(guest(127.0.0.10)) servers(127.0.0.10,127.0.0.11) success accountAddPersonalAccess own11 $a0 --osh accountAddPersonalAccess --account $account2 --host 127.0.0.11 --user $account2 --port 22 contain "adding the access blindly" - json .command accountAddPersonalAccess .error_code OK .value null + json .command accountAddPersonalAccess .error_code OK .value.account $account2 .value.ip 127.0.0.11 .value.user $account2 .value.port 22 # just try the ttl param - success accountAddPersonalAccess own11 $a0 --osh accountAddPersonalAccess --account $account2 --host 127.7.0.11 --user $account2 --port 22 --ttl 3 + success accountAddPersonalAccess own11_ttl $a0 --osh accountAddPersonalAccess --account $account2 --host 127.7.0.11 --user $account2 --port 22 --ttl 3 contain "adding the access blindly" contain "expires in 00:00:0" - json .command accountAddPersonalAccess .error_code OK .value null + json .command accountAddPersonalAccess .error_code OK .value.account $account2 .value.ip 127.7.0.11 .value.user $account2 .value.port 22 .value.ttl 3 # group1: a1(owner,aclkeeper,gatekeeper,member) a2(guest(127.0.0.10)) servers(127.0.0.10,127.0.0.11) # account1: perso(account1@127.0.0.11:22)