varnish-cache icon indicating copy to clipboard operation
varnish-cache copied to clipboard

gzip: Heal-the-BREACH mitigation

Open dridi opened this issue 1 year ago • 6 comments

Having the ability to add noise to compressed cache hits was requested a couple times, and after my recent foray into zlib territory I decided to tackle it.

https://en.wikipedia.org/wiki/BREACH

dridi avatar Jul 03 '24 16:07 dridi

This is lacking test coverage in a sub-request, it should probably be limited to the top one.

dridi avatar Jul 03 '24 17:07 dridi

The commit message of 724293b013470b6c604d4ff0464ac9a448814f98 says "experimental::gzip_breach" but I meant "feature::gzip_breach". I confused it with the experimental flag introduced in #3873.

dridi avatar Jul 04 '24 16:07 dridi

Is it a good idea to pad with 1 .. BREACH_PAD?

This is how we get the variable length.

The original paper uses a maximum of HTB = 100, is BREACH_PAD == 256 a good idea?

My understanding was that 100 forced a high enough number of requests to perform the attack. I didn't remember seeing advice against going higher. No objection on picking 100, I simply went with a round number.

Calling VRND_RandomTestable() for each byte looks like significant overhead. Did you consider other options like seeding AES?

I picked what we have to offer in vrnd.h, and I wanted something that would allow test coverage, but I'm open to new solutions.

I wonder if the VDP_PUSH of the gzip header could somehow weaken the mitigation? In other words, do we leak a side channel by informing eve of where the actual data starts?

Yes, I was bothered by that. We could maybe reserve storage in the workspace to avoid this flush.

Similarly, do we leak a timing side channel by generating bytes inline (should we do this upfront?) and depending on the random pad length (should we maybe always generate BREACH_PAD bytes and just send a fraction?)

I guess once we have pre-allocated that space we can avoid that. The problem with the comment is that it needs to be null-terminated. We could of course generate gzip headers with a comment of 0..MAX bytes (plus null character) ahead of time and just randomly pick from that array. It could even be at compile time, no opinion.

dridi avatar Jul 08 '24 13:07 dridi

bugwash: we should also teach HTB to the gzip VFP for cache misses and passes, and evaluate whether this is appropriate for cache hits.

dridi avatar Jul 08 '24 13:07 dridi

I revisited the patch series to leave it in a better state in my absence. The mitigation is present in both VFP and VDP, but for a cache miss yielding a beresp gzipped by the backend, we may not have access to OA_GZIPBITS immediately.

The VDP is no longer requiring a flush after the gzip header's random comment, the comment finds stable storage inside the task's workspace.

I did not address the points on constant-time operations or pseudo-random algorithm. We could very well just make a copy of a predefined BREACH_PAD-long string and stick a null character at a random position. I will leave that change to reviewers. :grin:

dridi avatar Jul 10 '24 13:07 dridi

I forgot to link to the paper author's mitigation:

https://github.com/iit-asi/PAPER-Heal-the-Breach/blob/main/gzip-randomizer.c

It should be noted that when I tried it way back then I ended up with invalid gzip.

dridi avatar Jul 22 '24 07:07 dridi