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

IHttpClientBuilder should have extension to redact query parameter from logging #68675

Closed
anktsrkr opened this issue Apr 28, 2022 · 7 comments
Closed
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-Extensions-HttpClientFactory
Milestone

Comments

@anktsrkr
Copy link

The current implementation of Microsoft.Extensions.Http logging framework redact headers value based on user input however it does not support redact sensitive information from query parameters, which is kind of security issue.

image

For customers that are more concerned about this logging risk or have to meet audit requirements for all their integrated services it is important to redact query parameters value based on users input.

The problem lies here -

private static string? GetUriString(Uri? requestUri)

We could implement this feature same way as we have a extension in IHttpClientBuilder to redact from header.

public static IHttpClientBuilder RedactLoggedHeaders(this IHttpClientBuilder builder, Func<string, bool> shouldRedactHeaderValue)

We might name this extension RedactLoggedQueryParameters

Thanks,
Ankit S

@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Net.Http untriaged New issue has not been triaged by the area owner labels Apr 28, 2022
@ghost
Copy link

ghost commented Apr 28, 2022

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

The current implementation of Microsoft.Extensions.Http logging framework redact headers value based on user input however it does not support redact sensitive information from query parameters, which is kind of security issue.

image

For customers that are more concerned about this logging risk or have to meet audit requirements for all their integrated services it is important to redact query parameters value based on users input.

The problem lies here -

private static string? GetUriString(Uri? requestUri)

We could implement this feature same way as we have a extension in IHttpClientBuilder to redact from header.

public static IHttpClientBuilder RedactLoggedHeaders(this IHttpClientBuilder builder, Func<string, bool> shouldRedactHeaderValue)

We might name this extension RedactLoggedQueryParameters

Thanks,
Ankit S

Author: anktsrkr
Assignees: -
Labels:

area-System.Net.Http, untriaged

Milestone: -

@MihaZupan MihaZupan added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-Extensions-HttpClientFactory labels May 2, 2022
@ghost
Copy link

ghost commented May 2, 2022

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

The current implementation of Microsoft.Extensions.Http logging framework redact headers value based on user input however it does not support redact sensitive information from query parameters, which is kind of security issue.

image

For customers that are more concerned about this logging risk or have to meet audit requirements for all their integrated services it is important to redact query parameters value based on users input.

The problem lies here -

private static string? GetUriString(Uri? requestUri)

We could implement this feature same way as we have a extension in IHttpClientBuilder to redact from header.

public static IHttpClientBuilder RedactLoggedHeaders(this IHttpClientBuilder builder, Func<string, bool> shouldRedactHeaderValue)

We might name this extension RedactLoggedQueryParameters

Thanks,
Ankit S

Author: anktsrkr
Assignees: -
Labels:

api-suggestion, area-System.Net.Http, untriaged, area-Extensions-HttpClientFactory

Milestone: -

@CarnaViire
Copy link
Member

Thanks for raising the concern @anktsrkr.

As a quick workaround for the time being, you can consider removing the logging entirely by putting that code after all your HttpClient registrations in ConfigureServices:

services.RemoveAll<IHttpMessageHandlerBuilderFilter>();

@CarnaViire
Copy link
Member

Triage: URIs aren’t meant to contain secrets for various reasons, e.g. proxies. But we still might want to add some way to control this. Given there's a workaround, moving to future.

@CarnaViire CarnaViire removed the untriaged New issue has not been triaged by the area owner label May 17, 2022
@CarnaViire CarnaViire added this to the Future milestone May 17, 2022
@CarnaViire
Copy link
Member

Note that #74339 asked for redacting not only query parameters, but path as well

@MihaZupan
Copy link
Member

FWIW, even the new HTTP logging in ASP.NET defaults to logging the path (though it is configurable).

@CarnaViire
Copy link
Member

Duplicate of #77312

@CarnaViire CarnaViire marked this as a duplicate of #77312 Jun 30, 2023
@CarnaViire CarnaViire closed this as not planned Won't fix, can't repro, duplicate, stale Jun 30, 2023
@karelz karelz modified the milestones: Future, 8.0.0 Jul 3, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Aug 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-Extensions-HttpClientFactory
Projects
None yet
Development

No branches or pull requests

4 participants