Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Send authentication log to the syslog #29

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

j1pe
Copy link
Contributor

@j1pe j1pe commented Apr 24, 2020

if you want, I've added a method to send logs (Public IP and username) during authentication to a syslog with udp socket.

if you want, I've added a method to send logs (Public IP and username) during authentication.
@yanncam
Copy link
Owner

yanncam commented Apr 25, 2020

Hello @j1pe,

Thank you for the PR, it's a good idea !
I think you can improve the Syslog integration by defining parameter in the config.php file, like this kind of new constants :

[...]
define("SYSLOG_AUTHD_ENABLE", true);
define("SYSLOG_AUTHD_HOST", "127.0.0.1");
define("SYSLOG_AUTHD_PORT", "514");
[...]

Plus, in your code added, I think you need to:

  • Sanitize the {$_SERVER['HTTP_X_FORWARDED_FOR']} and {$_POST['user']} variables to prevent vulnerability and attack as log injection, because these variables can be define client(attacker)-side ;
  • Add more control to prevent PHP-error if the Syslog server isn't reachable ;
  • Fix the second msg sent to Syslog if the authentication is successful, because you currently send kodi_authd Client authentication failure and it's not a failure :)
  • Plus, the code for sending a Syslog msg can be factorized in a new function I think.

Available to discuss it, I think it's a good feature.

If you can adjust your pull request, I would integrate and merge it with the project.

Thank you !

index.php Outdated
exit;
} else {
echo "<span class='success'>" . AUTHENTICATION_SUCCESS_REDIRECT . "&nbsp;&nbsp;&nbsp;</span><img src='./images/loading-login.gif' />";
echo "<script type='text/javascript'>document.location='./';</script>";
#sending a log to the syslog when authentication is correct
$sock = socket_create(AF_INET, SOCK_DGRAM, SOL_UDP);
$msg = "kodi_authd Client authentication failure: {$_SERVER['HTTP_X_FORWARDED_FOR']} ({$_POST['user']})";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this message isn't the right one. It's not a failure in this case, but a success, no ? :)

index.php Outdated
$sock = socket_create(AF_INET, SOCK_DGRAM, SOL_UDP);
$msg = "kodi_authd Client authentication failure: {$_SERVER['HTTP_X_FORWARDED_FOR']} ({$_POST['user']})";
$len = strlen($msg);
socket_sendto($sock, $msg, $len, 0, '127.0.0.1', 514);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the Syslog server isn't reachable, PHP-error can be triggered. Several control need to be added I think.

index.php Outdated
@@ -13,10 +13,23 @@
sleep(2);
if(!checkAuthentication(trim(strval($_POST['user'])), trim(strval($_POST['pass'])))){
echo "<span class='error'>" . AUTHENTICATION_ERROR_CREDENTIAL . "</span>";
#sending a log to the syslog when authentication is incorrect
$sock = socket_create(AF_INET, SOCK_DGRAM, SOL_UDP);
$msg = "kodi_authd Client authentication failure: {$_SERVER['HTTP_X_FORWARDED_FOR']} ({$_POST['user']})";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The $_SERVER['HTTP_X_FORWARDED_FOR'] and $_POST['user'] are defined client(attacker)-side, so it's necessary to add more control / check / sanitization over these variables to prevent log injection attack.

index.php Outdated
$sock = socket_create(AF_INET, SOCK_DGRAM, SOL_UDP);
$msg = "kodi_authd Client authentication failure: {$_SERVER['HTTP_X_FORWARDED_FOR']} ({$_POST['user']})";
$len = strlen($msg);
socket_sendto($sock, $msg, $len, 0, '127.0.0.1', 514);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Syslog server and port can be defined in the config.php file as constant.

index.php Outdated
@@ -13,10 +13,23 @@
sleep(2);
if(!checkAuthentication(trim(strval($_POST['user'])), trim(strval($_POST['pass'])))){
echo "<span class='error'>" . AUTHENTICATION_ERROR_CREDENTIAL . "</span>";
#sending a log to the syslog when authentication is incorrect
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A dedicated function to factorize code can be defined I think.

@j1pe
Copy link
Contributor Author

j1pe commented Apr 25, 2020

Hello,
I'll try to see what I can do within my limited skills.

@j1pe
Copy link
Contributor Author

j1pe commented Apr 28, 2020

Hi,
so I updated the files:
functions.php
index.php
config.php

So, I created a function allowing to send log to the Syslog server with verification of the values to have no injection.
If the server is down, there is no error.

I updated the index.php by only using the function
I updated config.php so that the user can enable or disable the sending of logs by setting the IP address and the UDP port of the server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants