Skip to content

Un-redact safe request/response headers in HttpLoggingMiddleware #36307

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.

Sign up for GitHub

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

Jump to bottom
Merged
wtgodbe merged 2 commits into from
Sep 9, 2021
Merged

Un-redact safe request/response headers in HttpLoggingMiddleware #36307

wtgodbe merged 2 commits into from
Sep 9, 2021

Conversation

@wtgodbe
Copy link
Member

@wtgodbe wtgodbe commented Sep 8, 2021

Fixes #36156 based on decisions made about safe headers for Yarp by @samsp-msft. Here's an example console log, from the SignalR SocialWeather sample:

info: Microsoft.AspNetCore.HttpLogging.HttpLoggingMiddleware[1]
Request:
Protocol: HTTP/1.1
Method: GET
Scheme: http
PathBase:
Path: /weather
Connection: Upgrade
Host: localhost:5000
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/92.0.4515.159 Safari/537.36
Accept-Encoding: gzip, deflate, br
Accept-Language: en-US,en;q=0.9
Cache-Control: no-cache
Origin: [Redacted]
Pragma: [Redacted]
Upgrade: websocket
Sec-WebSocket-Version: [Redacted]
Sec-WebSocket-Key: [Redacted]
Sec-WebSocket-Extensions: [Redacted]

info: Microsoft.AspNetCore.HttpLogging.HttpLoggingMiddleware[2]
Response:
StatusCode: 101
Connection: [Redacted]
Date: Wed, 08 Sep 2021 22:42:54 GMT
Server: Kestrel
Upgrade: [Redacted]
Sec-WebSocket-Accept: [Redacted]

@ghost ghost added the area-runtime label Sep 8, 2021
@wtgodbe
Copy link
Member Author

wtgodbe commented Sep 8, 2021

CC @blowdart - any concerns on these?

@wtgodbe wtgodbe requested a review from blowdart September 8, 2021 22:47
@Tratcher Tratcher requested a review from samsp-msft September 8, 2021 22:56
@Tratcher
Copy link
Member

Tratcher commented Sep 8, 2021

Request:
Sec-WebSocket-Version
Sec-WebSocket-Extensions

Response:
Connection
Upgrade

@wtgodbe
Copy link
Member Author

wtgodbe commented Sep 8, 2021

Any concerns with Sec-WebSocket-Version or Sec-WebSocket-Extensions @blowdart?

@sebastienros
Copy link
Member

sebastienros commented Sep 8, 2021

What is a "safe" header. One that doesn't contain personal information?

@blowdart
Copy link
Contributor

blowdart commented Sep 9, 2021

Nope, no concerns

@wtgodbe
Copy link
Member Author

wtgodbe commented Sep 9, 2021

What is a "safe" header. One that doesn't contain personal information?

Yes

@wtgodbe wtgodbe enabled auto-merge (squash) September 9, 2021 17:06
@wtgodbe
Copy link
Member Author

wtgodbe commented Sep 9, 2021

/backport to release/6.0

@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2021

Started backporting to release/6.0: https://github.com/dotnet/aspnetcore/actions/runs/1218231063

@github-actions github-actions bot mentioned this pull request Sep 9, 2021
Merged
@wtgodbe wtgodbe merged commit 24628d8 into main Sep 9, 2021
@wtgodbe wtgodbe deleted the wtgodbe/HeadersLoggin branch September 9, 2021 18:41
@ghost ghost added this to the 7.0-preview1 milestone Sep 9, 2021
@amcasey amcasey added area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@blowdart blowdart blowdart approved these changes

@sebastienros sebastienros sebastienros approved these changes

@Tratcher Tratcher Tratcher approved these changes

@adityamandaleeka adityamandaleeka Awaiting requested review from adityamandaleeka

@HaoK HaoK Awaiting requested review from HaoK

@BrennanConroy BrennanConroy Awaiting requested review from BrennanConroy BrennanConroy is a code owner

+1 more reviewer

@samsp-msft samsp-msft samsp-msft approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares

Projects

None yet

Milestone

7.0-preview1

Development

Successfully merging this pull request may close these issues.

HttpLoggingMiddleware is overly aggressive at redacting headers

7 participants

@wtgodbe @Tratcher @sebastienros @blowdart @samsp-msft @amcasey