Better default values
julien-f opened this issue · 11 comments
In the README, you suggest setting maxAge to at least 18 weeks (SSL Labs suggest at least 180 days) and includeSubDomains to true, why aren't those the default values?
In Helmet, I try to set options that are as non-destructive as possible. For example, in the frameguard middleware, I default the header to SAMEORIGIN instead of DENY; while the latter is probably more secure, it might also break an existing website.
In that spirit, I set the maxAge to a pretty short amount of time—what if they did it wrong?—and I omit includeSubdomains because I don't want to assume that we want the header there.
I'd argue that the "don't break stuff" design is better than the "maximum security first" design for this module, but I could be convinced otherwise.
Either way, I think the documentation needs to prepare a user to make these decisions.
What do you think?
To be honest, I don't know much about HTTP security, you might have made the right choice.
But people might expect (especially using the high level helmet package) to have the best options picked for them by default.
IMHO the document of this package is quite good but it could be improved for helmet for people to know that there are some important stuff to configure is they want their application to be as secure as possible.
I totally agree that Helmet should pick the best defaults it can. I've chosen to define "best" as "the least likely to break your existing website" over "the most secure, with potential problems".
Some of the defaults you've suggested are absolutely more secure and a better way to use this module. I'm hesitant to add them because they might break somebody's site (and, depending on how they mess up their SSL stuff, that can be pretty bad).
What do you think? Should the defaults be more secure?
I am not sure I fully grasp the consequences of making it more secure by default and I would not want to break any setup so I trust you to judge (yep, I'm a coward 😛).
Perhaps the default values should depend on the environment (NODE_ENV being either development or production).
The real grief I have is the impossibility to have the best configuration using directly helmet (helmetjs/helmet#112).
It sounds like HSTS's defaults should stay the same but the configuration passing is the more important part. If that's the case, let's close this issue and I'll get to the configuration passing PR.
I'm hesitant regarding the current HSTS defaults of 1 day. Using such a short max age kinda defeats the purpose of using HSTS which is to make sure your site is accessed using HTTPS. Most HSTS preload lists require to use a max age of minimum 18 weeks.
https://hstspreload.appspot.com/
https://blog.mozilla.org/security/2012/11/01/preloading-hsts/
Sorry to be slow to respond here.
First, the newly-released helmet@2.0.0 lets developers override defaults. For example:
app.use(helmet({
hsts: {
maxAge: ms('18 weeks')
}
}))This (hopefully) addresses @julien-f's issue.
@siboulet, I agree that 1 day is too short and defeats a lot of HSTS's benefit. Let me see if I can explain my rationale:
If a developer knows how long they want to set the HSTS header, this issue isn't a problem. They set the maxAge option.
There are developers that don't know what their HSTS maxAge should be, either because they don't know/understand HSTS or don't know the right amount of time. (I think we need to consider this person too!)
This means that we need to pick a default maxAge.
- If we pick a default
maxAgethat is too short, this middleware is basically useless. - If we pick a default
maxAgethat is too long, this middleware can mess up somebody's website if they've screwed up their HTTPS somehow.
I don't like either of these options but I prefer the first one. That's why I set a short default maxAge.
There's no documentation that shows how to use hsts with a default maxAge, which I think helps with this problem.
Alternatively, we could throw an error if a developer doesn't specify a maxAge. I haven't really considered this but it would require major version bumps of hsts and helmet.
Thoughts?
@EvanHahn maybe all we need is a more explicit explanation about HSTS default being 1 day, and that you most probably want to bump it to at least 18 weeks (optionally linking to Chrome and Firefox links above).
Okay. It sounds like I need to update the docs to strongly recommend 18+ weeks and add a note about the default values. Sound right?