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.

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

Merged
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]

@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

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

@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
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HttpLoggingMiddleware is overly aggressive at redacting headers
6 participants