Wireshark · Wireshark-dev: Re: [Wireshark-dev] Wireshark "Decode As"

[João Valverde <joao.valverde@xxxxxxxxxxxxxxxxxx>]

On 09/15/2015 09:05 PM, Pascal Quantin wrote:
> 2015-09-15 21:15 GMT+02:00 Jo�o Valverde
> <joao.valverde@xxxxxxxxxxxxxxxxxx
> <mailto:joao.valverde@xxxxxxxxxxxxxxxxxx>>:
>
>
>
>     On 09/15/2015 07:38 PM, Pascal Quantin wrote:
>
>         Hi Jo�o,
>
>         Le 15 sept. 2015 4:41 PM, "Jo�o Valverde"
>         <joao.valverde@xxxxxxxxxxxxxxxxxx
>         <mailto:joao.valverde@xxxxxxxxxxxxxxxxxx>
>         <mailto:joao.valverde@xxxxxxxxxxxxxxxxxx
>         <mailto:joao.valverde@xxxxxxxxxxxxxxxxxx>>> a �crit :
>           >
>           > Hi,
>           >
>           > I'm trying to understand and troubleshoot some "Decode As"
>         issues. To
>         give an example consider the packet:
>           >
>           > IPv6 | IPv6 | UDP
>           >
>           > Wouldn't the second IPv6 layer overwrite the Decode As protocol
>         number for the first layer, giving:
>           >
>           > IPv6 (Decode IP protocol 17 As:) | IPv6 (Decode IP protocol
>         17 As:)
>         | UDP
>           >
>           > Instead of the correct:
>           >
>           > IPv6 (Decode IP protocol 41 As:) | IPv6 (Decode IP protocol
>         17 As:) | UDP
>
>         Did you try it? In that case what result did you get? Can you
>         share a pcap?
>
>           >
>           > Is this a bug, a known limitation of the current code, or
>         just my
>         misunderstanding?
>
>         My understanding of the 'Decode As' functionality is that it
>         will allow
>         you to override the default protocol id / next header value <-> sub
>         dissector mapping with another sub dissector call.
>         It means that in your example (if I understood it correctly) if you
>         change the sub dissector called for next header 17 it should not
>         impact
>         another next header value (value 41 in first IPv6 header).
>         Note that I do not have access to the code right now to verify the
>         behavior, but that's what I would expect (it works like this for
>         Ethertype for example).
>
>         Regards,
>         Pascal.
>
>
>     Hi Pascal,
>
>     Looking at the code to handle this:
>
>     static void ipv6_prompt(packet_info *pinfo, gchar* result)
>     {
>          g_snprintf(result, MAX_DECODE_AS_PROMPT_LEN, "IP protocol %u as",
>              GPOINTER_TO_UINT(p_get_proto_data(pinfo->pool, pinfo,
>     proto_ipv6, IPV6_PROTO_VALUE)));
>     }
>
>     (...)
>          p_add_proto_data(wmem_packet_scope(), pinfo, proto_ipv6,
>     IPV6_PROTO_VALUE, GUINT_TO_POINTER((guint)nxt));
>     (...)
>
>     If you call p_add_proto_data() twice, first for the outer header
>     with nxt == 41 (IPv6 Encapsulation) and after that for the inner
>     header with nxt == 17 (UDP), only the UDP value will survive.
>
>     Both network layers will show a "Decode IP protocol 17 As" (UDP),
>     that seems incorrect. This first (outer) network layer should show
>     "Decode IP protocol 41 As" (IPv6 Encapsulation)...
>
>     It's not just a display issue. Selecting "Decode As" for the outer
>     IPv6 header, decode as TCP for example, I'd expect the inner IPv6
>     header to be decoded as TCP (and throw some errors for a malformed
>     TCP header). That's my understanding of how the feature should work,
>     "decode next layer as...". But instead the UDP (transport) layer is
>     decoded as TCP, ignoring the inner (network) header. So it seems to
>     me that the code doesn't handle a different layering other than:
>
>     <Network>
>     <Transport>
>
>     When you have:
>
>     <Network>
>     <Network>
>     <Transport>
>
>     as in this example, it breaks. The solution doesn't seem trivial...
>     my first impression is that p_add_proto_data() needs to store
>     multiple "nxt" values somehow for 'proto_ipv6', then using some
>     field in *pinfo to return the correct network layer value in
>     ipv6_prompt(), and maybe improving the layer decoding logic to
>     handle the second case correctly?
>
>     Hope that makes sense.
>
>
> Then it sounds like an issue specific to IPv6 (and not an issue with the
> 'Decode As' functionality by itself).

