lib/filters.c returning incorrect length filters

[Mike Mitchell]

Hi Alan,

Thanks for the response. See comments below.
 
> 
> "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.


I'll try to set up a clean install and retest, but I'm pretty sure the
behaviour I have observed is as stated. By analysis:

sizeof(ascend_ip_filter_t)      = 22 bytes
sizeof(ascend_ipx_filter_t)     = 26 bytes
sizeof(ascend_generic_filter_t) = 22 bytes

and, according to:

typedef struct ascend_filter_t {
	uint8_t 	type;
	uint8_t		forward;
	uint8_t		direction;
	uint8_t		fill;
	union {
		ascend_ip_filter_t   	 ip;
		ascend_ipx_filter_t   	 ipx;
		ascend_generic_filter_t	generic;
		uint8_t			data[28]; /* ensure it's 32 bytes */
	} u;
} ascend_filter_t;


sizeof(ascend_filter_t) = 32 bytes (4 + size of the union)

In ascend_parse_filter():

 pair->length is set to sizeof(filter), and
 sizeof(filter) bytes are copied to pair->vp_filter

where filter is of type ascend_filter_t.


So, maybe the actual problem is grouping ascend_ipx_filter_t with ip and
generic filters, which are a different format/size?


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

In earlier versions of the file, yes I agree. But in CVS HEAD (version 1.45)
it compares sizeof(filter). Unless my winCVS has gone wacky. It changed
going from 1.42 to 1.43.


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

Understood! The change may not be 100% necessary. In our case, the filters
are still applied (Juniper ERX), but produces warning messages indicating
that the format may not be correct. So it may be a matter of "correctness"
rather than being 100% necessary.


regards,
Mike