Ethereal-dev: RE: [Ethereal-dev] Re: Patch for indefinite length forpacket-ber.candvarious fix
Hi,
First off, Tim, many thanks for adding the support for indefinite length -
it makes life a whole lot easier!
I have found two problems with the patch however:
1) Constructed SEQUENCEs
In dissect_ber_sequence():
if(len==0) {
offset = eoffset+(2*ind_field);
} else {
offset = hoffset+count;
}
should be
if(len==0) {
offset = eoffset+(2*ind_field);
} else {
offset = hoffset+count+(2*ind_field);
}
Or even simplified further.
2) Zero length elements (e.g. an empty SET)
The logic is slightly wrong in get_ber_length():
/* ok in here we can traverse the BER to find the length, this will
fix most indefinite length issues */
/* Assumption here is that indefinite length is always used on
constructed types*/
while ((tvb_reported_length_remaining(tvb,offset)>0) && (
tvb_get_guint8(tvb, offset)) && (tvb_get_guint8(tvb,offset+1)))
/* check for EOC */
This will cause it to bail out if it finds a legal length of 0 (at offset+1)
for a given class/tag.
It should be:
/* ok in here we can traverse the BER to find the length, this will
fix most indefinite length issues */
/* Assumption here is that indefinite length is always used on
constructed types*/
while ((tvb_reported_length_remaining(tvb,offset)>0) && ((
tvb_get_guint8(tvb, offset)) || (tvb_get_guint8(tvb,offset+1))))
/* check for EOC */
i.e. we want both bytes to be zero - not just one - for EOC.
These changes may help with the CMIP/FTAM/GSMxx captures - it certain works
with my X.400 captures which next indefinite lengths 6 deep.
Graeme
P.S. I have added support for the reassembly of constructed octet strings to
packet-ber.c, which I will submit later on.
> -----Original Message-----
> From: ethereal-dev-bounces@xxxxxxxxxxxx
> [mailto:ethereal-dev-bounces@xxxxxxxxxxxx] On Behalf Of
> ronnie sahlberg
> Sent: 08 August 2005 07:57
> To: tim@xxxxxxxxxxxxxxx; Ethereal development
> Subject: [Ethereal-dev] Re: Patch for indefinite length
> forpacket-ber.candvarious fixes to tcap dissector.
>
> The patch makes several of my CMIP/FTAM/GSMxx captures to fail.
>
> I will need some time to see why the patch breaks these protocols.
>
>
>
> On 8/6/05, Tim <tim@xxxxxxxxxxxxxxx> wrote:
> > Luis,
> >
> > Here is an updated patch, this includes a oversight I must have made
> > when looking at the NULL dissector, also there are more problem with
> > indefinite length combined with Sequence of with implicit
> tags. I have
> > put a quick work around in get_ber_length for now but will
> be looking
> > for a much improved solution in the future.
> >
> > The issue is that a combination of implicit tags and
> indefinite length
> > can cause a sub-dissector to receive the EOC but not
> receive the tags
> > and length fields...
> >
> > The trace you sent over now appears to dissect correctly.
> >
> > Patch still against svn diff...
> >
> > Tim
> >
> > -----Original Message-----
> > From: LEGO [mailto:luis.ontanon@xxxxxxxxx]
> > Sent: 06 August 2005 00:07
> > To: tim@xxxxxxxxxxxxxxx
> > Subject: Re: [Ethereal-dev] Patch for indefinite length for
> > packet-ber.candvarious fixes to tcap dissector.
> >
> >
> > Tim,
> >
> > I've tried your patch, I've found some problems at least with the
> > attached capture (It has EOCs), the fact is that it breaks
> dissection
> > of things that without it were working.
> >
> > The packets that use indefinite lenghts are the responses that match
> > "gsm_map.invokeCmd == 55" (I got the requests fixed myself, I just
> > haven't checked that in yet ).
> >
> > Lius
> >
> > On 8/5/05, Tim <tim@xxxxxxxxxxxxxxx> wrote:
> > > Here is the patch against the current SVN.
> > >
> > > Tim
> > >
> > > -----Original Message-----
> > > From: ethereal-dev-bounces@xxxxxxxxxxxx
> > > [mailto:ethereal-dev-bounces@xxxxxxxxxxxx] On Behalf Of Tim
> > > Sent: 29 July 2005 19:23
> > > To: 'Ethereal development'
> > > Subject: RE: [Ethereal-dev] Patch for indefinite length for
> > > packet-ber.candvarious fixes to tcap dissector.
> > >
> > >
> > > With some fixes.
> > >
> > > Tim
> > >
> > > -----Original Message-----
> > > From: ethereal-dev-bounces@xxxxxxxxxxxx
> > > [mailto:ethereal-dev-bounces@xxxxxxxxxxxx] On Behalf Of Tim
> > > Sent: 26 July 2005 10:34
> > > To: 'Ethereal development'
> > > Subject: [Ethereal-dev] Patch for indefinite length for
> packet-ber.c
> > > andvarious fixes to tcap dissector.
> > >
> > >
> > >
> > > I have updated the packet-ber.c to deal with indefinite
> length, now
> > the
> > > get_ber_length function should return the correct length for
> > indefinite
> > > length encoding.
> > >
> > > The tcap dissector has been updated to use this length. I have not
> > > tested other asn.1 dissectors to ensure that they
> correctly use the
> > > indefinite encoding flag instead of the length value
> returning zero.
> > >
> > > There may also be some problems when re-assembly is
> needed, but the
> > > ability to deal with indefinite length is much more useful.
> > >
> > > For developers the get_ber_length now returns the length
> of the pdu
> > > including the EOC, where you have dissectors that use
> packet-ber.c the
> > > eoc may need to be dealt with separately.
> > >
> > > The tcap dissector has had numerous changes to make it
> less cluttered,
> > > and the useful feature of the previous version where a
> dialogue could
> > be
> > > filtered out by selecting either the source or
> destination transaction
> > > ID has been incorporated into this version.
> > >
> > > Tim
> > >
> > >
> > >
> > > _______________________________________________
> > > Ethereal-dev mailing list
> > > Ethereal-dev@xxxxxxxxxxxx
> > > http://www.ethereal.com/mailman/listinfo/ethereal-dev
> > >
> > >
> > >
> > >
> >
> >
> > --
> > This information is top security. When you have read it, destroy
> > yourself.
> > -- Marshall McLuhan
> >
> >
>
> _______________________________________________
> Ethereal-dev mailing list
> Ethereal-dev@xxxxxxxxxxxx
> http://www.ethereal.com/mailman/listinfo/ethereal-dev
>
