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

["Harms, Kyle J" <Kyle.J.Harms@xxxxxxxxxx>]

I decided that dissect_cigi should dissect the packet not discover what
it is.  I figure that part should be in another function.  In the
process I have tried to strength the heuristics.

If they are strong enough, should I remove the port preference?  Because
even if you use it now, since we return 0 if the packet is not
recognized as CIGI it will not force CIGI dissection as before.
Currently this preference is quite useless, unless it is possible to
force CIGI dissection even if it is rejected as being CIGI.

The IG Status code has values 0 to 255 so it is of no use in heuristics.
I was able to use the IG Mode because its valid values are 0, 1, or 2
(not 3).  And for CIGI 3 the specification says that all reserved fields
must be 0.  ( this is not a requirement for other versions of CIGI).
The other bit fields can be 0 or 1 so they are of no use.

I would give an svn diff as a patch however, we are not allowed to
connect Linux boxes onto the network.  Otherwise I have to go to great
lengths to do `svn diff`.  I can do a `diff -u old-file new-file >
file.patch` if that would be preferred.

As far as Unicode goes.  I think it might be worthwhile to consider some
of the character set translation utilities.  Such as
http://www.cl.cam.ac.uk/~mgk25/download/transtab.tar.gz, maybe there are
better ones out there.  Maybe on platforms that do not support Unicode a
preprocessor could be run on all "" strings that would translate double
arrow to => and mu to u, etc.  Obviously there should be a way to handle
this, I just don't have enough experience to know.  How do we currently
handle the AUTHORS file since it is Unicode?

Kyle

> -----Original Message-----
> From: ronnie sahlberg [mailto:ronniesahlberg@xxxxxxxxx]
> Sent: Monday, December 05, 2005 7:15 PM
> To: Ethereal development
> Cc: Harms, Kyle J
> Subject: Re: New dissector for CIGI
> 
> 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
> > > >
> >
> >

Attachment: packet-cigi.c.gz
Description: packet-cigi.c.gz