Wireshark · Ethereal-dev: [Ethereal-dev] Re: New dissector for CIGI

[ronnie sahlberg <ronniesahlberg@xxxxxxxxx>]

Very nice.

I have checked it in.


In the StartOfFrame   byte 4 is the ig status code. Is this type
restricted and can it be used in the heuristics?
byte 5 in that PDU seems to be a bitmap where only the low 4 bits are defined.
Heuristics here could check that
if(bitmap&0xf0) return0     not cigi

Similarly in the IGcontrol there is a bitmap that only covers 3 bits of byte 4
and for example the two least significant bits can not both be 1  (ig mode)


As for the heurstics being strong enough or not,   there are other
dissectors with weaker heurisdtics than cigi has.
This is not to say we should not try to make them sharper anyways.



Did you do the WIKI page for CIGI?
That page is very nice.



On 12/5/05, Harms, Kyle J <Kyle.J.Harms@xxxxxxxxxx> wrote:
> I've made the changes requested.  I've tried to strengthen the
> heuristics however this is something that my team and I discussed
> earlier.  Originally we wanted to create a heuristic dissector since
> CIGI doesn't ride on a defined set of ports, however the large variety
> in the protocol didn't seem like it would lend itself very well to being
> picked up based on its footprint.  Because of this we decided to go with
> the port preference.
>
> About the heuristics... I've tried to limit this as much as possible but
> it is difficult since so many of the values can take the entire range of
> the data type.
>
> These are the things we know about any CIGI transmission.  The first
> packet in a block must be a packet ID of 1 or 101 which is the IG
> Control or Start of Frame.  We know that is all versions of CIGI thus
> far the IG Control and SOF are 16 bytes except CIGI 1 where the SOF is
> 12 bytes.  We know that all CIGI packets will have the packet ID in the
> first byte and the packet size in the second byte.  We know that in the
> IG Control and SOF the third byte is the CIGI Version and the fourth is
> the Database Number.  Currently Valid CIGI versions are 1, 2, 3.
> Database Numbers can be any number from -128 to 127.  In CIGI 3 we know
> that the seventh and eighth bytes are the byte swap.  We know that in
> CIGI 1, 2 & 3 bytes 9 to 12 are the Frame Counter which valid values
> would be between 0 and 4,294,967,295.  We also know that CIGI 2 & 3
> bytes 13 to 16 valid values can be between 0 and 3.4028235 x 10e38.
>
> To me this doesn't seem like it gives enough unique information to
> correctly identify CIGI packets without giving false positives.  But in
> all truthfulness I don't use Ethereal enough to know.

