Closed Bug 531801 Opened 15 years ago Closed 14 years ago

Should cache SSL content to disk even without Cache-Control: public

Categories

(Core :: Networking: HTTP, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.3a3
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: Biesinger, Assigned: Biesinger)

References

(Depends on 1 open bug)

Details

(Keywords: perf)

Attachments

(3 files)

Currently, Firefox only caches SSL content to disk if it was sent with Cache-Control: public. On the other hand, IE and Chrome cache SSL content even without that header. And this header was not designed to affect client caches (only proxies).

The reason why I implemented it this way was to avoid caching secure content to disk without an explicit hint that this is OK to do so, but considering that especially IE doesn't require this header, it seems OK to remove this requirement.
This effectively reverts most of the patch for bug 345181.
Attachment #425001 - Flags: review?(bzbarsky)
I'd really like Jesse and Daniel to give this a once-over...
Comment 0 sounds good to me.  I don't like unnecessary differences between http and https, because they can make https a disadvantage.  (I care less about differences that are just defaults, though.)

But I don't understand why you're flipping the pref to true *and* removing code that's needed in the case where the pref is set to false.  If we don't want to support having the pref set to false, we should remove the pref.
We'd still support setting the pref set to false, it'd just mean we never cache SSL content to disk (like we used to).
Attachment #424998 - Flags: review?(bzbarsky) → review+
Comment on attachment 424998 [details] [diff] [review]
part 1: set the pref to cache all SSL content
[Checkin: Comment 8]

r=bzbarsky
Comment on attachment 425001 [details] [diff] [review]
part 2: remove the code for cache-control public
[Checkin: Comment 8]

r=bzbarsky
Attachment #425001 - Flags: review?(bzbarsky) → review+
thanks, checked in

http://hg.mozilla.org/mozilla-central/rev/51f9d293c939
http://hg.mozilla.org/mozilla-central/rev/c4a6cd1c88ec
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a2
Should this:
// Only cache SSL content on disk if the pref is set
if (!gHttpHandler->IsPersistentHttpsCachingEnabled())
not also check whether is mConnectionInfo->UsingSSL()?
Now when 'HttpsCachingEnabled' is false, everything not cached?
Oops. Excellent catch.
Attachment #426899 - Flags: review?(bzbarsky)
Attachment #426899 - Flags: review?(bzbarsky) → review+
This bug should not be RESOLVED FIXED.
I'm actually going to reopen this until the patch from comment 10 lands.  That landing needs to block 1.9.3.
Status: RESOLVED → REOPENED
blocking2.0: --- → ?
Resolution: FIXED → ---
Blocks: 547537
This patch seems to have caused bug 547537 where having set browser.cache.disk_cache_ssl to false also prevented http files from being written to disk cache.
Contrary to SeaMonkey trunk Firefox trunk can be run with browser.cache.disk_cache_ssl set to false without problems.
Attachment #424998 - Attachment description: part 1: set the pref to cache all SSL content → part 1: set the pref to cache all SSL content [Checkin: Comment 8]
Attachment #425001 - Attachment description: part 2: remove the code for cache-control public → part 2: remove the code for cache-control public [Checkin: Comment 8]
> This patch seems to have caused bug 547537

Yes, because the patch from comment 10 did not land...
No longer blocks: 547537
Depends on: 547537
Since the tree's closed right now, marking checkin-needed so that someone else can get to this before I can.
Keywords: checkin-needed
Pushed that patch as http://hg.mozilla.org/mozilla-central/rev/a6ff3b5d364b

Possible to write tests for it?
Status: REOPENED → RESOLVED
Closed: 14 years ago14 years ago
Flags: in-testsuite?
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: mozilla1.9.3a2 → mozilla1.9.3a3
blocking2.0: ? → betaN+
I know I'm late to the party here, but I have a few questions regarding this feature.  My concern is that there appears to be no discussion from a security point of view on whether this is a good idea or not.  The argument is simply that IE and Chrome do it, so why don't we?

I realize a bulk of SSL traffic can be cached without any security issues and would lead to a nice performance improvement for SSL sites.  However, there is clearly some SSL content that contains private or sensitive information.  If that is cached and some virus, malware, etc. reads the cache, we open the users to possible data theft of their sensitive information.

Will the SSL cache be encrypted?  Are we relying on the Cache-Control (no-cache/no-store) HTTP header to prohibit caching of some SSL data?
See comment 4 and my comments in bug 424872.  I'm on MoCo's security team :)

Among sites that don't use cache-control:no-store, the correlation between "SSL" and "sensitive" is very low.  Furthermore, protecting against future malware infections doesn't buy you much; the malware can just wait for you to visit the sensitive site again once you're infected.

Penalizing sites' performance for using SSL has had a real effect on the overall security of the internet (see e.g. bug 358384).
Thanks for the info Jesse.  As long as any no-store/no-cache/private data is not cached, I'm comfortable with caching the SSL content.  That puts the decision into the content provider's hands - which is best as they know the sensitivity of the data.  Just for FYI, I'm really thinking about this from a corporate security perspective, which can be quite strict.
I think it's only "cache-control: no-store" that's not cached at all, fwiw.
You're correct - only no-store content should not be stored at the user agent.  Thanks for the clarification.
Depends on: CVE-2011-0082
Have I understood the current implementation correctly? Please, tell me if one or more of the following statements are incorrect:

- Allow writing cached copy to disk unless header "Cache-Control" contains "no-store". No exceptions (e.g. HTTPS).

- For example, following may be cached on disk, usable for 1h (still available after browser restart):
Cache-Control: private, max-age=3600, s-maxage=0

- For example, following may be cached on disk (to be user for e.g. Save As feature), will not be used to satisfy any subsequent request:
Cache-Control: private, no-cache, max-age=0, s-maxage=0

- For example, following may be cached in memory (to be user for e.g. Save As feature), will not be used to satisfy any subsequent request:
Cache-Control: private, no-store, max-age=0, s-maxage=0
I _think_ that all of those statements are correct.
Blocks: 810085
You need to log in before you can comment on or make changes to this bug.