From: "Tomas Kukosa"
Sent: Thursday, March 11, 2004 1:15 AM
Subject: [Ethereal-dev] BER and Kerberos
> Hello Ronnie,
> I have made more changes in the BER and Kerberos than I expected.
> Please, could you look into the packet-kerberos.c if changes are
acceptable for you?
> If you agree I will release the patch.
>
Hi,
Thanks for the interest in packet-ber.c.
I looked at the patch and it is really a step in the right direction.
The handling of bitstrings looks good but why not take it a few steps even
further:
It should be possible to make it even simpler while you are cleaning it up
properly anyway.
Change KDCOptions_bits to a NULL terminated array of pointers to gint :
so it becomes something like
static (*gint)[] = {
&hf_...,
...
NULL
};
Then change the call to :
offset=dissect_ber_named_bitstring32(FALSE, pinfo, tree, tvb, offset,
KDCOptions_bits, hf_krb_KDCOptions, ett_krb_KDC_Options, NULL);
and make dissect_ber_named_bistring32()
1, handle the bits in order until we come to the NULL entry.
2, the text to use to puch up on the expansion line can be found using
proto_registrar_get_nth(hf_index)->name (or is it abbrev?) so we
dont need to specify it a second time in the array as in your patch.
3, the dissect_ber_named_bistring32() should also for each hf_index chech
that the type is FT_BOOLEAN and that the number of bits was set to
32 if not do a g_assert() so we find the bug quickly.
I.e. since most bistrings are going to be named bistrings of length 32 bits,
why not make a special helper for that case
and let the other special cases either provide a new helper or do things
manually.
(in my new kerberos patch there is another bitstring (from inside the
encryted blobs))
The changes to the octetstring stuff, i have to admit i did not look too
closely on it and might be wrong
but Im not sure i like that.
The octet string thing really should take a callback function to call for
the content of the bitstring.
Not everything inside kerberos bitstrings are BER_APPLICATION, hey, not
everything that we need to pass to a subdissector is even ASN.1/BER encoded.
See in my latest kerberos patch from a day or two ago. Inside the PAC
structure there are octet strings that contain NDR encoded data. No
ASN.1 stuff at all.
Would you consider looking at making the bistring handling even simpler
while you are at cleaning ber up?
Could you also look at porting my packet-kerberos.c thing over to your new
api to check that the octetstring changes does not break it.
