Project

General

Profile

Actions

Bug #23917

closed

Failed userlogins are not written to syslog using rsaauth and saltedpasswords

Added by Torben Hansen over 13 years ago. Updated over 5 years ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
Authentication
Target version:
-
Start date:
2010-11-02
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
4.4
PHP Version:
5.3
Tags:
Complexity:
Is Regression:
Sprint Focus:

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

class.tx_saltedpasswords_sv1.php (13 KB) class.tx_saltedpasswords_sv1.php Administrator Admin, 2010-11-02 18:22
16223.diff (2.88 KB) 16223.diff Administrator Admin, 2010-12-02 22:42
16223_2.diff (1.69 KB) 16223_2.diff Administrator Admin, 2011-03-01 09:29

Related issues 1 (0 open1 closed)

Has duplicate TYPO3 Core - Bug #21972: No entry in sys_log at loginClosed2010-01-16

Actions
Actions #1

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.

Actions #2

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.

Actions #3

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?

Actions #4

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?

Actions #5

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.

Actions #6

Updated by Torben Hansen over 13 years ago

OK, thanks a lot Steffen. Lets wait and hear what Marcus says to this.

Actions #7

Updated by Torben Hansen about 13 years ago

Hi Steffen,

did you get any feedback?

Actions #8

Updated by Steffen Gebert about 13 years ago

nope

Actions #9

Updated by Marcus Krause about 13 years ago

I'll look at it today.

Actions #10

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.

Actions #11

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

Actions #12

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

Actions #13

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

Actions #14

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

Actions #15

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

Actions #16

Updated by Anonymous almost 13 years ago

  • Status changed from Accepted to Resolved
  • % Done changed from 0 to 100
Actions #17

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
Actions #18

Updated by Steffen Gebert over 12 years ago

  • Category changed from Backend User Interface to Authentication
Actions #19

Updated by Ernesto Baschny about 12 years ago

  • Target version changed from 1391 to 1264
Actions #20

Updated by Ernesto Baschny almost 11 years ago

  • Target version deleted (1264)
Actions #21

Updated by Benni Mack over 5 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF