Re: [RFC] Handle ACLs that are neither denied nor allowed

From: Alex Rousskov <rousskov_at_measurement-factory.com>
Date: Thu, 22 Mar 2012 01:10:14 -0600

On 03/20/2012 06:09 PM, Amos Jeffries wrote:
> On 21.03.2012 06:33, Alex Rousskov wrote:
>> For example, ssl_bump supports slow ACLs but it cannot trigger
>> authentication. Thus, it must use authentication information already
>> available and must not be forced to decide whether that information is
>> fresh or stale.
>
> The first response byte is the boundary for "too late" on challenge same
> as any other HTTP status codes.
>
> When ssl_bump is run we have not yet sent any response bytes to the
> client. Therefore it is not too late.

I agree that it is technically possible to teach ssl_bump to trigger
authentication. It is also possible to teach tcp_outgoing_tos and many
other ACL-driven options to trigger authentication. My "cannot trigger"
wording was inaccurate. Yes, authentication can be triggered until we
send something else to the client, but my argument is that we should not
trigger authentication anywhere except in a few specific, hand-picked
places such as http_accept (even if we can).

Kinkie: I also agree that separating authentication from access control
is worth considering, but I think it is kind of orthogonal to what is
being discussed here. Authentication may or may not be extracted into
its own step, but it should not spread around the code that has nothing
to do with it and is very likely to handle it wrong. In other words, it
would be wrong to try to support something like:

    tcp_outgoing_tos authenticate foobar

>> IMHO, if authentication is involved, the only sane option is to follow
>> this pattern:
>>
>> 1a. Authenticate (may require several HTTP transactions).
>>
>> 1b. Iff authenticated user is authorized to access Squid, continue.
>> Otherwise, deny further access.
>>
>> 2. Use authenticated user credentials as needed.
>> No authentication can be triggered at this stage.
>> Credentials cannot become invalid or stale here.

> They *can* become stale.

In my proposal, the authenticated and accepted credentials cannot become
stale in the transaction context/lifetime. They simply do not have any
expiration date from this design point of view. Once accepted for a
transaction, they stay valid for that transaction until the transaction
is over.

> But we may use them anyway when there is no
> security risk. When there is security involved we must not use them
> until revalidation. That is the point of EXPIRED_OK. It means we are
> waiting an async helper lookup to re-validate them, but at last
> validation they were okay. The access list context is where the
> information about security-risk is implicit/known.

In my model, this complexity (and the need to determine the level of
security risk) simply does not exist (because I see no reason to make
credentials state variable during a single transaction lifetime).

> Consider: Admin closes a users account and removed them from the system.
> Just before that the user opened a CONNECT and starts using it for a
> week non-stop. All of the bumped requests are authorized under the
> expired credentials?

This is not a very good example because individual bumped requests
inside a CONNECT tunnel are normally not authenticated (the CONNECT
tunnel itself is). So, under normal circumstances, the already
authorized tunnel will remain authorized (and so will any transactions
inside it, bumped or not).

If this is not desirable, we can add a feature to forcefully terminate
in-progress transactions (and tunnels) under some conditions, but such
termination will not be put inside ssl_bump, tcp_outgoing_tos, and
similar option handling code that should have nothing to do with
transaction lifetime and authentication/authorization.

If we consider an abnormal situation where requests inside a tunnel are
individually/independently authenticated, then the next request will
fail authentication, of course. However, again, this will not happen in
ssl_bump or similar option processing code. It will happen in
http_access or similar code dedicated to authentication.

In other words, we should not try to write this kind of code:

    if (option1 && authenticated issues) ...
    if (option2 && authenticated issues) ...
    if (option3 && authenticated issues) ...
    if (option4 && authenticated issues) ...
    if (option5 && authenticated issues) ...

we should try to write this kind of code instead:

    if (option1 && authenticated issues) ...
    if (option2) ...
    if (option3) ...
    if (option4) ...
    if (option5) ...

(I am simplifying in this sketch but I hope you get the idea).

> or credentials suddenly change to EXPIRED_BAD or
> AUTH_REQUIRED and the sequence gets aborted (no challenge inside the
> bumping).

In my model, credentials already assigned to a transaction never change.

> This same scenario could also happen with hijacked accounts getting an
> emergency change on the password. Or any routine operations doing the same.
>
>
> NOTE: I think your suggestion of passing some info to the checklist
> indicating what to do about auth could be handy here to simplify things
> and hide the EXPIRED_* handling inside the checklist.

Yes, but the other bit is as important: We should limit the code that
has to deal with authentication and access denials. Some special code
will "pass some info" and will be ready to handle the consequences
accordingly (e.g., send a fresh challenge to the user). All other code
will not have to deal with authentication and access control at all!

>> Http_access belongs to #1. ssl_bump belongs to #2. The high-level Squid
>> code that initiates ACL checking should know its category and use the
>> right API. The low-level ACL code cannot make this determination -- it
>> should just rely on the high-level code setting the appropriate options
>> to allow or prohibit outcomes that trigger authentication.

> I'm starting to think we should clean it into two separate enums.
> * The checklist results being allow/deny/dunno/auth-required like
> before but enumerated clearer.

I do not think auth-required should be a part of this enum.
allow/deny/other is sufficient (where "other" comes with error,
auth-required, and possibly other details that caller/main code handles
depending on that code semantics; e.g., ssl_bump would warn and not bump
in all "other" cases).

> * a match() results enum with the full set of case states for checklist
> to handle any tricky conversions internally.

Can you give an example where match() needs to return something other
than a "yes", "no", or "other" answer? I cannot think of any tricky
conversions the code would need to perform in my model, but it is likely
I am missing some cases.

Thank you,

Alex.
Received on Thu Mar 22 2012 - 07:10:27 MDT

This archive was generated by hypermail 2.2.0 : Thu Mar 22 2012 - 12:00:06 MDT