sesh / ready

Are you production ready?
ISC License
26 stars 4 forks source link

'Expires' header fail when both headers are present? (nginx specific) #24

Closed mejofi closed 4 months ago

mejofi commented 5 months ago

When using nginx, the Cache-Control header can be set, along with the Expires header, via the ngx_http_headers module. Since this allows for some applications that are not possible with static Cache-Control headers, such as setting a specific time of day, or modified + time, it may not be a bad idea to, when both headers are present, to turn it into a warning instead?

For dynamic content, the upstream application server would likely deal with this, but, for static content, setting it in the nginx configuration is a rather convenient shortcut, with no way to turn off the Expires header.

See; https://nginx.org/en/docs/http/ngx_http_headers_module.html#expires

sesh commented 5 months ago

This one is potentially a little spicy.

For documents, I believe the preferred behaviour is to use Cache-Control without an Expires header. If both are present the browser will use Cache-Control, so what's the point of having the second header...

Perhaps "deprecated" is too harsh? I can't find a reference that it's actually deprecated in some quick searching.

I need to look into why nginx specifically is returning both.

sesh commented 5 months ago

(mostly a note to myself)

The idea behind the warn_on_fail flag was really to allow warnings for things that are not yet widely supported, or where there's a preference for a specific technique but it's not widely adopted. The COOP, CORP, COEP headers fit well into this category.

I should review these and updated them.

sesh commented 5 months ago

And perhaps this issue is enough to trigger the ability to skip certain checks from the command line or by using a config file.

mejofi commented 5 months ago

Yeah, I couldn't find anything that suggests it's deprecated either, other than that Cache-Control basically succeeded it, and has more options for granular control of caching behaviour. I suspect that nginx does it the way it does to be backwards compatible, and that this setting has been in there for quite some time.

One option could perhaps be to allow for bypassing certain checks via, say, --skip-filter or something similar? 🤔 That would allow the tool to run successfully against a target where one is not able to fix specific FAIL conditions, because the software does not support it without major rework.

sesh commented 4 months ago

I have update the check with the following wording:

Expires header should not be used without Cache-Control

This will now pass if both Cache-Control and Expires are set, and should fail if only Expires is set.