Bug #23917
closedFailed userlogins are not written to syslog using rsaauth and saltedpasswords
100%
Description
I have installed the system-extensions rsaauth and saltedpasswords on a TYPO3 4.4 installation.
When I try to login as a known user (existing username in DB) with a wrong password, the failed login-attempt is not written to TYPO3´s syslog (no security warning). When I try to login as a unknown user (non-existing username in DB) with a random password, TYPO3 writes the login-attempt correctly to the syslog.
Multiple failed login-attempts does not result in emailing a warning-email to [warning_email_addr]
In TYPO3´s devlog the extension "saltedpassword" writes: BE Authentication failed - wrong password for username '<username-value>', but not to the syslog, where this information is must useful.
I have reproduced the problem on several TYPO3 installations using TYPO3 4.4 and rsaauth ([BE][loginSecurityLevel] = rsa).
(issue imported from #M16223)
Files
Updated by Torben Hansen over 13 years ago
I found out, that the systemextension "saltedpassword" used t3lib_div::sysLog to log the failed login. This functions just writes the failed login to the syslog (eg. file or local syslog on webserver), but NOT to TYPO3´s logtable "sys_log".
When you have a look at the function "writelog" in the class "class.t3lib_userauthgroup", then you see, that this writelog-function inserts serveral values into TYPO3´s internal syslog-tabel (like userid, typ3, action, error, ...). This function is called by TYPO3s default AUTH-services and then therefor writes directly to the table sys_log.
In case of AUTH-Services we are talking about system security, so it is absolutely necessary that every AUTH-Service writes failed login-attempts directly to TYPO3s logtable sys_log.
For salted passwords, you can rename the class-internal function to something else (e.g. writeLogTest()) and now you can use the correct writeLog() function, which is inherrited from class.t3lib_userauthgroup.
IMHO this should be the correct way of handling log-writing in AUTH-Services.
Updated by Torben Hansen over 13 years ago
I have uploaded a modified version og class.tx_saltedpasswords_sv1.php.
The class-internal function writeLog has been renamed to writeSysLog() and all internal calls have been updated.
Then, I´ve implemented the writeLog() call, which is being used in class.tx_sv_auth.php.
The patched version I´ve uploaded does the logging as it should (and as class.tx_sv_auth.php does)
Note: This is a quick and dirty solution. But I think this points the CORE developers in the right direction helping to fix this issue.
Updated by Steffen Gebert over 13 years ago
Hi Torben,
thanks for your contribution.
I've cleaned the file up a bit and attached it as a diff to not overwrite changes made in between.
I agree that this should be really changed. However some comments:- I still geht no entries in my Log as mentioned in #21972, do you?
- BUT, I see the entries in the database table sys_log, therefor +1 for the patch
- however we can't rename a method, because of backwards compatibility.
Could you please test it with the attached patch?
Updated by Torben Hansen over 13 years ago
Hi Steffen,
I have tested your patch but with no success. Now nothing is written to sys_log when I try to login. But I think, both errors described in our bugreports are related to the same reason.
I see the problem in class.tx_saltedpasswords_sv1.php, where the logging to TYPO3s sys_log has been forgotten. It may be, that there are some misunderstandings in the TYPO3 Core function t3lib_div::syslog(), which actually does NOT log to TYPO3s sys_log. When you look at the sourcecode of t3lib_div::syslog() you see, that this function does:
1. Logs to a syslog-file (defined in localconf.php)
2. Sends logentries by e-mail to an e-mail-recipient
3. Logs to PHP Error log
4. Logs to the servers syslog
So having a look back at the original version of saltedpasswords, all logging is just done by t3lib_div::syslog, so there is no chance that something is logged to TYPO3s sys_log.
The class "tx_saltedpasswords_sv1" extends "tx_sv_authbase", where you can see a function named writeLog(). This function in "tx_sv_authbase" calls the writeLog() of the parent object which is inherited from "t3lib_svbase". So the main problem is, that "tx_saltedpasswords_sv1" has its own function called writeLog(), which replaces the original logging capabilities inherited from the parent classes. So actually it is not possible to call the original writeLog() to do the logging to TYPO3s sys_log.
I think, the writeLog() in "tx_saltedpasswords_sv1" must be renamed - else it would no be possible to use the inherited writeLog()-function. I see no problems in backwards compatibility, since writeLog() in "tx_saltedpasswords_sv1" is only used locally.
I have done a lot of testing with the version I´ve modified/uploaded and it seems, that all logging to TYPO3s sys_log works correct (both with known and unknows users).
How can we get far with this issue?
Updated by Steffen Gebert over 13 years ago
Hi Torben,
thanks for your explanation. I made the failure to first test it and afterwards rename the writeSysLog() back to writeLog().
I was pretty sure that PHP is case-sensitive for function names, like it is for variables - however it's not (....).
We could call the parent implementation of writelog() using parent::writelog(). IMHO it was a bad decision, to overwrite the function, however I think Marcus (I think it's his code..), wasn't aware of the existing parent::writelog() function, having different parameters.
I will point Marcus to this issue, what he thinks. However, as already pointed out, just renaming it, is a clean solution, however breaks backwars-compatibility, if sb. XCLASSed this file.
Updated by Torben Hansen over 13 years ago
OK, thanks a lot Steffen. Lets wait and hear what Marcus says to this.
Updated by Torben Hansen about 13 years ago
Hi Steffen,
did you get any feedback?
Updated by Torben Hansen about 13 years ago
I have attached a patch, which uses writeLog() from the parent object. This works without modifying the implemented writeLog() function.
I would be very happy, if this issue would could be fixed. We use saltedpasswords in every TYPO3 installation and it´s a great security-enhancement. But without the logging functionality to TYPO3´s log, it is a step backwards.
If I can help with this issue in any way, don´t hesitate to contact me.
Updated by Mr. Hudson almost 13 years ago
Patch set 1 of change Id3209d09c366c3a662e6a2e037ebe2c74fc2dd6c has been pushed to the review server.
It is available at http://review.typo3.org/1643
Updated by Mr. Hudson almost 13 years ago
Patch set 2 of change Id3209d09c366c3a662e6a2e037ebe2c74fc2dd6c has been pushed to the review server.
It is available at http://review.typo3.org/1643
Updated by Mr. Hudson almost 13 years ago
Patch set 3 of change Id3209d09c366c3a662e6a2e037ebe2c74fc2dd6c has been pushed to the review server.
It is available at http://review.typo3.org/1643
Updated by Mr. Hudson almost 13 years ago
Patch set 4 of change Id3209d09c366c3a662e6a2e037ebe2c74fc2dd6c has been pushed to the review server.
It is available at http://review.typo3.org/1643
Updated by Mr. Hudson almost 13 years ago
Patch set 1 of change Ic05b05873e3fd20df675db908ba76b7dd0e5548f has been pushed to the review server.
It is available at http://review.typo3.org/1795
Updated by Anonymous almost 13 years ago
- Status changed from Accepted to Resolved
- % Done changed from 0 to 100
Applied in changeset 07caa233ab8fa0b5b85519423d109fe5588ed750.
Updated by Steffen Gebert over 12 years ago
- Category set to Backend User Interface
- Assignee deleted (
Marcus Krause) - Target version changed from 0 to 1391
Updated by Steffen Gebert over 12 years ago
- Category changed from Backend User Interface to Authentication
Updated by Ernesto Baschny about 12 years ago
- Target version changed from 1391 to 1264