Page MenuHomePhabricator

Don't include all headers when logging a request
Closed, ResolvedPublic

Description

Currently, the service template logs all of the request's headers. However, as pointed out by @csteipp in T109023#1610593 these may contain sensitive information (user information, cookies, session ID, etc) which should not be stored, nor on the production or in logstash.

The template needs to provide a way to log a minimalistic set of safe headers. Additionally, the set should be configurable so it can be adapted to various services' needs.

Event Timeline

mobrovac created this task.Sep 10 2015, 9:47 AM
mobrovac claimed this task.
mobrovac raised the priority of this task from to High.
mobrovac updated the task description. (Show Details)
mobrovac added subscribers: mobrovac, csteipp, Mholloway.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 10 2015, 9:47 AM

PR 47 proposes to blacklist certain headers known to expose sensitive information.

I would certainly prefer a whitelist, which services can expand if they need a particular addition. Both as a protection against forgetting something in your blacklist (for example, your PR isn't blacklisting Authorization, which if any services use OAuth will be sensitive. We don't have any services using OAuth, so it would be stupid to add to the current blacklist, but someone needs to remember to blacklist it whenever one is added), as well as making the security review process simpler-- if a new service is adding headers to their logging config, they should be able to justify why they need it vs. me having to identify that they are sending/receiving a particular header, figure out if the header contains sensitive data, and checking that they added it to the blacklist if so.

Good point @csteipp , will modify the PR accordingly. BTW, have got a good "whitelist list" for a sane-ish default?

PR 48 sets up header whitelisting. The list of headers logged by default is:

  • cache-control
  • content-type
  • content-length
  • if-match
  • user-agent
  • x-request-id
mobrovac closed this task as Resolved.Oct 19 2015, 2:19 PM

PR merged, resolving. This functionality is available in service-template-node v0.2.2.