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

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index a6a0c75..b4d625c 100644 (file)
--- 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 b9a1c37..3ceb558 100644 (file)
--- 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 83c30ab..2dc8910 100644 (file)
--- 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 16beb72..4424735 100644 (file)
--- 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 ),
+               );
+       }
 }