Extract real IP behind a load balancer

[Denis-de-Bernardy]

​http://wordpress.org/extend/plugins/real-ip/

It's a 4 liner, that seems like good candidate for inclusion in wp-settings.php after the IIS fixes.

Rather than modifying the $_SERVER variable, wouldn't it be better to introduce (or modify) some sort of get_ip() function?

I doubt it. you could reasonably expect a beginner to think that the $_SERVER variable contains relevant data. Few php coders think about sanitizing things such as the referrer... Even less imagine that the IPs in $_SERVER are incorrect.

See also: #4602

HTTP_X_FORWARDED_FOR can be a comma separated list.

new patch should deal with this fine.

From what I've been told, you can't rely on HTTP_X_FORWARDED_FOR being set to something sane. This should be enabled only when you are sure the host uses HTTP_X_FORWARDED_FOR and uses it correctly. If we build this into core, we would probably need a USE_FORWARDED_IP flag that users can define in wp-config.php.

Seems there are a few more, too:

​http://coding-talk.com/f14/vbulletin-fix-for-apache-nginx-3384/

Let's punt this one to 2.9 early.

I've gotten to deal with WordPress behind a CDN or reverse proxy a few times now. From what I've seen there are at least a dozen different HTTP headers that can be used for the origin/source IP. WordPress can't "out of the box" handle everyone's situation perfectly, so it should be made easy to adjust:

The most flexible would be to switch to a get_ip() or wp_get_ip() function, then let users be able to override it with their own by creating a plugin:

function get_ip(){
  return $_SERVER['X-REAL-IP'];
}

Though not sure if that would happen early enough to be useful for init.

The other option is to leave a mapping option in wp-config.php with the default and let users change if needed. Something like:

// If your server sits behind a CDN or proxy you may need to adjust this
$real_ip = $_SERVER['REMOTE_ADDR']

The easiest fix I've seen right now is to simply overwrite $_SERVERREMOTE_ADDR? via wp-config.php, as REMOTE_ADDR is rarely if ever useful in this situation.

I like the idea of a mapping function in wp-config.php.

We could also use a define, actually. It would make things even easier. As in:

// add a WP_REMOTE_ADDR in your wp-config.php
if ( !defined('WP_REMOTE_ADDR') ) {
  // run our best guess as what the IP is
}

That way, users can override the WP procedure if it doesn't work for them.

That would work as well, though I think this looks somewhat odd looks somewhat odd:

define('WP_REMOTE_ADDR', $_SERVER['X-REAL-IP']);

But that's likely because you don't normally assign predefined variables to constants.

Perhaps leave an example in wp-config.php commented out so that it's discoverable?

Hmm, should we even try to set REMOTE_ADDR if WP_REMOTE_ADDR is not defined? I don't think we can safely do anything aside from leaving REMOTE_ADDR alone.

Replying to ryan:

Hmm, should we even try to set REMOTE_ADDR if WP_REMOTE_ADDR is not defined? I don't think we can safely do anything aside from leaving REMOTE_ADDR alone.

Agreed. Not to mention you shouldn't blindly trust HTTP_X_FORWARDED_FOR. This can cause trouble:

​http://marc.info/?l=bugtraq&m=108239864203144&w=2

I don't think anything but REMOTE_ADDR should be done on it's own. Leave it up to the user to decide if they should be trusting an arbitrary header.

That said, it might be best to do some sort of validation if a non-remote_addr is used to ensure the response is sane. I think remote_addr is considered safe because it's calculated by PHP. Other arbitrary headers are not.

Amen to that. New patch sticks to adding a comment into the wp-config-sample.php file to highlight extra defines and potential IP address issues for people behind load balancers.

There isn't much point in adding a define if it's were not trying to fix anything: we might as well have users set the $_SERVER variable as needed.

Just kinda freehanding... but maybe this can be simplified to handle things like TP_X_FORWARDED_FOR a little easier...

I just freehanded this for a proposed example... not saying this is perfect, or even sane... just putting ideas forward.

/**
 * If you're behind a load-balancer or CDN, you may want to set
 * WP_REMOTE_ADDR_HEADER to the correct header.  If comma delimited,
 * it will fetch the first IP address.
 */
define('WP_REMOTE_ADDR_HEADER', 'X-Real-IP');

/**
 * There are quite a few more defines for advanced users. See the wp-settings.php
 * file for details.
 */

/**
 * Fix remote address behind a load balancer
 *
 * If what follows doesn't work with your setup, configure the define in your wp-config.php file
 *
 * @since 2.9
 */
