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

Consider disabling hidePoweredBy in next major release #334

Closed
Gaelan opened this issue Oct 30, 2021 · 3 comments
Closed

Consider disabling hidePoweredBy in next major release #334

Gaelan opened this issue Oct 30, 2021 · 3 comments

Comments

@Gaelan
Copy link

Gaelan commented Oct 30, 2021

As the docs say, disabling X-Powered-By has little to no security benefit. I'd argue that, in fact, including hidePoweredBy by default does more harm than good:

  • It contributes to the belief that it's a useful security change ("see, helmet does it!")
  • It makes helmet look bad to people who know what they're doing ("if this thing disables X-Powered-By, how many of its other features are pointless?")

It seems to me that it's worth at least considering no longer including hidePoweredBy by default—thoughts?

@EvanHahn
Copy link
Member

I have a number of thoughts here which I'll share in more detail soon. Just want to let you know that I've seen this comment, think it's good feedback, and plan to respond more.

@Fdawgs
Copy link
Contributor

Fdawgs commented Nov 2, 2021

It is useful as a very very very minor form of "security through obscurity" so it still has reason to be enabled by default (Helmet is all about setting security-related headers after all) but i agree it offers no real protection against anything.

I disagree that including it makes Helmet look bad though, it's clearly documented that the option has limited security benefits.

@EvanHahn
Copy link
Member

EvanHahn commented Nov 7, 2021

tl;dr: I'm going to keep things as is.

The core of this issue is a question: does helmet.hidePoweredBy do more harm than good?

Pros:

  • It offers a tiny security benefit
  • It saves a small amount of bandwidth
  • It's been in Helmet for years; removing it could cause minor disruption when users upgrade (not strictly a benefit, but important to consider IMO)

Cons:

  • It slightly increases server CPU usage
  • Its existence implies that Helmet believes it to be a security boon, which could (1) look bad to people who understand it (2) be security theater for people who do not
  • It slightly increases maintainer burden
  • It slightly increases documentation size

A reasonable person could come down either way on this, I think.

To me, the pros outweigh the cons. Helmet is optimized for the drop-in app.use(helmet()) use case and I think it makes more sense to keep it for this kind of user.

I'm going to close this issue because I've considered this and don't think I'm going to change anything. But if you disagree with this decision or have additional thoughts, please let me know.

@EvanHahn EvanHahn closed this as completed Nov 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants