[squid-dev] [PATCH] Secure ICAP

[Tsantilas Christos]

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
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: Secure-ICAP-t5.patch
Type: text/x-patch
Size: 103684 bytes
Desc: not available
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20150423/957cde71/attachment-0001.bin>