guzba / zippy

Pure Nim implementation of deflate, zlib, gzip and zip.
MIT License
246 stars 29 forks source link

[FR] HTB (Heal-the-BREACH) support #61

Closed ITwrx closed 1 year ago

ITwrx commented 1 year ago

It seems that mummy is using this library to automatically perform on-the-fly gzip compression for HTTP responses which is great for performance, but in certain scenarios can be a serious security vulnerability due to BREACH

The BREACH link mentions HTB (Heal-the-BREACH) as an inexpensive, but effective mitigation. It would be nice if this is implemented, so that gzip compression can be used safely without people having to worry about the specifics of the application, threat model, etc.

thanks

guzba commented 1 year ago

Thanks for the note about this. This appears to be a straightforward mitigation to implement but I'll need to sit down and get it working and tested before I know anything for real.

guzba commented 1 year ago

I have added the mitigation explained in the Proposed Mitigation Method section of the page linked above: https://github.com/guzba/zippy/pull/62

Let me know if you have any feedback on the PR. I'll be getting this merged and tagged in a release soon.

ITwrx commented 1 year ago

i'm not an expert on this stuff, but it looks good to me (pls see concerns about using time as seed below). The attack and mitigation doc is pretty interesting reading, huh? if a little long-winded in places. :)

Thanks for implementing this. I/We don't know how automated and widespread this attack may be by now. It's been known to infosec people for a while. Pretty juicy and widely-deployed target too.

ITwrx commented 1 year ago

assuming a PRNG, xoroshiro128+ in this case, is good enough for this use-case. PRNG is fine with a suitably random seed.

[continuing with orig comment] This also makes me wonder if some(more) randomness could be introduced into the seed for initRand()...

ITwrx commented 1 year ago

here's the randomizer they used. https://github.com/iit-asi/PAPER-Heal-the-Breach/blob/main/gzip-randomizer.c I don't know C, but it looks like they used /dev/random for the seed too, if i'm reading that right.

unsigned int seed;
    FILE* random = fopen("/dev/random", "r");
    if (random==NULL) {
        printf("Cannot access /dev/random you may change the C program to use /dev/urandom");
        exit(-1);
    }
    fread(&seed, sizeof(int), 1, random);
    fclose(random);
    srand(seed);
    //srand((unsigned)time(0));  //This was crap random initialization, just for testing
ITwrx commented 1 year ago

the updates on the readme for haveged are interesting. This should mean that using /dev/random on recent kernels has enough entropy, and is non-blocking, as well.

ITwrx commented 1 year ago

Although time is often used for the seed, it seems it is not all that suitable. I've seen this mentioned a few places. Here's one such example from this blog post

Starting With A Seed

And all pseudo-random number generators need to start somewhere; they need to be seeded and that's where Hacker News failed. The random number generator was seeded with the time in milliseconds when the Hacker News software was last started. By some careful work, the attacker managed to make Hacker News crash and could then predict when it restarted within a window of about one minute.

guzba commented 1 year ago

The issue with the suggestion to use /dev/random etc is that I cannot assume Linux. I started with using https://nim-lang.org/docs/sysrand.html however that is only available on Nim 1.6+ and Zippy works back to Nim 1.0. I could use that for Nim 1.6+ and fall back to std/random before that.

I am also not sure that the strength of the random is critical here, since it is only generating a number from 0 to 25 in this case, not exactly a lot of entropy. This is pre-TLS stuff which definitely has the cryptographic requirement, I just don't know if it is vital here. It may be, and it wouldn't hurt, but it is not easy to get good entropy on all Nim targets pre-1.6.

guzba commented 1 year ago

I have updated the PR to include a std/sysrand path when available

ITwrx commented 1 year ago

The issue with the suggestion to use /dev/random etc is that I cannot assume Linux. I started with using https://nim-lang.org/docs/sysrand.html however that is only available on Nim 1.6+ and Zippy works back to Nim 1.0. I could use that for Nim 1.6+ and fall back to std/random before that.

Oh yeah, i was just commenting on what i'm focused on/interested in. I figured there might be a std lib that would handle platform compat. Thanks for going to the trouble to use the nim 1.6 stuff.

I am also not sure that the strength of the random is critical here, since it is only generating a number from 0 to 25 in this case, not exactly a lot of entropy. This is pre-TLS stuff which definitely has the cryptographic requirement, I just don't know if it is vital here. It may be, and it wouldn't hurt, but it is not easy to get good entropy on all Nim targets pre-1.6.

I'm not entirely sure either, but i feel better about it now. :) Can't help it if something is using older nim.

guzba commented 1 year ago

I have merged the mitigation PR and tagged it in release 0.10.9. Closing this issue as resolved and moving on to the Mummy part of this.