From 262e545bbbc2942cdd119b7dab7933cf4fc3aa9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lesimple?= Date: Wed, 31 May 2023 12:55:19 +0000 Subject: [PATCH] feat: add dryrun in access_modify() and widest prefix precondition check --- lib/perl/OVH/Bastion.pm | 7 ++-- lib/perl/OVH/Bastion/allowkeeper.inc | 54 ++++++++++++++++++++++++---- 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/lib/perl/OVH/Bastion.pm b/lib/perl/OVH/Bastion.pm index e354128..6395b36 100644 --- a/lib/perl/OVH/Bastion.pm +++ b/lib/perl/OVH/Bastion.pm @@ -632,7 +632,6 @@ sub is_valid_ip { my $fast = $params{'fast'}; # fast mode: avoid instantiating Net::IP... except if ipv6 if ($fast and $ip !~ m{:}) { - # fast asked and it's not an IPv6, regex ftw ## no critic (ProhibitUnusedCapture) if ( @@ -663,11 +662,13 @@ sub is_valid_ip { return R('KO_INVALID_IP', msg => "Invalid IP address ($ip)"); } if (defined $+{'slash'} and not defined $+{'prefix'}) { - # got a / in $ip but it's not followed by \d+ return R('KO_INVALID_IP', msg => "Invalid IP address ($ip)"); } - return R('OK', value => {ip => $ip}) if (defined $+{'prefix'} && $+{'prefix'} != 32); + + if (defined $+{'prefix'} && $+{'prefix'} != 32) { + return R('OK', value => {ip => $ip, prefix => $+{'prefix'}}); + } return R('OK', value => {ip => $+{'shortip'}}); } return R('KO_INVALID_IP', msg => "Invalid IP address ($ip)"); diff --git a/lib/perl/OVH/Bastion/allowkeeper.inc b/lib/perl/OVH/Bastion/allowkeeper.inc index 8447bdd..0f1c487 100644 --- a/lib/perl/OVH/Bastion/allowkeeper.inc +++ b/lib/perl/OVH/Bastion/allowkeeper.inc @@ -315,6 +315,15 @@ sub access_modify { my $forceKey = $params{'forceKey'}; my $forcePassword = $params{'forcePassword'}; + my $dryrun = $params{'dryrun'}; # don't do anything, just check params and prereqs + my $sudo = $params{'sudo'}; # passed as-is to subs we use + + # deny accesses wider than these prefixes + my %widestVxPrefix = ( + 4 => $params{'widestV4Prefix'}, + 6 => $params{'widestV6Prefix'}, + ); + my $fnret; foreach my $mandatoryParam (qw/action ip way/) { @@ -323,6 +332,9 @@ sub access_modify { } } + # if undef, default to sudo==1 + $sudo //= 1; + # due to how plugins work, sometimes user and port are just '', make them undef in those cases undef $user if (defined $user && $user eq ''); undef $port if (defined $port && $port eq ''); @@ -355,6 +367,20 @@ sub access_modify { return $fnret unless $fnret; $ip = $fnret->value->{'ip'}; + if ($fnret->value->{'type'} eq 'prefix') { + my $ipVersion = $fnret->value->{'version'}; + if (defined $widestVxPrefix{$ipVersion} && $fnret->value->{'prefixlen'} < $widestVxPrefix{$ipVersion}) { + return R( + 'ERR_INVALID_PARAMETER', + msg => sprintf( + "Specified prefix (/%d) is too wide, maximum allowed for IPv%d is /%d by this bastion policy", + $fnret->value->{'prefixlen'}, + $ipVersion, $widestVxPrefix{$ipVersion} + ), + ); + } + } + # check port if (defined $port) { $fnret = OVH::Bastion::is_valid_port(port => $port); @@ -418,7 +444,13 @@ sub access_modify { # ... 2. or $> is $grouptomodify and $ENV{'SUDO_USER'} is the requesting account my ($running_as) = (getpwuid($>))[0] =~ /([0-9a-zA-Z_.-]+)/; - my ($requester) = $ENV{'SUDO_USER'} =~ /([0-9a-zA-Z_.-]+)/; + my $requester; + if ($sudo) { + ($requester) = $ENV{'SUDO_USER'} =~ /([0-9a-zA-Z_.-]+)/; + } + else { + $requester = $running_as; + } # requester can never be a realm_* account, because it's shared and should not be able to add access to anything return R('ERR_SECURITY_VIOLATION', msg => "Requester can't be a realm user") if $requester =~ /^realm_/; @@ -432,7 +464,7 @@ sub access_modify { OVH::Bastion::is_user_in_group( user => $requester, group => 'osh-self' . ucfirst($action) . 'PersonalAccess', - sudo => 1 + sudo => $sudo, ); } @@ -441,25 +473,30 @@ sub access_modify { OVH::Bastion::is_user_in_group( user => $requester, group => 'osh-account' . ucfirst($action) . 'PersonalAccess', - sudo => 1 + sudo => $sudo ); } elsif ($way eq 'group') { $expected_running_as = $group; push @one_should_succeed, - OVH::Bastion::is_group_aclkeeper(account => $requester, group => $shortGroup, superowner => 1, sudo => 1); + OVH::Bastion::is_group_aclkeeper(account => $requester, group => $shortGroup, superowner => 1, sudo => $sudo); } elsif ($way eq 'groupguest') { push @one_should_succeed, - OVH::Bastion::is_group_gatekeeper(account => $requester, group => $shortGroup, superowner => 1, sudo => 1); + OVH::Bastion::is_group_gatekeeper( + account => $requester, + group => $shortGroup, + superowner => 1, + sudo => $sudo + ); } - if ($running_as ne $expected_running_as) { + if ($running_as ne $expected_running_as && !$dryrun) { warn_syslog("Security violation: current running user ($running_as) unexpected (wanted $expected_running_as)"); return R('ERR_SECURITY_VIOLATION', msg => "Current running user unexpected"); } - if (grep({ $_ } @one_should_succeed) == 0 && $requester ne 'root') { + if (grep({ $_ } @one_should_succeed) == 0 && $requester ne 'root' && !$dryrun) { warn_syslog( "Security violation: requesting user '$requester' doesn't have the right to do that (way=$way, group=" . ($shortGroup ? '' : $shortGroup) @@ -467,6 +504,9 @@ sub access_modify { return R('ERR_SECURITY_VIOLATION', msg => "You're not allowed to do that"); } + # end of dryrun + return R('OK', msg => "Would have added the access but we've been called with dryrun") if $dryrun; + # now, check if the access we're being asked to change is already in place or not osh_debug( "for action $action of $user\@$ip:$port of way $way with account=$account and group=$group, checking if already granted"