[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wireshark-dev
Subject:    [Wireshark-dev] Using plugin support to handle pcap-ng block types
From:       Paul Offord <Paul.Offord () advance7 ! com>
Date:       2018-01-29 22:40:17
Message-ID: CWLP265MB0099C1F87FE81C4D152F3B25C8E50 () CWLP265MB0099 ! GBRP265 ! PROD ! OUTLOOK ! COM
[Download RAW message or body]

Hi,

I am writing a plugin that handles two new PCAP-NG block types - details here: \
https://wiki.wireshark.org/Adding%20Support%20for%20a%20New%20Block%20Type

I'm using Guy's code that adds support for plugins to handle pcap-ng block types, \
described in https://code.wireshark.org/review/#/c/1775/

I have written a read block handler and registered it with Wireshark using \
register_pcapng_block_type_handler() and the handler gets called correctly.  Once the \
block has been read by my handler, I want Wireshark to continue to process it, but \
currently the block is ignored and Wireshark just moves on to the next block.

The blocks are read via a while loop in the pcapng_read(...) function of pcapng.c.  \
After calling a function to read the block there is a "switch (wblock.type)" section \
of code that decides what next to do with the block.  My new block is not defined as \
a switch case (not surprising), but I believe the default case should take into \
account the fact that a block type may have been registered.  The code in this area \
currently looks like this:

        switch (wblock.type) {

            case(BLOCK_TYPE_SHB):
                pcapng_debug("pcapng_read: another section header block");
                g_array_append_val(wth->shb_hdrs, wblock.block);
                break;

            case(BLOCK_TYPE_PB):
            case(BLOCK_TYPE_SPB):
            case(BLOCK_TYPE_EPB):
            case(BLOCK_TYPE_SYSDIG_EVENT):
            case(BLOCK_TYPE_SYSDIG_EVF):
                /* packet block - we've found a packet */
                goto got_packet;

            case(BLOCK_TYPE_IDB):
                /* A new interface */
                pcapng_debug("pcapng_read: block type BLOCK_TYPE_IDB");
                pcapng_process_idb(wth, pcapng, &wblock);
                wtap_block_free(wblock.block);
                break;

            case(BLOCK_TYPE_NRB):
                /* More name resolution entries */
                pcapng_debug("pcapng_read: block type BLOCK_TYPE_NRB");
                if (wth->nrb_hdrs == NULL) {
                    wth->nrb_hdrs = g_array_new(FALSE, FALSE, sizeof(wtap_block_t));
                }
                g_array_append_val(wth->nrb_hdrs, wblock.block);
                break;

            case(BLOCK_TYPE_ISB):
                /* Another interface statistics report */
                pcapng_debug("pcapng_read: block type BLOCK_TYPE_ISB");
                if_stats_mand_block = \
(wtapng_if_stats_mandatory_t*)wtap_block_get_mandatory_data(wblock.block);

                ... lines deleted ...

                }
                wtap_block_free(wblock.block);
                break;

            default:
                /* XXX - improve handling of "unknown" blocks */
                pcapng_debug("pcapng_read: Unknown block type 0x%08x", wblock.type);
                break;
        }
    }

got_packet:

    /*pcapng_debug("Read length: %u Packet length: %u", bytes_read, \
wth->phdr.caplen);*/  pcapng_debug("pcapng_read: data_offset is finally %" \
G_GINT64_MODIFIER "d", *data_offset);

    return TRUE;


I believe that the default condition should check for a plugin registered block type \
and then goto got_packet.  Also, as the code stands we never free wblock.block.

Is this a bug, or is my understanding of what's intended wrong?

Thanks and regards...Paul

______________________________________________________________________

This message contains confidential information and is intended only for the \
individual named. If you are not the named addressee you should not disseminate, \
distribute or copy this e-mail. Please notify the sender immediately by e-mail if you \
have received this e-mail by mistake and delete this e-mail from your system.

Any views or opinions expressed are solely those of the author and do not necessarily \
represent those of Advance Seven Ltd. E-mail transmission cannot be guaranteed to be \
secure or error-free as information could be intercepted, corrupted, lost, destroyed, \
arrive late or incomplete, or contain viruses. The sender therefore does not accept \
liability for any errors or omissions in the contents of this message, which arise as \
a result of e-mail transmission.

Advance Seven Ltd. Registered in England & Wales numbered 2373877 at Endeavour House, \
Coopers End Lane, Stansted, Essex CM24 1SJ

______________________________________________________________________
This email has been scanned by the Symantec Email Security.cloud service.
For more information please visit http://www.symanteccloud.com
______________________________________________________________________


