From 6a55506b0cf4e327de9f7c92a87023fc779727ac Mon Sep 17 00:00:00 2001 From: Sebastian Grewe Date: Fri, 7 Feb 2014 10:14:56 +0100 Subject: [PATCH] [IMPROVED] Logging format * Added IP address to all log output * Added Page and Action to all log output * Modified log messages * Added Error and Fatal handlers * Raised failed logins to Error * Added KLogger default log levels * Made it most verbose --- public/include/bootstrap.php | 2 +- public/include/classes/invitation.class.php | 4 +- public/include/classes/logger.class.php | 17 ++++- public/include/classes/notification.class.php | 2 +- public/include/classes/payout.class.php | 4 +- public/include/classes/user.class.php | 68 ++++++++++--------- public/include/config/security.inc.dist.php | 12 ++-- public/include/pages/account/edit.inc.php | 2 +- public/include/pages/admin/settings.inc.php | 2 +- 9 files changed, 66 insertions(+), 47 deletions(-) diff --git a/public/include/bootstrap.php b/public/include/bootstrap.php index 4d418ce4..fc300de6 100644 --- a/public/include/bootstrap.php +++ b/public/include/bootstrap.php @@ -22,7 +22,7 @@ if (@file_exists(BASEPATH . 'include/config/security.inc.php')) include_once(BAS $session_start = @session_start(); session_set_cookie_params(time()+$config['cookie']['duration'], $config['cookie']['path'], $config['cookie']['domain'], $config['cookie']['secure'], $config['cookie']['httponly']); if (!$session_start) { - $log->log("info", "Forcing session id regeneration for ".$_SERVER['REMOTE_ADDR']." [hijack attempt?]"); + $log->log("info", "Forcing session id regeneration, session failed to start [hijack attempt?]"); session_destroy(); session_regenerate_id(true); session_start(); diff --git a/public/include/classes/invitation.class.php b/public/include/classes/invitation.class.php index dfa0fe93..2d0c6896 100644 --- a/public/include/classes/invitation.class.php +++ b/public/include/classes/invitation.class.php @@ -114,14 +114,14 @@ class Invitation extends Base { } $aData['username'] = $this->user->getUserName($account_id); $aData['subject'] = 'Pending Invitation'; - $this->log->log("info", $this->user->getUserName($account_id)." sent an invitation from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("info", $this->user->getUserName($account_id)." sent an invitation"); if ($this->mail->sendMail('invitations/body', $aData)) { $aToken = $this->token->getToken($aData['token'], 'invitation'); if (!$this->createInvitation($account_id, $aData['email'], $aToken['id'])) return false; return true; } else { - $this->log->log("warn", $this->user->getUserName($account_id)." sent an invitation but the mailing failed from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("warn", $this->user->getUserName($account_id)." sent an invitation but failed to send e-mail"); $this->setErrorMessage($this->getErrorMsg('E0028')); } $this->setErrorMessage($this->getErrorMsg('E0029')); diff --git a/public/include/classes/logger.class.php b/public/include/classes/logger.class.php index 84de7244..35624a44 100644 --- a/public/include/classes/logger.class.php +++ b/public/include/classes/logger.class.php @@ -10,13 +10,24 @@ class Logger { } } public function log($type, $message) { + // Logmask, we add some infos into the KLogger + $strMask = "[ %12s ] [ %8s | %-8s ] : %s"; + $strIPAddress = isset($_SERVER['REMOTE_ADDR']) ? $_SERVER['REMOTE_ADDR'] : 'unknown'; + $strPage = isset($_REQUEST['page']) ? $_REQUEST['page'] : 'none'; + $strAction = isset($_REQUEST['action']) ? $_REQUEST['action'] : 'none'; if ($this->logging) { switch ($type) { case 'info': - $this->KLogger->LogInfo($message); + $this->KLogger->LogInfo(sprintf($strMask, $strIPAddress, $strPage, $strAction, $message)); break; case 'warn': - $this->KLogger->LogWarn($message); + $this->KLogger->LogWarn(sprintf($strMask, $strIPAddress, $strPage, $strAction, $message)); + break; + case 'error': + $this->KLogger->LogError(sprintf($strMask, $strIPAddress, $strPage, $strAction, $message)); + break; + case 'fatal': + $this->KLogger->LogFatal(sprintf($strMask, $strIPAddress, $strPage, $strAction, $message)); break; } return true; @@ -26,4 +37,4 @@ class Logger { } } $log = new Logger($config); -?> \ No newline at end of file +?> diff --git a/public/include/classes/notification.class.php b/public/include/classes/notification.class.php index d66dd4ec..ec09bad9 100644 --- a/public/include/classes/notification.class.php +++ b/public/include/classes/notification.class.php @@ -120,7 +120,7 @@ class Notification extends Mail { $this->setErrorMessage($this->getErrorMsg('E0047', $failed)); return $this->sqlError(); } - $this->log->log("info", "User $account_id updated notification settings from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("info", "User $account_id updated notification settings"); return true; } diff --git a/public/include/classes/payout.class.php b/public/include/classes/payout.class.php index d862ec45..f56ca577 100644 --- a/public/include/classes/payout.class.php +++ b/public/include/classes/payout.class.php @@ -33,12 +33,12 @@ class Payout Extends Base { if ($delete) { return true; } else { - $this->log->log("info", "User $account_id requested manual payout but the token deletion failed from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("info", "User $account_id requested manual payout but failed to delete payout token"); $this->setErrorMessage('Unable to delete token'); return false; } } else { - $this->log->log("info", "User $account_id requested manual payout using an invalid token from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("info", "User $account_id requested manual payout using an invalid payout token"); $this->setErrorMessage('Invalid token'); return false; } diff --git a/public/include/classes/user.class.php b/public/include/classes/user.class.php index de244853..0e02fd0f 100644 --- a/public/include/classes/user.class.php +++ b/public/include/classes/user.class.php @@ -69,17 +69,17 @@ class User extends Base { } public function changeNoFee($id) { $field = array('name' => 'no_fees', 'type' => 'i', 'value' => !$this->isNoFee($id)); - $this->log->log("warn", $this->getUserName($id)." changed no_fees to ".$this->isNoFee($id)." from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("warn", $this->getUserName($id)." changed no_fees to ".$this->isNoFee($id)); return $this->updateSingle($id, $field); } public function setLocked($id, $value) { $field = array('name' => 'is_locked', 'type' => 'i', 'value' => $value); - $this->log->log("warn", $this->getUserName($id)." changed is_locked to $value from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("warn", $this->getUserName($id)." changed is_locked to $value"); return $this->updateSingle($id, $field); } public function changeAdmin($id) { $field = array('name' => 'is_admin', 'type' => 'i', 'value' => !$this->isAdmin($id)); - $this->log->log("warn", $this->getUserName($id)." changed is_admin to ".$this->isAdmin($id)." from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("warn", $this->getUserName($id)." changed is_admin to ".$this->isAdmin($id)); return $this->updateSingle($id, $field); } public function setUserFailed($id, $value) { @@ -149,7 +149,7 @@ class User extends Base { $this->updateLoginTimestamp($uid); $getIPAddress = $this->getUserIp($uid); if ($getIPAddress !== $_SERVER['REMOTE_ADDR']) { - $this->log->log("warn", "$username has logged in with a different IP [".$_SERVER['REMOTE_ADDR']."] saved is [$getIPAddress]"); + $this->log->log("warn", "$username has logged in with a different IP, saved is [$getIPAddress]"); } $setIPAddress = $this->setUserIp($uid, $_SERVER['REMOTE_ADDR']); $this->createSession($username, $getIPAddress, $lastLoginTime); @@ -178,13 +178,13 @@ class User extends Base { } } $this->setErrorMessage("Invalid username or password"); - $this->log->log("info", "$username failed login from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log('error', "Authentication failed for $username"); if ($id = $this->getUserId($username)) { $this->incUserFailed($id); // Check if this account should be locked if (isset($this->config['maxfailed']['login']) && $this->getUserFailed($id) >= $this->config['maxfailed']['login']) { $this->setLocked($id, 1); - $this->log->log("warn", "$username locked via failed logins from [".$_SERVER['REMOTE_ADDR']."] saved is [".$this->getUserIp($this->getUserId($username))."]"); + $this->log->log("warn", "$username locked due to failed logins, saved is [".$this->getUserIp($this->getUserId($username))."]"); if ($token = $this->token->createToken('account_unlock', $id)) { $aData['token'] = $token; $aData['username'] = $username; @@ -213,12 +213,12 @@ class User extends Base { $this->setUserPinFailed($userId, 0); return ($pin_hash === $row_pin); } - $this->log->log("info", $this->getUserName($userId)." incorrect pin from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log('info', $this->getUserName($userId).' incorrect pin'); $this->incUserPinFailed($userId); // Check if this account should be locked if (isset($this->config['maxfailed']['pin']) && $this->getUserPinFailed($userId) >= $this->config['maxfailed']['pin']) { $this->setLocked($userId, 1); - $this->log->log("warn", $this->getUserName($userId)." was locked via incorrect pins from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("warn", $this->getUserName($userId)." was locked due to incorrect pins"); if ($token = $this->token->createToken('account_unlock', $userId)) { $username = $this->getUserName($userId); $aData['token'] = $token; @@ -247,16 +247,16 @@ class User extends Base { if ($this->checkStmt($stmt) && $stmt->bind_param('sis', $newpin, $userID, $current) && $stmt->execute()) { if ($stmt->errno == 0 && $stmt->affected_rows === 1) { if ($this->mail->sendMail('pin/reset', $aData)) { - $this->log->log("info", $this->getUserName($userID)." was sent a pin reset from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("info", "$username was sent a pin reset e-mail"); return true; } else { - $this->log->log("warn", $this->getUserName($userID)." request a pin reset but the mailing failed from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("warn", "$username request a pin reset but failed to send mail"); $this->setErrorMessage('Unable to send mail to your address'); return false; } } } - $this->log->log("warn", $this->getUserName($userID)." incorrect pin reset attempt from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("warn", "$username incorrect pin reset attempt"); $this->setErrorMessage( 'Unable to generate PIN, current password incorrect?' ); return false; } @@ -341,16 +341,16 @@ class User extends Base { default: $aData['subject'] = ''; } - $this->log->log("info", $this->getUserName($userID)." was sent a $strType token from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("info", $aData['username']." was sent a $strType token e-mail"); if ($this->mail->sendMail('notifications/'.$strType, $aData)) { return true; } else { $this->setErrorMessage('Failed to send the notification'); - $this->log->log("warn", $this->getUserName($userID)." requested a $strType token but the mailing failed from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("warn", $aData['username']." requested a $strType token but sending mail failed"); return false; } } - $this->log->log("warn", $this->getUserName($userID)." attempted to request multiple $strType tokens from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("warn", $this->getUserName($userID)." attempted to request multiple $strType tokens"); $this->setErrorMessage('A request has already been sent to your e-mail address. Please wait an hour for it to expire.'); return false; } @@ -380,15 +380,15 @@ class User extends Base { $tValid = $this->token->isTokenValid($userID, $strToken, 6); if ($tValid) { if ($this->token->deleteToken($strToken)) { - $this->log->log("info", $this->getUserName($userID)." deleted change password token from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("info", $this->getUserName($userID)." deleted change password token"); // token deleted, continue } else { - $this->log->log("warn", $this->getUserName($userID)." change password token failed to delete from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("warn", $this->getUserName($userID)." failed to delete the change password token"); $this->setErrorMessage('Token deletion failed'); return false; } } else { - $this->log->log("warn", $this->getUserName($userID)." attempted to use an invalid change password token from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("error", $this->getUserName($userID)." attempted to use an invalid change password token"); $this->setErrorMessage('Invalid token'); return false; } @@ -398,12 +398,12 @@ class User extends Base { $stmt->bind_param('sis', $new, $userID, $current); $stmt->execute(); if ($stmt->errno == 0 && $stmt->affected_rows === 1) { - $this->log->log("info", $this->getUserName($userID)." updated password from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("info", $this->getUserName($userID)." updated password"); return true; } $stmt->close(); } - $this->log->log("warn", $this->getUserName($userID)." incorrect password update attempt from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("warn", $this->getUserName($userID)." incorrect password update attempt"); $this->setErrorMessage( 'Unable to update password, current password wrong?' ); return false; } @@ -473,15 +473,15 @@ class User extends Base { $tValid = $this->token->isTokenValid($userID, $strToken, 5); if ($tValid) { if ($this->token->deleteToken($strToken)) { - $this->log->log("info", $this->getUserName($userID)." deleted account update token for [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("info", $this->getUserName($userID)." deleted account update token"); } else { $this->setErrorMessage('Token deletion failed'); - $this->log->log("warn", $this->getUserName($userID)." updated their account details but token deletion failed from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("warn", $this->getUserName($userID)." updated their account details but failed to delete token"); return false; } } else { $this->setErrorMessage('Invalid token'); - $this->log->log("warn", $this->getUserName($userID)." attempted to use an invalid token account update token from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("warn", $this->getUserName($userID)." attempted to use an invalid token account update token"); return false; } } @@ -489,7 +489,7 @@ class User extends Base { // We passed all validation checks so update the account $stmt = $this->mysqli->prepare("UPDATE $this->table SET coin_address = ?, ap_threshold = ?, donate_percent = ?, email = ?, is_anonymous = ? WHERE id = ?"); if ($this->checkStmt($stmt) && $stmt->bind_param('sddsii', $address, $threshold, $donate, $email, $is_anonymous, $userID) && $stmt->execute()) { - $this->log->log("info", $this->getUserName($userID)." updated their account details from [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("info", $this->getUserName($userID)." updated their account details"); return true; } // Catchall @@ -717,7 +717,7 @@ class User extends Base { } if (!$this->token->deleteToken($strToken)) { $this->setErrorMessage('Unable to remove used token'); - $this->log->log("warn", "$username tried to register but the token failed to delete [".$_SERVER['REMOTE_ADDR']."]"); + $this->log->log("warn", "$username tried to register but failed to delete the invitation token"); return false; } } @@ -839,9 +839,9 @@ class User extends Base { $aData['username'] = $this->getUserName($this->getUserId($username, true)); $aData['subject'] = 'Password Reset Request'; if ($_SERVER['REMOTE_ADDR'] !== $this->getUserIp($this->getUserId($username, true))) { - $this->log->log("warn", "$username requested password reset from [".$_SERVER['REMOTE_ADDR']."] saved is [".$this->getUserIp($this->getUserId($username, true))."]"); + $this->log->log("warn", "$username requested password reset, saved IP is [".$this->getUserIp($this->getUserId($username, true))."]"); } else { - $this->log->log("info", "$username requested password reset from [".$_SERVER['REMOTE_ADDR']."] saved is [".$this->getUserIp($this->getUserId($username, true))."]"); + $this->log->log("info", "$username requested password reset, saved IP is [".$this->getUserIp($this->getUserId($username, true))."]"); } if ($this->mail->sendMail('password/reset', $aData)) { return true; @@ -861,17 +861,21 @@ class User extends Base { **/ public function isAuthenticated($logout=true) { $this->debug->append("STA " . __METHOD__, 4); - if (@$_SESSION['AUTHENTICATED'] == true && - !$this->isLocked($_SESSION['USERDATA']['id']) && - $this->getUserIp($_SESSION['USERDATA']['id']) == $_SERVER['REMOTE_ADDR'] && - (!$this->config['protect_session_state'] || ($this->config['protect_session_state'] && $_SESSION['STATE'] == md5($_SESSION['USERDATA']['username'].$_SESSION['USERDATA']['id'].@$_SERVER['HTTP_USER_AGENT']))) + if ( @$_SESSION['AUTHENTICATED'] == true && + !$this->isLocked($_SESSION['USERDATA']['id']) && + $this->getUserIp($_SESSION['USERDATA']['id']) == $_SERVER['REMOTE_ADDR'] && + ( ! $this->config['protect_session_state'] || + ( + $this->config['protect_session_state'] && $_SESSION['STATE'] == md5($_SESSION['USERDATA']['username'].$_SESSION['USERDATA']['id'].@$_SERVER['HTTP_USER_AGENT']) + ) + ) ) return true; // Catchall - $this->log->log("warn", "Forcing logout, user is locked or IP changed mid session from [".$_SERVER['REMOTE_ADDR']."] [hijack attempt?]"); + $this->log->log('warn', 'Forcing logout, user is locked or IP changed mid session [hijack attempt?]'); if ($logout == true) $this->logoutUser(); return false; } - + /** * Convenience function to get IP address, no params is the same as REMOTE_ADDR * @param trustremote bool must be FALSE to checkclient or checkforwarded diff --git a/public/include/config/security.inc.dist.php b/public/include/config/security.inc.dist.php index 6c98eb09..28607523 100644 --- a/public/include/config/security.inc.dist.php +++ b/public/include/config/security.inc.dist.php @@ -12,11 +12,15 @@ $config['protect_session_state'] = false; /** * Logging - * Log security issues - 0 = disabled, 2 = everything, 3 = warnings only - * + * 0 = Nothing + * 1 = Everything + * 2 = INFO only + * 3 = + WARN + * 4 = + ERROR + * 5 = + FATAL */ $config['logging']['enabled'] = true; -$config['logging']['level'] = 3; +$config['logging']['level'] = 5; $config['logging']['path'] = realpath(BASEPATH.'../logs'); $config['logging']['file'] = date('Y-m-d').'.security.log'; @@ -60,4 +64,4 @@ $config['twofactor']['options']['changepw'] = true; $config['maxfailed']['login'] = 3; $config['maxfailed']['pin'] = 3; -?> \ No newline at end of file +?> diff --git a/public/include/pages/account/edit.inc.php b/public/include/pages/account/edit.inc.php index b6cf8158..cde09a77 100644 --- a/public/include/pages/account/edit.inc.php +++ b/public/include/pages/account/edit.inc.php @@ -101,7 +101,7 @@ if ($user->isAuthenticated()) { } else { $aBalance = $transaction->getBalance($_SESSION['USERDATA']['id']); $dBalance = $aBalance['confirmed']; - $user->log->log("info", $_SESSION['USERDATA']['username']." requesting manual payout from [".$_SERVER['REMOTE_ADDR']."]"); + $user->log->log("info", $_SESSION['USERDATA']['username']." requesting manual payout"); if ($dBalance > $config['txfee_manual']) { if (!$oPayout->isPayoutActive($_SESSION['USERDATA']['id'])) { if (!$config['csrf']['enabled'] || $config['csrf']['enabled'] && $csrftoken->valid) { diff --git a/public/include/pages/admin/settings.inc.php b/public/include/pages/admin/settings.inc.php index 4e02af22..e36b0f6a 100644 --- a/public/include/pages/admin/settings.inc.php +++ b/public/include/pages/admin/settings.inc.php @@ -8,7 +8,7 @@ if (!$user->isAuthenticated() || !$user->isAdmin($_SESSION['USERDATA']['id'])) { } if (@$_REQUEST['do'] == 'save' && !empty($_REQUEST['data'])) { - $user->log->log("warn", @$_SESSION['USERDATA']['username']." changed admin settings from [".$_SERVER['REMOTE_ADDR']."]"); + $user->log->log("warn", @$_SESSION['USERDATA']['username']." changed admin settings"); foreach($_REQUEST['data'] as $var => $value) { $setting->setValue($var, $value); }