[squid-dev] Efficient FD annotations

[Alex Rousskov]

On 1/10/20 2:37 AM, Amos Jeffries wrote:
> On 8/01/20 3:39 am, Alex Rousskov wrote:
>> On 1/7/20 1:39 AM, Amos Jeffries wrote:
>>> On 7/01/20 4:28 am, Alex Rousskov wrote:
>>>> For the record: The ideas below are superseded by the concept of the
>>>> code context introduced in commit ccfbe8f, including the
>>>> fde::codeContext field. --Alex
>>
>>> If you want to go that way (replace fde:note with fde:codeContext)
>>
>> I would not replace fde::note with fde::codeContext. I would keep
>> fde::note as a basic indication of the current FD purpose/scope. This
>> can be done cheaply using string literals or constant SBufs.

> In that case, why is this thread being revived?

FWIW, I was not trying to revive this thread. I simply added a reference
to the (official) next generation of the ideas expressed on that old
thread. IIRC, I posted this "closure" in part because PR 270 is open,
and one of my old comments there refers to this thread.


> AIUI your proposals were alternatives to PR 270 - ways to replace the
> fde::note field instead of just updating it to SBuf.

This thread kept the fde::note field and its SBuf conversion introduced
in PR 270. The thread focused on adding transaction information without
the performance sacrifices. Please see the original message for details:
http://lists.squid-cache.org/pipermail/squid-dev/2019-February/009503.html


>>> we are going to have to do a security audit on the values displayed
>>> by the CodeContext objects. That is due to how the fde::note are sent
>>> over the public network in clear-text transactions for
>>> mgr:filedescriptors report.
>>
>> Overall, I doubt such an audit is a good idea -- only the Squid admin
>> can correctly decide whether it is OK to expose transaction information
>> in cache manager responses[1,2].
> 
> see responses below to [1] and [2]. Since you are apparently not seeking
> to replace the mgr:filedescriptors report fde::note field with
> fde::codeContext display - then please take it as a general security
> approach policy for handling that type of change.

Sorry, I do not understand what "it" and "that type of change" refers to
in your last sentence, especially since those references follow a text
that describes what is _not_ going to happen (i.e. "you are apparently
not ..."). My argument was (and is) that an audit without a
configuration option (or an equivalent shared definition of "sensitive"
and "exposure") would be largely impractical or meaningless because the
auditor would not be able to properly identify problems with exposure of
sensitive info.


>> If there is demand for limiting that
>> exposure, I would rather add a configuration directive that would allow
>> the admin to control whether Squid is allowed to report context in cache
>> manager responses, error pages, etc.

> We do not have to default such an option to publish everything on the
> off chance it is safe. Our objective here is to prevent CVEs occuring.
> To achieve that we should be defaulting to elide sensitive details from
> public view unless configured to show them.

I have no problems with the "do not publish" default for areas where the
information may go to a 3rd party unexpectedly (for a reasonable admin).
You started your paragraph with "we do not have to X". I hope I did not
imply anywhere that we have to X.


> The details I am thinking of most prominently here are things like
> credentials and tokens in the auth processing contexts. URL history and
> keys in the SSL-Bump context. Access to Squid internal memory spaces on
> the server. There are probably others we will uncover later. Anything
> that could be reported as a sensitive information leak and assigned a CVE.

I think we are on the same page here although, IIRC, we already report
URLs in fde::notes.


>> [1] Some of the transaction context is already exposed in the current
>> cache manager responses. We may want to add more details or report fewer
>> details, but there is no paradigm shift here.

> "some" renders this argument irrelevant. The leak issues will be
> cropping up about the *new* data if we let that contain anything sensitive.

I do not follow. If we already expose sensitive data in interface X,
then a reasonable admin would be expected to protect that interface and,
hence, we can add similarly sensitive details there with little risk.
The only other reasonable action that I can see is to file a CVE for
what we are already doing, admitting that we have exposed too much (even
though nobody complained [loudly enough] about that old exposure).


>> [2] In some deployment environments, cache manager responses are
>> delivered over secure channels.

> "some" renders this argument irrelevant. The leak issues will be
> cropping up where they are *not* secured.

I think the context got lost here. The argument was that, in general,
the admin has to decide: In some environments, any exposure is safe,
even new exposure, because the admin has already protected (or disabled)
the affected interface (e.g., [2]). In other environments, any exposure
is undesirable, especially new exposure. Squid cannot distinguish one
environment from another without a new configuration option.

Alex.