Re: [PATCH] Support PROXY protocol

On 5/08/2014 2:47 a.m., Alex Rousskov wrote:
> On 07/30/2014 09:02 AM, Amos Jeffries wrote:
>
>> +NAME: proxy_forwarded_access follow_x_forwarded_for
>
>> Requests may pass through a chain of several other proxies
>> + before reaching us. The original source details may by sent in:
>> + * HTTP message Forwarded header, or
>> + * HTTP message X-Forwarded-For header, or
>> + * PROXY protocol connection header.
>
> ...
>
>> + For proxy-surrogate ports an allow match is required for Squid to
>> + permit the corresponding TCP connection, before Squid even looks for
>> + HTTP request headers. If there is an allow match, Squid starts using
>> + PROXY header information to determine the source address of the
>> + connection for all future ACL checks.
>
>> + On each HTTP request Squid checks for X-Forwarded-For header fields.
>
>
> Given the "first evaluate proxy_forwarded_access for connection, then
> evaluate proxy_forwarded_access for each HTTP request header"
> functionality described above I wonder whether it is a good idea to
> merge both evaluations into one directive. Would not it be easier for
> admin to write correct ACL rules if we keep them separate?
>
> For example, let's assume an admin wants to trust a PROXY client device
> at (and only at) address A and XFF-setting child proxy at (and only at)
> address B. If we split the functionality into proxy_forwarded_access and
> follow_x_forwarded_for, then the admin can do:
>
> proxy_forwarded_access allow A
> #proxy_forwarded_access deny all
>
> follow_x_forwarded_for allow B
> #follow_x_forwarded_for deny B
>
> Right? What about the merged implementation proposed in the patch? How
> can the admin express the above logic? AFAICT, the following will _not_
> work:
>
> proxy_forwarded_access allow A
> proxy_forwarded_access allow B
> #proxy_forwarded_access deny all
>
> because it will allow PROXY clients at B and XFF setting by A.
>

Good point. I have been considering these lookups as stateless. However,
XFF does offer the unusual property that regex can check for specific
ordering of IPs. We do not have that with PROXY+XFF.

I am adding proxy_protocol_access as the first access control, reverting
follow_x_forwarded_for for the second. But retaining the new description
texts.

Also for lack of anything better in the last week I am using
require-proxy-header for the port option.

New patch attached.

Amos