[squid-dev] [PATCH] Secure ICAP

[Tsantilas Christos]

If there are not any objections  I will apply the latest patch to 
squid-trunk.

On 04/23/2015 08:58 PM, Tsantilas Christos wrote:
> A new patch  for Secure ICAP.
> This is synced with the latest trunk.
>
> Also handles Amos requests.
>
> Regards,
>     Christos
>
> On 04/10/2015 04:51 AM, Amos Jeffries wrote:
>> On 10/04/2015 2:43 a.m., Tsantilas Christos wrote:
>>>
>>> This patch adds support for ICAP services that require SSL/TLS transport
>>> connections.
>>>
>>> To mark an ICAP service as "secure", use an "icaps://" service URI
>>> scheme when listing your service via an icap_service directive.
>>
>> The squid.conf parsing code contradicts that - it expects "... ssl
>> icap://...".
>
> Inside Adaptation::Icap::ServiceRep::finalize() method if the protocol
> is the "icaps" then sets the secure.encryptTransport flag to true. IT
> can be used to enable secure icap without any ssl option.
>
>>
>>
>>>
>>> Squid uses port 11344 for Secure ICAP by default, following another
>>> popular proxy convention. The old 1344 default for plain ICAP ports has
>>> not changed.
>>>
>>> This patch should applied after the "server_name" and "splicing resumed
>>> sessions" patches applied to trunk, and after re-merged with the trunk.
>>> However we can start the discussion if you agree.
>>>
>>>
>>> Technical Details
>>> ==================
>>>
>>> This patch:
>>>    - Splits Ssl::PeerConnector class into Ssl::PeerConnector parent and
>>> two kids: Ssl::BlindPeerConnector, a basic SSL connector for
>>> cache_peers, and Ssl::PeekingPeerConnector, a peek-and-splice SSL
>>> connector for HTTP servers.
>>>
>>>    - Adds a third Ssl::IcapPeerConnector kid to connect to Secure ICAP
>>> servers.
>>>
>>>    - Fixes ErrorState class to avoid crashes on nil ErrorState::request
>>> member. (Ssl::IcapPeerConnector may generate an ErrorState with a nil
>>> request).
>>>
>>>    - Modifies the ACL peername to use the Secure ICAP server name as
>>> value while connecting to an ICAP server. This is useful to make SSL
>>> certificate  policies based on ICAP server name. However, this change is
>>> undocumented until we decide whether a dedicated ACL would be better.
>>>
>>>
>>> This is a Measurement Factory project.
>>>
>>
>> Initial audit:
>>
>> * Watch for HERE macros in the new, changed or moved code. I see at
>> least one being added in PeerConnector.cc
>
> fixed
>
>
>
>>
>>
>> in src/acl/FilledChecklist.h:
>>
>> * please use an SBuf for dst_peer_name
>>   - that will also prevent memory leaks in
>> Ssl::IcapPeerConnector::initializeSsl().
>
> ok.
>
>
>>
>> in src/adaptation/ServiceConfig.cc:
>>
>> * the new "ssl" option needs some polishing.
>>   - for the dependency WARNING it should have:
>> debugs(3, DBG_PARSE_NOTE(DBG_IMPORTANT), "WARNING: ... ICAP service
>> option ignored.");
>>   - since this is all new code please use "tls-" as the TLS/SSL options
>> prefix.
>
> OK
> I am not documenting "tls-" prefix of the options in cf.data.pre.
> But it works.
>
>>
>>
>> in src/cf.data.pre:
>> * the new securtiy options section title should have s/HTTPS/ICAPS/
>
> OK.
>
>
>>
>>
>> in src/adaptation/icap/ServiceRep.cc:
>>
>> * please create/use #define DEFAULT_ICAP_PORT and DEFAULT_ICAPS_PORT
>> instead of magic port numbers.
>>
>>
>
> ok.
>
>> in src/adaptation/icap/ServiceRep.h:
>>
>> * please use Security::ContextPointer instead of SSL_CTX* for sslContext.
>>   - and it can go outside the USE_OPENSSL wrapper
>
> ok
>
>>
>>
>> in src/adaptation/icap/Xaction.cc:
>>
>> * Adaptation::Icap::Xaction::noteCommClosed() does not need to wrap
>> securer use with USE_OPENSSL.
>
> fixed.
>
>
>>
>>
>> in src/err_detail_type.h:
>> * whats with the change to ERR_DETAIL_REDIRECTOR_TIMEDOUT ?
>>    - if thats a bug-fix you found along the way it should go in separate
>> for backporting.
>
> Yes it is a bug.
> I committed as separate patch. It is a trunk only bug.
>
>
>>
>>
>> NOTE: I've not done much audit on the PeerConnector changes, my brain is
>> too fuzzy right now. Though I am liking the class split.
>>
>>
>> Amos
>> _______________________________________________
>> squid-dev mailing list
>> squid-dev at lists.squid-cache.org
>> http://lists.squid-cache.org/listinfo/squid-dev
>>
>
>
>
> _______________________________________________
> squid-dev mailing list
> squid-dev at lists.squid-cache.org
> http://lists.squid-cache.org/listinfo/squid-dev
>