DrHeader icon indicating copy to clipboard operation
DrHeader copied to clipboard

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

Open manuel-sommer opened this issue 2 years ago • 12 comments

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 https://github.com/Santandersecurityresearch/DrHeader/issues/250

manuel-sommer avatar Jun 24 '22 05:06 manuel-sommer

@javixeneize could you review this PR?

manuel-sommer avatar Jun 24 '22 18:06 manuel-sommer

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

manuel-sommer avatar May 02 '23 05:05 manuel-sommer

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

emilejq avatar Jul 31 '23 13:07 emilejq

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.

manuel-sommer avatar Aug 31 '23 07:08 manuel-sommer

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.

pealtrufo avatar Sep 04 '23 09:09 pealtrufo

@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 avatar Nov 14 '23 09:11 manuel-sommer

Please review again @pealtrufo :-)

manuel-sommer avatar Nov 14 '23 09:11 manuel-sommer

Reminder

manuel-sommer avatar Dec 06 '23 12:12 manuel-sommer

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.

emilejq avatar Dec 14 '23 15:12 emilejq

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 avatar Dec 14 '23 15:12 manuel-sommer

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

manuel-sommer avatar Feb 22 '24 07:02 manuel-sommer

Friendly reminder @emilejq

manuel-sommer avatar Jun 20 '24 20:06 manuel-sommer