Open klauco opened 1 year ago
Tagging subscribers to this area: @dotnet/ncl See info in area-owners.md if you want to be subscribed.
Author: | klauco |
---|---|
Assignees: | - |
Labels: | `api-suggestion`, `area-System.Net` |
Milestone: | - |
cc @geeknoid @xakep139
This has been discussed before (at least for PII in exceptions) -- it is good to restart the discussion. What would a safe to log exception look like? Is callstack+exception type safe to log? In the past we have discussed having the runtime do this stripping at the point of throw, based on some global flag. But given arbitrary logging may contain PII, there presumably should still be redaction at the logger level as well.
Separately we are missing guidance for what is OK to have in an exception message. A secret like a password clearly is not. A user name may not be. What about host name? We have had requests to add that when a host cannot be found, to say what the host is. What we do in this repo right now is not really consistent. If there was a way to strip messages globally, it would be easier to add hostname to an exception message.
We know from talking to customers that those that are concerned with PII in logs (eg., in a business like healthcare), often just disable logging in production. If work like this is successful, would it be possible for them to enable logging, or would they still be concerned about PII in other messages, in which case it would not be an improvement? Some customers do filter, eg., StackExchange https://github.com/NickCraver/StackExchange.Exceptional
also -- often developers can't easily debug in production (eg., they need approval to deploy tools) or don't want to - in that case it's helpful to be able to disable stripping and enable detailed logging temporarily, by just changing config or environment.
searching email, this has come up several times in the last few years.
cc @blowdart
Let's also add the ability to adjust at runtime without an app restart, and to turn on and off individual log messages without an app restart, and without having to write a custom filtering class.
One thing which I did not add to the proposal is the fail-safe. Same as when you are trying to strip sensitive info after it was generated, there could be a mechanism which would kick off right before the logging of the exception is happening. Let's call it ExceptionSanitizer:
public class ExceptionSanitizer {
public Exception Sanitize(Exception ex) {
// returns sanitized exception based on the registered sanitizers for a given exception type or assembly the exception was fired from or other information
return ex;
}
}
Alternatively, when it comes to figuring out what is safe to include in the exception message and what not, I could imagine fully typed ExceptionMessage object which would define template + typed arguments of the template. @danmoseley - in your example above, host could be a type which you could then register as a type which is safe or unsafe-to-log. So similarly to how data classification works based on attributes, then the definition could be externalized and exposed to end user.
URIs are used to identify resources on the web, and they often contain sensitive data, such as query parameters, authentication tokens, or personal information. Logging such data can pose security and privacy risks, as well as violate compliance regulations.
One of the basic rules of handling data securely is using POST
requests for confidential data instead of GET
requests since those leak it in the addresses. Adding APIs that encourage unsecure behaviour shouldn't be a goal for the runtime.
@MichalPetryka This request does not come out of the blue, but from the real experience of service owners. The simplest example where GET exposes sensitive information is Microsoft Graph to GET a user:
GET https://graph.microsoft.com/v1.0/users/<some user id>
The proposed APIs are not to encourage unsecure behaviour, they are there to enable real-world service owners use .NET in a real-world scenarios, where libraries/SDKs/clients are designed in a way which is not 100% in line with the security recommendations.
Additionally, route templates will reduce cardinality of the route dimensions in outgoing HTTP metrics. Even OpenTelemetry specification recommends using low-cardinality values whenever possible, so using route templates or URL templates instead of real high-cardinality URLs.
A user ID is usually not considered sensitive information, though.
@Joe4evr UserId is in the category of End User Pseudonymous identifiers and is considered as personal data. Consider a different example:
/users?$filter=mail eq 'mail@contoso.onmicrosoft.com'
I'll just add that "End User Pseudonymous identifier" is a Microsoft term (an M365 term, to be more precise), and logging EUPI results in a Privacy incident. That would be painful enough, but some logging some kinds of EUPI, in combination with other data, can yield GDPR violations.
Not sure if this is just me, but the concern of knowing whether some value is safe or not to log should not be on the value itself. So, the proposal to create specialized, "safe-to-log" versions of Uri
and string
is a terrible idea to me.
If we need extra metadata about some type (like route template information for a Uri
) that should not be part of a new Uri
type but of some container type that has the Uri
in it alongside the other data. Composition instead of inheritance.
In other words, I agree with the idea behind this proposal, but not with the solution it proposes.
We will not implement this in the proposed form. The proposal should be broken down to multiple issues.
The primary requests are:
url.template
We should also think about the following requests, although I'm sceptical they will result in BCL features:
For URL redaction we are implementing #103769 to ensure basic privacy, the rest of the work will happen in .NET 10.
Background and motivation
At the moment, .NET users need to ensure that PII data is handled accordingly. When it comes to logging, service owners remove PII data to ensure that GDPR or other privacy standards are followed. There are two main sources of PII data in services running on .NET:
To overcome these issues, service owners need to find ways to ensure that their services don’t leak sensitive data, in particular:
The following requirements should be met:
API Proposal
Define a SafeToLogString type, which together with code-gen, will allow users to define a string template + parameters of the template which can be set during runtime. Stringified version of the instance of the type would return only the template:
API Usage
Alternative Designs
As an alternative to a generic-purpose (and still very useful) SafeToLogString type, Exception constructor could accept a new ExceptionMessage type
Exception class would accept the ExceptionMessage object:
Users could either instantiate the ExceptionMessage directly, or have code-gen based helpers which would enable filling in Message and AdditionalData properties, i.e.:
In the sample above, Message would be filled from the attribute argument and AdditionalData property would be filled from MyProperty and OtherProperty.
Risks
No response