🎉 Advance DrHeader to evalute HSTS max-age #250:bug

[manuel-sommer]

This adds an evaluation for the value "max-age".
As long as the evaluated max-age of HSTS is greater or equal to the yml max-age of HSTS, then, everything is fine. Otherwise, it triggers a finding.

• [x ] New feature (non-breaking change which adds functionality)

Link to the github issue
#250

[manuel-sommer]

@javixeneize could you review this PR?

[manuel-sommer]

@javixeneize and @amias-channer could you please review this PR?

[emilejq]

I'm not sure I see the value in this. 1 year is a well established benchmark that's pretty ubiquitous, and it's unlikely that anyone would want to set a max-age that equates to 1 year + an arbitrary number of seconds.

I'd support increasing the expected value in the default rules to 63072000 (2 years), which is the recommendation from Google when using the preload list https://hstspreload.org/#deployment-recommendations

[manuel-sommer]

Hi @emilejq
I guess you understand this wrong. I also think that 1 year is enough. However, if you have set 1 year, but the scanned target returns 2 years, then a finding is reported. To me, then a finding should not be reported, as the default 1 year should not be an exact value, but rather a "1 year and above" is accepted.

Example (Current implementation):
--> target.com has 1 year and 2 month for HSTS max-age, and rules.yml evaluates if the max-age is equal to 1 year, then a finding is reported.

Example (PR):
--> target.com has 1 year and 2 month for HSTS max-age, and rules.yml evaluates if the max-age is 1 year or above, then a finding is not reported.

[pealtrufo]

Hi @manuel-sommer

My take on it is that your suggestion could be confusing and/or leading to not wanted behaviours. I definitely don't think it is a bug.

If you set the rule to have a one year max-age for HSTS header, then a scenario where a 10 years max-age is returned would be expected to fail. The fact it is more than one year doesn't make it always good. The same is expected to happen for anything different to the value set in the rule really. This is the reason why it is not a bug, as it is the way the tool has been implemented to work.

Now if you wanted a behaviour like that, then I believe it would be more appropriate to extend DrHeader with a new directive 'value-greater-than' or 'value-between', but if I am honest, there's not a lot of value I can see with that at the moment.

Also note that the logic in your PR changes would likely create conflicts with max-age in Cache-Control header.

[manuel-sommer]

@pealtrufo Sorry, it took a while, but I implemented now a greaterequal-age and it works very solid, I tested it. Hopefully we can merge this now.

[manuel-sommer]

Please review again @pealtrufo :-)

[manuel-sommer]

Reminder

[emilejq]

Hi @manuel-sommer,

I just don't see what the use case of this is.

1 year is a de facto standard for HSTS max-age; it is recommended and used extensively including in examples from OWASP, Mozilla and CIO.

As I said in my first message, we can open a discussion about increasing the default value to 2 years, which is the recommendation from Google when using the preload list and therefore increasingly common, but to my eyes, supporting any arbitrary value in between is solving a problem that does not need solving.

[manuel-sommer]

Hi @emilejq,

if you use a default setting and if you use this default setting to scan various services (e.g. through an automation), I don't want to have a finding returned if one service implements HSTS with 1 year and one service implements it for 1.5 years and one service implements it for 2 years as I simply don't care about that. So this feature helps to suppress false positive findings. This feature brings the benefit to use DrHeader more broadly.

[manuel-sommer]

Hi @emilejq , this is a friendly reminder. :-)

[manuel-sommer]

Friendly reminder @emilejq