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

Prevent sensitive request headers from being logged in debug level #9652

Open
nulltrope opened this issue Jan 10, 2020 · 6 comments
Open

Prevent sensitive request headers from being logged in debug level #9652

nulltrope opened this issue Jan 10, 2020 · 6 comments
Labels
area/security design proposal Needs design doc/proposal before implementation help wanted Needs help!

Comments

@nulltrope
Copy link

Description:
Hey all, first just some quick background before getting into the issue we're experiencing. We wrote a small service that is called by the ext_authz filter to perform an OIDC Authorization Flow, and verify existing JWTs stored as cookies in request headers.

The JWT cookie contains sensitive information and therefore shouldn't be logged, however we noticed that when envoy is run with -l debug, various components will log the full request headers including the JWT, e.g.:

envoy_1         | [2020-01-10 16:29:21.082][14][debug][http] [source/common/http/conn_manager_impl.cc:246] [C5] new stream
envoy_1         | [2020-01-10 16:29:21.082][14][debug][http] [source/common/http/conn_manager_impl.cc:619] [C5][S6425470214323392008] request headers complete (end_stream=true):
envoy_1         | ':authority', 'localhost:9002'
envoy_1         | ':path', '/say'
envoy_1         | ':method', 'GET'
envoy_1         | 'user-agent', 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:71.0) Gecko/20100101 Firefox/71.0'
envoy_1         | 'accept', 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8'
envoy_1         | 'accept-language', 'en-US,en;q=0.5'
envoy_1         | 'accept-encoding', 'gzip, deflate'
envoy_1         | 'dnt', '1'
envoy_1         | 'connection', 'keep-alive'
envoy_1         | 'cookie', 'oidc-service-token=supersecrettoken'
envoy_1         | 'upgrade-insecure-requests', '1'
envoy_1         | 'x-svc-cluster', 'sayer'

and

envoy_1         | [2020-01-10 16:29:21.083][14][debug][router] [source/common/router/router.cc:514] [C5][S6425470214323392008] router decoding headers:
envoy_1         | ':authority', 'localhost:9002'
envoy_1         | ':path', '/say'
envoy_1         | ':method', 'GET'
envoy_1         | ':scheme', 'http'
envoy_1         | 'user-agent', 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:71.0) Gecko/20100101 Firefox/71.0'
envoy_1         | 'accept', 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8'
envoy_1         | 'accept-language', 'en-US,en;q=0.5'
envoy_1         | 'accept-encoding', 'gzip, deflate'
envoy_1         | 'dnt', '1'
envoy_1         | 'cookie', 'oidc-service-token=supersecrettoken'
envoy_1         | 'upgrade-insecure-requests', '1'
envoy_1         | 'x-svc-cluster', 'sayer'
envoy_1         | 'x-forwarded-for', '172.22.0.1'
envoy_1         | 'x-forwarded-proto', 'http'
envoy_1         | 'x-envoy-internal', 'true'
envoy_1         | 'x-request-id', '635a3677-6c36-404e-9421-8c7a1f37304e'
envoy_1         | 'x-envoy-expected-rq-timeout-ms', '20000'

Normally we don't run our production envoy's with debug level, however there are instances where we will temporarily set the level to debug when troubleshooting issues etc.

I'm wondering if there is some way to exclude particular headers from being logged in debug level? The only documentation I can seem to find for configuring logging is for access logs, not envoy's application logs.

Thanks!

@mattklein123 mattklein123 added area/security design proposal Needs design doc/proposal before implementation help wanted Needs help! labels Jan 10, 2020
@mattklein123
Copy link
Member

Thanks for raising @nulltrope. In general today we assume that debug/trace logs carry sensitive/trusted information and are primarily used for development, though I think it's reasonable to (optionally) do better here. Marking this as needing a design proposal.

@pzmi
Copy link

pzmi commented Jan 13, 2020

Hi. I was about to report a similar issue.
I was pointed to a related issue #4757 and a PR #9315. However these address only logging proto messages.

For sure we need a possibility of specifying sensitive headers through some configuration as there are a variety of different deployment scenarios and some of them find the full authorization logging useful.
Secondly it would be helpful to use a common headers (or any other sensitive information) logging interface. Bonus points for a generalized log sanitization mechanism through some listeners or visitors. Maybe there's a way of utilizing the proto redaction functionality mentioned earlier.
Also we need a way of enforcing usage of provided tooling as anyone could log sensitive information plain text.

Also another issue regarding sanitization of config_dump #7365

@kfaseela
Copy link
Contributor

Having similar concerns about the debug logs. Not sure if someone is looking into this.

@tsaarni
Copy link
Member

tsaarni commented Jun 14, 2023

FYI: I had an attempt at this issue, to provide way to exclude particular headers from being logged in debug level, in #27579 and #27820.

Feedback from #27579 (comment) is that this approach is discouraged, and wrapper script to process logs is encouraged.

@pzmi
Copy link

pzmi commented Jun 14, 2023

Feedback from #27579 (comment) is that this approach is discouraged, and wrapper script to process logs is encouraged.

Ultimately, this is how I solved this issue myself.

@nil-scan
Copy link

We stumbled onto this issue as well.

It is common to enable the debug log level for troubleshooting issues, but is can also be impractical to write a wrapper script. Not to mention that many users are unaware that sensitive info is being logged, and might be sending them to 3rd parties.

Maybe an easy way to solve this would be to change the level of that particular log to trace, and have a summarized version at the debug level? Something like header=hash or header=size

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/security design proposal Needs design doc/proposal before implementation help wanted Needs help!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants