Skip to content

Commit

Permalink
Don't log credentials of LoginController::tryLogin - fixes #25895
Browse files Browse the repository at this point in the history
  • Loading branch information
@DeepDiver1975
DeepDiver1975 committed Aug 22, 2016
1 parent a9e633d commit b29f1c9
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 6 deletions.
7 changes: 5 additions & 2 deletions lib/private/Log.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ class Log implements ILogger {
'calculateHMAC',
'encrypt',
'decrypt',

//LoginController
'tryLogin'
];

/**
Expand Down Expand Up @@ -304,14 +307,14 @@ public function log($level, $message, array $context = array()) {
* @since 8.2.0
*/
public function logException($exception, array $context = array()) {
$exception = array(
$exception = [
'Exception' => get_class($exception),
'Message' => $exception->getMessage(),
'Code' => $exception->getCode(),
'Trace' => $exception->getTraceAsString(),
'File' => $exception->getFile(),
'Line' => $exception->getLine(),
);
];
$exception['Trace'] = preg_replace('!(' . implode('|', $this->methodsWithSensitiveParameters) . ')\(.*\)!', '$1(*** sensitive parameters replaced ***)', $exception['Trace']);
$msg = isset($context['message']) ? $context['message'] : 'Exception';
$msg .= ': ' . json_encode($exception);
Expand Down
27 changes: 23 additions & 4 deletions tests/lib/LoggerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,17 @@
namespace Test;

use OC\Log;
use OCP\IConfig;
use OCP\Util;

class LoggerTest extends TestCase {
/**
* @var \OCP\ILogger
*/
/** @var \OCP\ILogger */
private $logger;
static private $logs = array();

/** @var IConfig | \PHPUnit_Framework_MockObject_MockObject */
private $config;

protected function setUp() {
parent::setUp();

Expand All @@ -40,7 +43,7 @@ public function testAppCondition() {
$this->config->expects($this->any())
->method('getValue')
->will(($this->returnValueMap([
['loglevel', \OCP\Util::WARN, \OCP\Util::WARN],
['loglevel', Util::WARN, Util::WARN],
['log.condition', [], ['apps' => ['files']]]
])));
$logger = $this->logger;
Expand Down Expand Up @@ -122,4 +125,20 @@ public function testDetectvalidateUserPass($user, $password) {
$this->assertContains('validateUserPass(*** sensitive parameters replaced ***)', $logLine);
}
}

/**
* @dataProvider userAndPasswordData
*/
public function testDetecttryLogin($user, $password) {
$e = new \Exception('test');
$this->logger->logException($e);
$logLines = $this->getLogs();

foreach($logLines as $logLine) {
$this->assertNotContains($user, $logLine);
$this->assertNotContains($password, $logLine);
$this->assertContains('tryLogin(*** sensitive parameters replaced ***)', $logLine);
}
}

}

1 comment on commit b29f1c9

@kostas707
Copy link

@kostas707 kostas707 commented on b29f1c9 Aug 23, 2016 •
edited by DeepDiver1975
Loading

Choose a reason for hiding this comment

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

Thanks, for quick response. I've checked #25902. When login with fake user, no password is showing. But when connect with exist Domain username and password is correct, then password is still in log.

{"reqId":"MTRK5tqbshYzakztCAoW","remoteAddr":"192.168.100.15",
"app":"user_ldap",
"message":"Exception: {
"Exception":"Exception",
"Message":"No user available for the given login name on 10.90.2.23:10389",
"Code":0,"Trace":"
#0 /var/www/owncloud/apps/user_ldap/lib/User_LDAP.php(120): 
        OCA-User_LDAP-User_LDAP->getLDAPUserByLoginName('domain.username...') 
#1 [internal function]: OCA-User_LDAP-User_LDAP->checkPassword(*** sensitive parameters replaced ***) 
#2 /var/www/owncloud/apps/user_ldap/lib/User_Proxy.php(67): call_user_func_array(Array, Array) 
#3 /var/www/owncloud/apps/user_ldap/lib/Proxy.php(139): 
        OCA-User_LDAP-User_Proxy->walkBackends('domain.username...', 'checkPassword', Array) 
#4 /var/www/owncloud/apps/user_ldap/lib/User_Proxy.php(182): 
        OCA-User_LDAP-Proxy->handleRequest('domain.username...', 'checkPassword', Array) 
#5 /var/www/owncloud/lib/private/User/Manager.php(190): 
        OCA-User_LDAP-User_Proxy->checkPassword(*** sensitive parameters replaced ***) 
#6 /var/www/owncloud/core/Controller/LoginController.php(177): 
        OC-User-Manager->checkPassword(*** sensitive parameters replaced ***) 
#7 [internal function]: OC-Core-Controller-LoginController->tryLogin(*** sensitive parameters replaced ***) 
#8 /var/www/owncloud/lib/private/AppFramework/Http/Dispatcher.php(159): call_user_func_array(Array, Array) 
#9 /var/www/owncloud/lib/private/AppFramework/Http/Dispatcher.php(89): 
        OC-AppFramework-Http-Dispatcher-> 
        executeController(Object(OC-Core-Controller-LoginController), 'tryLogin') 
#10 /var/www/owncloud/lib/private/AppFramework/App.php(110): 
        OC-AppFramework-Http-Dispatcher->dispatch(Object(OC-Core-Controller-LoginController), 'tryLogin') 
#11 /var/www/owncloud/lib/private/AppFramework/Routing/RouteActionHandler.php(46):      
        OC-AppFramework-App::main('LoginController', 'tryLogin', 
        Object(OC-AppFramework-DependencyInjection-DIContainer), Array) 
#12 [internal function]: OC-AppFramework-Routing-RouteActionHandler->__invoke(Array) 
#13 /var/www/owncloud/lib/private/Route/Router.php(280): 
        call_user_func(Object(OC-AppFramework-Routing-RouteActionHandler), Array) 
#14 /var/www/owncloud/lib/base.php(891): OC-Route-Router->match('/login') 
#15 /var/www/owncloud/index.php(39): OC::handleRequest() 
#16 {main}","File":"/var/www/owncloud/apps/user_ldap/lib/User_LDAP.php","Line":104}",
"level":3,
"time":"2016-08-23T06:03:08+00:00",
"method":"POST",
"url":"/index.php/login",
"user":"--"}

{
"reqId":"MTRK5tqbshYzakztCAoW",
"remoteAddr":"192.168.100.15",
"app":"user_ldap",
"message":"Exception: 
{"Exception":"Exception",
"Message":"No user available for the given login name on 10.90.2.23:10389",
"Code":0,"Trace":"
#0 /var/www/owncloud/apps/user_ldap/lib/User_LDAP.php(120): 
        OCA-User_LDAP-User_LDAP->getLDAPUserByLoginName('domain.username...') 
#1 [internal function]: OCA-User_LDAP-User_LDAP->checkPassword(*** sensitive parameters replaced ***) 
#2 /var/www/owncloud/apps/user_ldap/lib/User_Proxy.php(67): call_user_func_array(Array, Array) 
#3 /var/www/owncloud/apps/user_ldap/lib/Proxy.php(139): 
        OCA-User_LDAP-User_Proxy->walkBackends('domain.username...', 'checkPassword', Array) 
#4 /var/www/owncloud/apps/user_ldap/lib/User_Proxy.php(182): 
        OCA-User_LDAP-Proxy->handleRequest('domain.username...', 'checkPassword', Array) 
#5 /var/www/owncloud/lib/private/User/Manager.php(190): 
        OCA-User_LDAP-User_Proxy->checkPassword(*** sensitive parameters replaced ***) 
#6 /var/www/owncloud/lib/private/User/Session.php(427): 
        OC-User-Manager->checkPassword(*** sensitive parameters replaced ***) 
#7 /var/www/owncloud/lib/private/User/Session.php(287): 
        OC-User-Session->loginWithPassword('domain.username...', 

---> 'plain_password!!!') ---<

#8 /var/www/owncloud/core/Controller/LoginController.php(196): 
        OC-User-Session->login(*** sensitive parameters replaced ***) 
#9 [internal function]: OC-Core-Controller-LoginController->tryLogin(*** sensitive parameters replaced ***) 
#10 /var/www/owncloud/lib/private/AppFramework/Http/Dispatcher.php(159): call_user_func_array(Array, Array) 
#11 /var/www/owncloud/lib/private/AppFramework/Http/Dispatcher.php(89): 
        OC-AppFramework-Http-Dispatcher-> executeController(Object(OC-Core-Controller-LoginController), 'tryLogin') 
#12 /var/www/owncloud/lib/private/AppFramework/App.php(110): 
        OC-AppFramework-Http-Dispatcher->dispatch(Object(OC-Core-Controller-LoginController), 'tryLogin') 
#13 /var/www/owncloud/lib/private/AppFramework/Routing/RouteActionHandler.php(46):          
         OC-AppFramework-App::main('LoginController', 'tryLogin', Object
        (OC-AppFramework-DependencyInjection-DIContainer), Array) 
#14 [internal function]: OC-AppFramework-Routing-RouteActionHandler->__invoke(Array) 
#15 /var/www/owncloud/lib/private/Route/Router.php(280): 
        call_user_func(Object(OC-AppFramework-Routing-RouteActionHandler), Array) 
#16 /var/www/owncloud/lib/base.php(891): OC-Route-Router->match('/login') 
#17 /var/www/owncloud/index.php(39): OC::handleRequest() 
#18 {main}",
"File":"/var/www/owncloud/apps/user_ldap/lib/User_LDAP.php",
"Line":104
}",
"level":3,
"time":"2016-08-23T06:03:08+00:00",
"method":"POST",
"url":"/index.php/login","user":"--"}
{"reqId":"PEaoW0KkIpTbe3dR3OT7",
"remoteAddr":"192.168.100.15"
,"app":"user_ldap",
"message":"Exception: {
Exception":"Exception",
"Message":"No user available for the given login name on 10.90.2.23:10389",
"Code":0,"Trace":"
#0 /var/www/owncloud/apps/user_ldap/lib/User_LDAP.php(120): 
        OCA-User_LDAP-User_LDAP->getLDAPUserByLoginName('domain.username...') 
#1 [internal function]: OCA-User_LDAP-User_LDAP->checkPassword(*** sensitive parameters replaced ***) 
#2 /var/www/owncloud/apps/user_ldap/lib/User_Proxy.php(67): call_user_func_array(Array, Array) 
#3 /var/www/owncloud/apps/user_ldap/lib/Proxy.php(139): OCA-User_LDAP-User_Proxy->walkBackends('domain.username...', 'checkPassword', Array) 
#4 /var/www/owncloud/apps/user_ldap/lib/User_Proxy.php(182): 
        OCA-User_LDAP-Proxy->handleRequest('domain.username...', 'checkPassword', Array) 
#5 /var/www/owncloud/lib/private/User/Manager.php(190): 
        OCA-User_LDAP-User_Proxy->checkPassword(*** sensitive parameters replaced ***) 
#6 /var/www/owncloud/lib/private/User/Session.php(591): 
        OC-User-Manager->checkPassword(*** sensitive parameters replaced ***) 
#7 /var/www/owncloud/lib/private/User/Session.php(626): 
        OC-User-Session->checkTokenCredentials(Object(OC-Authentication-Token-DefaultToken), 'vphb8idv8kgpc4i...') 
#8 /var/www/owncloud/lib/private/User/Session.php(221): 
        OC-User-Session->validateToken(*** sensitive parameters replaced ***) 
#9 /var/www/owncloud/lib/private/User/Session.php(196): OC-User-Session->validateSession() 
#10 /var/www/owncloud/lib/private/App/AppManager.php(152): OC-User-Session->getUser() 
#11 /var/www/owncloud/lib/private/legacy/app.php(313): OC-App-AppManager->isEnabledForUser('user_webdavauth') 
#12 /var/www/owncloud/lib/public/App.php(131): OC_App::isEnabled('user_webdavauth') 
#13 /var/www/owncloud/apps/user_ldap/appinfo/app.php(72): OCP-App::isEnabled('user_webdavauth') 
#14 /var/www/owncloud/lib/private/legacy/app.php(186): require_once('/var/www/ownclo...') 
#15 /var/www/owncloud/lib/private/legacy/app.php(149): OC_App::requireAppFile('user_ldap') 
#16 /var/www/owncloud/lib/private/legacy/app.php(119): OC_App::loadApp('user_ldap') 
#17 /var/www/owncloud/lib/base.php(861): OC_App::loadApps(Array) 
#18 /var/www/owncloud/index.php(39): OC::handleRequest() 
#19 {main}","File":"/var/www/owncloud/apps/user_ldap/lib/User_LDAP.php","Line":104}",
"level":3,
"time":"2016-08-23T06:03:08+00:00",
"method":"GET",
"url":"/index.php/apps/files/",
"user":"domain.username"}

Please sign in to comment.