From 8c82c3441b39ed6923e0b3d661a161042f8e7e7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lesimple?= Date: Fri, 15 Jul 2022 11:22:48 +0000 Subject: [PATCH] fix: accountInfo wasn't showing TTL account expiration #329 --- bin/plugin/restricted/accountInfo | 31 +++++++++++++++- bin/plugin/restricted/accountList | 20 ++++++++-- bin/shell/osh.pl | 22 ++--------- lib/perl/OVH/Bastion.pm | 41 +++++++++++++++++++++ tests/functional/tests.d/325-accountinfo.sh | 24 +++++++++++- 5 files changed, 113 insertions(+), 25 deletions(-) diff --git a/bin/plugin/restricted/accountInfo b/bin/plugin/restricted/accountInfo index 00d08a6..6885b77 100755 --- a/bin/plugin/restricted/accountInfo +++ b/bin/plugin/restricted/accountInfo @@ -127,13 +127,40 @@ else { if (OVH::Bastion::is_auditor(account => $self)) { + $fnret = OVH::Bastion::is_account_ttl_nonexpired(sysaccount => $sysaccount, account => $account); + if ($fnret->is_ok && $fnret->err eq 'OK_NO_TTL') { + osh_info "This account has " . colored('no TTL set', 'green'); + $ret{'is_ttl_expired'} = 0; + } + elsif ($fnret->is_ok && $fnret->err eq 'OK_TTL_VALID') { + osh_info "This account " + . colored('TTL is still valid', 'green') + . " (for " + . $fnret->value->{'details'}{'human'} . ")"; + $ret{'is_ttl_expired'} = 0; + } + elsif ($fnret->is_ko) { + osh_info "This account " + . colored('TTL is EXPIRED', 'red') + . " (since " + . $fnret->value->{'details'}{'human'} . ")"; + $canConnect = 0; + $ret{'is_ttl_expired'} = 1; + } + else { + osh_warn "Error getting account TTL expiration info (" . $fnret->msg . ")"; + } + $ret{'ttl_timestamp'} = ($fnret && $fnret->value) ? $fnret->value->{'expiry_time'} : undef; + $fnret = OVH::Bastion::is_account_nonexpired(sysaccount => $sysaccount, remoteaccount => $remoteaccount); if ($fnret->is_ok) { - osh_info "This account is " . colored('not expired', 'green'); + osh_info "This account has seen recent-enough activity to " . colored('not be activity-expired', 'green'); $ret{'is_expired'} = 0; } elsif ($fnret->is_ko) { - osh_info "This account is " . colored('EXPIRED', 'red'); + osh_info "This account is " + . colored('EXPIRED', 'red') + . " activity-wise (it hasn't been seen for a long time)"; $canConnect = 0; $ret{'is_expired'} = 1; } diff --git a/bin/plugin/restricted/accountList b/bin/plugin/restricted/accountList index b5c82c8..eb694a3 100755 --- a/bin/plugin/restricted/accountList +++ b/bin/plugin/restricted/accountList @@ -105,6 +105,16 @@ foreach my $account (sort keys %$accounts) { } if ($audit) { + $fnret = OVH::Bastion::is_account_ttl_nonexpired(sysaccount => $account, account => $account); + $states{'is_ttl_expired'} = undef; + if ($fnret->is_ok) { + $states{'is_ttl_expired'} = 0; + } + elsif ($fnret->is_ko) { + $states{'is_ttl_expired'} = 1; + $states{'ttl_expired_details'} = $fnret->value->{'details'}; + } + $fnret = OVH::Bastion::is_account_nonexpired(sysaccount => $account); $states{'is_expired'} = undef; if ($fnret->is_ok) { @@ -140,7 +150,10 @@ foreach my $account (sort keys %$accounts) { } } - $states{'can_connect'} = ($states{'is_active'} && !$states{'is_expired'}) ? 1 : 0; + $states{'can_connect'} = 1; + $states{'can_connect'} = 0 if (!defined $states{'is_active'} || $states{'is_active'} == 0); + $states{'can_connect'} = 0 if (!defined $states{'is_expired'} || $states{'is_expired'} == 0); + $states{'can_connect'} = 0 if (!defined $states{'is_ttl_expired'} || $states{'is_ttl_expired'} == 0); $states{'mfa_password_required'} = OVH::Bastion::is_user_in_group( user => $account, @@ -202,11 +215,12 @@ foreach my $account (sort keys %$accounts) { push @mfaTOTP, 'bypass' if $states{'mfa_totp_bypass'}; osh_info sprintf( - "%-18s %6d active:%-12s expired:%-12s can_connect:%-12s already_seen:%-12s mfa_password:%-25s mfa_totp:%-25s pam_bypass:%-12s pubkey_auth_optional:%-12s pass_status:%-15s pass_changed:%-10s pass_min_days:%-3d pass_max_days:%-3d pass_warn_days:%-3d %s\n", + "%-18s %6d active:%-12s expired:%-12s ttl_expired:%-12s can_connect:%-12s already_seen:%-12s mfa_password:%-25s mfa_totp:%-25s pam_bypass:%-12s pubkey_auth_optional:%-12s pass_status:%-15s pass_changed:%-10s pass_min_days:%-3d pass_max_days:%-3d pass_warn_days:%-3d %s\n", $account, $accounts->{$account}{'uid'}, tristate2str($states{'is_active'}), - tristate2str($states{'is_expired'}, 1), + tristate2str($states{'is_expired'}, 1), + tristate2str($states{'is_ttl_expired'}, 1), tristate2str($states{'can_connect'}), tristate2str($states{'already_seen_before'}), @mfaPassword ? colored(join(',', @mfaPassword), 'green') : colored('-', 'blue'), diff --git a/bin/shell/osh.pl b/bin/shell/osh.pl index c1ac686..bd5bd60 100755 --- a/bin/shell/osh.pl +++ b/bin/shell/osh.pl @@ -175,25 +175,9 @@ if (-e '/home/allowkeeper/maintenance') { # Does the user have a TTL, and if yes, has it expired? # -$fnret = OVH::Bastion::account_config(account => $self, key => "account_ttl"); -if ($fnret) { - if ($fnret->value !~ /^\d+$/) { - main_exit(OVH::Bastion::EXIT_TTL_EXPIRED, - "ttl_expired", "Your TTL has an invalid value, access denied. Check with an administrator."); - } - my $ttl = $fnret->value; - - $fnret = OVH::Bastion::account_config(account => $self, key => "creation_timestamp"); - if ($fnret->value !~ /^\d+$/) { - main_exit(OVH::Bastion::EXIT_TTL_EXPIRED, "ttl_expired", - "Your account creation date has an invalid value, and you have a TTL set, access denied. Check with an administrator." - ); - } - my $created = $fnret->value; - - if ($created + $ttl < time()) { - main_exit(OVH::Bastion::EXIT_TTL_EXPIRED, "ttl_expired", "Sorry $self, your account has expired."); - } +$fnret = OVH::Bastion::is_account_ttl_nonexpired(account => $self, sysaccount => $sysself); +if (!$fnret) { + main_exit(OVH::Bastion::EXIT_TTL_EXPIRED, "ttl_expired", "Sorry $self, acccess denied (" . $fnret->msg . ")"); } # diff --git a/lib/perl/OVH/Bastion.pm b/lib/perl/OVH/Bastion.pm index 668a462..6bf399d 100644 --- a/lib/perl/OVH/Bastion.pm +++ b/lib/perl/OVH/Bastion.pm @@ -278,6 +278,47 @@ sub is_account_nonexpired { return R('ERR_INTERNAL_ERROR'); } +sub is_account_ttl_nonexpired { + my %params = @_; + my $account = $params{'account'}; + my $sysaccount = $params{'sysaccount'}; + my $fnret; + + if (!$sysaccount || !$account) { + return R('ERR_MISSING_PARAMETER', msg => "Missing a 'sysaccount' or 'account' parameter"); + } + + $fnret = OVH::Bastion::account_config(account => $sysaccount, key => "account_ttl"); + if ($fnret) { + my $ttl = $fnret->value; + if ($ttl !~ /^[0-9]+$/) { + warn_syslog("Invalid TTL value '$ttl' for account '$sysaccount'"); + return R('ERR_INVALID_TTL', msg => "Invalid TTL configuration, please check with an administrator"); + } + + $fnret = OVH::Bastion::account_config(account => $sysaccount, key => "creation_timestamp"); + my $created = $fnret->value; + if ($created !~ /^[0-9]+$/) { + warn_syslog("Invalid account creation time '$created' for account '$sysaccount'"); + return R('ERR_INVALID_TTL', msg => "Invalid TTL configuration, please check with an administrator"); + } + + my $expiryTime = $created + $ttl; + if ($expiryTime < time()) { + $fnret = OVH::Bastion::duration2human(seconds => time() - $expiryTime); + return R( + 'KO_TTL_EXPIRED', + msg => "Account TTL has expired since " . $fnret->value->{'human'}, + value => {expiry_time => $expiryTime, details => $fnret->value} + ); + } + + $fnret = OVH::Bastion::duration2human(seconds => $expiryTime - time()); + return R('OK_TTL_VALID', value => {expiry_time => $expiryTime, details => $fnret->value}); + } + return R('OK_NO_TTL'); +} + # Check whether a user is still active, if this feature has been enabled in the config sub is_account_active { my %params = @_; diff --git a/tests/functional/tests.d/325-accountinfo.sh b/tests/functional/tests.d/325-accountinfo.sh index 448c80b..470b60d 100644 --- a/tests/functional/tests.d/325-accountinfo.sh +++ b/tests/functional/tests.d/325-accountinfo.sh @@ -15,8 +15,17 @@ testsuite_accountinfo() # create another target account we'll use for accountInfo success a0_create_a2 $a0 --osh accountCreate --always-active --account $account2 --uid $uid2 --public-key "\"$(cat $account2key1file.pub)\"" --comment "\"'this is a comment'\"" json .error_code OK .command accountCreate .value null + + # create a third account with a ttl + success a0_create_a3 $a0 --osh accountCreate --always-active --account $account3 --uid $uid3 --public-key "\"$(cat $account3key1file.pub)\"" --ttl 15s + json .error_code OK .command accountCreate .value null + revoke accountCreate + # check that account3 can connect during their TTL + success a3_ttl_connect $a3 --osh info + json .error_code OK + # grant account0 as admin success set_a0_as_admin $r0 "\". $opt_remote_basedir/lib/shell/functions.inc; add_user_to_group_compat $account0 osh-admin\"" configchg 's=^\\\\x22adminAccounts\\\\x22.+=\\\\x22adminAccounts\\\\x22:[\\\\x22'"$account0"'\\\\x22],=' @@ -28,6 +37,10 @@ testsuite_accountinfo() success a0_grant_a0_accountinfo $a0 --osh accountGrantCommand --command accountInfo --account $account0 success a0_grant_a1_accountinfo $a0 --osh accountGrantCommand --command accountInfo --account $account1 + # check that account3 info has the ttl in it + success a1_info_a3_ttl $a1 --osh accountInfo --account $account3 + json .error_code OK .value.is_ttl_expired 0 + # a0 should see basic info about a2 success a0_accountinfo_a2_basic $a0 --osh accountInfo --account $account2 json_document '{"error_message":"OK","command":"accountInfo","error_code":"OK","value":{"always_active":1,"is_active":1,"allowed_commands":[],"groups":{}}}' @@ -41,7 +54,7 @@ testsuite_accountinfo() json .value.ingress_piv_enforced 0 .value.always_active 1 .value.creation_information.by "$account0" json .value.creation_information.comment "this is a comment" json .value.already_seen_before 0 .value.last_activity null - json .value.max_inactive_days null + json .value.max_inactive_days null .value.is_ttl_expired 0 .value.ttl_timestamp null if [ "$OS_FAMILY" = Linux ]; then json .value.password.date_changed $(date +%Y-%m-%d) fi @@ -105,10 +118,19 @@ testsuite_accountinfo() revoke accountModify + # check that account3 can no longer connect due to their TTL + run a3_ttl_connect_no $a3 --osh info + retvalshouldbe 121 + contain 'TTL has expired' + + success a1_info_a3_ttl_no $a1 --osh accountInfo --account $account3 + json .error_code OK .value.is_ttl_expired 1 + # delete account1 & account2 grant accountDelete success a0_delete_a1 $a0 --osh accountDelete --account $account1 --no-confirm success a0_delete_a2 $a0 --osh accountDelete --account $account2 --no-confirm + success a0_delete_a3 $a0 --osh accountDelete --account $account3 --no-confirm success a0_delete_a4 $a0 --osh accountDelete --account $account4 --no-confirm revoke accountDelete }