Wireshark · Ethereal-dev: RE: [Ethereal-dev] Re: Patch for indefinite lengthforpacket-ber.candvarious fixes to tcap dissector.

["Tim" <tim@xxxxxxxxxxxxxxx>]

Graeme,

Thanks for finding these, I'm sure Anders would apply a patch if you
send over a diff.

Can you grab the latest svn, as I have been making a number of fixes
over the last few weeks as different traces have been made
available(thanks). The first bug is dealt with in a different way now
but the second is valid.

I have attached my latest version with your fix done, can you check that
it still works with your nested indefinite length traces.

Tim



-----Original Message-----
From: ethereal-dev-bounces@xxxxxxxxxxxx
[mailto:ethereal-dev-bounces@xxxxxxxxxxxx] On Behalf Of Graeme Lunt
Sent: 21 August 2005 13:57
To: 'Ethereal development'
Subject: RE: [Ethereal-dev] Re: Patch for indefinite
lengthforpacket-ber.candvarious fixes to tcap dissector.


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
> 

_______________________________________________
Ethereal-dev mailing list
Ethereal-dev@xxxxxxxxxxxx
http://www.ethereal.com/mailman/listinfo/ethereal-dev

Attachment: packet-ber.c
Description: Binary data