Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP Span Attributes: http.url must not contain username / password #367

Open
pellared opened this issue Mar 9, 2021 · 9 comments
Open
Assignees
Labels
bug Something isn't working good first issue Good for newcomers security/privacy issue Issues that result in security vulnerability or PII leaks specification Issues that are needed to comply with the specifications triaged

Comments

@pellared
Copy link
Member

pellared commented Mar 9, 2021

As is stated in the recent specification change :

http.url MUST NOT contain credentials passed via URL in form of https://username:password@www.example.com/. In such case the attribute's value should be https://www.example.com/

@codeboten
Copy link
Contributor

Thanks for filing this! Moving this issue to the contrib repo as I don't believe anything in the core repo sets this attribute.

@codeboten codeboten transferred this issue from open-telemetry/opentelemetry-python Mar 9, 2021
@owais owais added bug Something isn't working security/privacy issue Issues that result in security vulnerability or PII leaks good first issue Good for newcomers labels Apr 6, 2021
@github-actions
Copy link

github-actions bot commented May 7, 2021

This issue was marked stale due to lack of activity. It will be closed in 30 days.

@lzchen lzchen added triaged specification Issues that are needed to comply with the specifications and removed backlog labels May 12, 2021
@ryokather
Copy link
Contributor

Hi @codeboten is this issue still open? I'd love to take on my first issue and be assigned to it. I noticed that its been fixed in other language implementations but not here yet.

cc: @alolita

@ryokather
Copy link
Contributor

Just wanted to follow up @lzchen or @owais that it's ok to take this on!

@codeboten
Copy link
Contributor

Yup, still open, all yours.

@marcorichetta
Copy link

This should be closed, right? @codeboten

@lzchen
Copy link
Contributor

lzchen commented Jan 3, 2022

@marcorichetta @ryokather
I believe httpx is the only instrumentation that still needs implementing.

@tammy-baylis-swi
Copy link
Contributor

I found this issue while doing a separate check for if http.url from instrumentation libraries contains sensitive information, so wanted to share some findings.

I agree with @lzchen; the use of remove_url_credentials util by several instrumentation libraries is already sanitizing http.url when written (aiohttp_client, asgi, requests, tornado, urllib, wsgi).

It seems like the httpx instrumentation library isn't exposing passwords but it still exposes usernames. http.url value is protected by HTTPX’s request implementation and URL implementation. Creation of a request like r = httpx.get("http://username:password@0.0.0.0:8000/home/") then calling r.url gives a protected URL('http://username:[secure]@0.0.0.0:8000/home/'), and this is what the instrumentation library's _extract_parameters method uses before its _prepare_attributes method.

@lzchen
Copy link
Contributor

lzchen commented Mar 28, 2023

@tammy-baylis-swi
Interesting. If that's the case then simply some tests should be added to confirm that it does indeed solve this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers security/privacy issue Issues that result in security vulnerability or PII leaks specification Issues that are needed to comply with the specifications triaged
Projects
None yet
Development

No branches or pull requests

7 participants