lib/filters.c returning incorrect length filters

[Alan DeKok]

"Mitchell, Michael J" <Michael.Mitchell at team.telstra.com> wrote:
> While testing Ascend Data Filters (IP filters) it became apparent that
> freeRADIUS was sending 32 byte long Ascend-Data-Filter attributes. In
> our case our NAS's expect 24 byte IP filters (while some newer NAS's
> apparently expect 26 bytes?) and complains when its gets something
> longer.

  That's the first I've heard of the problem.  The code has been in
the server (and in Cistron before that) for over 8 years.

> Taking a look at lib/filters.c it became apparent that the code makes no
> distinction between the various filter types when assigning the length
> of the attribute.

  From everything I've seen, that's correct, and should work.

> A bit of reading on the web revealed that some other RADIUS servers
> allow the fill size at the end of an IP filter to be configured (to
> allow for 24 or 26 byte filters more easily). This looks a bit difficult
> in freeRADIUS due to the code being in libradius rather than the core.
> It is also something that would need to be configured on a per client
> basis, which is rather nasty.

  A module could do this without too much trouble.

> I believe there is also a bug in the print_abinary() function. It was
> comparing the vp->length to sizeof(filter), but filter is now a pointer.

  No... it's comparing vp->length to SIZEOF_RADFILTER, which is a
macro defined to be 32.  See earlier code.

> Also, it is not possible to do a "!=" comparison here, as different
> filter types have different lengths. At worst we can do a ">"
> comparison, a better solution would be to do the comparison based on the
> actual filter type further down the code. My patch compares the length
> of the attribute against the filter type.

  I'm really surprised that this code is necessary.  I'd like to know
a lot more before making any changes.

  Alan DeKok.