From e8063a08462f588b40fa23809ea83b8ef9c70b21 Mon Sep 17 00:00:00 2001 From: Dayllan Maza Date: Thu, 17 May 2018 01:55:02 -0400 Subject: [PATCH] Send a cookie with IP/IP-Range blocks when blocking logged-out users A cookie will be set when ip users try to edit and their IP has been blocked or if they try to create an account and the block prevents account creation This feature is disabled by default and can be enabled by setting the new $wgCookieSetOnIpBlock config variable to true. Note: this is meant to discourage vandals that try to avoid blocks by switching their ip address while editing anonymously. Bug: T152462 Change-Id: I0b78a5e174bcd882edea39e868a08f9a347f5aba --- RELEASE-NOTES-1.32 | 5 + includes/DefaultSettings.php | 9 ++ includes/EditPage.php | 4 + includes/specials/SpecialCreateAccount.php | 4 + includes/user/User.php | 74 ++++++++++---- tests/phpunit/includes/user/UserTest.php | 109 +++++++++++++++++++++ 6 files changed, 185 insertions(+), 20 deletions(-) diff --git a/RELEASE-NOTES-1.32 b/RELEASE-NOTES-1.32 index f1efd4fc03..75f605a658 100644 --- a/RELEASE-NOTES-1.32 +++ b/RELEASE-NOTES-1.32 @@ -21,6 +21,9 @@ production. adds a defense-in-depth feature to stop an attacker who has found a bug in the parser allowing them to insert malicious attributes. Disabled by default, you can configure this via $wgCSPHeader and $wgCSPReportOnlyHeader. +* New configuration variable has been added: $wgCookieSetOnIpBlock. + This determines whether to set a cookie when an IP user is blocked. Doing so means + that a blocked user, even after moving to a new IP address, will still be blocked. === New features in 1.32 === * (T112474) Generalized the ResourceLoader mechanism for overriding modules @@ -28,6 +31,8 @@ production. * Added 'ApiParseMakeOutputPage' hook. * (T174313) Added checkbox on Special:ListUsers to display only users in temporary user groups. +* (T152462) A cookie can now be set when an IP user is blocked to track that user if + they move to a new IP address. This is disabled by default. === External library changes in 1.32 === * … diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 87ca0168bd..e529d2dd9b 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -6043,6 +6043,15 @@ $wgSessionName = false; */ $wgCookieSetOnAutoblock = false; +/** + * Whether to set a cookie when a logged-out user is blocked. Doing so means that a blocked user, + * even after moving to a new IP address, will still be blocked. This cookie will contain an + * authentication code if $wgSecretKey is set, or otherwise will just be the block ID (in which + * case there is a possibility of an attacker discovering the names of revdeleted users, so it + * is best to use this in conjunction with $wgSecretKey being set). + */ +$wgCookieSetOnIpBlock = false; + /** @} */ # end of cookie settings } /************************************************************************//** diff --git a/includes/EditPage.php b/includes/EditPage.php index 67ce1f3c11..0026ba87c7 100644 --- a/includes/EditPage.php +++ b/includes/EditPage.php @@ -587,6 +587,10 @@ class EditPage { $permErrors = $this->getEditPermissionErrors( $this->save ? 'secure' : 'full' ); if ( $permErrors ) { wfDebug( __METHOD__ . ": User can't edit\n" ); + + // track block with a cookie if it doesn't exists already + $this->context->getUser()->trackBlockWithCookie(); + // Auto-block user's IP if the account was "hard" blocked if ( !wfReadOnly() ) { DeferredUpdates::addCallableUpdate( function () { diff --git a/includes/specials/SpecialCreateAccount.php b/includes/specials/SpecialCreateAccount.php index 73beafce0f..2f87c478bb 100644 --- a/includes/specials/SpecialCreateAccount.php +++ b/includes/specials/SpecialCreateAccount.php @@ -63,6 +63,10 @@ class SpecialCreateAccount extends LoginSignupSpecialPage { $user = $this->getUser(); $status = AuthManager::singleton()->checkAccountCreatePermissions( $user ); if ( !$status->isGood() ) { + // track block with a cookie if it doesn't exists already + if ( $user->isBlockedFromCreateAccount() ) { + $user->trackBlockWithCookie(); + } throw new ErrorPageError( 'createacct-error', $status->getMessage() ); } } diff --git a/includes/user/User.php b/includes/user/User.php index 7189bea120..b5fa97ff2c 100644 --- a/includes/user/User.php +++ b/includes/user/User.php @@ -1345,32 +1345,54 @@ class User implements IDBAccessObject, UserIdentity { $user = $session->getUser(); if ( $user->isLoggedIn() ) { $this->loadFromUserObject( $user ); - - // If this user is autoblocked, set a cookie to track the Block. This has to be done on - // every session load, because an autoblocked editor might not edit again from the same - // IP address after being blocked. - $config = RequestContext::getMain()->getConfig(); - if ( $config->get( 'CookieSetOnAutoblock' ) === true ) { - $block = $this->getBlock(); - $shouldSetCookie = $this->getRequest()->getCookie( 'BlockID' ) === null - && $block - && $block->getType() === Block::TYPE_USER - && $block->isAutoblocking(); - if ( $shouldSetCookie ) { - wfDebug( __METHOD__ . ': User is autoblocked, setting cookie to track' ); - $block->setCookie( $this->getRequest()->response() ); - } + if ( $user->isBlocked() ) { + // If this user is autoblocked, set a cookie to track the Block. This has to be done on + // every session load, because an autoblocked editor might not edit again from the same + // IP address after being blocked. + $this->trackBlockWithCookie(); } // Other code expects these to be set in the session, so set them. $session->set( 'wsUserID', $this->getId() ); $session->set( 'wsUserName', $this->getName() ); $session->set( 'wsToken', $this->getToken() ); + return true; } + return false; } + /** + * Set the 'BlockID' cookie depending on block type and user authentication status. + */ + public function trackBlockWithCookie() { + $block = $this->getBlock(); + if ( $block && $this->getRequest()->getCookie( 'BlockID' ) === null ) { + $config = RequestContext::getMain()->getConfig(); + $shouldSetCookie = false; + + if ( $this->isAnon() && $config->get( 'CookieSetOnIpBlock' ) ) { + // If user is logged-out, set a cookie to track the Block + $shouldSetCookie = in_array( $block->getType(), [ + Block::TYPE_IP, Block::TYPE_RANGE + ] ); + if ( $shouldSetCookie ) { + $block->setCookie( $this->getRequest()->response() ); + + // temporary measure the use of cookies on ip blocks + $stats = MediaWikiServices::getInstance()->getStatsdDataFactory(); + $stats->increment( 'block.ipblock.setCookie.success' ); + } + } elseif ( $this->isLoggedIn() && $config->get( 'CookieSetOnAutoblock' ) ) { + $shouldSetCookie = $block->getType() === Block::TYPE_USER && $block->isAutoblocking(); + if ( $shouldSetCookie ) { + $block->setCookie( $this->getRequest()->response() ); + } + } + } + } + /** * Load user and user_group data from the database. * $this->mId must be set, this is how the user is identified. @@ -1896,12 +1918,24 @@ class User implements IDBAccessObject, UserIdentity { // An ID was found in the cookie. $tmpBlock = Block::newFromID( $blockCookieId ); if ( $tmpBlock instanceof Block ) { - // Check the validity of the block. - $blockIsValid = $tmpBlock->getType() == Block::TYPE_USER - && !$tmpBlock->isExpired() - && $tmpBlock->isAutoblocking(); $config = RequestContext::getMain()->getConfig(); - $useBlockCookie = ( $config->get( 'CookieSetOnAutoblock' ) === true ); + + switch ( $tmpBlock->getType() ) { + case Block::TYPE_USER: + $blockIsValid = !$tmpBlock->isExpired() && $tmpBlock->isAutoblocking(); + $useBlockCookie = ( $config->get( 'CookieSetOnAutoblock' ) === true ); + break; + case Block::TYPE_IP: + case Block::TYPE_RANGE: + // If block is type IP or IP range, load only if user is not logged in (T152462) + $blockIsValid = !$tmpBlock->isExpired() && !$this->isLoggedIn(); + $useBlockCookie = ( $config->get( 'CookieSetOnIpBlock' ) === true ); + break; + default: + $blockIsValid = false; + $useBlockCookie = false; + } + if ( $blockIsValid && $useBlockCookie ) { // Use the block. return $tmpBlock; diff --git a/tests/phpunit/includes/user/UserTest.php b/tests/phpunit/includes/user/UserTest.php index e819d35e32..3eb6abd0c1 100644 --- a/tests/phpunit/includes/user/UserTest.php +++ b/tests/phpunit/includes/user/UserTest.php @@ -1199,4 +1199,113 @@ class UserTest extends MediaWikiTestCase { $this->assertFalse( $user->isBlockedFrom( $ut ) ); } + /** + * Block cookie should be set for IP Blocks if + * wgCookieSetOnIpBlock is set to true + */ + public function testIpBlockCookieSet() { + $this->setMwGlobals( [ + 'wgCookieSetOnIpBlock' => true, + 'wgCookiePrefix' => 'wiki', + 'wgSecretKey' => MWCryptRand::generateHex( 64, true ), + ] ); + + // setup block + $block = new Block( [ + 'expiry' => wfTimestamp( TS_MW, wfTimestamp() + ( 5 * 60 * 60 ) ), + ] ); + $block->setTarget( '1.2.3.4' ); + $block->setBlocker( $this->getTestSysop()->getUser() ); + $block->insert(); + + // setup request + $request = new FauxRequest(); + $request->setIP( '1.2.3.4' ); + + // get user + $user = User::newFromSession( $request ); + $user->trackBlockWithCookie(); + + // test cookie was set + $cookies = $request->response()->getCookies(); + $this->assertArrayHasKey( 'wikiBlockID', $cookies ); + + // clean up + $block->delete(); + } + + /** + * Block cookie should NOT be set when wgCookieSetOnIpBlock + * is disabled + */ + public function testIpBlockCookieNotSet() { + $this->setMwGlobals( [ + 'wgCookieSetOnIpBlock' => false, + 'wgCookiePrefix' => 'wiki', + 'wgSecretKey' => MWCryptRand::generateHex( 64, true ), + ] ); + + // setup block + $block = new Block( [ + 'expiry' => wfTimestamp( TS_MW, wfTimestamp() + ( 5 * 60 * 60 ) ), + ] ); + $block->setTarget( '1.2.3.4' ); + $block->setBlocker( $this->getTestSysop()->getUser() ); + $block->insert(); + + // setup request + $request = new FauxRequest(); + $request->setIP( '1.2.3.4' ); + + // get user + $user = User::newFromSession( $request ); + $user->trackBlockWithCookie(); + + // test cookie was not set + $cookies = $request->response()->getCookies(); + $this->assertArrayNotHasKey( 'wikiBlockID', $cookies ); + + // clean up + $block->delete(); + } + + /** + * When an ip user is blocked and then they log in, cookie block + * should be invalid and the cookie removed. + */ + public function testIpBlockCookieIgnoredWhenUserLoggedIn() { + $this->setMwGlobals( [ + 'wgAutoblockExpiry' => 8000, + 'wgCookieSetOnIpBlock' => true, + 'wgCookiePrefix' => 'wiki', + 'wgSecretKey' => MWCryptRand::generateHex( 64, true ), + ] ); + + // setup block + $block = new Block( [ + 'expiry' => wfTimestamp( TS_MW, wfTimestamp() + ( 40 * 60 * 60 ) ), + ] ); + $block->setTarget( '1.2.3.4' ); + $block->setBlocker( $this->getTestSysop()->getUser() ); + $block->insert(); + + // setup request + $request = new FauxRequest(); + $request->setIP( '1.2.3.4' ); + $request->getSession()->setUser( $this->getTestUser()->getUser() ); + $request->setCookie( 'BlockID', $block->getCookieValue() ); + + // setup user + $user = User::newFromSession( $request ); + + // logged in users should be inmune to cookie block of type ip/range + $this->assertFalse( $user->isBlocked() ); + + // cookie is being cleared + $cookies = $request->response()->getCookies(); + $this->assertEquals( '', $cookies['wikiBlockID']['value'] ); + + // clean up + $block->delete(); + } } -- 2.20.1