Re: [PATCH] Support PROXY protocol

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.

Thank you,

Alex.
Received on Mon Aug 04 2014 - 14:48:00 MDT