if( defined('WP_REMOTE_ADDR_HEADER') ) {
	if( isset($_SERVER[WP_REMOTE_ADDR_HEADER]) ){
		if( strpos($_SERVER[WP_REMOTE_ADDR_HEADER],',') !== false){
			$remote_ip = explode(',', $_SERVER[WP_REMOTE_ADDR_HEADER]);
 			$remote_ip = $remote_ip[0];
		} else {
                	$remote_ip = $_SERVER['WP_REMOTE_ADDR_HEADER'];
		}
		if( preg_match('/(\d+).(\d+).(\d+).(\d+)/',$remote_ip) ){
			$_SERVER['REMOTE_ADDR'] = $remote_ip;
		}
		unset($remote_ip)
	}
}

There's actually a separate potential issue if we try to fix this -- we'd potentially need to strip out local addresses. The bottom-line is, as was suggested further up, that we probably shouldn't even try to fix this -- but 9235.4.diff could come in handy to highlight the variable we're using.

@ryan - shall we get the extra comment in wp-config-sample.php, or close as wontfix?

+1 for including a check in core.. -1 for just having a comment.

This doesn't only affect load balances, It also affects extracting the correct IP when the web browser is behind a proxy.

Look at what other software which relies on IP addresses a bit more has done, for example, docuwiki: ​http://dev.splitbrain.org/reference/dokuwiki/inc/common.php.source.html#l591

Sure, Its a bit complex for what we need.. but it shows that if IP's are to be stored/mentioned (ie. comments), WordPress might as well get it right.

It can even be in a function such as that and only called when needed (ie. comment being added) to reduce server overheads, most page loads dont need the "real ip"

Of course, If something like this was included in core, The "proxies" ip would also have to be taken into consideration for spam checks and coment-flow-control, Else a client could continuously spoof a proxied header resulting in bypassing the comment-flow limits..

Using another header for the real IP requires knowledge about how the server is setup. You'd need a whitelist of internal addresses so that people couldn't fool your system into accepting an incorrect IP address. Take a look at Apache's mod_rpaf:

​http://stderr.net/apache/rpaf/

That's what I use to give Apache the correct IP address from nginx. But it's whitelisted so that if you do manage to connect to Apache directly, you can't fill in that header and have Apache use it.

I don't think that this belongs at the application level. Closing as WONTFIX, but if someone feels really strongly about this, reopen and make your case for why doing it at the app level is better than doing it at the HTTP server level.

I'd like to see support for this (probably as commented out code in wp-config.php) . Doing this at the application level is a Good Thing. An example of another webapp that does this is MediaWiki, which uses a ​somewhat obscure setting to control what source IPs are considered 'trusted proxies'.

I don't think this is a good thing to hand off to the http layer. mod_rpaf obliterates the real source IP, so while you can see what the client's address is, it's no longer possible to tell which of your upstream servers a request came from. It's possible some web servers don't even support this, I know Apache & nginx do but unsure about IIS.

The correct solution here is very much server specific and not something to go into the core code.

Re-closing as WONTFIX

@westi I do agree but how do I fix this without hacking the core ?
I think that at least a hook/filter should be added.

I agree that a filterable get_ip() function would be useful.

Introduce wp_get_ip()

wp_get_ip.diff introduces wp_get_ip()

I think that filtering is a good solution here. It would be trivial to then create a plugin for any load balancer/proxy.

Patch makes sense to me.

I would go with probably wp_remote_ip(). We should probably eat our dog food and use this in place of REMOTE_ADDR checks.

See wp_remote_ip.diff.

wp-includes/comment.php is the only place where REMOTE_ADDR is used (that I could find).

Looks like it's also in ms-functions.php. (And also akismet, and referenced in simplepie and phpmailer.)

Why do we need a filter?

A plugin can play with the REMOTE_ADDR superglobal on plugins_loaded - is that not early enough?

Replying to westi:

Why do we need a filter?

A plugin can play with the REMOTE_ADDR superglobal on plugins_loaded - is that not early enough?

I agree, I was just coming here to make the same comment. A plugin should be fixing the global.

Cross referencing [15560] as well as this potential snippet, good for wp-config.

if ( ! empty( $_SERVER['HTTP_X_FORWARDED_FOR'] ) && preg_match( '/^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/', $_SERVER['HTTP_X_FORWARDED_FOR'] ) )
	$_SERVER['REMOTE_ADDR'] = $_SERVER['HTTP_X_FORWARDED_FOR'];

Again, specific to your environment. You also may want to check for HTTP_X_FORWARDED_PROTO for HTTPS.

Related: #16932

This is a pretty serious problem for those of us moving behind a load balancer, the comment anti-flood kind of creeps up on you. Ideally, Wordpress would notice 127.0.0.1 IPs and put up a little one-time alert in the admin interface and at a minimum provide an out-of-the-box wp-config solution, like the one nacin proposed (which worked great).

There is bad advice on the web (for example, modifying comments.php, and comment replies in that thread are closed), so if wp core can help, it'll be best for everyone.