search

logstash-plugins / logstash-output-http

Apache License 2.0
35 stars 82 forks source link

[Security] Headers content dumped in logs #106

Closed ftatard closed 3 years ago

ftatard commented 5 years ago

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 commented 4 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

thank you @jsvd, much appreciated!