92598 - 'cache-control: no-store' response should not be cached

Status: VERIFIED FIXED

(Core :: Networking: HTTP, defect, P1)

Description

[Darin Fisher]

mozilla currently caches everything a web server sends for the purposes of
session history (back/forward buttons), file->save_as, and view->page_source.
however, this leaves webservers with no way to instruct mozilla not to cache
sensitive information.  the http/1.1 response header 'cache-control: no-store'
is meant to inform the browser that the response is sensitive (however, the spec
still makes allowances for session history even in this case).

the spec is therefore somewhat contradictory: if 'no-store' means the content
is sensitive, then how can it be OK for it to be saved for the purposes of
session history.  so, i believe that we should disable caching when a server
sends a 'cache-control: no-store' header, hence providing servers with a way
to programmatically prevent mozilla from holding onto sensitive content.

this bug is somewhat related to bug 90288

Comment 1

[Darin Fisher]

Attachment: v1.0 fixes this problem

Comment 2

[Darin Fisher]

the v1.0 patch was checked in on the 0.9.2 branch per PDT2 approval.  i have
approval to land this for 0.9.3 and will be doing so once the verification
builds complete (r=bbaetz, sr=waterson, a=chofmann).

Comment 3

[Darin Fisher]

I have two simple testcases (so far):

1) Test GET request which results in a 'Cache-control: no-store' response
   http://g.mcom.com/test/no-store.cgi
   Just follow the link.  The response should not be cached (press Back,
   then Forward to verify).

2) Test POST request which results in a 'Cache-control: no-store' response 
   http://g.mcom.com/test/no-store.html
   Just follow the link and press Upload.  The response should not be 
   cached (press Back, then Forward to verify).

Comment 4

[Bradley Baetz (:bbaetz)]

I tested on linux last night on the branch, and this appeared to do the right thing.

I couldn't find any real life servers which sent this (including the login page
for various banks), so there should be no difference for the vast majority of sites.

I'll do some more testing when I get in.

Comment 5

[Gagan]

this should land on the trunk as well. 

Comment 6

[Darin Fisher]

fixed on the trunk

Comment 7

[Brendan Eich [:brendan]]

Just curious, not picking nits or asking for changes: why PL_strcasestr rather
than PL_strcasecmp, or perhaps something more complicated that strips leading
and trailing whitespace (tho I'd hope that PeekHeader does that for you)?  Are
all the values guaranteed not to be substrings of other legal values?  Can you
have random garbage around the no-store and get the desired results, according
to the spec?

/be

Comment 8

[Darin Fisher]

good point.  w/ http/1.1 we are ok, but future versions of the spec could of
course introduce other cache-control values that could cause false positives w/
this test... hmmm... thanks brendan, i'll file a bug to clean up this test as
well as similar tests scattered throughout the http code.

Comment 9

[lchiang]

tever or ben - can this be verified?  I think we've done really good testing on
this, including some tests jrgm did.

Adding PDT+ so this bug can be included on any bug stats we do in the future for
bugs fixed.

Comment 10

[benc]

VERIFIED:
after talking w/ tever and bbaetz, + many others who have also given feedback.

There are some other new issues, bbaetz is covering them.

Comment 11

[Boris Zbarsky [:bzbarsky]]

http://hg.mozilla.org/mozilla-central/rev/f290559c01d1 and http://hg.mozilla.org/mozilla-central/rev/f90c51ed3cd2 added a test for this bug.