april / laboratory

Because good website security shouldn't only be available to mad scientists!
https://addons.mozilla.org/en-US/firefox/addon/laboratory-by-mozilla/
Mozilla Public License 2.0
175 stars 13 forks source link

Overridden CSP headers are cached by the browser #5

Closed edmorley closed 7 years ago

edmorley commented 7 years ago

Sorry for the vague STR, but I did something like: 1) Install addon v1.0.5 2) Visit https://brasstacks.mozilla.com/orangefactor/ 3) Enable recording 4) Refresh page and navigate around 5) Stop recording (pretty sure I did this, but perhaps I forgot?) 6) Disable addon in addons manager 7) Close tab containing orangefactor 8) Some time later, re-open https://brasstacks.mozilla.com/orangefactor/ 9) Noticed that the orangefactor nginx logs contained recent entries of form POST /laboratory-fake-csp-report (from step 8) 10) Tried re-enabling the addon, pressing "record", then stopping recording. 11) Refreshed https://brasstacks.mozilla.com/orangefactor/

After step 11 OrangeFactor doesn't load any CSS/JS at all (eg unstyled page), unless I enable recording, and then reverts back to unstyled page once recording stopped again.

I presume after restarting the browser this will go back to normal, however ideally: 1) Pressing stop should reset any overridden CSP headers 2) Disabling the addon should do the same in case the user forgot to press stop

This is using yesterday's nightly (20170521030205).

edmorley commented 7 years ago

Ah the unstyled page was caused by #6.

The later requests to /laboratory-fake-csp-report seem to have been caused by the browser caching the CSP header that the addon used. Perhaps the addon should force the browser to not cache the loaded page?

edmorley commented 7 years ago

Perhaps the addon should force the browser to not cache the loaded page?

@april, what are your thoughts on this?

april commented 7 years ago

I could probably do that (by setting Cache-Control: no-cache), but is this a browser bug? It's possible that it should be fixed there instead.

I'll poke the content security team and see if they have an idea on this.

april commented 7 years ago

This was a tricky one, but I think I've fixed the issue. Can you take the no-cache branch and verify that it works for you?

https://github.com/april/laboratory/tree/no-cache

Thanks so much!

edmorley commented 7 years ago

Sorry for the delayed reply!

The OrangeFactor site changed its caching headers in the meantime which would make the existing STR not work. I tried using another site that did have >0s caching and didn't see any warnings, but wasn't sure if the STR I was using would have reproduced anyway. To try and re-confirm the STR I used the release version of the addon, but that's broken with Nightly (content panel is empty). So not sure what else to try.

Happy to just call this fixed :-)

april commented 7 years ago

Great! I've got some sweet features coming btw, I can't wait for you to try them out!

edmorley commented 7 years ago

Looking forward to them!