laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.22k stars 10.91k forks source link

request()->getClientIp() is returning right-most IP instead of left-most IP. #25684

Closed andreshg112 closed 6 years ago

andreshg112 commented 6 years ago

Description:

I have logged this in a AWS server.

{
"getOriginalClientIp":"190.9.199.83",
"getClientIp":"70.132.5.137",
"getClientIps":["70.132.5.137","190.9.199.83"],
"x-forwarded-for":"190.9.199.83, 70.132.5.137"
}

getOriginalClientIp() is a custom function. getClientIp is request()->getClientIp(). You can see it is returning the right-most IP in x-forwarded-for. getClientIps is request()->getClientIps(). You can see is sorting inversely IPs that comes from x-forwarded-for. x-forwarded-for is request()->header('x-forwarded-for'). This is the header received from AWS API Gateway.

Custom function:

function getOriginalClientIp(Request $request = null) : string
    {
        $request = $request ?? request();
        $xForwardedFor = $request->header('x-forwarded-for');
        if (empty($xForwardedFor)) {
            // Si está vacío, tome la IP del request.
            $ip = $request->ip();
        } else {
            // Si no, viene de API gateway y se transforma para usar.
            $ips = is_array($xForwardedFor) ? $xForwardedFor : explode(', ', $xForwardedFor);
            $ip = $ips[0];
        }
        return $ip;
    }

getClientIp():

/**
     * Returns the client IP address.
     *
     * This method can read the client IP address from the "X-Forwarded-For" header
     * when trusted proxies were set via "setTrustedProxies()". The "X-Forwarded-For"
     * header value is a comma+space separated list of IP addresses, the left-most
     * being the original client, and each successive proxy that passed the request
     * adding the IP address where it received the request from.
     *
     * @return string|null The client IP address
     *
     * @see getClientIps()
     * @see http://en.wikipedia.org/wiki/X-Forwarded-For
     */
    public function getClientIp()
    {
        $ipAddresses = $this->getClientIps();

        return $ipAddresses[0];
    }

app/Http/Middleware/TrustedProxies.php:

<?php

namespace App\Http\Middleware;

use Fideloper\Proxy\TrustProxies as Middleware;
use Illuminate\Http\Request;

class TrustedProxies extends Middleware
{
    /**
     * The trusted proxies for this application.
     *
     * @var array
     */
    protected $proxies = '**';

    /**
     * The headers that should be used to detect proxies.
     *
     * @var string
     */
    protected $headers = Request::HEADER_X_FORWARDED_ALL;
}

I want to use this package https://github.com/antonioribeiro/firewall, but It uses $request->getClientIp(). So I have to make it work in order to use the package.

Steps To Reproduce:

image

staudenmeir commented 6 years ago

getClientIp() is a Symfony\Component\HttpFoundation\Request method. Please use their repository.

A quick search turned up this: https://github.com/symfony/symfony/issues/27867

staudenmeir commented 6 years ago

Please close the issue.

sisve commented 6 years ago

I believe this to be the correct behavior. This header is used by proxies, but we have no way of trusting the values. Anyone can set a X-Forwarded-For-header, and the only way we trust those values is if the value comes from a trusted proxy. Even then we can only trust the last value, the one inserted by your trusted proxy. Every other address is a potential lie from a malicious user.

eleracmp commented 3 years ago

I have the same Issue. I think it is a bug. The header has the format: X-Forwarded-For: , , and the name of the method id getClientIp not getMostTrustedIp or getLastProxyIp so, it has to give the client ip. It doesn't matter what people trust.

jleonardolemos commented 1 year ago

Sorry about resurrect this issue again. I have two proxies(1 - Cloudflare, 2 - AWS Load Balancer). What would be the best and secure way to retrieve the real client IP in this scenario???

Is there a easy way to retrieve the client real IP from request()->ip()??? For sure i can do a "smart logic" to realize this but is there a "Laravel Way" for do this?

jleonardolemos commented 1 year ago

Just an update from anyone that see this thread in the future. I was able to solve the problem setting the "preserve mode" attribute for XFF header in my load balancer. This can be a serious security issue because allow tempering the XFF header then you need to make sure that your load balancer is receiving requests only from the CloudFlare. Just allow only the cloudflare IPS in the LB security group.