Normalize remote IP addresses when writing to logs, etc
Summary:
Ref T11939. IPv4 addresses can normally only be written in one way, but IPv6 addresses have several formats.
For example, the addresses "FFF::", "FfF::", "fff::", "0ffF::", "0fFf:0::", and "0FfF:0:0:0:0:0:0:0" are all the same address.
Normalize all addresses before writing them to logs, etc, so we store the most-preferred form ("fff::", above).
Test Plan:
Ran an SSH clone over IPv6:
```
$ git fetch ssh://local@::1/diffusion/26/locktopia.git
```
It worked; verified that address read out of `SSH_CLIENT` sensibly.
Faked my remote address as a non-preferred-form IPv6 address using `preamble.php`.
Failed to login, verified that the preferred-form version of the address appeared in the user activity log.
Made IPv6 requests over HTTP:
```
$ curl -H "Host: local.phacility.com" "http://[::1]/"
```
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11939
Differential Revision: https://secure.phabricator.com/D16987
This commit is contained in:
@@ -557,11 +557,13 @@ final class AphrontRequest extends Phobject {
|
||||
}
|
||||
|
||||
public function getRemoteAddress() {
|
||||
$address = $_SERVER['REMOTE_ADDR'];
|
||||
if (!strlen($address)) {
|
||||
$address = PhabricatorEnv::getRemoteAddress();
|
||||
|
||||
if (!$address) {
|
||||
return null;
|
||||
}
|
||||
return substr($address, 0, 64);
|
||||
|
||||
return $address->getAddress();
|
||||
}
|
||||
|
||||
public function isHTTPS() {
|
||||
|
||||
@@ -106,10 +106,18 @@ abstract class AphrontApplicationConfiguration extends Phobject {
|
||||
PhabricatorAccessLog::init();
|
||||
$access_log = PhabricatorAccessLog::getLog();
|
||||
PhabricatorStartup::setAccessLog($access_log);
|
||||
|
||||
$address = PhabricatorEnv::getRemoteAddress();
|
||||
if ($address) {
|
||||
$address_string = $address->getAddress();
|
||||
} else {
|
||||
$address_string = '-';
|
||||
}
|
||||
|
||||
$access_log->setData(
|
||||
array(
|
||||
'R' => AphrontRequest::getHTTPHeader('Referer', '-'),
|
||||
'r' => idx($_SERVER, 'REMOTE_ADDR', '-'),
|
||||
'r' => $address_string,
|
||||
'M' => idx($_SERVER, 'REQUEST_METHOD', '-'),
|
||||
));
|
||||
|
||||
|
||||
@@ -108,18 +108,28 @@ final class PhabricatorUserLog extends PhabricatorUserDAO
|
||||
$log->setUserPHID((string)$object_phid);
|
||||
$log->setAction($action);
|
||||
|
||||
$log->remoteAddr = (string)idx($_SERVER, 'REMOTE_ADDR', '');
|
||||
$address = PhabricatorEnv::getRemoteAddress();
|
||||
if ($address) {
|
||||
$log->remoteAddr = $address->getAddress();
|
||||
} else {
|
||||
$log->remoteAddr = '';
|
||||
}
|
||||
|
||||
return $log;
|
||||
}
|
||||
|
||||
public static function loadRecentEventsFromThisIP($action, $timespan) {
|
||||
$address = PhabricatorEnv::getRemoteAddress();
|
||||
if (!$address) {
|
||||
return array();
|
||||
}
|
||||
|
||||
return id(new PhabricatorUserLog())->loadAllWhere(
|
||||
'action = %s AND remoteAddr = %s AND dateCreated > %d
|
||||
ORDER BY dateCreated DESC',
|
||||
$action,
|
||||
idx($_SERVER, 'REMOTE_ADDR'),
|
||||
time() - $timespan);
|
||||
$address->getAddress(),
|
||||
PhabricatorTime::getNow() - $timespan);
|
||||
}
|
||||
|
||||
public function save() {
|
||||
|
||||
17
src/infrastructure/env/PhabricatorEnv.php
vendored
17
src/infrastructure/env/PhabricatorEnv.php
vendored
@@ -818,12 +818,12 @@ final class PhabricatorEnv extends Phobject {
|
||||
return false;
|
||||
}
|
||||
|
||||
$address = idx($_SERVER, 'REMOTE_ADDR');
|
||||
$address = self::getRemoteAddress();
|
||||
if (!$address) {
|
||||
throw new Exception(
|
||||
pht(
|
||||
'Unable to test remote address against cluster whitelist: '.
|
||||
'REMOTE_ADDR is not defined.'));
|
||||
'REMOTE_ADDR is not defined or not valid.'));
|
||||
}
|
||||
|
||||
return self::isClusterAddress($address);
|
||||
@@ -844,6 +844,19 @@ final class PhabricatorEnv extends Phobject {
|
||||
->containsAddress($address);
|
||||
}
|
||||
|
||||
public static function getRemoteAddress() {
|
||||
$address = idx($_SERVER, 'REMOTE_ADDR');
|
||||
if (!$address) {
|
||||
return null;
|
||||
}
|
||||
|
||||
try {
|
||||
return PhutilIPAddress::newAddress($address);
|
||||
} catch (Exception $ex) {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/* -( Internals )---------------------------------------------------------- */
|
||||
|
||||
|
||||
|
||||
@@ -95,7 +95,13 @@ abstract class PhabricatorSSHWorkflow extends PhabricatorManagementWorkflow {
|
||||
// This has the format "<ip> <remote-port> <local-port>". Grab the IP.
|
||||
$remote_address = head(explode(' ', $ssh_client));
|
||||
|
||||
return $remote_address;
|
||||
try {
|
||||
$address = PhutilIPAddress::newAddress($remote_address);
|
||||
} catch (Exception $ex) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return $address->getAddress();
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user