Re: [Ethereal-dev] ITU/ANSI TCAP dissector

[Guy Harris <guy@xxxxxxxxxxxx>]

On Fri, Sep 12, 2003 at 09:46:55AM -0400, Jeff Morriss wrote:
> Jeff Morriss wrote:
> > 
> > I'm not ASN.1 expert either, but it appears that the existing 
> > "asn1_tag_decode()" only works for the "high tag number" format 
> > (multi-octet) tag IDs.  In particular, it assumes that bit 8 will be set 
> > on low-tag-number identifiers (but this isn't necessarily the case).
> > 
> > Oops, no, that's not the problem...
> > 
> > The problem is that the TCAP dissector is using 'asn1_tag_decode()' 
> > instead of 'asn1_id_decode()' (which does the work for dealing with low 
> > and high tag number formats).
> > 
> > However, the TCAP dissector wants the tag to include the Class, 
> > Constructor, and Tag bits all combined together...
> > 
> > So there are 3 options:
> > 
> > 1) Rewrite a bunch of the TCAP dissector to look at tag IDs as a triplet 
> > (class, constructor, tag number)
> >     - this probably has negative filtering consequences: users will 
> > probably want to filter on "tcap.msgtype == 0x62" instead of just the 
> > tag number
> > 
> > 2) Make 'asn1_tag_decode_m()' a wrapper around 'asn1_id_decode()' that 
> > then squishes the class, constructor, and tag number back together
> > 
> > 3) Change the comment for (and maybe the name of) 'asn1_tag_decode_m()' 
> > to indicate why that function is used instead of 'asn1_id_decode()'
> 
> Or, here's the (obvious, some would say) option #4:  Add the new routine 
> from (3) to "asn1.c" for others to use if they want. (I couldn't think 
> of a better name for this routine than 'asn1_id_decode1()'.)

I've checked in Michael's original TCAP dissector, with both of your
patches to it, along with the ASN.1 BER changes - except that, as I
don't know whether any plugins not part of Ethereal use
"asn1_tag_decode()", I left its semantics the same (it zeroes out *tag),
and put the bulk of it into a "asn1_tag_get()" routine, internal to
asn1.c, and had "asn1_id_decode1()" use it.

If "asn1_tag_decode()" isn't being used, we should get rid of it; if
it's being used but stuff using it should be using something else, we
should, again, get rid of it.