On Wed, Aug 7, 2024 at 5:55 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> On 06/08/2024 03:58, Thomas Munro wrote:
> > On Tue, Aug 6, 2024 at 2:41 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> >> What if the message contains multiple attribute of the same type? If
> >> there's a duplicate Message-Authenticator, we should surely reject the
> >> packet. I don't know if duplicate attributes are legal in general.
> >
> > Duplicate attributes are legal in general per RFC 2865, which has a
> > table of attributes and their possible quantity; unfortunately this
> > one is an extension from RFC 2869, and I didn't find where it pins
> > that down.
There's a similar table near the end of 2869:
https://datatracker.ietf.org/doc/html/rfc2869#section-5.19
> > I suppose if we wanted to be extra fastidious, we could also test with
> > a gallery of malformed packets crafted by a Perl script, but that
> > feels like overkill. On the other hand it would be bad if you could
> > crash a server by lobbing UDP packets at it because of some dumb
> > thinko.
>
> This would also be a easy target for fuzz testing. I'm not too worried
> though, the packet format is pretty simple. Still, bugs happen. (Not a
> requirement for this patch in any case)
<tangent>
I've been working on fuzzing JSON, and I spent some time adapting that
to test this RADIUS code. No pressure to use any of it (the
refactoring to pull out the response validation is cowboy-quality at
best), but it might help anyone who wants to pursue it in the future?
This fuzzer hasn't been able to find anything in the response parser.
(But the modifications I made make that claim a little weaker, since
I'm not testing what's actually shipping.) The attached response
corpus could be used to seed a malformed-packet gallery like Thomas
mentioned; it's built with the assumption that the request packet was
all zeroes and the shared secret is `secret`.
I was impressed with how quickly LLVM was able to find the packet
shape, including valid signatures. The only thing I had to eventually
add manually was a valid Message-Authenticator case; I assume the
interdependency between the authenticator and the packet checksum was
a little too much for libfuzzer to figure out on its own.
</tangent>
--Jacob