• Resolved Peter

    (@pkthree)


    The client IP check function does the following:

    function lz_getip(){
    	if(isset($_SERVER["REMOTE_ADDR"])){
    		return $_SERVER["REMOTE_ADDR"];
    	}elseif(isset($_SERVER["HTTP_X_FORWARDED_FOR"])){
    		return $_SERVER["HTTP_X_FORWARDED_FOR"];
    	}elseif(isset($_SERVER["HTTP_CLIENT_IP"])){
    		return $_SERVER["HTTP_CLIENT_IP"];
    	}
    }

    Shouldn’t it check REMOTE_ADDR last? We have Varnish in front, and are setting the real client IP in HTTP_X_FORWARDED_FOR. Other reverse proxies set HTTP_CLIENT_IP, but we typically never overwrite or remove REMOTE_ADDR. Thus, REMOTE_ADDR should be a fallback. Otherwise when you block an IP in a reverse proxy situation, you are blocking everybody.

    Also, “Your IP Address” in the plugin settings display should use lz_getip; currently it’s hard-coded to show REMOTE_ADDR.

Viewing 6 replies - 1 through 6 (of 6 total)
  • Plugin Contributor loginizer

    (@loginizer)

    Hi,

    Sorry for the late reply.
    But you might be interested in this :
    https://stackoverflow.com/questions/3003145/how-to-get-the-client-ip-address-in-php

    Trusting the other values first has security implications. Hence we first use REMOTE_ADDR.

    Also, “Your IP Address” in the plugin settings display should use lz_getip; currently it’s hard-coded to show REMOTE_ADDR.

    I have updated this. Thanks.

    Thread Starter Peter

    (@pkthree)

    I understand your concern with other IP headers. However, as it stands your plugin is not usable for a site behind a CDN or reverse proxy, because when a block is triggered, it will block everybody. REMOTE_ADDR is typically the IP of the CDN or reverse proxy. In such setups, if you check REMOTE_ADDR first, you will never check the other headers.

    One way to address the concern of untrusted proxies causing a false positive is to have a list of trusted proxy IPs for which the X-Forwarded-For or Client-IP headers will be respected.

    Plugin Contributor loginizer

    (@loginizer)

    Hi,

    Knowing all proxy IP addresses is impossible.

    One way to address this would be to make a setting to let the admin decide the preference.

    What’s your thoughts ?

    Regards

    Thread Starter Peter

    (@pkthree)

    A setting essentially solves the issue!

    Every install is different, and often you do know the proxy IP addresses in the case where you’ve set up your own proxy with Varnish. But in any case, if the admin can decide to trust X-Forwarded-For, then that puts the power and responsibility in their hands.

    Plugin Contributor loginizer

    (@loginizer)

    Hi,

    This is now there in version 1.3.2.

    Regards

    Thread Starter Peter

    (@pkthree)

    Thanks!

Viewing 6 replies - 1 through 6 (of 6 total)
  • The topic ‘Client IP logic’ is closed to new replies.