Wireshark-dev: Re: [Wireshark-dev] Netflow dissector bug-to-be
From: Hadriel Kaplan <HKaplan@xxxxxxxxxxxxxx>
Date: Sun, 7 Nov 2010 16:11:25 -0500
On Nov 7, 2010, at 12:51 PM, Bill Meier wrote:

> Looking at the code a bit I see that currently "pen"
> seems to be effectively limited to 16 bits even though 32 bits are 
> fetched from the tvbuff:
> 
> dissect_v9_v10_template_fields(...) {
>         ...
> 	guint16      pen;
> 	...
> 	if ((ver == 10) && (type & 0x8000)) {  /* IPFIX only */
> 		pen = tvb_get_ntohl(tvb,offset+4);
> 		...
> 		}

Yup, another bug.

> 
> Is this a bug ? Are Enterprise Numbers greater that 16 bits allowed ? 
> (It would seem so).

Yup, enterprise numbers are 32 bits.

> 
> In practical terms: Are there currently values > 16 bits ?

No, the current greatest high IANA has is 36840.  But it's been growing (they're free after all :)


> If not, one hack might be to define pen_type in 
> dissect_v9_v10_pdu_data() as guint32 which gets around the "silent cast" 
> issue for the moment.

Huh.  I tried that and you're right it does - why?  


> Maybe a slightly better fix might be to have a separate function  to 
> handle the pen_type for each different Enterprise Number (even if 
> there's some duplication).

That's what my code does right now - I would submit it, but it's only a temporary situation for showcasing an IETF draft at this week's IETF meeting, using a research Enterprise number because this is still a draft.  If the draft gets adopted then IANA will assign real ipfix IE numbers for the fields and it won't use an Enterprise Number.

I think the "right" thing to do (or at least "ambitious" thing) would be not to have vendor-specific Netflow fields using private enterprise numbers written in static code to begin with, but rather loaded from dictionary files like RADIUS and DIAMETER dissectors do.  Netflow's model for this is effectively the same concept as RADIUS VSAs, DIAMETER vendor AVPs, and SNMP OIDs.  There are already a bunch of vendor-specific Netflow v9 fields being used which aren't in the dissector code, and making code changes for each one as they're encountered will be annoying.  I'm hoping to make that change to Netflow's dissector as soon as I get some free time. :)

-hadriel