Using the back button after logging out shows you pages from the authenticated user's session

OWASP has a page in the WSTG about this topic. Their recommendation:

The Back button can be stopped from showing sensitive data. This can be done by:

• Delivering the page over HTTPS.
• Setting Cache-Control: must-revalidate

This is basically what Drupal is doing already. Except that there are some additional directives on authenticated pages (Cache-Control: must-revalidate, no-cache, private). I wouldn't expect that the extra directives no-cache and private would reverse the effect of must-revalidate. So I looked at other response headers relevant to browser caching.

Turns out that we are missing Vary: Cookie on authenticated responses. The culprit is FinishResponseSubscriber::setResponseNotCacheable(). The comment in this method says:

    // There is no point in sending along headers necessary for cache
    // revalidation, if caching by proxies and browsers is denied in the first
    // place. Therefore remove ETag, Last-Modified and Vary in that case.

There is only one situation where this behavior would be correct: On a selected route which is delivered with Cache-Control: no-store in all cases (independently whether a session is open or not).

In all other situations, the Vary: Cookie header must be on every response. No matter whether a user is logged in or not or a session is open or not.

znerol → closed merge request !3504

MR !3505 adds Vary: Cookie to every response. Obviously needs manual browser testing.

MR !3505 doesn't seem to fix the problem. Interestingly this wasn't an issue on the previous build of the site I am mainly working on. I get a 403 (i.e., the expected behavior) with the following testing scenario in Firefox:

1. GET /
2. Log in -> 302 to /
3. Navigate to profile form (/user//edit)
4. Logout -> 302 to /
5. Hit browser back button -> 403

The legacy site was based on Drupal 7 and Authcache. Authcache encourages the browser to cache pages even for authenticated users. However, it also ensures that the ETag is different for each variant of a page.

I think that when Authcache is active, the browser is replacing the cached frontpage in step 2 (after login), and the cache is replaced again in step 4 (after logout). My hunch is that a cache replacement (in contrast to a cache insert) invalidates previous history and as a result the browser attempts to refetch the user page when the back button is hit.

Pure speculation though, more testing is clearly necessary. Also I am a bit disappointed by the OWASP page linked in #39. It doesn't seem to cover the full problem.

see https://www.drupal.org/project/drupal/issues/3130912 🐛 Incorrect Cache-Control headers for authenticated users Postponed

Should we consider Option 3:Cache-Control: no-store? On linked issue 🐛 Incorrect Cache-Control headers for authenticated users Postponed there's patches to implement it with test coverage. OWASP recommends the approach also on its FAQ page https://owasp.org/www-community/OWASP_Application_Security_FAQ#how-do-i-...

Twitter.com and Google seems to rely on Cache-Control: no-store header on authenticated services.

Fastly article from 2014 states about Vary: Cookie: "Cookie is probably one of the most unique request headers, and is therefore very bad."

I tested @znerol's MR !3505 with DDEV, it works only with HTTPS enabled, like mentioned on OWASP documentation. Tested with Firefox, Chrome, Edge. Unfortunately on Safari the back button showed the cached content.

Wrote on 🐛 Incorrect Cache-Control headers for authenticated users Postponed that we should implement the Cache Control header change on that issue. It will partially fix this issue. On Safari there's a bug https://discussions.apple.com/thread/251817133 and I hope we could narrow down this issue just for Safari.

sokru → changed the visibility of the branch 1912514-fix-vary-2 to hidden.