Hi,
I reworked the patch a little:
- No C++ comments
- No need to check for size < 11, that is already done
- Changed the comparison to the value taken from the value_string
Not much changes this way. Hope you can check it out before we commit
this. I don't have a sample capture.
Thanx,
Jaap
On Wed, 25 Apr 2007, Stefan Puiu wrote:
> Thanks for the reply,
>
> actually, both the secret ID and HMAC fields are not present in
> DHCPDISCOVER messages. I've attached a new stab at a patch against the
> SVN head - the previous one was against the 0.99.5 source.
>
> Stefan.
>
> On 4/24/07, Jaap Keuter <jaap.keuter@xxxxxxxxx> wrote:
> > Hi,
> >
> > What about the presentation of the HMAC MD5 Hash? That's got to be
> > conditional as well.
> > Please refer to the SVN version, your line numbers seem to indicate an
> > older revision of the file.
> >
> > Thanx,
> > Jaap
> >
> > On Mon, 23 Apr 2007, Stefan Puiu wrote:
> >
> > >
> > > Stefan Puiu <stefan.puiu@...> writes:
> > >
> > > >
> > > > Hi all,
> > > >
> > > > I captured some DHCP traffic using DHCP AUTH (option 90 - see
> > > > RFC3118) using tcpdump on a Linux device and was then trying to view
> > > > it from wireshark (0.99.4) on Windows. The problem is I'm getting some
> > > > warnings on the option length, and I think they are wrong.
> > >
> > > OK, I've decided to dive a bit into the code and see what's wrong. I seem to
> > > have found the offending code in epan/dissectors/proto-bootp.c.
> > >
> > > It seems that there's a general check on option 90 so that the length field is
> > > >= 11, which seems right according to RFC3118. Then, wireshark expects the
> > > option length to be >= 31 if HMAC-MD5 is used - trouble is, that is valid for
> > > packets of any other type besides DHCPDISCOVER. Here'an attempt at a patch -
> > > since I'm using cygwin on Windows, I can't verify it:
> > >
> > > --- packet-bootp.c.orig 2007-02-02 00:00:56.000000000 +0200
> > > +++ packet-bootp.c 2007-04-24 00:42:44.267830400 +0300
> > > @@ -1172,11 +1172,20 @@
> > > switch (algorithm) {
> > >
> > > case AUTHEN_DELAYED_ALGO_HMAC_MD5:
> > > - if (optlen < 31) {
> > > + if (!strcmp(*dhcp_type_p, "Discover")) {
> > > + if (optlen < 11) {
> > > + proto_item_append_text(vti,
> > > + " length isn't >= 11");
> > > + break;
> > > + }
> > > + }
> > > + else if (optlen < 31) {
> > > proto_item_append_text(vti,
> > > " length isn't >= 31");
> > > break;
> > > }
> > > +
> > > +
> > > proto_tree_add_text(v_tree, tvb, optoff, 4,
> > > "Secret ID: 0x%08x",
> > > tvb_get_ntohl(tvb, optoff));
> > >
> > > Basically, it uses 11 as the minimum size if the DHCP message type is
> > > DHCPDISCOVER, and 31 otherwise.
> > >
> > > Stefan.
> > >
> > >
> > >
> > >
> > > _______________________________________________
> > > Wireshark-users mailing list
> > > Wireshark-users@xxxxxxxxxxxxx
> > > http://www.wireshark.org/mailman/listinfo/wireshark-users
> > >
> > >
> >
> > _______________________________________________
> > Wireshark-users mailing list
> > Wireshark-users@xxxxxxxxxxxxx
> > http://www.wireshark.org/mailman/listinfo/wireshark-users
> >
>
Index: packet-bootp.c
===================================================================
--- packet-bootp.c (revision 21561)
+++ packet-bootp.c (working copy)
@@ -1188,21 +1188,25 @@
switch (algorithm) {
case AUTHEN_DELAYED_ALGO_HMAC_MD5:
- if (optlen < 31) {
- proto_item_append_text(vti,
- " length isn't >= 31");
+ if (!strcmp(*dhcp_type_p, val_to_str(1, opt53_text, ""))) {
+ /* Discover has no Secret ID nor HMAC MD5 Hash */
break;
+ } else {
+ if (optlen < 31) {
+ proto_item_append_text(vti,
+ " length isn't >= 31");
+ break;
+ }
+ proto_tree_add_text(v_tree, tvb, optoff, 4,
+ "Secret ID: 0x%08x",
+ tvb_get_ntohl(tvb, optoff));
+ optoff += 4;
+ optleft -= 4;
+ proto_tree_add_text(v_tree, tvb, optoff, 16,
+ "HMAC MD5 Hash: %s",
+ tvb_bytes_to_str(tvb, optoff, 16));
+ break;
}
- proto_tree_add_text(v_tree, tvb, optoff, 4,
- "Secret ID: 0x%08x",
- tvb_get_ntohl(tvb, optoff));
- optoff += 4;
- optleft -= 4;
- proto_tree_add_text(v_tree, tvb, optoff, 16,
- "HMAC MD5 Hash: %s",
- tvb_bytes_to_str(tvb, optoff, 16));
- break;
-
default:
if (optleft == 0)
break;
