Re: dissect_ip() and check for header length

[Guy Harris <guy () alum mit edu>]

On Sep 16, 2010, at 7:12 AM, Petr Sumbera wrote:

> Ops. As I was not subscribed to the alias I missed that the mail 
> actually went though and it even got answer from Guy:
>
> > ...which means, of course, that no outgoing IPv4 traffic *ever* has IP
> > options.
>
> > Otherwise, outgoing IPv4 packets would be undissectable unless the
> > IPv4 header length is available elsewhere; you cannot dissect an IPv4
> > header unless you know how big it is, as it's not fixed-length (it's a
> > fixed-length 20-byte portion followed by 0 or more bytes of options).
>
> I know how big IP packet is. tvb_length(tvb) tells me...

I didn't say how big the *packet* is, I said how big the *header* is.  IPv4 has two length fields, the header length, 
which is what you say is zero, and the total length.  If you only know the total length, you *CANNOT* determine the 
header length.  What you originally said was

> > But the problem is that stored packets for *outgoing* traffic doesn't
> > contain "Header length" (it's zero as at layer where it was captured it
> > wasn't filled, the same case is for "Header checksum").
> >
> > dissect_ip() for such packets will report just:
> > "Header length: %u bytes (bogus, must be at least %u)".

which was referring to the header length, not the total length.

> So shall I create new dissector which will do something like this?
>
>  if (!tvb_get_ntohs(tvb, 2)) /* ip packet length is zero */

You could do that, but it wouldn't solve the problem you discussed in your earlier message, where the IP header length 
- i.e., the low-order 4 bits of the *first* byte of the packet, not the 2-byte big-endian number in the third and 
fourth bytes of the packet, which is the total length - is zero.

Which is it?  If you're seeing

        Header length: 0 bytes (bogus, must be at least 20)

perhaps the total length is zero, or perhaps it isn't, but the *header* length is definitely zero.

>  {
>    length = (guint16*)tvb_get_ptr(tvb, 2, 2);
>    *length = (guint16)htons(tvb_length(tvb));

That would be completely illegal:

        1) there is no guarantee whatsoever that the result of tvb_get_ptr() is aligned on a 2-byte boundary, and there 
is no guarantee that a machine running Solaris will allow you to store a 2-byte integral value through a misaligned 
pointer (Oracle offers Solaris on 2 architectures, one of which will allow that, and one of which will *TRAP*, causing 
Wireshark to *CRASH*, if you try to do that; at least at one point there was a project to port Solaris to ARM, and I 
think ARM won't trap but also won't do what you might expect);

        2) Wireshark assumes that the raw packet data will not be modified by dissectors - that's why tvb_get_ptr() 
returns a const pointer - and will not promise that attempts to do so will not trap due to the memory being on 
read-only pages.

You would have to make a copy of the packet data and construct a new tvbuff - and even that, as noted, can not and will 
not fix

        Header length: 0 bytes (bogus, must be at least 20)

if that's what you're seeing.  If you're seeing that, *NOTHING*, short of fixing Solaris to fill in the IP header 
length, or changing it to guarantee that none of the packets on an IP tunnel link *EVER* have IP options or to somehow 
provide the IP header length by some other mechanism, can fix that.
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe