From 7fa26a80e0d69697dec6b9f5638b2d7f7ba60991 Mon Sep 17 00:00:00 2001 From: fogs Date: Thu, 14 Feb 2019 16:10:38 +0100 Subject: [PATCH] Bugfix #153: log messages must not contain clear text passwords --- Driver/ZendLdapDriver.php | 11 ++++++--- Exception/SanitizingException.php | 20 +++++++++++++++ Tests/Driver/ZendLdapDriverTest.php | 38 +++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 4 deletions(-) create mode 100755 Exception/SanitizingException.php diff --git a/Driver/ZendLdapDriver.php b/Driver/ZendLdapDriver.php index f7baab1..576f392 100644 --- a/Driver/ZendLdapDriver.php +++ b/Driver/ZendLdapDriver.php @@ -7,6 +7,7 @@ use Symfony\Component\Security\Core\User\UserInterface; use Zend\Ldap\Exception\LdapException as ZendLdapException; use Zend\Ldap\Ldap; +use FR3D\LdapBundle\Exception\SanitizingException; /** * This class adapt ldap calls to Zend Framework Ldap library functions. @@ -83,7 +84,7 @@ public function bind(UserInterface $user, $password) return ($bind instanceof Ldap); } catch (ZendLdapException $exception) { - $this->zendExceptionHandler($exception); + $this->zendExceptionHandler($exception, $password); } return false; @@ -94,19 +95,21 @@ public function bind(UserInterface $user, $password) * * @param ZendLdapException $exception */ - protected function zendExceptionHandler(ZendLdapException $exception) + protected function zendExceptionHandler(ZendLdapException $exception, $password) { + $sanitizedException = new SanitizingException($exception, $password); + switch ($exception->getCode()) { // Error level codes case ZendLdapException::LDAP_SERVER_DOWN: if ($this->logger) { - $this->logger->error('{exception}', ['exception' => $exception]); + $this->logger->error('{exception}', ['exception' => $sanitizedException]); } break; // Other level codes default: - $this->logDebug('{exception}', ['exception' => $exception]); + $this->logDebug('{exception}', ['exception' => $sanitizedException]); break; } } diff --git a/Exception/SanitizingException.php b/Exception/SanitizingException.php new file mode 100755 index 0000000..ae2e28f --- /dev/null +++ b/Exception/SanitizingException.php @@ -0,0 +1,20 @@ +actualException = $actualException; + $this->secret = $secret; + } + + public function __toString() + { + return str_replace($this->secret, '****', $this->actualException->__toString()); + } +} diff --git a/Tests/Driver/ZendLdapDriverTest.php b/Tests/Driver/ZendLdapDriverTest.php index 3acaef7..cea5422 100644 --- a/Tests/Driver/ZendLdapDriverTest.php +++ b/Tests/Driver/ZendLdapDriverTest.php @@ -7,6 +7,8 @@ use Symfony\Component\Security\Core\User\UserInterface; use Zend\Ldap\Exception\LdapException as ZendLdapException; use Zend\Ldap\Ldap; +use Psr\Log\LoggerInterface; +use FR3D\LdapBundle\Exception\SanitizingException; /** * Test class for ZendLdapDriver. @@ -88,4 +90,40 @@ public function testFailBindByDn(UserInterface $user, $password) self::assertFalse($this->driver->bind($user, $password)); } + + public function testZendExceptionHandler() + { + $password = 'veryverysecret'; + + $loggerMock = $this->getMockBuilder(LoggerInterface::class) + ->setMethods(['debug', 'error']) + ->getMockForAbstractClass(); + + $zendLdapExceptionMock = $this->getMockBuilder(ZendLdapException::class)->getMock(); + $zendLdapExceptionMock + ->method('__toString') + ->willReturn("Zend\Ldap\Exception\LdapException: fr3d/ldap-bundle/Driver/ZendLdapDriver.php(82): Zend\Ldap\Ldap->bind('fogs', '$password')") + ; + + $loggerMock->method('debug')->with('{exception}', $this->callback(function ($context) use ($password) { + if (!array_key_exists('exception', $context)) { + return $this->fail('Logger context must contain key "exception"'); + } + if (!$context['exception'] instanceof SanitizingException) { + return $this->fail('Logger context "exception" must contain object of class SanitizingException'); + } + if (strpos($context['exception']->__toString(), $password) !== false) { + return $this->fail('String representation of the SanitizingException must not contain the bind password'); + } + return true; + })); + + $reflectionClass = new \ReflectionClass($this->driver); + $reflectionProperty = $reflectionClass->getProperty('logger'); + $reflectionProperty->setAccessible(true); + $reflectionProperty->setValue($this->driver, $loggerMock); + $reflectionMethod = $reflectionClass->getMethod('zendExceptionHandler'); + $reflectionMethod->setAccessible(true); + $reflectionMethod->invoke($this->driver, $zendLdapExceptionMock, $password); + } }