[TYPO3-core] RFC: Fix bug #7397: Proxy servers replace REMOTE_ADDR with their own IP

[Martin Kutschker]

Michael Stucki schrieb:
> Hi Martin,
> 
> finally I could try this patch.
> 
> Martin Kutschker wrote:
> 
>>>> Problem:
>>>> When requesting the clients REMOTE_ADDR, it can happen that there is a
>>>> proxy in between server and client, which replaces the value with his
>>>> own IP, and puts the original IP in HTTP_X_FORWARDED_FOR instead.
>> It's not clearly stated here and in the bug report, but generally we
>> only want to handle well-know reverse proxies. Anything else would be a
>> security risk.
> 
> I absolutely agree!
> 
>>> Here's a new patch. This one is more secure as it ties TYPO3 to a set of
>>> know proxies. Furthermore you may define that one or more proxies use
>>> SSL in connection to the Internet. And additionally it's possibly to add
>>> a prefix for http and https proxies in case there is a (weird) path
>>> changing proxy setup in place (seems to be the case with some mass
>>> SSL-BE hosters).
>> The 3rd version uses now SYS[reverseProxy*] to signify more the real
>> usage.
>>
>> It also has now a config that defines how to deal with multiple values
>> in HTTP_X_FORWARDED_FOR/HTTP_X_FORWARDED_HOST. Possible options to use
>> the first, the last or none. None is the default.
> 
> Makes perfect sense. However, I (currently) have no server to test this.
> Will do later. What I did check was whether it will break some existing
> behaviours, and it does not! So I would say you should commit this patch.
> 
> However, the code has some issues.
> 
> Most important: Please _always_ check your patches before committing them!
> Just run "svn diff" and you would see this, for example: Your patch removes
> a line in config_default.php which should stay there (unless you provide a
> good reason for it):
> - 'SC_mod_web_perm_ajax::dispatch' => [...]

Fixed.

> config_default.php: The description for "reverseProxySSL" refers
> to "SYS[proxyIP]" intead of "SYS[reverseProxyIP]". Must be fixed.

Fixed.

> There seems to be a mistake in the case switch for "TYPO3_SSL": The
> comparison is always made against "SYS[reverseProxyIP]" disregarding of
> the "SYS[reverseProxyIP]" setting. Must be fixed.

Fixed. FYI: TYPO3_SSL checks for the IP addresses in reverseProxySSL. If 
this contains *, it will fall back to check against reverseProxyIP. This 
is for convenience reasons, so you don't have to ste the same IP list 
for both values, but is flexible enough if only some proxies serve SSL.

> Various things:  (all marginal, can be done later)
> - the cmpIP comparison against "reverseProxyIP" should only be only made if
>   the value is set

cmpIP now returns false if the list to compare against is empty. This is 
ok as only * as wild card should return true.

> - typo in config_default: "when SSL accessing the server via an SSL proxy"

Fixed.

Masi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bug_7397_v4.diff
Type: text/x-diff
Size: 17271 bytes
Desc: not available
Url : http://lists.netfielders.de/pipermail/typo3-team-core/attachments/20080301/f9eed5bf/attachment-0001.diff