Reverse proxy IP fix

[x-rated]

Hello,
i need an extension to fix showing incorrect IP addresses when server is behind reverse proxy.
We found out how to fix it with code edit, but it would be more usefull to have such option as an extension.

Pasting this piece of code

Code: Select all

if(!empty($_SERVER['HTTP_X_FORWARDED_FOR']) && $_SERVER['SERVER_ADDR'] == $_SERVER['REMOTE_ADDR'])
{
$_SERVER['REMOTE_ADDR'] = htmlspecialchars((string) $_SERVER['HTTP_X_FORWARDED_FOR']);
}

after

Code: Select all

if (!defined('IN_PHPBB'))
{
	exit;
}

in includes/startup.php works great.

Can someone make this as an extension please?
There should be just some checkbox or switch in ACP like in invision forum (by default disabled).

[martti]

• I'm not fan of this kind of trickery. The superglobals got set by PHP and have a specific meaning. Altering them could produce side-effects somewhere in the code. Developers are relying on the value set by PHP.
• If you are interested in the end-user's IPs (which is something you never can be sure of), I think it's a better solution to record them separately without changing any var.
• If you keep your code like this, I would additionally check if the value returned by HTTP_X_FORWARDED_FOR (or HTTP_CLIENT_IP or HTTP_X_REAL_IP) is a valid IP with

  Code: Select all

  filter_var($_SERVER['HTTP_X_FORWARDED_FOR'], FILTER_VALIDATE_IP)); 

  The value could be anything ranging from All who is reading this is a dirty pig. to DROP table users;. (If you are just recording the value entirely separately and you don't rely on it for any functionality, there's no need to check if it's a valid IP)
• $_SERVER['SERVER_ADDR'] == $_SERVER['REMOTE_ADDR'] is specific to your setup. You are using the same IP but a different port for the server and the proxy. (Btw. not all servers set SERVER_ADDR)
• If using an extension for this, there's no need for a switch as you are proposing. Just enable or disable the extension.
• Extensions would interact with the request object object, not the superglobals.

[Paul]

Instead of fixing it in phpbb, you should fix it within your reverse proxy

[martti]

@Paul. I don't think that's possible REMOTE_ADDR is always the client your server is talking to.

Probably you'll use this event:

Code: Select all

		// Why no forwarded_for et al? Well, too easily spoofed. With the results of my recent requests
		// it's pretty clear that in the majority of cases you'll at least be left with a proxy/cache ip.
		$ip = htmlspecialchars_decode($request->server('REMOTE_ADDR'));
		$ip = preg_replace('# {2,}#', ' ', str_replace(',', ' ', $ip));
		/**
		* Event to alter user IP address
		*
		* @event core.session_ip_after
		* @var	string	ip	REMOTE_ADDR
		* @since 3.1.10-RC1
		*/
		$vars = array('ip');
		extract($phpbb_dispatcher->trigger_event('core.session_ip_after', compact($vars)));

[x-rated]

• If using an extension for this, there's no need for a switch as you are proposing. Just enable or disable the extension.

good idea, if it is possible

the easiest way and best code tweaks are welcome
unfortunately i dont understand php, so if there is someone who can create it somehow, i will be happy

[3Di]

That's so simple, just modify the code of this extension of mine which uses the same event as above, about Cloudflare IPs though.

https://www.phpbb.com/customise/db/exte ... masked_ip/

[x-rated]

i believe it is not hard to do
so should i somehow modify this part of code? but how exactly?

Code: Select all

		$ip_check = $event['ip'];

		/**
		 * Normalizes user IPs when Cloudflare is running on the server.
		 *
		 * Ternary Operator improves performance.
		 * I can't use the Null Coalescing Operator (PHP7) due to BC with phpBB 3.1.x
		 */
		$ip_check = ($this->request->server('HTTP_CF_CONNECTING_IP') != '') ? htmlspecialchars_decode($this->request->server('HTTP_CF_CONNECTING_IP')) : htmlspecialchars_decode($this->request->server('REMOTE_ADDR'));

		$event['ip'] = (string) $ip_check;
	}
}

[martti]

I've made an Extension for this. To be tested.

[x-rated]

it seems to be working great, thank you