[Attachment #3 (text/html)]

<html xmlns:v="urn:schemas-microsoft-com:vml" \
xmlns:o="urn:schemas-microsoft-com:office:office" \
xmlns:w="urn:schemas-microsoft-com:office:word" \
xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" \
xmlns="http://www.w3.org/TR/REC-html40"> <head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
	{font-family:"Cambria Math";
	panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
	{font-family:Calibri;
	panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
	{margin:0cm;
	margin-bottom:.0001pt;
	font-size:11.0pt;
	font-family:"Calibri",sans-serif;
	mso-fareast-language:EN-US;}
a:link, span.MsoHyperlink
	{mso-style-priority:99;
	color:#0563C1;
	text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
	{mso-style-priority:99;
	color:#954F72;
	text-decoration:underline;}
span.EmailStyle17
	{mso-style-type:personal-compose;
	font-family:"Calibri",sans-serif;
	color:windowtext;}
.MsoChpDefault
	{mso-style-type:export-only;
	mso-fareast-language:EN-US;}
@page WordSection1
	{size:612.0pt 792.0pt;
	margin:72.0pt 72.0pt 72.0pt 72.0pt;}
div.WordSection1
	{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-GB" link="#0563C1" vlink="#954F72">
<div class="WordSection1">
<p class="MsoNormal">Hi,<o:p></o:p></p>
<p class="MsoNormal"><o:p>&nbsp;</o:p></p>
<p class="MsoNormal">I am writing a plugin that handles two new PCAP-NG block types \
&#8211; details here: <a \
href="https://wiki.wireshark.org/Adding%20Support%20for%20a%20New%20Block%20Type"> \
https://wiki.wireshark.org/Adding%20Support%20for%20a%20New%20Block%20Type</a><o:p></o:p></p>
 <p class="MsoNormal"><o:p>&nbsp;</o:p></p>
<p class="MsoNormal">I&#8217;m using Guy&#8217;s code that adds support for plugins \
to handle pcap-ng block types, described in <a \
href="https://code.wireshark.org/review/#/c/1775/">https://code.wireshark.org/review/#/c/1775/</a><o:p></o:p></p>
 <p class="MsoNormal"><o:p>&nbsp;</o:p></p>
<p class="MsoNormal">I have written a read block handler and registered it with \
Wireshark using register_pcapng_block_type_handler() and the handler gets called \
correctly.&nbsp; Once the block has been read by my handler, I want Wireshark to \
continue to process  it, but currently the block is ignored and Wireshark just moves \
on to the next block.<o:p></o:p></p> <p class="MsoNormal"><o:p>&nbsp;</o:p></p>
<p class="MsoNormal">The blocks are read via a while loop in the pcapng_read(&#8230;) \
function of pcapng.c.&nbsp; After calling a function to read the block there is a \
&#8220;switch (wblock.type)&#8221; section of code that decides what next to do with \
the block.&nbsp; My new block is  not defined as a switch case (not surprising), but \
I believe the default case should take into account the fact that a block type may \
have been registered.&nbsp; The code in this area currently looks like \
this:<o:p></o:p></p> <p class="MsoNormal"><o:p>&nbsp;</o:p></p>
<p class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; switch (wblock.type) \
{<o:p></o:p></p> <p class="MsoNormal"><o:p>&nbsp;</o:p></p>
<p class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
case(BLOCK_TYPE_SHB):<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
pcapng_debug(&quot;pcapng_read: another section header block&quot;);<o:p></o:p></p> \
<p class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
g_array_append_val(wth-&gt;shb_hdrs, wblock.block);<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
break;<o:p></o:p></p> <p class="MsoNormal"><o:p>&nbsp;</o:p></p>
<p class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
case(BLOCK_TYPE_PB):<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
case(BLOCK_TYPE_SPB):<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
case(BLOCK_TYPE_EPB):<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
case(BLOCK_TYPE_SYSDIG_EVENT):<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
case(BLOCK_TYPE_SYSDIG_EVF):<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
/* packet block - we've found a packet */<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
goto got_packet;<o:p></o:p></p> <p class="MsoNormal"><o:p>&nbsp;</o:p></p>
<p class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
case(BLOCK_TYPE_IDB):<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
/* A new interface */<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
pcapng_debug(&quot;pcapng_read: block type BLOCK_TYPE_IDB&quot;);<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
pcapng_process_idb(wth, pcapng, &amp;wblock);<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
wtap_block_free(wblock.block);<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
break;<o:p></o:p></p> <p class="MsoNormal"><o:p>&nbsp;</o:p></p>
<p class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
case(BLOCK_TYPE_NRB):<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
/* More name resolution entries */<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
pcapng_debug(&quot;pcapng_read: block type BLOCK_TYPE_NRB&quot;);<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
if (wth-&gt;nrb_hdrs == NULL) {<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
wth-&gt;nrb_hdrs = g_array_new(FALSE, FALSE, sizeof(wtap_block_t));<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
}<o:p></o:p></p> <p class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
g_array_append_val(wth-&gt;nrb_hdrs, wblock.block);<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
break;<o:p></o:p></p> <p class="MsoNormal"><o:p>&nbsp;</o:p></p>
<p class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
case(BLOCK_TYPE_ISB):<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
&nbsp;&nbsp;&nbsp;/* Another interface statistics report */<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
pcapng_debug(&quot;pcapng_read: block type BLOCK_TYPE_ISB&quot;);<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
if_stats_mand_block = \
(wtapng_if_stats_mandatory_t*)wtap_block_get_mandatory_data(wblock.block);<o:p></o:p></p>
 <p class="MsoNormal"><o:p>&nbsp;</o:p></p>
<p class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
&#8230; lines deleted &#8230;<o:p></o:p></p> <p \
class="MsoNormal"><o:p>&nbsp;</o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
}<o:p></o:p></p> <p class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
wtap_block_free(wblock.block);<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
break;<o:p></o:p></p> <p class="MsoNormal"><o:p>&nbsp;</o:p></p>
<p class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
default:<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
&nbsp;&nbsp;/* XXX - improve handling of &quot;unknown&quot; blocks */<o:p></o:p></p> \
<p class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
pcapng_debug(&quot;pcapng_read: Unknown block type 0x%08x&quot;, \
wblock.type);<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
break;<o:p></o:p></p> <p class="MsoNormal">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; \
}<o:p></o:p></p> <p class="MsoNormal">&nbsp;&nbsp;&nbsp; }<o:p></o:p></p>
<p class="MsoNormal"><o:p>&nbsp;</o:p></p>
<p class="MsoNormal">got_packet:<o:p></o:p></p>
<p class="MsoNormal"><o:p>&nbsp;</o:p></p>
<p class="MsoNormal">&nbsp;&nbsp;&nbsp; /*pcapng_debug(&quot;Read length: %u Packet \
length: %u&quot;, bytes_read, wth-&gt;phdr.caplen);*/<o:p></o:p></p> <p \
class="MsoNormal">&nbsp;&nbsp;&nbsp; pcapng_debug(&quot;pcapng_read: data_offset is \
finally %&quot; G_GINT64_MODIFIER &quot;d&quot;, *data_offset);<o:p></o:p></p> <p \
class="MsoNormal"><o:p>&nbsp;</o:p></p> <p class="MsoNormal">&nbsp;&nbsp;&nbsp; \
return TRUE;<o:p></o:p></p> <p class="MsoNormal"><o:p>&nbsp;</o:p></p>
<p class="MsoNormal"><o:p>&nbsp;</o:p></p>
<p class="MsoNormal">I believe that the default condition should check for a plugin \
registered block type and then goto got_packet.&nbsp; Also, as the code stands we \
never free wblock.block.<o:p></o:p></p> <p class="MsoNormal"><o:p>&nbsp;</o:p></p>
<p class="MsoNormal">Is this a bug, or is my understanding of what&#8217;s intended \
wrong?<o:p></o:p></p> <p class="MsoNormal"><o:p>&nbsp;</o:p></p>
<p class="MsoNormal">Thanks and regards&#8230;Paul<o:p></o:p></p>
</div>
<br clear="both">
______________________________________________________________________<BR>
<BR>
This message contains confidential information and is intended only for the \
individual named. If you are not the named addressee you should not disseminate, \
distribute or copy this e-mail. Please notify the sender immediately by e-mail if you \
have received this e-mail by mistake and delete this e-mail from your system.<BR> \
<BR> Any views or opinions expressed are solely those of the author and do not \
necessarily represent those of Advance Seven Ltd. E-mail transmission cannot be \
guaranteed to be secure or error-free as information could be intercepted, corrupted, \
lost, destroyed, arrive late or incomplete, or contain viruses. The sender therefore \
does not accept liability for any errors or omissions in the contents of this \
message, which arise as a result of e-mail transmission.<BR> <BR>
Advance Seven Ltd. Registered in England & Wales numbered 2373877 at Endeavour House, \
Coopers End Lane, Stansted, Essex CM24 1SJ<BR> <BR>
______________________________________________________________________<BR>
This email has been scanned by the Symantec Email Security.cloud service.<BR>
For more information please visit http://www.symanteccloud.com<BR>
______________________________________________________________________<BR>
</body>
</html>


[Attachment #4 (unknown)]

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev@wireshark.org>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request@wireshark.org?subject=unsubscribe

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic