OpenSSH

[Michal Zalewski]

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

[Kurt Roeckx]

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

[Konstantin Serebryany]

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:
> 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

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.

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.... 

 

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

--
You received this message because you are subscribed to the Google Groups "afl-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to afl-users+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Michal Zalewski]

> 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).

In 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

[Kurt Roeckx]

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

[Kurt Roeckx]

On Tue, Mar 21, 2017 at 01:29:46PM -0700, Konstantin Serebryany wrote:
> 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:
> > > 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
> >
> > 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.
>
>
> This is why you've asked to support up to 64-bytes in memcmp, right?

Yes.

> 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).

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

[Konstantin Serebryany]

I don't have first hand experience with this (most likely you and the boringssl folks know more about it).

https://github.com/google/boringssl/blob/master/FUZZING.md#fuzzer-mode

Can you give an example in pseudo code of what exactly problem we are talking about? 

[Kurt Roeckx]

I've read that document before, and want to do some of the things
they do differently.

> Can you give an example in pseudo code of what exactly problem we are
> talking about?

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

[Konstantin Serebryany]

Is it possible to simply fuzz the "then parse the decrypted packet" part? 

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

[Kurt Roeckx]

On Wed, Mar 22, 2017 at 03:22:29PM -0700, Konstantin Serebryany wrote:
> >
> > 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.
> >
>
> Is it possible to simply fuzz the "then parse the decrypted packet" part?

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

[Markus Teufelberger]

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.