From f22549a60539c9aa5c5390c8417c984ba8eef5b2 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Gerg=C5=91=20Tisza?= Date: Sun, 7 Feb 2016 11:24:05 -0800 Subject: [PATCH] Log multiple IPs using the same session or the same user account As an attempt to detect SessionManager errors that log people into the wrong account, log multiple IPs using the same session, or the same user account. Bug: T125455 Change-Id: I27468a3f6d582d9b46984227b9307dc71190fd6a --- includes/DefaultSettings.php | 22 +++++ includes/Setup.php | 5 + includes/session/SessionManager.php | 91 +++++++++++++++++++ .../includes/session/SessionManagerTest.php | 70 ++++++++++++++ 4 files changed, 188 insertions(+) diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index a6a0c75b54..b4d625c9cb 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -2305,6 +2305,28 @@ $wgSessionHandler = null; */ $wgPHPSessionHandling = 'enable'; +/** + * The number of different IPs in the same session within a period of $wgSuspiciousIpExpiry + * that should cause warnings to be logged. This is meant more for debugging errors in the + * authentication system than for detecting abuse. + * @since 1.27 + */ +$wgSuspiciousIpPerSessionLimit = 2; + +/** + * Like $wgSuspiciousIpPerSessionLimit but over all requests from the same user within + * $wgSuspiciousIpExpiry, whether they are in the same session or not. + * @since 1.27 + */ +$wgSuspiciousIpPerUserLimit = 5; + +/** + * Time in seconds to remember IPs for, for the purposes of $wgSuspiciousIpPerSessionLimit and + * $wgSuspiciousIpPerUserLimit. + * @since 1.27 + */ +$wgSuspiciousIpExpiry = 600; + /** * If enabled, will send MemCached debugging information to $wgDebugLogFile */ diff --git a/includes/Setup.php b/includes/Setup.php index b9a1c377bb..3ceb5585de 100644 --- a/includes/Setup.php +++ b/includes/Setup.php @@ -809,5 +809,10 @@ if ( !defined( 'MW_NO_SESSION' ) && !$wgCommandLineMode ) { wfDebug( "Fully initialised\n" ); $wgFullyInitialised = true; +// T125455 +if ( !defined( 'MW_NO_SESSION' ) && !$wgCommandLineMode ) { + MediaWiki\Session\SessionManager::singleton()->checkIpLimits(); +} + Profiler::instance()->scopedProfileOut( $ps_extensions ); Profiler::instance()->scopedProfileOut( $ps_setup ); diff --git a/includes/session/SessionManager.php b/includes/session/SessionManager.php index 83c30aba3b..2dc8910c0a 100644 --- a/includes/session/SessionManager.php +++ b/includes/session/SessionManager.php @@ -24,6 +24,7 @@ namespace MediaWiki\Session; use Psr\Log\LoggerInterface; +use Psr\Log\LogLevel; use BagOStuff; use CachedBagOStuff; use Config; @@ -1053,6 +1054,96 @@ final class SessionManager implements SessionManagerInterface { self::$globalSessionRequest = null; } + /** + * Do a sanity check to make sure the session is not used from many different IP addresses + * and store some data for later sanity checks. + * FIXME remove this once SessionManager is considered stable + * @private For use in Setup.php only + * @param Session $session Defaults to the global session. + */ + public function checkIpLimits( Session $session = null ) { + $session = $session ?: self::getGlobalSession(); + + try { + $ip = $session->getRequest()->getIP(); + } catch ( \MWException $e ) { + return; + } + if ( $ip === '127.0.0.1' || \IP::isConfiguredProxy( $ip ) ) { + return; + } + $now = time(); + + // Record (and possibly log) that the IP is using the current session. + // Don't touch the stored data unless we are adding a new IP or re-adding an expired one. + // This is slightly inaccurate (when an existing IP is seen again, the expiry is not + // extended) but that shouldn't make much difference and limits the session write frequency + // to # of IPs / $wgSuspiciousIpExpiry. + $data = $session->get( 'SessionManager-ip', array() ); + if ( + !isset( $data[$ip] ) + || $data[$ip] < $now + ) { + $data[$ip] = time() + $this->config->get( 'SuspiciousIpExpiry' ); + foreach ( $data as $key => $expires ) { + if ( $expires < $now ) { + unset( $data[$key] ); + } + } + $session->set( 'SessionManager-ip', $data ); + + $logger = \MediaWiki\Logger\LoggerFactory::getInstance( 'session-ip' ); + $logLevel = count( $data ) >= $this->config->get( 'SuspiciousIpPerSessionLimit' ) + ? LogLevel::WARNING : ( count( $data ) === 1 ? LogLevel::DEBUG : LogLevel::INFO ); + $logger->log( + $logLevel, + 'Same session used from {count} IPs', + array( + 'count' => count( $data ), + 'ips' => $data, + 'session' => $session->getId(), + 'user' => $session->getUser()->getName(), + 'persistent' => $session->isPersistent(), + ) + ); + } + + // Now do the same thing globally for the current user. + // We are using the object cache and assume it is shared between all wikis of a farm, + // and further assume that the same name belongs to the same user on all wikis. (It's either + // that or a central ID lookup which would mean an extra SQL query on every request.) + if ( $session->getUser()->isLoggedIn() ) { + $userKey = 'SessionManager-ip:' . md5( $session->getUser()->getName() ); + $data = $this->store->get( $userKey ) ?: array(); + if ( + !isset( $data[$ip] ) + || $data[$ip] < $now + ) { + $data[$ip] = time() + $this->config->get( 'SuspiciousIpExpiry' ); + foreach ( $data as $key => $expires ) { + if ( $expires < $now ) { + unset( $data[$key] ); + } + } + $this->store->set( $userKey, $data, $this->config->get( 'SuspiciousIpExpiry' ) ); + $logger = \MediaWiki\Logger\LoggerFactory::getInstance( 'session-ip' ); + $logLevel = count( $data ) >= $this->config->get( 'SuspiciousIpPerUserLimit' ) + ? LogLevel::WARNING : ( count( $data ) === 1 ? LogLevel::DEBUG : LogLevel::INFO ); + $logger->log( + $logLevel, + 'Same user had sessions from {count} IPs', + array( + 'count' => count( $data ), + 'ips' => $data, + 'session' => $session->getId(), + 'user' => $session->getUser()->getName(), + 'persistent' => $session->isPersistent(), + ) + ); + } + } + } + /**@}*/ } diff --git a/tests/phpunit/includes/session/SessionManagerTest.php b/tests/phpunit/includes/session/SessionManagerTest.php index 16beb7299e..4424735461 100644 --- a/tests/phpunit/includes/session/SessionManagerTest.php +++ b/tests/phpunit/includes/session/SessionManagerTest.php @@ -3,6 +3,7 @@ namespace MediaWiki\Session; use AuthPlugin; +use MediaWiki\Logger\LoggerFactory; use MediaWikiTestCase; use Psr\Log\LogLevel; use User; @@ -1677,4 +1678,73 @@ class SessionManagerTest extends MediaWikiTestCase { $logger->clearBuffer(); } + /** + * @dataProvider provideCheckIpLimits + */ + public function testCheckIpLimits( $ip, $sessionData, $userData, $logLevel1, $logLevel2 ) { + $this->setMwGlobals( array( + 'wgSuspiciousIpPerSessionLimit' => 5, + 'wgSuspiciousIpPerUserLimit' => 10, + 'wgSuspiciousIpExpiry' => 600, + 'wgSquidServers' => array( '11.22.33.44' ), + ) ); + $manager = new SessionManager(); + $logger = $this->getMock( '\Psr\Log\LoggerInterface' ); + $this->setLogger( 'session-ip', $logger ); + $request = new \FauxRequest(); + $request->setIP( $ip ); + + $session = $manager->getSessionForRequest( $request ); + /** @var SessionBackend $backend */ + $backend = \TestingAccessWrapper::newFromObject( $session )->backend; + $data = &$backend->getData(); + $data = array( 'SessionManager-ip' => $sessionData ); + $backend->setUser( User::newFromName( 'UTSysop' ) ); + $manager = \TestingAccessWrapper::newFromObject( $manager ); + $manager->store->set( 'SessionManager-ip:' . md5( 'UTSysop' ), $userData ); + + $logger->expects( $this->exactly( isset( $logLevel1 ) + isset( $logLevel2 ) ) )->method( 'log' ); + if ( $logLevel1 ) { + $logger->expects( $this->at( 0 ) )->method( 'log' )->with( $logLevel1, + 'Same session used from {count} IPs', $this->isType( 'array' ) ); + } + if ( $logLevel2 ) { + $logger->expects( $this->at( isset( $logLevel1 ) ) )->method( 'log' )->with( $logLevel2, + 'Same user had sessions from {count} IPs', $this->isType( 'array' ) ); + } + + $manager->checkIpLimits( $session ); + } + + public function provideCheckIpLimits() { + $future = time() + 1000; + $past = time() - 1000; + return array( + // DEBUG log for first new IP + array( '1.2.3.4', array(), array(), LogLevel::DEBUG, LogLevel::DEBUG ), + // no log for same IP + array( '1.2.3.4', array( '1.2.3.4' => $future ), array( '1.2.3.4' => $future ), + null, null ), + array( '1.2.3.4', array(), array( '1.2.3.4' => $future ), + LogLevel::DEBUG, null ), + // INFO log for second new IP + array( '1.2.3.4', array( '10.20.30.40' => $future ), array( '10.20.30.40' => $future ), + LogLevel::INFO, LogLevel::INFO ), + // WARNING above $wgSuspiciousIpPerSessionLimit + array( '1.2.3.4', array_fill_keys( range( 1, 5 ), $future ), + array_fill_keys( range( 1, 5 ), $future ), LogLevel::WARNING, LogLevel::INFO ), + // WARNING above $wgSuspiciousIpPerUserLimit + + array( '1.2.3.4', array_fill_keys( range( 1, 2 ), $future ), + array_fill_keys( range( 1, 12 ), $future ), LogLevel::INFO, LogLevel::WARNING ), + // expired keys ignored + array( '1.2.3.4', array( '1.2.3.4' => $past ), array( '1.2.3.4' => $past ), + LogLevel::DEBUG, LogLevel::DEBUG ), + array( '1.2.3.4', array_fill_keys( range( 1, 5 ), $past ), + array_fill_keys( range( 1, 5 ), $past ), LogLevel::DEBUG, LogLevel::DEBUG ), + // special IPs are ignored + array( '127.0.0.1', array(), array(), null, null ), + array( '11.22.33.44', array(), array(), null, null ), + ); + } } -- 2.20.1