saper has uploaded a new change for review.
https://gerrit.wikimedia.org/r/53050
Change subject: Refactor XFF processing into clientBehindProxy
......................................................................
Refactor XFF processing into clientBehindProxy
Refactor getClientIPfromXFF() into clientBehindProxy()
and remove copy+paste logic to fill in `cuc_xff`
and `cuc_xff_hex` fields of the database.
Introduce RL stylesheet to get rid of embedded
background colors.
Conflicts:
CheckUser.hooks.php
Change-Id: I6f655bade49887ad9941bde09911c42ada1c700b
---
M CheckUser.hooks.php
M CheckUser.php
M CheckUser_body.php
A checkuser.css
4 files changed, 75 insertions(+), 35 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/CheckUser
refs/changes/50/53050/1
diff --git a/CheckUser.hooks.php b/CheckUser.hooks.php
index 50f3121..f9c7059 100644
--- a/CheckUser.hooks.php
+++ b/CheckUser.hooks.php
@@ -11,9 +11,10 @@
extract( $rc->mAttribs );
// Get IP
$ip = $wgRequest->getIP();
- // Get XFF header
- $xff = $wgRequest->getHeader( 'X-Forwarded-For' );
- list( $xff_ip, $isSquidOnly ) = self::getClientIPfromXFF( $xff
);
+ // Handle XFF header
+ list( $cuc_xff_chain, $cuc_xff_hex ) = self::clientBehindProxy(
+ $wgRequest->getHeader( 'X-Forwarded-For' )
+ );
// Get agent
$agent = $wgRequest->getHeader( 'User-Agent' );
// Store the log action text for log events
@@ -48,8 +49,8 @@
'cuc_timestamp' => $rc_timestamp,
'cuc_ip' => IP::sanitizeIP( $ip ),
'cuc_ip_hex' => $ip ? IP::toHex( $ip ) : null,
- 'cuc_xff' => !$isSquidOnly ? $xff : '',
- 'cuc_xff_hex' => ( $xff_ip && !$isSquidOnly ) ?
IP::toHex( $xff_ip ) : null,
+ 'cuc_xff' => $cuc_xff_chain,
+ 'cuc_xff_hex' => $cuc_xff_hex,
'cuc_agent' => $agent
);
# On PG, MW unsets cur_id due to schema incompatibilites. So it
may not be set!
@@ -68,9 +69,10 @@
public static function updateCUPasswordResetData( User $user, $ip,
$account ) {
global $wgRequest;
- // Get XFF header
- $xff = $wgRequest->getHeader( 'X-Forwarded-For' );
- list( $xff_ip, $isSquidOnly ) = self::getClientIPfromXFF( $xff
);
+ // Handle XFF header
+ list( $cuc_xff_chain, $cuc_xff_hex ) = self::clientBehindProxy(
+ $wgRequest->getHeader( 'X-Forwarded-For' )
+ );
// Get agent
$agent = $wgRequest->getHeader( 'User-Agent' );
$dbw = wfGetDB( DB_MASTER );
@@ -90,8 +92,8 @@
'cuc_timestamp' => $dbw->timestamp( wfTimestampNow() ),
'cuc_ip' => IP::sanitizeIP( $ip ),
'cuc_ip_hex' => $ip ? IP::toHex( $ip ) : null,
- 'cuc_xff' => !$isSquidOnly ? $xff : '',
- 'cuc_xff_hex' => ( $xff_ip && !$isSquidOnly ) ?
IP::toHex( $xff_ip ) : null,
+ 'cuc_xff' => $cuc_xff_chain,
+ 'cuc_xff_hex' => $cuc_xff_hex,
'cuc_agent' => $agent
);
$dbw->insert( 'cu_changes', $rcRow, __METHOD__ );
@@ -113,9 +115,10 @@
$hash = md5( $userTo->getEmail() . $userTo->getId() .
$wgSecretKey );
// Get IP
$ip = $wgRequest->getIP();
- // Get XFF header
- $xff = $wgRequest->getHeader( 'X-Forwarded-For' );
- list( $xff_ip, $isSquidOnly ) = self::getClientIPfromXFF( $xff
);
+ // Handle XFF header
+ list( $cuc_xff_chain, $cuc_xff_hex ) = self::clientBehindProxy(
+ $wgRequest->getHeader( 'X-Forwarded-For' )
+ );
// Get agent
$agent = $wgRequest->getHeader( 'User-Agent' );
$dbw = wfGetDB( DB_MASTER );
@@ -135,8 +138,8 @@
'cuc_timestamp' => $dbw->timestamp( wfTimestampNow() ),
'cuc_ip' => IP::sanitizeIP( $ip ),
'cuc_ip_hex' => $ip ? IP::toHex( $ip ) : null,
- 'cuc_xff' => !$isSquidOnly ? $xff : '',
- 'cuc_xff_hex' => ( $xff_ip && !$isSquidOnly ) ?
IP::toHex( $xff_ip ) : null,
+ 'cuc_xff' => $cuc_xff_chain,
+ 'cuc_xff_hex' => $cuc_xff_hex,
'cuc_agent' => $agent
);
if ( trim( $wgCUPublicKey ) != '' ) {
@@ -184,9 +187,11 @@
// Get IP
$ip = $wgRequest->getIP();
- // Get XFF header
- $xff = $wgRequest->getHeader( 'X-Forwarded-For' );
- list( $xff_ip, $isSquidOnly ) = self::getClientIPfromXFF( $xff
);
+ // Handle XFF header
+ list( $cuc_xff_chain, $cuc_xff_hex ) = self::clientBehindProxy(
+ $wgRequest->getHeader( 'X-Forwarded-For' )
+ );
+
// Get agent
$agent = $wgRequest->getHeader( 'User-Agent' );
$dbw = wfGetDB( DB_MASTER );
@@ -207,8 +212,8 @@
'cuc_timestamp' => $dbw->timestamp( wfTimestampNow() ),
'cuc_ip' => IP::sanitizeIP( $ip ),
'cuc_ip_hex' => $ip ? IP::toHex( $ip ) : null,
- 'cuc_xff' => !$isSquidOnly ? $xff : '',
- 'cuc_xff_hex' => ( $xff_ip && !$isSquidOnly ) ?
IP::toHex( $xff_ip ) : null,
+ 'cuc_xff' => $cuc_xff_chain,
+ 'cuc_xff_hex' => $cuc_xff_hex,
'cuc_agent' => $agent
);
$dbw->insert( 'cu_changes', $rcRow, __METHOD__ );
@@ -231,22 +236,23 @@
}
/**
- * Locates the client IP within a given XFF string
+ * Process X-Forwarded-For header
+ * Return XFF header and the IP address in hex of
+ * the first host behind a trusted proxy, in case it can
+ * be identified
* @param string $xff
- * @return array( string, bool )
+ * @return array( string, string )
*/
- public static function getClientIPfromXFF( $xff ) {
+ public static function clientBehindProxy( $xff ) {
if ( !$xff ) {
- return array( null, false );
+ return array( '', null );
}
// Avoid annoyingly long xff hacks
$xff = trim( substr( $xff, 0, 255 ) );
$client = null;
$isSquidOnly = true;
- // @todo Unused variable?
- $trusted = true;
// Check each IP, assuming they are separated by commas
$ips = explode( ',', $xff );
foreach ( $ips as $ip ) {
@@ -261,13 +267,36 @@
}
} elseif ( !wfIsTrustedProxy( $ip ) )
{
- $isSquidOnly = false;
- break;
+ return array( $xff, IP::toHex($client)
);
}
}
}
-
- return array( $client, $isSquidOnly );
+ # $client $isSquidOnly
+ # +-------+--------------+
+ # | null | false | 1) XFF is empty
+ # +-------+--------------+
+ # | null | true | 2) All IP addresses in XFF chain
are RFC1918
+ # +-------+--------------+
+ # | IP | false | 3) IP is the first non-RFC1918
address in the XFF chain
+ # +-------+--------------+ that is *not* a trusted proxy
+ # | IP | true | 4) IP is the first non-RFC1918
address in the XFF chain
+ # +-------+--------------+ and all of them are trusted
proxies
+ #
+ # 'cuc_xff' => !$isSquidOnly ? $xff : '',
+ # 'cuc_xff_hex' => ( $xff_ip && !$isSquidOnly ) ?
IP::toHex( $xff_ip ) : null,
+ #
+ # We want:
+ # | cuc_xff | cuc_xff_hex |
+ # +-------+--------------+--------------------+
+ # | 1) | '' | null |
+ # +-------+--------------+--------------------+
+ # | 2) | '' | null |
+ # +-------+--------------+--------------------+
+ # | 3) | $xff | IP::toHex($client) |
+ # +-------+--------------+--------------------+
+ # | 4) | '' | null |
+ # +-------+--------------+--------------------+
+ return array( '', null );
}
public static function checkUserSchemaUpdates( DatabaseUpdater $updater
) {
diff --git a/CheckUser.php b/CheckUser.php
index b140696..85c3a81 100644
--- a/CheckUser.php
+++ b/CheckUser.php
@@ -84,6 +84,7 @@
$wgResourceModules['ext.checkUser'] = array(
'scripts' => 'checkuser.js',
+ 'styles' => 'checkuser.css',
'dependencies' => array( 'mediawiki.util' ), // IP stuff
'localBasePath' => dirname( __FILE__ ),
'remoteExtPath' => 'CheckUser',
diff --git a/CheckUser_body.php b/CheckUser_body.php
index d9eab8e..8e0ceb8 100644
--- a/CheckUser_body.php
+++ b/CheckUser_body.php
@@ -1044,8 +1044,12 @@
if ( $set[1] ) {
# Flag our trusted proxies
list( $client, $trusted ) =
CheckUserHooks::getClientIPfromXFF( $set[1], $set[0] );
- $c = $trusted ? '#F0FFF0' :
'#FFFFCC';
- $s .= '   <span
style="background-color: ' . $c . '"><strong>XFF</strong>: ';
+ if( is_null( $cuc_xff_hex ) ) {
+ $c =
'mw-checkuser-trusted-xff';
+ } else {
+ $c =
'mw-checkuser-not-trusted-xff';
+ }
+ $s .= '   <span
class="' . $c . '"><strong>XFF</strong>: ';
$s .= Linker::linkKnown(
$this->getTitle(),
htmlspecialchars(
$set[1] ),
@@ -1220,9 +1224,13 @@
# XFF
if ( $row->cuc_xff != null ) {
# Flag our trusted proxies
- list( $client, $trusted ) =
CheckUserHooks::getClientIPfromXFF( $row->cuc_xff, $row->cuc_ip );
- $c = $trusted ? '#F0FFF0' : '#FFFFCC';
- $line .= '   <span
class="mw-checkuser-xff" style="background-color: ' . $c . '">' .
+ list( $cuc_xff_chain, $cuc_xff_hex ) =
CheckUserHooks::clientBehindProxy( $row->cuc_xff );
+ if( is_null( $cuc_xff_hex ) ) {
+ $c = 'mw-checkuser-trusted-xff';
+ } else {
+ $c = 'mw-checkuser-not-trusted-xff';
+ }
+ $line .= '   <span
class="mw-checkuser-xff ' . $c . '">' .
'<strong>XFF</strong>: ';
$line .= Linker::linkKnown(
$cuTitle,
diff --git a/checkuser.css b/checkuser.css
new file mode 100644
index 0000000..37efa3f
--- /dev/null
+++ b/checkuser.css
@@ -0,0 +1,2 @@
+.mw-checkuser-trusted-xff { background-color: #f0fff0; }
+.mw-checkuser-not-trusted-xff { background-color: #ffffcc; }
--
To view, visit https://gerrit.wikimedia.org/r/53050
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6f655bade49887ad9941bde09911c42ada1c700b
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/CheckUser
Gerrit-Branch: master
Gerrit-Owner: saper <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
