Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RequestFactory::fromLambaEvent not aggregating sourceIp from request context -> identity #130

Open
krcrawford opened this issue Dec 29, 2018 · 0 comments

Comments

@krcrawford
Copy link

krcrawford commented Dec 29, 2018

Is there a reason that additional values from the request context in the lambda event are not read into the server global?
In order to determine any sort of trusted proxy strategy, Symfony demands a $_SERVER['REMOTE_ADDR']
Aside from event['requestContext']['identity']['sourceIp'], there is really no way to populate a value for the $_SERVER['REMOTE_ADDR']
Possibly we could check headers for X-Forwarded-For, execute some parsing for multi-values, and set the $_SERVER['REMOTE_ADDR'] to either a scalar X-Forwarded-For or the last item in a multi-value X-Forwarded-For
RequestFactory::fromLambdaEvent is not currently ingesting this information, so is a PR welcome to expand that, or is this an intentional strategy?

Would the following introduce any more security risks than are likely already present in any strategy to extract the client IP?

        ...
        $server = [
            'SERVER_PROTOCOL' => $protocolVersion,
            'REQUEST_METHOD' => $method,
            'REQUEST_TIME' => $event['requestContext']['requestTimeEpoch'] ?? time(),
            'REQUEST_TIME_FLOAT' => microtime(true),
            'QUERY_STRING' => $queryString,
            'DOCUMENT_ROOT' => getcwd(),
            'REQUEST_URI' => $uri,
        ];

        if (isset($event['requestContext']['identity']['sourceIp'])) {
            $server['REMOTE_ADDR'] = $event['requestContext']['identity']['sourceIp'];
        }
        if (array_key_exists('X-Forwarded-For', $headers)) {
            $ip = [$headers['X-Forwarded-For']];
            if (preg_match('/,\s?/', $ip[0])) {
                $ip = explode(', ', $ip[0]);
            }
            if (count($ip) > 1) {
                // we've been proxied. symfony demands a remote addr to check
                // against trusted proxy set
               $last = count($ip) - 1;
               $server['REMOTE_ADDR'] = $ip[$last];
            }
        }

        if (isset($headers['Host'])) {
            $server['HTTP_HOST'] = $headers['Host'];
        }
        ...

As an aside, and if this strategy is feasible, we would also want to look at supporting trusted proxies in the RequestFactory::fromLambaEvent operation. Looks like the default symfony public/index.php file looks for those values in the $_SERVER['TRUSTED_PROXIES'] or $_ENV['TRUSTED_PROXIES']. Well, we could just expect any consumer to customize the bref.php or public/index.php file as well. So maybe this last item is a non-starter.

@krcrawford krcrawford changed the title RequestFactory::fromLambaEvent not aggregating ALL request context RequestFactory::fromLambaEvent not aggregating sourceIp from request context -> identity Dec 31, 2018
@krcrawford krcrawford changed the title RequestFactory::fromLambaEvent not aggregating sourceIp from request context -> identity RequestFactory::fromLambaEvent not aggregating sourceIp from request context -> identity Dec 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants