http.url MUST NOT contain credentials

[pellared]

Why

Example request's URL: https://username:password@example.com:8080/path/to/file.aspx?query=1#fragment

What is the expected attribute's value in such a case? For sure we should not pass the password. The username should not there as well per #1502 (comment).

Why do I think it is important? Because I saw this in OTel .NET SDK: https://github.com/open-telemetry/opentelemetry-dotnet/blob/main/src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs#L115

Reference:

• https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md#common-attributes
• https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/span-general.md#general-identity-attributes

What

Mandate to get rid of username (thanks to @yurishkuro) and password if it is present in the URL.

[jsuereth]

Would you mind updating changelog with this change?

[pellared]

Would you mind updating changelog with this change?

Done 1bcdc0b

[pjanotti]

cc @cijothomas for the OTel.NET SDK mention

[yurishkuro]

I would suggest going with a more strict rule and require stripping everything before @. If people want username they can always add it separately, but logging username by default could be a huge privacy concern given recent EU regulations like ePD.

[pellared]

I agree, GDPR etc. It would be compatible with https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/span-general.md#general-identity-attributes:

Given the sensitive nature of this information, SDKs and exporters SHOULD drop these attributes by default and then provide a configuration parameter to turn on retention for use cases where the information is required and would not violate any policies or regulations.

[pellared]

@yurishkuro
Addressed.
Could you please double-check?

[mrveera]

I understand because of sensitivity of info we have to clear everything but for enduser it won't be evident that URL was containing username and password when it got here. Instead if we have redacted version like https://***:***@www.example.com/ it will be more evident in expressing URL contains username and password but they are redacted for security reasons.

If we don't go with redacted version it might cause ambiguity for someone checking if url contains creds or not because in span with creds and without creds will be same.

options:

1. Use redacted version like https://***:***@www.example.com/
2. An attribute to say that credential info is removed (As @matej-g suggested here)

[pellared]

@veera83372

Regarding Option 1: I suggest NOT to redact it to https://***:***@www.example.com/. Why?

1. If a malicious user accesses this data, he would gain knowledge that could help him in making deeper attacks. Removing it completely reduces the attack surface.
2. Doing *** is anonymization which AFAIK according to GDPR is a kind of personal data processing. Not even touching this info makes sure that auth data won't be processed.
3. Probably (I am not sure here) if you sniff the traffic the URL would be https://www.example.com/ and the auth data would be passed via HTTP Headers.

Option 2 is better. However, I think such an attribute would HAVE TO be optional (disabled by default) in order to make sure that the default configuration is as secure as possible.

[Oberon00]

2. Doing *** is anonymization which AFAIK according to GDPR is a kind of personal data processing. Not even touching this info makes sure that auth data won't be processed.

At the point where it reaches your instrumentation/exporter/... code it has already been processed (e.g. the HTTP request containing it has been parsed). IANAL, but I would be extremely surprised if the additional "processing" of doing anonymization would be a GDPR problem in itself (if the processing to get it to the point where you can anonymize it wasn't already).

[mrveera]

I am fine option 2 as well but just want to clarify *** i didn't mean process any credentials data. We have to generate URL string without creds anyway, What i am trying to say is just add ***:***@ before actual URL if creds are present. Don't need to do processing on data.

2. Doing `***` is anonymization which AFAIK according to GDPR is a kind of personal data processing. Not even touching this info makes sure that auth data won't be processed.

[pellared]

@veera83372 I understand. My biggest concern is:

If a malicious user accesses this data, he would gain knowledge that could help him in making deeper attacks. Removing it completely reduces the attack surface.

@Oberon00
You are correct. I state say it differently.
IANAL, but I guess that from a GDPR perspective, not persisting the data (or not passing data further) is better (safer) than anonymization.

[Oberon00]

I guess that from a GDPR perspective, not persisting the data (or not passing data further) is better (safer) than anonymization.

👍 I agree. But my understanding of the *** suggestion was to actually replace the user and password with the string ***:***, discarding the original data, leaving only the info that user+password was there at all but not what it was.

[pellared]

@Oberon00 @veera83372

But my understanding of the *** suggestion was to actually replace the user and password with the string :, discarding the original data, leaving only the info that user+password was there at all but not what it was.

This is what anonymization is. It leaves traces that there was "something"😉 Malicious user could dig deeper to try to get this info from other sources. Defence in depth.

@yurishkuro Do you think what I write here makes sense?