[squid-dev] [PATCH] connections_encrypted ACL

[Amos Jeffries]

On 5/01/2016 5:25 a.m., Christos Tsantilas wrote:
> On 01/02/2016 05:23 AM, Amos Jeffries wrote:
>> On 2015-12-30 23:23, Christos Tsantilas wrote:
>>> This patch initialy discussed in squid-dev under the thread "[PATCH]
>>> received_encrypted ACL" some months ago:
>>> http://lists.squid-cache.org/pipermail/squid-dev/2015-July/002680.html
>>>
>>> The "received_encrypted" was the original name of a new ACL which in
>>> this patch (t7) renamed to connections_encrypted
>>>
>>> I am reposting here as new patch.
>>>
>>> (New) Patch description:
>>>
>>> The new connections_encrypted ACL matches transactions where all HTTP
>>> messages were received over TLS transport connections, including
>>> messages received from ICAP servers.
>>>
>>> Some ICAP/eCAP services receive data from unencrypted sources. Some
>>> ICAP/eCAP services are "secure". By default we assume that all eCAP
>>> services and all ICAP services on TLS transport connections  are
>>> "secure" unless the user uses the "connection_encryption" option in
>>> service configuration line.
>>>
>>> This is a Measurement Factory project.
>>
>>
>> in src/acl/ConnectionsEncrypted.h:
>>
>> * the #ifndef wrapper string s/ENTRYPTED/ENCRYPTED/
>>
>> * please use a space between "operator="
>>
>> * please add empty line between end of namespace and file wrapper #endif
>>
>>
>> in src/adaptation/ServiceConfig.h:
>>
>> * something seems wrong about the use of connectionsEncryptedSet member.
>>   - it is using twice as much memory to configure a boolean option than
>> it should be able to.
> 
> Probably the connectionsEncryptedSet should renamed to
> overwriteConnectionsEncrypted os something like that.
> 

What I meant was that it should not be necessary at all. Its very
existence increases the number of variables used to say whether the
connection is encrypted. But...

>>
>> The options are documented as being configured *after* the service URI.
>> So it seems to me that the service URI should set the flag default based
>> on the icap/icaps (or ecap/ecaps) nature. Then the user option set/unset
>> it later if they want to override. No need for a separate "user set it"
>> member, or for special casing of ICAP/eCAP non-S.
> 
> 
> An ICAP service can configured to use TLS even if the icaps is not used
> in URI scheme. Just the "tls" option in icap_service configuration line
> is enough to enable TLS for service.
> 
> The ecaps is not used in ecap URIs, the ECAP services by default
> considered always as secure because no connection to external sources is
> required.
> 
> The final decision about encrypting or not the ICAP connections in the
> ICAP related squid code does not taken while the configuration line is
> parsed, but in later step, just before the service configured and starts.
> 

Okay. Thats a pretty nasty design problem. Fair enough on keeping the
member then. Just add a // comment about why it exists.

> 
>>
>> On the dumpCfg() side the service URI type could be checked to see if it
>> the value needs to be dumped out or not. The user setting it to the
>> default is a no-op.
> 
> hmm.....
> The Adaptation::Config::dumpService() functions which prints an
> adaptation service looks wrong ...
> 
> If no problem I will not touch this method now, and I will make a new
> patch which fixes this method.
> 

Okay for fixing how it dumps other settings, but please make it dump the
new variable as correctly as you can.

>>
>> in src/cf.data.pre:
>>
>> * the documentation implies that connection_encrypted=on turns the taint
>> check *OFF* for ICAPS connections. But does not un-taint ICAP
>> connections. The code does not seem to match.
> 
> You are right I confused too reading these lines.
> 
> * For secure ICAP service the  connection_encrypted=on option is the
> default, the user can use the connection_encrypted=off to change the
> default behaviour.
> * For plain ICAP services the default is the connection_encrypted=off
> and the user can use the connection_encrypted=on to change the default
> behaviour.
> 
> Are the following lines enough for ICAP service:
> "The default is "on" for Secure ICAP services (i.e., those with the
> icaps:// service URIs scheme) and "off" for plain ICAP services."
> 

Yes that is fine.


> The existing related documentation line for ECAP looks OK to me.

Yes.

Amos