Wireshark · Wireshark-users: Re: [Wireshark-users] bad handling of DHCP option 90?

[Jaap Keuter <jaap.keuter@xxxxxxxxx>]

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;