From 41121f77233a46b23010e5b5c5da2e309310ee5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lesimple?= Date: Thu, 7 Jan 2021 17:11:43 +0000 Subject: [PATCH] fix: proper sqlite log location for invalid realm accounts --- bin/shell/osh.pl | 17 +++++++++--- lib/perl/OVH/Bastion/log.inc | 50 ++++++++++++++++++++++++------------ 2 files changed, 47 insertions(+), 20 deletions(-) diff --git a/bin/shell/osh.pl b/bin/shell/osh.pl index 37a54dd..45fff77 100755 --- a/bin/shell/osh.pl +++ b/bin/shell/osh.pl @@ -112,9 +112,20 @@ my $osh_debug = $config->{'debug'}; # and the real remote account name (which doesn't have an account here because it's from another realm) # is passed through LC_BASTION if ($self =~ /^realm_([a-zA-Z0-9_.-]+)/) { - $self = sprintf("%s/%s", $1, $ENV{'LC_BASTION'}); - $fnret = OVH::Bastion::is_bastion_account_valid_and_existing(account => $self, realmOnly => 1); - $fnret or main_exit(OVH::Bastion::EXIT_ACCOUNT_INVALID, "account_invalid", "The realm-scoped account '$self' is invalid (" . $fnret->msg . ")"); + if ($ENV{'LC_BASTION'}) { + + # don't overwrite $self just yet because it might end up being invalid, and when we'll call main_exit 2 lines down, + # we won't log to the proper place if sql logs or access logs are enabled per account. + my $potentialSelf = sprintf("%s/%s", $1, $ENV{'LC_BASTION'}); + $fnret = OVH::Bastion::is_bastion_account_valid_and_existing(account => $potentialSelf, realmOnly => 1); + $fnret or main_exit(OVH::Bastion::EXIT_ACCOUNT_INVALID, "account_invalid", "The realm-scoped account '$self' is invalid (" . $fnret->msg . ")"); + + # $potentialSelf is valid, we can use it + $self = $potentialSelf; + } + else { + main_exit(OVH::Bastion::EXIT_ACCOUNT_INVALID, "account_invalid", "Attempted to use a realm account but not from another bastion"); + } } else { # non-realm case diff --git a/lib/perl/OVH/Bastion/log.inc b/lib/perl/OVH/Bastion/log.inc index 67fd07f..65002de 100644 --- a/lib/perl/OVH/Bastion/log.inc +++ b/lib/perl/OVH/Bastion/log.inc @@ -327,7 +327,7 @@ sub _sql_log_insert_file { # if this is set, we probably reached max retry in previous loop without succeeding if ($DBI::err) { - warn_syslog("Failed after multiple retries [$sqltype] err $DBI::err while doing [$doing]: $DBI::errstr"); + warn_syslog("Failed after multiple retries [$sqltype] err $DBI::err while doing [$doing]: $DBI::errstr ($file)"); return R('ERR_SQL_EXECUTE', msg => "SQL error [$sqltype] err $DBI::err while doing [$doing]: $DBI::errstr"); } @@ -346,14 +346,25 @@ sub log_access_insert { # if not, or if its invalid, still try to log what we can (hence don't return here) my ($remoteaccount, $sysaccount); if (defined $account) { - if ($fnret = OVH::Bastion::is_bastion_account_valid_and_existing(account => $account)) { - $account = $fnret->value->{'account'}; - $sysaccount = $fnret->value->{'sysaccount'}; - $remoteaccount = $fnret->value->{'remoteaccount'}; + if ($fnret = OVH::Bastion::is_bastion_account_valid_and_existing(account => $account, accountType => "normal")) { + + # normal accounts + ; + } + elsif ($fnret = OVH::Bastion::is_bastion_account_valid_and_existing(account => $account, accountType => "realm")) { + + # $account can also be a realm_xyz account, in case osh.pl is erroring early and couldn't resolve the + # proper realm-qualified account name (realm/user) + ; } else { undef $account; } + if ($fnret) { + $account = $fnret->value->{'account'}; + $sysaccount = $fnret->value->{'sysaccount'}; + $remoteaccount = $fnret->value->{'remoteaccount'}; + } } $loghome ||= $sysaccount; @@ -386,17 +397,22 @@ sub log_access_insert { # first, log in account db my $accountsql = 'no'; if (OVH::Bastion::config('enableAccountSqlLog')->value()) { - $fnret = _sql_log_insert_file( - %params, - sqltype => "local", - file => sprintf("/home/%s/%s-log-%04d%02d.sqlite", $loghome, $remoteaccount || $loghome, $localtime[5] + 1900, $localtime[4] + 1) - ); - if ($fnret) { - ($insert_id, $db_name) = ($fnret->value->{'id'}, $params{'file'}); - $accountsql = 'ok'; + if ($loghome && -d "/home/$loghome") { + $fnret = _sql_log_insert_file( + %params, + sqltype => "local", + file => sprintf("/home/%s/%s-log-%04d%02d.sqlite", $loghome, $remoteaccount || $loghome, $localtime[5] + 1900, $localtime[4] + 1) + ); + if ($fnret) { + ($insert_id, $db_name) = ($fnret->value->{'id'}, $params{'file'}); + $accountsql = 'ok'; + } + else { + $accountsql = 'error ' . $fnret->msg; + } } else { - $accountsql = 'error ' . $fnret->msg; + warn_syslog("Attempted to log to account $account sqlite database without a valid loghome ($loghome)"); } } @@ -528,7 +544,7 @@ sub _sql_log_update_file { # if this is set, we probably reached max retry in previous loop without succeeding if ($DBI::err) { - warn("Failed after multiple retries [updating] err $DBI::err while doing [$doing]: $DBI::errstr"); + warn_syslog("Failed after multiple retries [updating] err $DBI::err while doing [$doing]: $DBI::errstr ($file)"); return R('ERR_SQL_EXECUTE', msg => "SQL error [updating] err $DBI::err while doing [$doing]: $DBI::errstr"); } @@ -591,7 +607,7 @@ EOS # if this is set, we probably reached max retry in previous loop without succeeding if ($DBI::err) { - warn("Failed after multiple retries [plugins] err $DBI::err while doing [$doing]: $DBI::errstr"); + warn_syslog("Failed after multiple retries [plugins] err $DBI::err while doing [$doing]: $DBI::errstr ($file)"); return R('ERR_SQL_EXECUTE', msg => "SQL error [plugins] err $DBI::err while doing [$doing]: $DBI::errstr"); } } @@ -885,7 +901,7 @@ sub _sql_log_fetch_from_file { # if this is set, we probably reached max retry in previous loop without succeeding if ($DBI::err) { - warn("Failed after multiple retries [$sqltype] err $DBI::err while doing [$doing]: $DBI::errstr"); + warn_syslog("Failed after multiple retries [$sqltype] err $DBI::err while doing [$doing]: $DBI::errstr ($file)"); return R('ERR_SQL_EXECUTE', msg => "SQL error [$sqltype] err $DBI::err while doing [$doing]: $DBI::errstr"); }