Document Exception Summary security expectation

[sebastienros]

Add documentation to explain it doesn't provide any security guaranty and it's not meant to hide data.

[geeknoid]

I presume you mean "privacy" rather than "security".

The component is specifically meant to produce non-PII data, so I'm not sure what would need to be documented.

[sebastienros]

Exception messages might contain technical information that should not be disclosed. We know the current implementations of IExceptionSummaryProvider won't produce any PII or copy anything from the exception description and stack trace. However, it is necessary to document that IExceptionSummarizer does not provide any security or privacy guaranty explicitly.

This is a request from threat modeling. I assume the component might otherwise lead someone to believe the opposite and we can't make any guaranty, so this has to be explicit. It's easy enough and removes any concerns they had.

[danmoseley]

Should we document that the current implementations do not leak PII (even though other implementation may do so)? That's the intent, right? In a look at the code, it's just the type and some meta info like status.

We know from past customer development that it's typical for customers processing sensitive information to not log at all in production, or at least, take care that exceptions aren't logged. We have never had a clear bar for what is OK to include in exception messages, so while you hopefully won't get passwords, you might get something else interesting. For such customers presumably exception summarization (as implemented by us) will not be useful without a statement like the above.

Or is exception summarization in all implementations not designed to protect PII/security, but merely for diagnostic convenience by trusted users? Perhaps that's what the request is -- to document that. Not just for customers but also for folks implementing IExceptionSummarizer in future.

[sebastienros]

/cc @blowdart to confirm what is acceptable here

[blowdart]

Or is exception summarization in all implementations not designed to protect PII/security, but merely for diagnostic convenience by trusted users

This is well put :)

[geeknoid]

The design of the interface is specifically created to avoid PII leaks in the description. Here's some background.

Summarization returns this:

public readonly struct ExceptionSummary : IEquatable<ExceptionSummary>
{
    public string ExceptionType { get; }
    public string Description { get; }
    public string AdditionalDetails { get; }
}

The exception type is a string derived from the type name of the exception. Description is a static string registered by summarizers at startup and is thus guaranteed to not hold PII.

AdditionalDetails can contain things like a status code, and could potentially contain PII if a particular summarizer decides to put it there. The in-box summarizers do not introduce anything close to PII in that field, but 3rd party summarizers could. Applications are in full control of the summarizers used in their process based on what they link in and what they register.

[danmoseley]

Right, it is easy to imagine a summarizer putting some piece from an exception message into AdditionalDetails. We could document that summarizers must not, but since we don't control them I think we have to document that they may -- that ExceptionSummarys may contain sensitive data.

We could also choose to document that the ExceptionSummarys that you will get when you do AddExceptionSummarizer(sc => sc.AddHttpProvider()); (or AddResilienceHandler(..) which ends up doing this) will not contain sensitive data. And then we are held to this promise.

summarizers used in their process

I'm not very familiar with DI, but can you have more than one? I see ServiceDescriptor.Singleton<IExceptionSummaryProvider, T>

[geeknoid]

@danmoseley Yes, you can have multiple summarizers. They register themselves with the exception type they know how to summarize.

[joperezr]

The issue here seems to be just that we need to adjust documentation to match what the API is doing.

[IEvangelist]

I have called this out in my draft article I'm working: https://github.com/dotnet/docs/pull/37455

[sebastienros]

Fixed in https://github.com/dotnet/extensions/pull/4559