Skip to content

Commit

Permalink
enh: expiration: reorder code and remove legacy checks
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
speed47 committed Jan 31, 2025
1 parent 4dbc32e commit f3a7353
Show file tree
Hide file tree
Showing 4 changed files with 164 additions and 82 deletions.
33 changes: 19 additions & 14 deletions bin/helper/osh-accountUnexpire
Original file line number Diff line number Diff line change
Expand Up @@ -49,35 +49,40 @@ 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");
}

#<RIGHTSCHECK

#>PARAMS:ACCOUNT
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'};

#<PARAMS:ACCOUNT

my $accounthome = $fnret->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});
29 changes: 25 additions & 4 deletions bin/plugin/restricted/accountUnexpire
Original file line number Diff line number Diff line change
Expand Up @@ -41,22 +41,43 @@ 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);

$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) {
Expand Down
20 changes: 7 additions & 13 deletions bin/shell/osh.pl
Original file line number Diff line number Diff line change
Expand Up @@ -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'}) {
Expand All @@ -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
#
Expand Down
164 changes: 113 additions & 51 deletions lib/perl/OVH/Bastion.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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)) {
Expand All @@ -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;

Expand All @@ -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 {
Expand Down Expand Up @@ -786,6 +842,7 @@ sub touch_file {
else {
$ret = sysopen($fh, $file, O_RDWR | O_CREAT);
}
my $err = $!;

if ($ret) {
close($fh);
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit f3a7353

Please sign in to comment.