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

On 19/01/20 21:15, Christian Hopps wrote:
> Hi,
>
> I'm writing a dissector for IPTFS (https://tools.ietf.org/html/draft-ietf-ipsecme-iptfs-00) and inside the dissector I need to get the ESP sequence number. I've looked around some (not exhaustively) and don't see an easy way to get lower layer info from inside my dissector. Right now I'm using this hack:
>
>    tvbuff_t *esptvb = get_esp_tvb(tvb, pinfo);
>    guint32 seq_num = esptvb ? tvb_get_ntohl(esptvb, 4) : ~0u;
>
> where get_esp_tvb is:
>
>    static tvbuff_t *
>    get_esp_tvb(tvbuff_t *tvb, packet_info *pinfo)
>    {
>        /* get_data_source_tvb_by_name(pinfo, "Frame") has bug use when fixed */
>        for (GSList *e = pinfo->data_src; e; e = e->next) {
>            tvbuff_t *esptvb = get_data_source_tvb((struct data_source *)e->data);
>            for (; esptvb->next; esptvb = esptvb->next)
>                /* look for when our original tvb is the next one (decrypted)
>                 * the current one should be ESP prior to decrypt copy. */
>                if (esptvb->next->ds_tvb == tvb->ds_tvb)
>                    return esptvb;
>        }
>        return NULL;
>    }
>
> This is pretty ugly as it counts on the ESP implementation creating a new tvbuff_t chain for the decrypted data (that's why the code is checking for the ds_tvb to change).
>
> Is there a better way to get at the lower layer protocol data? Is the "proper" way to do this to just create an esp_seq_num field in the protocol info structure? This would sort of mimic the already existing copying of addresses and ports from the lower layers that upper layers use to look-back like I'm trying to do. Seems like there should be a more generic way to do this though (e.g., that allows future plugin functionality w/o modifications to the protocol info structure).


Interesting work and good question. There is currently no generic 
non-hacky way for upper layer dissectors to fetch lower layer dissector 
data, unfortunately. The logic today is lower dissectors push up the 
data (instead of upper dissectors pulling it). There are a few problems 
with this approach IMO, which I won't get into here. To use the packet 
info structure I think you'll have to provide strong justification why 
that is the best solution or can't be done another way (on the grounds 
that the struct shouldn't grow in size indefinitely).

My suggestion is to patch the ESP dissector to use 
call_dissector_with_data() and pass an ancillary data structure with the 
relevant ESP fields. But I'm interested to hear what others think.


> Thanks,
> Chris.
>
> P.S. get_data_source_tvb_by_name is broken otherwise i'd do the above as:
>
>    static tvbuff_t *
>    get_esp_tvb(tvbuff_t *tvb, packet_info *pinfo)
>    {
>        tvbuff_t *esptvb = get_data_source_tvb_by_name(pinfo, "Frame");
>        for (; esptvb->next; esptvb = esptvb->next)
>            /* look for when our original tvb is the next one (decrypted)
>             * the current one should be ESP prior to decrypt copy. */
>            if (esptvb->next->ds_tvb == tvb->ds_tvb)
>                return esptvb;
>        return NULL;
>    }
>
> I can submit a patch for fixing get_data_source_tvb_by_name too.
>
> Thanks,
> Chris.
> ___________________________________________________________________________
> Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
> Archives:    https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
>               mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe