From: Aaron Schulz Date: Fri, 29 Mar 2013 19:13:35 +0000 (+0000) Subject: Revert "Apply IP blocks to X-Forwarded-For header" X-Git-Tag: 1.31.0-rc.0~20174^2 X-Git-Url: https://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/banques/ajouter.php?a=commitdiff_plain;h=4ba66e41b5b52ac17a3bcdc58937b2739743ccc7;p=lhc%2Fweb%2Fwiklou.git Revert "Apply IP blocks to X-Forwarded-For header" Test are now starting to fail for everything. This reverts commit a5d70e3ae6b43743b63f8d4e8efdfd6e26e35d40 Change-Id: I30c9eb9c00be12ff080e85452e17c2a310f03bd3 --- diff --git a/RELEASE-NOTES-1.21 b/RELEASE-NOTES-1.21 index ff0569e7b8..56d812cb46 100644 --- a/RELEASE-NOTES-1.21 +++ b/RELEASE-NOTES-1.21 @@ -125,8 +125,6 @@ production. correctly. * (bug 45803) Whitespace within == Headline == syntax and within headings is now non-significant and not preserved in the HTML output. -* (bug 23343) Implemented ability to apply IP blocks to the contents of X-Forwarded-For headers - by adding a new configuration variable $wgApplyIpBlocksToXff (disabled by default). === Bug fixes in 1.21 === * (bug 40353) SpecialDoubleRedirect should support interwiki redirects. diff --git a/includes/Block.php b/includes/Block.php index fc8ece6c07..7ee36ce95b 100644 --- a/includes/Block.php +++ b/includes/Block.php @@ -1074,187 +1074,6 @@ class Block { return null; } - - /** - * Get all blocks that match any IP from an array of IP addresses. Blocks are - * sorted so that hard blocks are before soft blocks, and blocks that disable - * account creation are before those that don't. - * - * @param Array $ipChain list of IPs (strings), usually retrieved from the - * X-Forwarded-For header of the request - * @param Bool $isAnon Exclude anonymous-only blocks if false - * @param Bool $fromMaster Whether to query the master or slave database - * @return Array of Blocks - * @since 1.21 - */ - public static function getBlocksForIPList( array $ipChain, $isAnon, $fromMaster = false ) { - if ( !count( $ipChain ) ) { - return array(); - } - - wfProfileIn( __METHOD__ ); - $conds = array(); - foreach ( array_unique( $ipChain ) as $ipaddr ) { - # Discard invalid IP addresses. Since XFF can be spoofed and we do not - # necessarily trust the header given to us, make sure that we are only - # checking for blocks on well-formatted IP addresses (IPv4 and IPv6). - # Do not treat private IP spaces as special as it may be desirable for wikis - # to block those IP ranges in order to stop misbehaving proxies that spoof XFF. - if ( !IP::isValid( $ipaddr ) ) { - continue; - } - # Don't check trusted IPs (includes local squids which will be in every request) - if ( wfIsTrustedProxy( $ipaddr ) ) { - continue; - } - # Check both the original IP (to check against single blocks), as well as build - # the clause to check for rangeblocks for the given IP. - $conds['ipb_address'][] = $ipaddr; - $conds[] = self::getRangeCond( IP::toHex( $ipaddr ) ); - } - - if ( !count( $conds ) ) { - wfProfileOut( __METHOD__ ); - return array(); - } - - if ( $fromMaster ) { - $db = wfGetDB( DB_MASTER ); - } else { - $db = wfGetDB( DB_SLAVE ); - } - $conds = $db->makeList( $conds, LIST_OR ); - if ( !$isAnon ) { - $conds = array( $conds, 'ipb_anon_only' => 0 ); - } - $selectFields = array_merge( - array( 'ipb_range_start', 'ipb_range_end' ), - Block::selectFields() - ); - $rows = $db->select( 'ipblocks', - $selectFields, - $conds, - __METHOD__, - array( 'ORDER BY' => 'ipb_anon_only ASC, ipb_create_account DESC' ) - ); - - $blocks = array(); - foreach ( $rows as $row ) { - $block = self::newFromRow( $row ); - if ( !$block->deleteIfExpired() ) { - $blocks[] = $block; - } - } - - wfProfileOut( __METHOD__ ); - return $blocks; - } - - /** - * From a list of multiple blocks, find the most exact and strongest Block. - * The logic for finding the "best" block is: - * - Blocks that match the block's target IP are preferred over ones in a range - * - Hardblocks are chosen over softblocks that prevent account creation - * - Softblocks that prevent account creation are chosen over other softblocks - * - Other softblocks are chosen over autoblocks - * - If there are multiple exact or range blocks at the same level, the one chosen - * is random - - * @param Array $ipChain list of IPs (strings). This is used to determine how "close" - * a block is to the server, and if a block matches exactly, or is in a range. - * The order is furthest from the server to nearest e.g., (Browser, proxy1, proxy2, - * local-squid, ...) - * @param Array $block Array of blocks, sorted with hard blocks before soft blocks, - * and ones that disable account creation before those that don't. The db query - * probably had 'ORDER BY' => 'ipb_anon_only ASC, ipb_create_account DESC' - * @return Block|null the "best" block from the list - */ - public static function chooseBlock( array $blocks, array $ipChain ) { - if ( !count( $blocks ) ) { - return null; - } elseif ( count( $blocks ) == 1 ) { - return $blocks[0]; - } - - wfProfileIn( __METHOD__ ); - $blocksListExact = array( - 'hard' => false, - 'disable_create' => false, - 'other' => false, - 'auto' => false - ); - $blocksListRange = array( - 'hard' => false, - 'disable_create' => false, - 'other' => false, - 'auto' => false - ); - $ipChain = array_reverse( $ipChain ); - - foreach ( $blocks as $block ) { - // Stop searching if we have already have a "better" block. This - // is why the order of the blocks matters - if ( !$block->isHardblock() && $blocksListExact['hard'] ) { - break; - } elseif ( !$block->prevents( 'createaccount' ) && $blocksListExact['disable_create'] ) { - break; - } - - foreach ( $ipChain as $checkip ) { - $checkipHex = IP::toHex( $checkip ); - if ( (string)$block->getTarget() === $checkip ) { - if ( $block->isHardblock() ) { - $blocksListExact['hard'] = $blocksListExact['hard'] ?: $block; - } elseif ( $block->prevents( 'createaccount' ) ) { - $blocksListExact['disable_create'] = $blocksListExact['disable_create'] ?: $block; - } elseif ( $block->mAuto ) { - $blocksListExact['auto'] = $blocksListExact['auto'] ?: $block; - } else { - $blocksListExact['other'] = $blocksListExact['other'] ?: $block; - } - // We found closest exact match in the ip list, so go to the next Block - break; - } elseif ( array_filter( $blocksListExact ) == array() - && $block->getRangeStart() <= $checkipHex - && $block->getRangeEnd() >= $checkipHex - ) { - if ( $block->isHardblock() ) { - $blocksListRange['hard'] = $blocksListRange['hard'] ?: $block; - } elseif ( $block->prevents( 'createaccount' ) ) { - $blocksListRange['disable_create'] = $blocksListRange['disable_create'] ?: $block; - } elseif ( $block->mAuto ) { - $blocksListRange['auto'] = $blocksListRange['auto'] ?: $block; - } else { - $blocksListRange['other'] = $blocksListRange['other'] ?: $block; - } - break; - } - } - } - - if ( array_filter( $blocksListExact ) == array() ) { - $blocksList = &$blocksListRange; - } else { - $blocksList = &$blocksListExact; - } - - $chosenBlock = null; - if ( $blocksList['hard'] ) { - $chosenBlock = $blocksList['hard']; - } elseif ( $blocksList['disable_create'] ) { - $chosenBlock = $blocksList['disable_create']; - } elseif ( $blocksList['other'] ) { - $chosenBlock = $blocksList['other']; - } elseif ( $blocksList['auto'] ) { - $chosenBlock = $blocksList['auto']; - } else { - throw new MWException( "Proxy block found, but couldn't be classified." ); - } - - wfProfileOut( __METHOD__ ); - return $chosenBlock; - } - /** * From an existing Block, get the target and the type of target. * Note that, except for null, it is always safe to treat the target diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 2fce7f72c5..26fe197d38 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -4324,13 +4324,6 @@ $wgSorbsUrl = array(); */ $wgProxyWhitelist = array(); -/** - * Whether to look at the X-Forwarded-For header's list of (potentially spoofed) - * IPs and apply IP blocks to them. This allows for IP blocks to work with correctly-configured - * (transparent) proxies without needing to block the proxies themselves. - */ -$wgApplyIpBlocksToXff = false; - /** * Simple rate limiter options to brake edit floods. * diff --git a/includes/User.php b/includes/User.php index 8809fe96a1..6b7348a8fb 100644 --- a/includes/User.php +++ b/includes/User.php @@ -1271,7 +1271,7 @@ class User { * done against master. */ private function getBlockedStatus( $bFromSlave = true ) { - global $wgProxyWhitelist, $wgUser, $wgApplyIpBlocksToXff; + global $wgProxyWhitelist, $wgUser; if ( -1 != $this->mBlockedby ) { return; @@ -1317,25 +1317,6 @@ class User { } } - # (bug 23343) Apply IP blocks to the contents of XFF headers, if enabled - if ( !$block instanceof Block - && $wgApplyIpBlocksToXff - && $ip !== null - && !$this->isAllowed( 'proxyunbannable' ) - && !in_array( $ip, $wgProxyWhitelist ) - ) { - $xff = $this->getRequest()->getHeader( 'X-Forwarded-For' ); - $xff = array_map( 'trim', explode( ',', $xff ) ); - $xff = array_diff( $xff, array( $ip ) ); - $xffblocks = Block::getBlocksForIPList( $xff, $this->isAnon(), !$bFromSlave ); - $block = Block::chooseBlock( $xffblocks, $xff ); - if ( $block instanceof Block ) { - # Mangle the reason to alert the user that the block - # originated from matching the X-Forwarded-For header. - $block->mReason = wfMessage( 'xffblockreason', $block->mReason )->text(); - } - } - if ( $block instanceof Block ) { wfDebug( __METHOD__ . ": Found block.\n" ); $this->mBlock = $block; @@ -1349,7 +1330,7 @@ class User { $this->mAllowUsertalk = false; } - // Extensions + # Extensions wfRunHooks( 'GetBlockedStatus', array( &$this ) ); wfProfileOut( __METHOD__ ); diff --git a/languages/messages/MessagesEn.php b/languages/messages/MessagesEn.php index b6fff489f6..52082afcc0 100644 --- a/languages/messages/MessagesEn.php +++ b/languages/messages/MessagesEn.php @@ -3307,7 +3307,6 @@ Please contact your Internet service provider or technical support of your organ 'sorbsreason' => 'Your IP address is listed as an open proxy in the DNSBL used by {{SITENAME}}.', 'sorbs_create_account_reason' => 'Your IP address is listed as an open proxy in the DNSBL used by {{SITENAME}}. You cannot create an account', -'xffblockreason' => 'An IP address present in the X-Forwarded-For header, either yours or that of a proxy server you are using, has been blocked. The original block reason was: $1', 'cant-block-while-blocked' => 'You cannot block other users while you are blocked.', 'cant-see-hidden-user' => "The user you are trying to block has already been blocked and hidden. Since you do not have the hideuser right, you cannot see or edit the user's block.", diff --git a/languages/messages/MessagesQqq.php b/languages/messages/MessagesQqq.php index 700282a1de..14f8b255c3 100644 --- a/languages/messages/MessagesQqq.php +++ b/languages/messages/MessagesQqq.php @@ -5374,9 +5374,6 @@ See also: * {{msg-mw|Sorbsreason}} * {{msg-mw|Sorbs create account_reason}}', 'cant-see-hidden-user' => 'Used as (red) error message on [[Special:Block]] when you try to change (as sysop without the hideuser right) the block of a hidden user.', -'xffblockreason' => "This text is shown to the user as a block reason and describes that the user is being blocked because an IP in the X-Forwarded-For header -(which lists the user's IP as well as all IPs of the transparent proxy servers they went through) sent when they loaded the page has been blocked: -* $1 is the original block reason for the IP address matched in the X-Forwarded-For header", 'ipbblocked' => 'Error message shown when a user tries to alter block settings when they are themselves blocked.', 'ipbnounblockself' => 'Error message shown when a user without the unblockself right tries to unblock themselves.', diff --git a/maintenance/language/messages.inc b/maintenance/language/messages.inc index ae33cca3bd..7c16df6cf4 100644 --- a/maintenance/language/messages.inc +++ b/maintenance/language/messages.inc @@ -2291,7 +2291,6 @@ $wgMessageStructure = array( 'sorbs', 'sorbsreason', 'sorbs_create_account_reason', - 'xffblockreason', 'cant-block-while-blocked', 'cant-see-hidden-user', 'ipbblocked', diff --git a/tests/phpunit/includes/BlockTest.php b/tests/phpunit/includes/BlockTest.php index 704c912a9a..19c9b6874d 100644 --- a/tests/phpunit/includes/BlockTest.php +++ b/tests/phpunit/includes/BlockTest.php @@ -228,117 +228,4 @@ class BlockTest extends MediaWikiLangTestCase { $this->assertEquals( 'MetaWikiUser', $block->getByName(), 'Correct blocker name' ); $this->assertEquals( 0, $block->getBy(), 'Correct blocker id' ); } - - function testBlocksOnXff() { - - $blockList = array( - array( 'target' => '70.2.0.0/16', - 'type' => Block::TYPE_RANGE, - 'desc' => 'Range Hardblock', - 'ACDisable' => false, - 'isHardblock' => true, - 'isAutoBlocking' => false, - ), - array( 'target' => '2001:4860:4001::/48', - 'type' => Block::TYPE_RANGE, - 'desc' => 'Range6 Hardblock', - 'ACDisable' => false, - 'isHardblock' => true, - 'isAutoBlocking' => false, - ), - array( 'target' => '60.2.0.0/16', - 'type' => Block::TYPE_RANGE, - 'desc' => 'Range Softblock with AC Disabled', - 'ACDisable' => true, - 'isHardblock' => false, - 'isAutoBlocking' => false, - ), - array( 'target' => '50.2.0.0/16', - 'type' => Block::TYPE_RANGE, - 'desc' => 'Range Softblock', - 'ACDisable' => false, - 'isHardblock' => false, - 'isAutoBlocking' => false, - ), - array( 'target' => '50.1.1.1', - 'type' => Block::TYPE_IP, - 'desc' => 'Exact Softblock', - 'ACDisable' => false, - 'isHardblock' => false, - 'isAutoBlocking' => false, - ), - ); - - foreach ( $blockList as $insBlock ) { - $target = $insBlock['target']; - - if ( $insBlock['type'] === Block::TYPE_IP ) { - $target = User::newFromName( IP::sanitizeIP( $target ), false )->getName(); - } elseif ( $insBlock['type'] === Block::TYPE_RANGE ) { - $target = IP::sanitizeRange( $target ); - } - - $block = new Block(); - $block->setTarget( $target ); - $block->setBlocker( 'testblocker@global' ); - $block->mReason = $insBlock['desc']; - $block->mExpiry = 'infinity'; - $block->prevents( $insBlock['ACDisable'] ); - $block->isHardblock( $insBlock['isHardblock'] ); - $block->isAutoblocking( $insBlock['isAutoBlocking'] ); - $block->insert(); - } - - $xffHeaders = array( - array( 'xff' => '1.2.3.4, 70.2.1.1, 60.2.1.1, 2.3.4.5', - 'count' => 2, - 'result' => 'Range Hardblock' - ), - array( 'xff' => '1.2.3.4, 50.2.1.1, 60.2.1.1, 2.3.4.5', - 'count' => 2, - 'result' => 'Range Softblock with AC Disabled' - ), - array( 'xff' => '1.2.3.4, 70.2.1.1, 50.1.1.1, 2.3.4.5', - 'count' => 2, - 'result' => 'Exact Softblock' - ), - array( 'xff' => '1.2.3.4, 70.2.1.1, 50.2.1.1, 50.1.1.1, 2.3.4.5', - 'count' => 3, - 'result' => 'Exact Softblock' - ), - array( 'xff' => '1.2.3.4, 70.2.1.1, 50.2.1.1, 2.3.4.5', - 'count' => 2, - 'result' => 'Range Hardblock' - ), - array( 'xff' => '1.2.3.4, 70.2.1.1, 60.2.1.1, 2.3.4.5', - 'count' => 2, - 'result' => 'Range Hardblock' - ), - array( 'xff' => '50.2.1.1, 60.2.1.1, 2.3.4.5', - 'count' => 2, - 'result' => 'Range Softblock with AC Disabled' - ), - array( 'xff' => '1.2.3.4, 50.1.1.1, 60.2.1.1, 2.3.4.5', - 'count' => 2, - 'result' => 'Exact Softblock' - ), - array( 'xff' => '1.2.3.4, <$A_BUNCH-OF{INVALID}TEXT\>, 60.2.1.1, 2.3.4.5', - 'count' => 1, - 'result' => 'Range Softblock with AC Disabled' - ), - array( 'xff' => '1.2.3.4, 50.2.1.1, 2001:4860:4001:802::1003, 2.3.4.5', - 'count' => 2, - 'result' => 'Range6 Hardblock' - ), - ); - - foreach ( $xffHeaders as $test ) { - $list = array_map( 'trim', explode( ',', $test['xff'] ) ); - $xffblocks = Block::getBlocksForIPList( $list, true ); - $this->assertEquals( $test['count'], count( $xffblocks ), 'Number of blocks for ' . $test['xff'] ); - $block = Block::chooseBlock( $xffblocks, $list ); - $this->assertEquals( $test['result'], $block->mReason, 'Correct block type for XFF header ' . $test['xff'] ); - } - - } }