Make it easier to parse "X-Forwarded-For" with one or more load balancers
Summary:
Fixes T13392. If you have 17 load balancers in sequence, Phabricator will receive requests with at least 17 "X-Forwarded-For" components in the header.
We want to select the 17th-from-last element, since prior elements are not trustworthy.
This currently isn't very easy/obvious, and you have to add a kind of sketchy piece of custom code to `preamble.php` to do any "X-Forwarded-For" parsing. Make handling this correctly easier.
Test Plan:
- Ran unit tests.
- Configured my local `preamble.php` to call `preamble_trust_x_forwarded_for_header(4)`, then made `/debug/` dump the header and the final value of `REMOTE_ADDR`.
```
$ curl http://local.phacility.com/debug/
<pre>
HTTP_X_FORWARDED_FOR =
FINAL REMOTE_ADDR = 127.0.0.1
</pre>
```
```
$ curl -H 'X-Forwarded-For: 1.1.1.1, 2.2.2.2, 3.3.3.3, 4.4.4.4, 5.5.5.5, 6.6.6.6' http://local.phacility.com/debug/
<pre>
HTTP_X_FORWARDED_FOR = 1.1.1.1, 2.2.2.2, 3.3.3.3, 4.4.4.4, 5.5.5.5, 6.6.6.6
FINAL REMOTE_ADDR = 3.3.3.3
</pre>
```
```
$ curl -H 'X-Forwarded-For: 5.5.5.5, 6.6.6.6' http://local.phacility.com/debug/
<pre>
HTTP_X_FORWARDED_FOR = 5.5.5.5, 6.6.6.6
FINAL REMOTE_ADDR = 5.5.5.5
</pre>
```
Maniphest Tasks: T13392
Differential Revision: https://secure.phabricator.com/D20785
2019-09-05 12:43:22 +02:00
|
|
|
<?php
|
|
|
|
|
|
|
|
/**
|
|
|
|
* Parse the "X_FORWARDED_FOR" HTTP header to determine the original client
|
|
|
|
* address.
|
|
|
|
*
|
|
|
|
* @param int Number of devices to trust.
|
|
|
|
* @return void
|
|
|
|
*/
|
|
|
|
function preamble_trust_x_forwarded_for_header($layers = 1) {
|
|
|
|
if (!is_int($layers) || ($layers < 1)) {
|
|
|
|
echo
|
|
|
|
'preamble_trust_x_forwarded_for_header(<layers>): '.
|
|
|
|
'"layers" parameter must an integer larger than 0.'."\n";
|
|
|
|
echo "\n";
|
|
|
|
exit(1);
|
|
|
|
}
|
|
|
|
|
|
|
|
if (!isset($_SERVER['HTTP_X_FORWARDED_FOR'])) {
|
|
|
|
return;
|
|
|
|
}
|
|
|
|
|
|
|
|
$forwarded_for = $_SERVER['HTTP_X_FORWARDED_FOR'];
|
PHP 8.1: fixes for strlen() not accepting NULL anymore, part 2
Summary:
This is a fix for PHP 8.1 deprecation of strlen(NULL), for these Phorge components:
- scripts
- aphront
- project
The strlen() was used in Phabricator to check if a generic value was a non-empty string.
For this reason, Phorge adopts phutil_nonempty_string() that checks that.
Note: this may highlight other absurd input values that might be worth correcting
instead of just ignoring. If your phutil_nonempty_string() throws an exception, just
report it to Phorge to evaluate and fix together that specific corner case.
Closes T15223
Ref T15190
Ref T15064
Test Plan: - check with your big eyes that there are no obvious typos
Reviewers: O1 Blessed Committers, avivey
Reviewed By: O1 Blessed Committers, avivey
Subscribers: speck, tobiaswiese, Matthew, Cigaryno
Maniphest Tasks: T15223, T15190, T15064
Differential Revision: https://we.phorge.it/D25105
2023-04-01 15:19:43 +02:00
|
|
|
if (!phutil_nonempty_string($forwarded_for)) {
|
Make it easier to parse "X-Forwarded-For" with one or more load balancers
Summary:
Fixes T13392. If you have 17 load balancers in sequence, Phabricator will receive requests with at least 17 "X-Forwarded-For" components in the header.
We want to select the 17th-from-last element, since prior elements are not trustworthy.
This currently isn't very easy/obvious, and you have to add a kind of sketchy piece of custom code to `preamble.php` to do any "X-Forwarded-For" parsing. Make handling this correctly easier.
Test Plan:
- Ran unit tests.
- Configured my local `preamble.php` to call `preamble_trust_x_forwarded_for_header(4)`, then made `/debug/` dump the header and the final value of `REMOTE_ADDR`.
```
$ curl http://local.phacility.com/debug/
<pre>
HTTP_X_FORWARDED_FOR =
FINAL REMOTE_ADDR = 127.0.0.1
</pre>
```
```
$ curl -H 'X-Forwarded-For: 1.1.1.1, 2.2.2.2, 3.3.3.3, 4.4.4.4, 5.5.5.5, 6.6.6.6' http://local.phacility.com/debug/
<pre>
HTTP_X_FORWARDED_FOR = 1.1.1.1, 2.2.2.2, 3.3.3.3, 4.4.4.4, 5.5.5.5, 6.6.6.6
FINAL REMOTE_ADDR = 3.3.3.3
</pre>
```
```
$ curl -H 'X-Forwarded-For: 5.5.5.5, 6.6.6.6' http://local.phacility.com/debug/
<pre>
HTTP_X_FORWARDED_FOR = 5.5.5.5, 6.6.6.6
FINAL REMOTE_ADDR = 5.5.5.5
</pre>
```
Maniphest Tasks: T13392
Differential Revision: https://secure.phabricator.com/D20785
2019-09-05 12:43:22 +02:00
|
|
|
return;
|
|
|
|
}
|
|
|
|
|
|
|
|
$address = preamble_get_x_forwarded_for_address($forwarded_for, $layers);
|
|
|
|
|
|
|
|
$_SERVER['REMOTE_ADDR'] = $address;
|
|
|
|
}
|
|
|
|
|
|
|
|
function preamble_get_x_forwarded_for_address($raw_header, $layers) {
|
|
|
|
// The raw header may be a list of IPs, like "1.2.3.4, 4.5.6.7", if the
|
|
|
|
// request the load balancer received also had this header. In particular,
|
|
|
|
// this happens routinely with requests received through a CDN, but can also
|
|
|
|
// happen illegitimately if the client just makes up an "X-Forwarded-For"
|
|
|
|
// header full of lies.
|
|
|
|
|
|
|
|
// We can only trust the N elements at the end of the list which correspond
|
|
|
|
// to network-adjacent devices we control. Usually, we're behind a single
|
|
|
|
// load balancer and "N" is 1, so we want to take the last element in the
|
|
|
|
// list.
|
|
|
|
|
|
|
|
// In some cases, "N" may be more than 1, if the network is configured so
|
|
|
|
// that that requests are routed through multiple layers of load balancers
|
|
|
|
// and proxies. In this case, we want to take the Nth-to-last element of
|
|
|
|
// the list.
|
|
|
|
|
|
|
|
$addresses = explode(',', $raw_header);
|
|
|
|
|
|
|
|
// If we have more than one trustworthy device on the network path, discard
|
|
|
|
// corresponding elements from the list. For example, if we have 7 devices,
|
|
|
|
// we want to discard the last 6 elements of the list.
|
|
|
|
|
|
|
|
// The final device address does not appear in the list, since devices do
|
|
|
|
// not append their own addresses to "X-Forwarded-For".
|
|
|
|
|
|
|
|
$discard_addresses = ($layers - 1);
|
|
|
|
|
|
|
|
// However, we don't want to throw away all of the addresses. Some requests
|
|
|
|
// may originate from within the network, and may thus not have as many
|
|
|
|
// addresses as we expect. If we have fewer addresses than trustworthy
|
|
|
|
// devices, discard all but one address.
|
|
|
|
|
|
|
|
$max_discard = (count($addresses) - 1);
|
|
|
|
|
|
|
|
$discard_count = min($discard_addresses, $max_discard);
|
|
|
|
if ($discard_count) {
|
|
|
|
$addresses = array_slice($addresses, 0, -$discard_count);
|
|
|
|
}
|
|
|
|
|
|
|
|
$original_address = end($addresses);
|
|
|
|
$original_address = trim($original_address);
|
|
|
|
|
|
|
|
return $original_address;
|
|
|
|
}
|