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

[Security] Headers content dumped in logs #106

Closed
ftatard opened this issue Nov 8, 2019 · 7 comments · Fixed by #122
Closed

[Security] Headers content dumped in logs #106

ftatard opened this issue Nov 8, 2019 · 7 comments · Fixed by #122

Comments

@ftatard
Copy link

ftatard commented Nov 8, 2019

Hello, in the event of a connection issue with the endpoint, the error message dumps the headers content in logs, which might lead to the exposure of the endpoint API key when headers are use for basic authentication and that logs are collected.

Would it be possible to add an option to make the inclusion of these headers conditional ? Thank you.

@borissnd
Copy link

Good point!
I would also like to suggest making the inclusion of request body optional.
They inflate logs beyond proportion and make them difficult to read.
When messages are encrypted with SSL, request body is just kilobytes of gibberish. Most of these messages are retried (when allow retry is enabled) and are successfully delivered, therefore a detailed error becomes unnecessary.

@farrp
Copy link

farrp commented Apr 29, 2021

Both the headers and body content represent a security risk. Some of our data is sensitive and having it dumped into the logs in cleartext is a huge risk. Logging the API key and other header info is also a security issue.

@farrp
Copy link

farrp commented May 17, 2021

Hi, Any update on this? Writing the content of the events to the log is a huge issue to us ad a blocker for moving off the Lumberjack plugin to this one.

@jsvd
Copy link
Member

jsvd commented May 17, 2021

Hi friends, I created #122 to change move headers and backtraces to the debug level.
There are some comments here about how the body is also sensitive, and while I can see that, in Logstash there are many plugins that will dump event content on errors, especially on input and output plugins to help users figure out why there was an error in the first place.
We could hide the body for this plugin but it's very likely another plugin may end up dumping it too.
Is this PR, that prevents headers from being logged, a reasonable compromise for most?

@borissnd
Copy link

thanks @jsvd. this is definitely an improvement.
However, I would argue that there should be an option to exclude the message body from error messages.

When http compression is enabled, message body contains unreadable content and is useless.
When disabled, the body may contain sensitive information which one might wish to avoid exposing outside the HTTPS channel.

@jsvd
Copy link
Member

jsvd commented May 26, 2021

I agree that it certain scenarios the body won't be useful. I've updated the PR to only log the body in debug.

@borissnd
Copy link

thank you @jsvd, much appreciated!

@jsvd jsvd closed this as completed in #122 May 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants