Repository navigation
Conversation
arturobernalg
force-pushed
the
fix/httpd-70289-cache-vary-star-err-headers
branch
from
October 31, 2026 16:08
1ecc52a to
a802ccf
Compare
The cacheability check only looked at r->headers_out for a Vary: * response, so a Vary: * set via "Header always" (which writes to r->err_headers_out) was missed. Storage itself does merge err_headers_out into headers_out, so the response got stored and could later be served to a different client under the same key, handing one client's response to another. Also fix cache_select(): it tokenized a stored response's Vary header and, for a "*" token, looked up a request header literally named "*". Since no such header exists on either side, both sides were NULL and the code treated that as a match, serving a stored Vary: * entry to a mismatched request instead of always treating it as a mismatch per RFC 9111 4.1. This closes the read side for any Vary: * entry already on disk (e.g. from before this fix, or written during a rolling upgrade), the same way the save-time check above closes the write side. PR 70289
arturobernalg
force-pushed
the
fix/httpd-70289-cache-vary-star-err-headers
branch
from
October 31, 2026 16:47
a802ccf to
1d2a892
Compare
covener
approved these changes
Nov 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
mod_cache's cacheability check for
Vary: *only looked atr->headers_out. WhenVary: *is set withHeader always set Vary "*"instead ofHeader set, it lands inr->err_headers_outinstead, so the check missed it - butstore_headers()does mergeerr_headers_outin (viaap_cache_cacheable_headers_out()/cache_merge_headers_out()), so the response got stored anyway, and could later be served to a different client from the cache.On top of that,
cache_select()had a related read-side bug: when matching a stored response'sVaryheader against the current request, a*token was looked up as a literal request header named*. Since no such header ever exists, both sides came backNULLand the code treated that as a match instead of RFC 9111 §4.1's "a stored response withVary: *always fails to match" rule. So aVary: *entry already on disk - written before this fix, or during a rolling upgrade across mixed versions - could still be served to the wrong client even with the store-side fix alone.Impact: reproduced end to end against a real httpd with a disk cache in front of a proxied backend. A backend that varies its body by a cookie while marking the response
Cache-Control: public, max-age=600(already questionable backend behaviour, but not something httpd can see), behindHeader always set Vary "*": a request withCookie: session=victimgets cached, and a following request with no cookie getsX-Cache: HITand the victim's body back. This is a cross-client information disclosure through the shared cache, gated behind that specific admin configuration (Header always + a backend that marks per-identity content as publicly cacheable). Separately reproduced thecache_select()gap with a two-binary test: a pre-fix build wrote aVary: *entry to disk, then a post-fix build serving from the sameCacheRootstill served it to a mismatched request, untilcache_select()was fixed too.Fix: check
err_headers_outtoo incache_save_filter(), matching how this same function already treatsExpires/Last-Modified/Etag/Cache-Control/Pragmaa few lines above (err_headers_out first, falling back to headers_out). And incache_select(), treat a storedVary: *token as an automatic, unconditional mismatch rather than an absent-header no-op match, per RFC 9111 §4.1. Bothmod_cache_diskandmod_cache_socacheshare this onecache_select(), so one fix covers both providers.Test: added
test/modules/proxy/test_08_cache_vary_star.py, modeled on the existingtest_07_cache_control.py: the original cross-client leak (Header always), a positive control proving the disk cache genuinely works absentVary: *, and the pre-existingHeader set(headers_out) case for regression coverage. Thecache_select()fix itself isn't separately covered by an automated test - with the save-time gate in place no live HTTP path can produce aVary: *disk entry anymore, and exercising the read path directly would require hand-constructingmod_cache_disk's private on-disk struct; it was instead verified with the manual two-binary reproduction above.PR 70289
(see
SECURITY.mdandhttps://www.apache.org/security/#reporting-a-vulnerability).
Leaving this unchecked on purpose: this does have real cross-client
impact, see "Impact" above. BZ 70289 was already filed as a public,
non-embargoed Bugzilla report with a full PoC by the original
reporter, so I'm not disclosing anything new here, but I'd rather
flag it than self-certify.
APLOGNO()tag;numbers are assigned by a committer at merge time (see
docs/log-message-tags/README).(One new log message was added, in
cache_select(); itsAPLOGNO()isleft empty for a committer to assign at merge time.)
changes-entries/*.txtfile is included,following the template in
README.CHANGES(not needed otherwise).test/modules/xxxdirectory matching themodules/xxxmodule source,or
test/modules/corefor core server changes. On Unix, verify thesepass locally with e.g.
make check-pytest PYTEST_DIRS=test/modules/xxx.(Verified both fixes against a real build with standalone end-to-end
reproductions equivalent to this test, including the cache_select()
upgrade scenario described above; could not run the pyhttpd suite
itself in this sandbox, which lacks
uv/the project's test venv.)