Re: [RFC] Breaking forwarding loops in transparent proxies

On 06/26/2009 11:00 PM, Amos Jeffries wrote:
> Alex Rousskov wrote:
>> Hello,
>>
>> Squid detects forwarding loops in most configurations, but breaks
>> them (using a customizable HTTP_FORBIDDEN response) only when working as
>> an accelerator. Squid does not break loops when working as a transparent
>> proxy. Interestingly enough, the breaking code comment (in the patch
>> below) says that both cases are covered. Perhaps the exclusion of
>> transparent mode was not done on purpose.
>>
>> I understand that forwarding loops in transparent environment are
>> usually caused by misconfiguration. However, when an admin is unable to
>> fix the problem promptly, should not we help her by breaking the loop?
>>
>> Please note that a persistent loop is going to be broken anyway, when
>> the Via and X-Forwarded-For headers exceed header size limit, but that
>> wastes a lot of resources and may also crash Squid.
>>
>> Should we break forwarding loops in transparent mode?
>
> Yes.

Committed to Squid3 trunk (r9782). Please commit to v3.1.

> Keep in mind the fact that any crash during header processing opens the
> doorway for DDoS sooner rather than later. I would go so far as to
> suggest making it abort cleanly on mode. Not just these two.

I agree, but wonder what the original rationale was behind the decision
to limit loop breaking to accelerator setups? Does anybody know?

> Might be time for a configuration error page to accompany this change as
> well. Not just access denied. The wiki search queries are growing for
> people looking for "access denied" message then some peer or
> interception config option.

Yeah. It would be nice to have a generic mechanism where admins can
specify an error page to use for virtually all error contexts. The core
code will only provide a unique context ID, and there will be default
context_ID:error_page mapping that can be customized in squid.conf. This
way, it would be much easier to customize error pages without the code
bloat.

Thank you,

Alex.

>> === modified file 'src/client_side_reply.cc'
>> --- src/client_side_reply.cc 2009-04-10 09:23:50 +0000
>> +++ src/client_side_reply.cc 2009-06-24 20:33:37 +0000
>> @@ -640,7 +640,8 @@
>> /*
>> * Deny loops when running in accelerator/transproxy mode.
>> */
>> - if (http->flags.accel && r->flags.loopdetect) {
>> + if (r->flags.loopdetect &&
>> + (http->flags.accel || http->flags.transparent)) {
>> http->al.http.code = HTTP_FORBIDDEN;
>> err =
>> clientBuildError(ERR_ACCESS_DENIED, HTTP_FORBIDDEN, NULL,
>>
>
> Amos
Received on Thu Jul 02 2009 - 15:11:26 MDT