From f3a7353f9f06dcc576eb2a58dc59c0486b62163a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lesimple?= Date: Tue, 30 Apr 2024 09:47:14 +0000 Subject: [PATCH] enh: expiration: reorder code and remove legacy checks The accountCreate.comment file no longer exists, and migration has been handled by the install script since several years, so get rid of the code that still reference it. Defaulting on the ttyrec folder date to guess last login time is not needed, checking the lastlog file is enough. Add a syslog warning for accounts where we can't determine the last login date. Still allow them to login if no expiration policy is set, deny them otherwise. --- bin/helper/osh-accountUnexpire | 33 +++--- bin/plugin/restricted/accountUnexpire | 29 ++++- bin/shell/osh.pl | 20 ++-- lib/perl/OVH/Bastion.pm | 164 ++++++++++++++++++-------- 4 files changed, 164 insertions(+), 82 deletions(-) diff --git a/bin/helper/osh-accountUnexpire b/bin/helper/osh-accountUnexpire index cf475e7cb..d1bf78055 100755 --- a/bin/helper/osh-accountUnexpire +++ b/bin/helper/osh-accountUnexpire @@ -49,11 +49,6 @@ else { } } -# for this special helper, $account must be equal to $ENV{'USER'} -if (OVH::Bastion::get_user_from_env()->value ne $account) { - HEXIT('ERR_SECURITY_VIOLATION', msg => "You're not allowed to run this on $account, dear $self"); -} - #PARAMS:ACCOUNT @@ -61,23 +56,33 @@ osh_debug("Checking account"); $fnret = OVH::Bastion::is_bastion_account_valid_and_existing(account => $account); $fnret or HEXIT($fnret); $account = $fnret->value->{'account'}; # untainted +my $sysaccount = $fnret->value->{'sysaccount'}; +my $remoteaccount = $fnret->value->{'remoteaccount'}; +my $accounthome = $fnret->value->{'dir'}; #value->{'dir'}; +# for this special helper, $sysaccount must be equal to $ENV{'USER'} +if (OVH::Bastion::get_user_from_env()->value ne $sysaccount) { + HEXIT('ERR_SECURITY_VIOLATION', msg => "You're not allowed to run this on $account, dear $self"); +} + if (!-d $accounthome) { HEXIT('ERR_INVALID_HOME', msg => "Invalid HOME directory for this account"); } -$fnret = OVH::Bastion::is_account_nonexpired(sysaccount => $account); -$fnret->is_err and HEXIT($fnret); # couldn't read file or other error -$fnret->is_ok and HEXIT($fnret); # wasn't expired +$fnret = OVH::Bastion::is_account_nonexpired( + sysaccount => $sysaccount, + remoteaccount => $remoteaccount, + update_lastlog => 1, + no_create_remote => 1, +); +use Data::Dumper; +print Dumper($fnret); +$fnret->is_err and HEXIT($fnret); # couldn't read file or other error +$fnret->is_ok and HEXIT($fnret); # wasn't expired # is_ko: is expired -my $days = $fnret->value->{'days'}; -my $filepath = $fnret->value->{'filepath'}; - -$fnret = OVH::Bastion::touch_file($filepath); -$fnret or HEXIT($fnret); +my $days = $fnret->value->{'days'}; HEXIT('OK', value => {account => $account, days => $days}); diff --git a/bin/plugin/restricted/accountUnexpire b/bin/plugin/restricted/accountUnexpire index 4d9b1622e..abdccec2b 100755 --- a/bin/plugin/restricted/accountUnexpire +++ b/bin/plugin/restricted/accountUnexpire @@ -41,9 +41,10 @@ if (not $account) { $fnret = OVH::Bastion::is_bastion_account_valid_and_existing(account => $account); $fnret or osh_exit $fnret; $account = $fnret->value->{'account'}; +my $sysaccount = $fnret->value->{'sysaccount'}; my @command = qw{ sudo -n -u }; -push @command, ($account, '--'); +push @command, ($sysaccount, '--'); push @command, qw{ /usr/bin/env perl -T }; push @command, $OVH::Bastion::BASEPATH . '/bin/helper/osh-accountUnexpire'; push @command, ('--account', $account); @@ -51,12 +52,32 @@ push @command, ('--account', $account); $fnret = OVH::Bastion::helper(cmd => \@command); $fnret or osh_exit $fnret; +my $retvalue = {account => $account}; +$retvalue->{'days'} = $fnret->value->{'days'} if ($fnret->value && defined $fnret->value->{'days'}); + if ($fnret->err eq 'OK') { - my $days = $fnret->value->{'days'}; osh_ok R( 'OK', - value => {account => $account, days => $days}, - msg => "Account $account was expired ($days days without connection), it is now active again." + value => $retvalue, + msg => "Account $account was expired (" + . $retvalue->{'days'} + . ") days without connection), it is now active again." + ); +} +elsif ($fnret->err eq 'OK_EXPIRATION_NOT_CONFIGURED') { + osh_ok R( + $fnret->err, + value => $fnret->value, + msg => + "Expiration is not configured on this bastion, but account ${account}'s expiration has been refreshed nevertheless." + ); +} +elsif ($fnret->err eq 'OK_NOT_EXPIRED') { + osh_ok R( + $fnret->err, + value => $fnret->value, + msg => + "Expiration is not configured on this bastion, but account ${account}'s expiration has been refreshed nevertheless." ); } elsif ($fnret->is_ok) { diff --git a/bin/shell/osh.pl b/bin/shell/osh.pl index 73dd07f66..e38ad4fd8 100755 --- a/bin/shell/osh.pl +++ b/bin/shell/osh.pl @@ -199,18 +199,21 @@ sub main_exit { # # Second check : has account logged-in recently enough to be allowed ? # -$fnret = OVH::Bastion::is_account_nonexpired(sysaccount => $sysself, remoteaccount => $remoteself); +$fnret = OVH::Bastion::is_account_nonexpired( + sysaccount => $sysself, + remoteaccount => $remoteself, + update_lastlog => 1, + ipfrom => $ipfrom, + hostfrom => $hostfrom +); if ($fnret->is_err) { - # internal error, warn and pass osh_warn($fnret); } elsif ($fnret->is_ko) { - # expired main_exit OVH::Bastion::EXIT_ACCOUNT_EXPIRED, 'account_expired', $fnret->msg; } -my $lastlog_filepath = $fnret->value->{'filepath'}; my $lastlogmsg = sprintf("Welcome to $bastionName, $self, this is your first connection"); if ($fnret && $fnret->value && $fnret->value->{'seconds'}) { @@ -223,15 +226,6 @@ sub main_exit { ); } -# ok not expired, so we update lastlog -if ($lastlog_filepath && open(my $lastlogfh, '>', $lastlog_filepath)) { - print $lastlogfh sprintf("%s(%s)", $ipfrom, $hostfrom); - close($lastlogfh); -} -else { - osh_warn "Couldn't update your lastlog ($lastlog_filepath: $!), contact a bastion admin"; -} - # # Fetch command options # diff --git a/lib/perl/OVH/Bastion.pm b/lib/perl/OVH/Bastion.pm index aedbb573e..6bfaad6e0 100644 --- a/lib/perl/OVH/Bastion.pm +++ b/lib/perl/OVH/Bastion.pm @@ -210,9 +210,13 @@ sub is_account_nonfrozen { # checks whether an account is expired (inactivity) if that's configured on this bastion sub is_account_nonexpired { - my %params = @_; - my $sysaccount = $params{'sysaccount'}; - my $remoteaccount = $params{'remoteaccount'}; + my %params = @_; + my $sysaccount = $params{'sysaccount'}; + my $remoteaccount = $params{'remoteaccount'}; + my $updateLastlog = $params{'update_lastlog'}; # update lastlog and lastlog_$remote + my $noCreateRemote = $params{'no_create_remote'}; # don't create lastlog_$remote if doesn't exist already + my $ipfrom = $params{'ipfrom'}; # only used for update_lastlog + my $hostfrom = $params{'hostfrom'}; # only used for update_lastlog if (not $sysaccount) { return R('ERR_MISSING_PARAMETER', msg => "Missing 'sysaccount' argument"); @@ -227,18 +231,41 @@ sub is_account_nonexpired { # some accounts might have a specific configuration overriding the global one $fnret = OVH::Bastion::account_config(account => $sysaccount, %{OVH::Bastion::OPT_ACCOUNT_MAX_INACTIVE_DAYS()}); - if ($fnret) { - $accountMaxInactiveDays = $fnret->value; + $accountMaxInactiveDays = $fnret->value if $fnret; + + my @lastlogFilesToRead; + my @lastlogFilesToUpdate; + if ($remoteaccount) { + # if we have a remoteaccount, this file takes precedence, however if it doesn't + # exist, we'll fallback to the lastlog file of the shared realm account itself + my $lastlogRemote = "/home/$sysaccount/lastlog_$remoteaccount"; + push @lastlogFilesToRead, $lastlogRemote; + + # if noCreateRemote, don't add it to @lastlogFilesToUpdate if it doesn't exist. + # don't use test -f to check that, in case we have -r+x on the folder: + # directly try to open it instead + if ($noCreateRemote) { + if (open(my $fh, "<", $lastlogRemote)) { + push @lastlogFilesToUpdate, $lastlogRemote; + close($fh); + } + } + else { + push @lastlogFilesToUpdate, $lastlogRemote; + } } + # the main lastlogfile of the sysaccount must be read and updated at all times + push @lastlogFilesToRead, "/home/$sysaccount/lastlog"; + push @lastlogFilesToUpdate, "/home/$sysaccount/lastlog"; - my $isFirstLogin; my $lastlog; - my $filepath = "/home/$sysaccount/lastlog" . ($remoteaccount ? "_$remoteaccount" : ""); - my $value = {filepath => $filepath}; - if (-e $filepath) { - $isFirstLogin = 0; - $lastlog = (stat(_))[9]; - osh_debug("is_account_nonexpired: got lastlog date: $lastlog"); + my $value = {}; + # use the first lastlog file that exists in the list + foreach my $filepath (@lastlogFilesToRead) { + next if !-e $filepath; + + $lastlog = (stat(_))[9]; + osh_warn("is_account_nonexpired: got lastlog date: $lastlog from file $filepath"); # if lastlog file is available, fetch some info from it if (open(my $lastloginfh, "<", $filepath)) { @@ -247,13 +274,23 @@ sub is_account_nonexpired { close($lastloginfh); $value->{'info'} = $info; } + + # no need to continue once we have the proper info + last; } - else { + + my $isFirstLogin = 0; + if (not defined $lastlog) { + # the lastlog file doesn't exist, or maybe we just don't have access to it. + # by trying to chdir() to the folder, we're confirming we do have -x on it, + # otherwise that would explain why we can't open lastlog or even know whether it exists: my ($previousDir) = getcwd() =~ m{^(/[a-z0-9_./-]+)}i; if (!chdir("/home/$sysaccount")) { osh_debug("is_account_nonexpired: no exec access to the folder!"); return R('ERR_NO_ACCESS', msg => "No read access to this account folder to compute last login time"); } + # access rights to the home of the account confirmed, so lastlog doesn't exist, + # so this is the first login chdir($previousDir); $isFirstLogin = 1; @@ -263,51 +300,70 @@ sub is_account_nonexpired { $lastlog = $fnret->value; osh_debug("is_account_nonexpired: got creation date from config.creation_timestamp: $lastlog"); } - elsif (-e "/home/$sysaccount/accountCreate.comment") { - - # fall back to the stat of the accountCreate.comment file - $lastlog = (stat(_))[9]; - osh_debug("is_account_nonexpired: got creation date from accountCreate.comment stat: $lastlog"); - } - else { - # last fall back to the stat of the ttyrec/ folder - $lastlog = (stat("/home/$sysaccount/ttyrec"))[9]; - osh_debug("is_account_nonexpired: got creation date from ttyrec/ stat: $lastlog"); - } } - my $seconds = time() - $lastlog; - my $days = int($seconds / 86400); - $value->{'days'} = $days; - $value->{'seconds'} = $seconds; $value->{'already_seen_before'} = !$isFirstLogin; - osh_debug("Last account activity: $days days ago"); + if (defined $lastlog) { + $value->{'seconds'} = time() - $lastlog; + $value->{'days'} = int($value->{'seconds'} / 86400); + osh_debug("Last account activity: " . $value->{'days'} . " days ago"); + } + else { + # can't tell the last log nor the creation date + syslog_warn("Couldn't tell the last login date of $sysaccount" . ($remoteaccount ? "/$remoteaccount" : "")); + } - if ($accountMaxInactiveDays == 0) { + if (not defined $lastlog) { + # last login date is unknown, but there is an expiration policy configured, + # err to the side of caution and return an error to deny login + return R('KO_UNKNOWN_LAST_LOGIN', msg => "Couldn't determine last login date"); + } + # called at several places below, only when we return OK + my $updateFunc = sub { + return if !$updateLastlog; + foreach my $file (@lastlogFilesToUpdate) { + if ($ipfrom && $hostfrom) { + if (open(my $lastlogfh, '>', $file)) { + print $lastlogfh sprintf("%s(%s)", $ipfrom, $hostfrom); + close($lastlogfh); + } + else { + warn_syslog("Couldn't update the lastlog file '$file' ($!)"); + } + } + else { + # just touch it (i.e. accountUnexpire case) + $fnret = OVH::Bastion::touch_file($file); + if (!$fnret) { + warn_syslog("Couldn't update the lastlog file '$file' ($fnret)"); + } + } + } + }; + + if ($accountMaxInactiveDays == 0) { # no expiration configured, allow login and return some info + $updateFunc->(); return R('OK_FIRST_LOGIN', value => $value) if $isFirstLogin; return R('OK_EXPIRATION_NOT_CONFIGURED', value => $value); } - else { - if ($days < $accountMaxInactiveDays) { - # expiration configured, but account not expired, allow login - return R('OK_NOT_EXPIRED', value => $value); - } - else { - # account expired, deny login - my $msg = OVH::Bastion::config("accountExpiredMessage")->value; - $msg = "Sorry, but your account has expired (#DAYS# days), access denied by policy." if !$msg; - $msg =~ s/#DAYS#/$days/g; - return R( - 'KO_EXPIRED', - value => $value, - msg => $msg, - ); - } + if ($value->{'days'} < $accountMaxInactiveDays) { + # expiration configured, but account not expired, allow login + $updateFunc->(); + return R('OK_NOT_EXPIRED', value => $value); } - return R('ERR_INTERNAL_ERROR'); + + # account expired, deny login + my $msg = OVH::Bastion::config("accountExpiredMessage")->value; + $msg = "Sorry, but your account has expired (#DAYS# days), access denied by policy." if !$msg; + $msg =~ s/#DAYS#/$value->{'days'}/g; + return R( + 'KO_EXPIRED', + value => $value, + msg => $msg, + ); } sub is_account_ttl_nonexpired { @@ -786,6 +842,7 @@ sub touch_file { else { $ret = sysopen($fh, $file, O_RDWR | O_CREAT); } + my $err = $!; if ($ret) { close($fh); @@ -796,8 +853,8 @@ sub touch_file { } # else - warn_syslog(sprintf("Couldn't touch file '%s' with perms %o: %s", $file, $perms, $!)); - return R('KO', msg => "Couldn't create file $file: $!"); + warn_syslog(sprintf("Couldn't touch file '%s' with perms %o: %s", $file, $perms, $err)); + return R('KO', msg => "Couldn't create file $file: $err"); } sub create_file_if_not_exists { @@ -968,8 +1025,13 @@ sub can_account_execute_plugin { # restricted plugins (osh-* system groups based) if (-f ($path_plugin . '/restricted/' . $plugin)) { if (OVH::Bastion::is_user_in_group(user => $account, group => "osh-$plugin", cache => $cache)) { - return R('OK', - value => {fullpath => $path_plugin . '/restricted/' . $plugin, type => 'restricted', plugin => $plugin} + return R( + 'OK', + value => { + fullpath => $path_plugin . '/restricted/' . $plugin, + type => 'restricted', + plugin => $plugin + } ); } else {