Good point. The problem (as such) might be related to the presence of 
the pseudo-layer "IPv6 Next Header" (or it might not, I don't see why it 
would be at a glance). I need to test and compare the same behaviour 
with IPv4.

> The key argument of the p_(add|get)_proto_data is already used by some
> other dissectors as a way to identify multiple encapsulated messages
> (like in SIP messages).
> So instead of using
>      p_get_proto_data(pinfo->pool, pinfo, proto_ipv6, IPV6_PROTO_VALUE);
> and
>      p_add_proto_data(wmem_packet_scope(), pinfo, proto_ipv6,
> IPV6_PROTO_VALUE, GUINT_TO_POINTER((guint)nxt));
> you could give a try to:
>      p_get_proto_data(pinfo->pool, pinfo, proto_ipv6,
> (IPV6_PROTO_VALUE<<24) | pinfo->curr_layer_num);
> and
>      p_add_proto_data(wmem_packet_scope(), pinfo, proto_ipv6,
> (IPV6_PROTO_VALUE<<24) | pinfo->curr_layer_num,
> GUINT_TO_POINTER((guint)nxt));
>
> And see if it helps (did not verify this, I'm just thinking loudly).

Thanks for the pointer. It helps a lot anyway to know how other protocol 
handle this use case.

> Pascal.
>
>     Thanks,
>
>     Jo�o V.
>
>           >
>           > I don't think it is specific to IPv6, it's just an example.
>         If it is
>         a limitation it seems the Decode As proto_value() interface
>         needs to be
>         extended somehow, feedback very welcome.
>           >
>           > Thanks,
>           >
>           > Jo�o V.
>           >
>         ___________________________________________________________________________
>           > Sent via:    Wireshark-dev mailing list
>         <wireshark-dev@xxxxxxxxxxxxx <mailto:wireshark-dev@xxxxxxxxxxxxx>
>         <mailto:wireshark-dev@xxxxxxxxxxxxx
>         <mailto:wireshark-dev@xxxxxxxxxxxxx>>>
>           > Archives: https://www.wireshark.org/lists/wireshark-dev
>           > Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>           >             mailto:wireshark-dev-request@xxxxxxxxxxxxx
>         <mailto:wireshark-dev-request@xxxxxxxxxxxxx>
>         <mailto:wireshark-dev-request@xxxxxxxxxxxxx
>         <mailto:wireshark-dev-request@xxxxxxxxxxxxx>>?subject=unsubscribe
>
>
>
>         ___________________________________________________________________________
>         Sent via:    Wireshark-dev mailing list
>         <wireshark-dev@xxxxxxxxxxxxx <mailto:wireshark-dev@xxxxxxxxxxxxx>>
>         Archives: https://www.wireshark.org/lists/wireshark-dev
>         Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>                        mailto:wireshark-dev-request@xxxxxxxxxxxxx
>         <mailto:wireshark-dev-request@xxxxxxxxxxxxx>?subject=unsubscribe
>
>     ___________________________________________________________________________
>     Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx
>     <mailto:wireshark-dev@xxxxxxxxxxxxx>>
>     Archives: https://www.wireshark.org/lists/wireshark-dev
>     Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>                  mailto:wireshark-dev-request@xxxxxxxxxxxxx
>     <mailto:wireshark-dev-request@xxxxxxxxxxxxx>?subject=unsubscribe
>
>
>
>
> ___________________________________________________________________________
> Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
> Archives:    https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>               mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe