Wireshark-dev: Re: [Wireshark-dev] pcapng - packet comment terminator; packet list equiv for de
From: chuck c <bubbasnmp@xxxxxxxxx>
Date: Fri, 25 Sep 2020 10:01:54 -0500
"That's a file detail that's overkill for reading capture files."
Agreed. I was looking more to diagnose the pcapng issue that Wireshark flags as trailing characters.
There doesn't seem to be a common code base for writing pcapng so as more applications support the file format there might be other issues to investigate in the future.

Thanks for the insight.
I'm still confused about the difference between "Fileshark" and TRB - https://gitlab.com/wireshark/wireshark/-/wikis/TRB-Protocol
Maybe that's a future lesson. :-)

thanks
chuck


On Fri, Sep 25, 2020 at 12:06 AM Guy Harris <gharris@xxxxxxxxx> wrote:
On Sep 24, 2020, at 5:41 PM, chuck c <bubbasnmp@xxxxxxxxx> wrote:

> vmware has a packet capture utility (pktcap-uw) which adds packet comments when writing a capture as pcapng.
> Looks like the code that writes the comments is reusing a buffer so that when a smaller comment is written there are leftover characters from the previous comment.
> The code is also adding a null terminator to the comment string.
> When the capture is reloaded in View->Reload as File Format/Capture, the comments are flagged with "Trailing stray characters".
>
> Question #1.
> The pcapng draft standard states:
> "If an option's value is a string, the value is not necessarily zero-terminated. Software that reads these files MUST NOT assume that strings are zero-terminated, and MUST treat a zero-value octet as a string terminator."
>
> but also:
> "opt_comment:
>
> The opt_comment option is a UTF-8 string containing human-readable comment text that is associated to the current block. Line separators SHOULD be a carriage-return + linefeed ('\r\n') or just linefeed ('\n'); either form may appear and be considered a line separator. The string is not zero-terminated."

"The string is not zero-terminated" is a restatement of "If an option's value is a string, the value is not necessarily zero-terminated."  It should probably be removed, with the general statement about *all* string options covering comments.

> Is Wireshark handling comments with embedded nulls properly?

Prior to the master branch,

        If an option's value is a string, the value is not necessarily zero-terminated. Software that reads these files MUST NOT assume that strings are zero-terminated, and MUST treat a zero-value octet as a string terminator.

translates to FT_STRINGZPAD.  However, "frame.comment" is of type FT_STRING, so that's incorrect.

In the master branch, we have FT_STRINGZPAD and FT_STRINGZTRUNC; the former means "padded, if necessary, entirely with null characters" and the latter means "truncated, if necessary, with a null character, with no guarantee that anything that *follows* the null character is also null", so FT_STRINGZTRUNC would be appropriate there.

> Question #2.
> Viewing the pcapng internals in the Wireshark gui is great

Presumably you mean "opening a pcapng file using View > Reload as File Format/Capture".

> but ....
>
> I can view frame.comment in the "Capture" view but not pcapng.options.option.length

That's a file detail that's overkill for reading capture files.

> In the "File Format" view I can add option attributes as a column but get the values for all the blocks in a single entry.

There is an entry called "Options", which includes all the options, each one as a separate entry.

> Has it ever been discussed to turn the Packet List pane into a Block List pane?

That's a third issue; comments wouldn't show up separately from the record in which they appear.

Any record (a capture-file-type-neutral term I've been using, and that's used in libwiretap) that has a time stamp should probably appear in the summary pane, as it's probably some sort of event.

For other records, it's less clear.

> Is #1 worthy of opening a bug/issue?

Yes.

> (or alternative, try to open bug with vmware - ugh)

They're not violating the current pcapng spec; we *could* change it to require null padding, but 1) Wireshark can handle null-truncated-but-not-null-padded strings and 2) most other software should be able to handle that with little difficulty, so I wouldn't be inclined to make that change.

> Is #2 worthy of an enhancement request?

As per the above, I don't see any need to display the details of comments when treating a pcapng file as a capture.  Note that comments won't necessarily be pcapng-specific - other capture file formats may have them, and they might be in a format that doesn't have anything directly corresponding to the option type or length.  libwiretap is intended to abstract away as many file-type-specific details as possible.

When treating a pcapng file as "Fileshark" input ("View > Reload as File Format/Capture" can be thought of as a first step towards Fileshark), the only way not to have "all the blocks in a single entry" would be to remove the top-level "Options" entry and put the individual option items directly under the "Block Data" item.
___________________________________________________________________________
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