| OpenSSH | Michal Zalewski | 19/03/17 19:43 | In case people have not seen this elsewhere - here's a very good blog
post covering the non-trivial task of meaningfully fuzzing a complex, crypto-themed network service: http://vegardno.blogspot.com/2017/03/fuzzing-openssh-daemon-using-afl.html /mz |
| Re: [afl-users] OpenSSH | Kurt Roeckx | 21/03/17 12:49 | When fuzzing openssl I have actually taking a different approach.
In that post he explains things like disabling the CRC and MAC. I don't disable them because you don't end up in the error cases anymore and have less coverage. The error cases are usually also the most interesting because they are less well tested. Instead I replace the const time memcmp() with a real memcmp(). Libfuzzer will add this to a temporary dictionary and then is usually able to fill in the correct one. There are also cases where the MAC / CRC is encrypted, in which case this doesn't work so well. I have some vague plan to reencrypt it with the correct MAC, and then call memcmp(), but I'm not sure how easy that is going to be. In any case, without actually removing the MAC/CRC checks afl and libfuzzer are both already very good in finding the correct values to get good coverage. Kurt |
| Re: [afl-users] OpenSSH | Konstantin Serebryany | 21/03/17 13:30 | On Tue, Mar 21, 2017 at 12:49 PM, Kurt Roeckx <ku...@roeckx.be> wrote:On Sun, Mar 19, 2017 at 07:43:21PM -0700, Michal Zalewski wrote: This is why you've asked to support up to 64-bytes in memcmp, right? Indeed, this approach works, but it is also somewhat wasteful because the data-flow-guided mutations happen only with some probability (say, 1/100, or 1/1000, I don't know the exact value). OTOH, if the CRC or some such don't match we exit early, so maybe it's not that wasteful after all....
|
| Re: [afl-users] OpenSSH | Michal Zalewski | 21/03/17 14:12 | > Indeed, this approach works, but it is also somewhat wasteful becauseIn general, if somebody wants to test the handling of checksum errors, it's probably better to replace: if (header_cksum != computed_checksum) bork(); ...with: if (!(header_checksum % 100)) bork(); ...or something like that. Still exercises that code path every now and then, but doesn't waste CPU time trying to solve a pointless puzzle. /mz |
| Re: [afl-users] OpenSSH | Kurt Roeckx | 21/03/17 14:49 | It's actually things like a sha256 sum, so a 32 byte string. I
guess you suggest that I just compare the first byte of that string to for instance if it's == 0 and if it matches return that it doesn't match and otherwise that it does? That should be easy enough to change. Kurt |
| Re: [afl-users] OpenSSH | Kurt Roeckx | 21/03/17 15:16 | On Tue, Mar 21, 2017 at 01:29:46PM -0700, Konstantin Serebryany wrote:Yes. It seems to be high enough to find the cases were it increases the coverage fast. But I'm also wondering how the data-flow-guided mutations work when using encryption. Does it still work properly? Is there something we can do to improve it? Kurt |
| Re: [afl-users] OpenSSH | Konstantin Serebryany | 21/03/17 16:25 | I don't have first hand experience with this (most likely you and the boringssl folks know more about it). Can you give an example in pseudo code of what exactly problem we are talking about?
|
| Re: [afl-users] OpenSSH | Kurt Roeckx | 22/03/17 12:01 | I've read that document before, and want to do some of the things
they do differently. There are various combinations of how this works exactly, but it boils down that you get a packet, decrypt that, and then parse the decrypted packet. At least with TLS 1.2 and older there was an option to do use the "NULL cipher" in which case it's not encrypted, but that doesn't seem to exist in TLS 1.3. So I might need to take a look at how I can do that in TLS 1.3. Kurt |
| Re: [afl-users] OpenSSH | Konstantin Serebryany | 22/03/17 15:22 | Is it possible to simply fuzz the "then parse the decrypted packet" part?
|
| Re: [afl-users] OpenSSH | Kurt Roeckx | 23/03/17 11:24 | On Wed, Mar 22, 2017 at 03:22:29PM -0700, Konstantin Serebryany wrote:I guess it's possible to set up a fuzzer for only parsing the client hello message and so on. But with the parsed data it's normally going to do things, based on that it will enable certain things, send back 1 or more packets (that's not in the input file), get other packets in return, and so on. There is a whole statemachine that is affected by the parsed data, by things from a previous packet and so on. And I really want to fuzz as much as possible of that, which includes all the interactions between the different packets. There is possibly a large different in place of where the data is parsed and where it has an effect in the flow. I assume that the fuzzers are smart enough to see if I change a bit at some place, it has an effect much later then were it's parsed. But with encryption, changing a bit will actually change the whole decrypted packet, and I guess it can't see anymore how changing that has an effect. And that in the encrypted part is currently just dumb luck that it finds something useful. So clearly having a mode where the encryption doesn't do anything is useful. I added the NULL ciphers, but I think I should at least also have an option to turn all ciphers into a NULL cipher. Kurt |
| Re: [afl-users] OpenSSH | Markus Teufelberger | 27/03/17 02:25 | It would be great if you could add the FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION #ifdef suggested by libFuzzer (http://llvm.org/docs/LibFuzzer.html#fuzzer-friendly-build-mode) to your patches on Github, since it might help in the future to look for examples on how to disable these CRC checks etc. just by looking for code with this string in it. It also would have a much higher probability of being accepted upstream eventually. |