Re: Question ICAP-client

On Sat, 2007-03-10 at 16:00 +0200, Tsantilas Christos wrote:

> I think that client address/port and squid address/port must copied.
> They can not (and must not) changed by an ICAP server.
> The same about authentication information because referred to users
> authenticated on squid and this info must not changed by an ICAP server.

<rant>
We are looking at this from different angles. You perceive an ICAP
server as an extension of a client: a client should have no problem with
any adaptation of its request by such as friendly server.

I am sure we will eventually see compromised or otherwise unfriendly
ICAP servers that do nasty things. Such servers would love to do nasty
things "on behalf" of a client, using client identity if possible. Thus,
I have a problem with blindly copying sensitive client information into
requests generated (originated from) the ICAP server.

As an ICAP server developer and seller, I know that many proxy
installations today use 3rd party ICAP servers, and many proxy admins
have very little knowledge about what that ICAP server is actually
doing. Add remote updates to ICAP server software, and you have a
perfect location for a man-in-the-middle attack point.

> Not coping addresses and auth info, we are breaking the ACL checking in
> later steps (e.g in FwdState::fwdStart).

>From your point of view, the solution is to copy. From my point of view,
this breakage is a sign that we should be extra careful with copying
such information. That is, there may be a _good_ reason why ACLs do not
"just work" for requests initiated by the ICAP server.
</rant>

> Also I think, there is not any problem with HttpRequest.flags. A small
> research about the flags show me that only the flags.redirected,
> flags.accelerated, flags.transparent, flags.internal,
> flags.internalclient are computed before the ICAP reqmod called. I do
> not think that any of this flags can modified by an ICAP
> server. The other flags set after the ICAP reqmod. (I kept a list with
> flags and section code they set....)

I was concerned about this too. Thank you for doing this research! To
increase chances of preserving the above conclusion, I added a
request_flags::cloneAdaptationImmune() method and a comment. Hopefully,
when new flags are added, folks will read the comment and adjust
cloneAdaptationImmune() if needed.

> > I have no problems with committing a fix that always copies this
> > information as a temporary measure, but I think we need to at least put
> > some comments in the code that we are copying user-specific information
> > to a request that did not originate from the user.
> >
> I think it not so problem, the new request can appeared as originated by
> the same user and the same IP as the original request...

Yes, the request will appear as if it was originated from the user.
Whether that is a problem for requests that actually originated from the
ICAP server depends on the environment.

Christos, I have applied your patch, with minor modifications, to
squid3-icap. Thanks a lot for writing the patch!

Stefan, please test and file a bug report if something does not work
right after this change.

Alex.
Received on Wed Mar 21 2007 - 10:24:43 MDT