>
> If you can see any additional unique information to add to the
> heuristics let me know and I will fix it.  Here is packet-cigi.c as it
> stands now.
>
> Thanks for your help.
>
> Kyle
>
> > -----Original Message-----
> > From: ronnie sahlberg [mailto:ronniesahlberg@xxxxxxxxx]
> > Sent: Saturday, December 03, 2005 5:09 AM
> > To: Ethereal development
> > Subject: [Ethereal-dev] Re: New dissector for CIGI
> >
> > Kyle,
> >
> > I have attached a new version of the dissector that uses heuristics to
> > determine if it is CIGI or not.
> > No need for specifying a range of ports.
> >
> > Can you have a look at maybe sharpening the heuristics if possible and
> > comment?
> >
> >
> >
> > On 12/2/05, Harms, Kyle J <Kyle.J.Harms@xxxxxxxxxx> wrote:
> > > I have a few questions before I look at this some more.
> > >
> > > 1,
> > > The reason I did Booleans this way is because if I use the
> FT_BOOLEAN it
> > > only shows '. = IG Mode...' in the window as apposed to '.... ...1 =
> IG
> > > Mode...' which is very useful in debugging CIGI.  Is there a way to
> use
> > > FT_BOOLEAN and still see which bits it is actually using i.e. '....
> > > ...1' ?
> > >
> > > 2,
> > > README.developer does not mention anything about a new style
> dissector.
> > > Where might I look at an example of such a dissector?  I might
> suggest
> > > that the README.developer stub be updated to be a new style
> dissector.
> > >
> > > 3,
> > > I don't think there is really a clean way to do a 'range' check in a
> > > case/switch statement, other than default,  but I need that for
> else.
> > >
> > >         } else if ( packet_id >= CIGI2_PACKET_ID_USER_DEFINABLE_MIN
> &&
> > > packet_id <= CIGI2_PACKET_ID_USER_DEFINABLE_MAX ) {
> > >             hf_cigi2_packet = hf_cigi2_user_definable;
> > >             packet_length = packet_size;
> > >         } else {
> > >             hf_cigi2_packet = hf_cigi_unknown;
> > >             packet_length = packet_size;
> > >         }
> > >
> > > Is there such a way to make this clean as apposed to
> > > case 236:
> > > case 237:
> > > case 238:
> > > case 239:
> > > ...
> > > case 255:
> > >
> > > ?
> > >
> > > I will try and address these issues as soon as I can.
> > >
> > > Kyle
> > >
> > > -----Original Message-----
> > > From: ronnie sahlberg [mailto:ronniesahlberg@xxxxxxxxx]
> > > Sent: Friday, December 02, 2005 3:41 PM
> > > To: Ethereal development
> > > Subject: [Ethereal-dev] Re: New dissector for CIGI
> > >
> > > Hi,
> > >
> > >
> > > 1,
> > > In some places you specify booleans in a bit suboptimal way:
> > > Please see :
> > > the definition of cigi_valid_vals[]   and
> > > hf_cigi3_ig_control_timestamp_valid.
> > >
> > > These ones should instead be using a true_false_string   and the
> > > hf_field should be    TFS(&cigi_valid_tfs), and be of type
> FT_BOOLEAN.
> > >
> > >
> > > 2,
> > > we have many many dissetors now and we get more and more
> "collissions"
> > > when ehtereal takes the wrong dissector.
> > >
> > > Can you change dissect_cigi()   to being a new-style dissector (i.e.
> > > returning int) and using new_create_dissector_handle() to register
> the
> > > handle.
> > > The new signature for dissecvt_cigi() would then be static int.
> > >
> > > Then dissect_cigi would return 0 if it was not a cigi packet (and
> > > letting ethereal try something else) or x   for x number of bytes
> > > eaten by the dissector.
> > >
> > > in the beginning of dissect_cigi()  before you start manipulating
> > > col_info/col_protocol and before starting dissection  you should add
> > > some heuristics to verify (as far as this is possible) that this
> does
> > > indeed look like a cigi packet.  and return 0 if not.
> > >
> > >
> > > 3,
> > > you use very long chains of if()  {} else if {} when demultiplexing
> > > things like packet_id and calling the subdissectors.
> > > change this to a switch/case
> > >
> > >
> > >
> > > apart from that the dissector looks good
> > > can you please address the 3 issues above?
> > >
> > >
> > >
> > >
> > >
> > > On 12/2/05, Harms, Kyle J <Kyle.J.Harms@xxxxxxxxxx> wrote:
> > > > Hi,
> > > >
> > > > This patch is for a CIGI dissector (complete versions 2 and 3).
> It
> > > has
> > > > been [fuzz] tested on GNU/Linux using the Ethereal 0.10.13
> codebase.
> > > > However, the patch here is against the svn repository.
> > > >
> > > > More information about CIGI can be found at
> > > http://cigi.sourceforge.net/
> > > >
> > > > Kyle Harms
> > > >
> > > >
> > >
> > > _______________________________________________
> > > Ethereal-dev mailing list
> > > Ethereal-dev@xxxxxxxxxxxx
> > > http://www.ethereal.com/mailman/listinfo/ethereal-dev
> > >
> > > _______________________________________________
> > > Ethereal-dev mailing list
> > > Ethereal-dev@xxxxxxxxxxxx
> > > http://www.ethereal.com/mailman/listinfo/ethereal-dev
> > >
>
>