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

[ronnie sahlberg <ronniesahlberg@xxxxxxxxx>]

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' ?

Ah, when you specify FT_BOOLEAN   the next field is an integer where
you specify how many bits are in the actual bitmap.
Specify it as ... , FT_BOOLEAN, 32, ...
to display it as
.... ....  .... ..X.  .... ....  .... ....

Then dissect it with
proto_tree_add_item(... , offset, 4, ...
to tell ethereal it should highlight all 4 bytes in the hex pane.


see for example packet-smb2.c  that uses booleans this way.



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


Ok,  remind me i must update that.
It is real easy.
You just change the signature to static int and then just return the
number of bytes eaten or 0 if the dissector rejects it (==this is NOT
my protocol, try something else).

See   packet-xot.c for an example of  new_create_dissector_handle()

the function dissect_xot()   does some very primitive heuristics to
check it is indeed an xot packet.   it just checks that it has at
least 2 bytes and if not reject it as not xot.
Please make your heuristics stronger than that. :-)

packet-ucp.c   has muich more sophisticated heuristics in   dissect_ucp_heur()

iFCP has decent heuristics fort this also
see
packet-ifcp.c   dissect_ifcp() and get_next_ifcp_header_offset()


I.e.   just change it to the signature xot uses   and just add code to
the start of the dissector to check that the headers look sane   and
reject it by return 0   othervise.
Things like if byte 4-5 must have values in the range 7-12   check
this and return 0 without dissecting anything othervise.

The stronger the tests are the better.


If you can make the test real strong with very very little chance of
false positives   then you could get rid og the ports for the protocol
completely and just register that new-style dissector as a heuristic
dissector.
Then ethereal would find your protocol automatically,   unless some
other dissector claims the packet as theirs first.




>
> 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:
>
> ?

ok  leave it as it is if you cant find a clean solution.



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