[squid-dev] [PATCH] received_encrypted ACL

[Tsantilas Christos]

On 07/23/2015 07:21 PM, Alex Rousskov wrote:
> On 07/23/2015 07:41 AM, Amos Jeffries wrote:
>> On 23/07/2015 3:32 a.m., Alex Rousskov wrote:
>>> On 07/21/2015 04:25 AM, Amos Jeffries wrote:
>>>> On 21/07/2015 9:42 a.m., Alex Rousskov wrote:
>>>>>>> adaptation_access icapS aclIcap
>>>>>>> adaptation_access icapN !aclIcap
>>>>>>>
>>>>>>>
>>>>>>> aclIcap can be a received_encrypted ACL. What ACL expression would you
>>>>>>> suggest for aclIcap if received_encrypted is not available?
>>>
>>>
>>>>>>    # top 5 criteria - TLS transactions
>>>>>>    acl aclIcap allof HTTPS
>>>
>>>>> What is "HTTPS" ACL? If it is based on the URI scheme, then AFAIK, it is
>>>>> not going to work because it will not match bumped http:// requests (at
>>>>> least).
>>>
>>>> Good you have seen those. That is what the HTTPbis WG call
>>>> "opportunistic encryption" traffic.
>>>
>>>
>>> I bet many (most?) of them are actually regular HTTP requests sent over
>>> TLS/SSL connections. There is nothing opportunistic about them.
>>
>> That is the very definition of "opportunistic" in the related HTTP
>> documentation.
>
>
> Hi Amos,
>
>      The opportunistic security (OS) documents you site talk about
> optionally applying encryption to traffic X, to arrive at traffic XE. A
> general-purpose intermediary cannot use those documents to go backwards
> and say that any applied encryption was optional if we see traffic XE!
> This backward direction is possible only when X or XE includes some sort
> of an "it is OK to decrypt at any time" flag. Until such a flag is
> defined for HTTP and/or TLS, and Squid learns to interpret it, the
> discussion about opportunistic encryption is pretty much not relevant to
> the received_encrypted ACL.
>
> Moreover, the received_encrypted ACL itself and the related use cases do
> not become wrong or obsolete when/if "it is OK to decrypt at any time"
> flag is supported.
>
> Is "http://" scheme such a flag? Not for Squid. That URI scheme existed
> long before any of the HTTP WG proposals you site were written, so Squid
> should not assume that an encrypted http:// request is the result of OS.
> Either a more reliable flag or an admin decision is required to enable
> such behavior. All of this is orthogonal to received_encrypted.
>
>
>
>>>> This is where the security requirements and your ACL use-case differ.
>>>> Security requirements for those requests is that they be handled as per
>>>> any other http:// - any encrytpion or security applied is *optional*.
>>>
>>> No requirement can _force_ something to be optional! If the use case
>>
>> I think you misunderstand me.
>>
>> The forcing applied is: optional->mandatory.
>>
>> It is done by using the received_encrypted ACL. Apparently without admin
>> intending to.
>
> Any assertion that uses a "received ACL forces ..." pattern is wrong. An
> ACL is a yes/no test, nothing more. Yet, you keep blaming the ACL for
> doing something other than testing. That does not compute for me.
>
>
>>> demands that requests received over TLS are sent to icapS service, then
>>> they should be sent to the icapS service, even if some of the
>>> corresponding request owners would not mind to see their requests going
>>> to the icapN service.
>>
>> If you recall my very first words on this thread. I question the use-case.
>
> What does that mean? Are you saying the use case does not exist? That I
> made it up? Or that we should not allow the admins to do what they want
> to do? Something else?
>
>
>> Look closely:
>
>> +    if (!(filled->request->sources & HttpMsg::srcUnsafe)  ||
>> +        (filled->reply && !(filled->reply->sources & HttpMsg::srcUnsafe)))
>> +        return 1;
>> +
>> +    return 0;


This is a stupid bug, probably caused while I was trying to convert a 
more complex expression to a simpler....

>
> That code does not make sense to me. I think it should be written like
> this instead:
>
>      const bool safeRequest =
>          !(filled->request->sources & HttpMsg::srcUnsafe);
>      const bool safeReply = !filled->reply ||
>          !(filled->reply->sources & HttpMsg::srcUnsafe);
>      return (safeRequest && safeReply) ? 1 : 0;

Yes!

>
> Furthermore, the values of "unsafe" srcX enum constants should be
> increased to actually match the srcUnsafe mask (16 is still smaller than
> 0xFFFF).

This is should be OK.
The safe flags are from:
  (1 << (16 + 0)) = (1 << (16 + 0)) & 0xFFFF0000
to:
   (1 << (16 + 15)) = (1 << (16 + 15)) & 0xFFFF0000

Am I loosing something?
(It is late here, this is not a time for playing with bits and shifts, 
my mind is broken ...)

>
>
>> While I personally like the outcome (more security). Its clearly not
>> matching the use case presented as *sole* reason for the ACLs existence.
>> Nor the apparent admin policy in adaptation_access.
>
> To me, it just looks like an implementation bug. Christos, do you think
> the code should be rewritten as in my sketch above?
>
>
>> So I ask; why are we bothering with this ACL?
>>   Just use an https:// scheme match, which will only be true in the top
>> line of above if()-matrix. Admin *wanting* "opportunistic security"
>> should not be the ones trying to avoid using icapS in the first place.
>
> I have answered that question several times already, including providing
> specific examples requested by Kinkie: The URI scheme alone does not
> match what the ACL is defined to match.
>
>
> Cheers,
>
> Alex.
>
>