desowin / usbpcap

USB packet capture for Windows
http://desowin.org/usbpcap
902 stars 170 forks source link

Wireshark integration: support new extcap option "--extcap-version=x.x" #51

Closed theXappy closed 6 years ago

theXappy commented 6 years ago

The latest build version of Wireshark (2.9.0) contains a change to the extcap API which breaks compatabiliy with usbpcap.

Until now, USBPcapCMD.exe was queried for it's extcap interfaces using: "USBPcapCMD.exe --extcap-interfaces" But from version 2.9 it will change to: "USBPcapCMD.exe --extcap-interfaces --extcap-version=2.9"

Currently running USBPcapCmd.exe with these args returns this error: "USBPcapCMD.exe: --extcap-version: unknown option" This leads to wireshark rejecting the executable and not allowing capturing using it's GUI.

desowin commented 6 years ago

Before there is a new release, one might want to use the latest AppVeyor build of USBPcapCMD (x64 only; Windows 10 warns about lack of signature - but works).

https://ci.appveyor.com/project/desowin/usbpcap

theXappy commented 6 years ago

For what it's worth, this also works on my (Win10 x64) machine with the master branch of Wireshark 2.9

desowin commented 6 years ago

Conversation moved from Pull Request

@guyharris wrote:

Note that 1) the version argument to --extcap-version is optional, so if there's an argument to the --extcap-version option, it shouldn't print the extcap version (it should presumably check whether it supports that version and fail if it doesn't), and if there isn't an argument to that option, it should continue to report the version.

Note also that the only way to pass an optional argument to an option that is supported by all versions of getopt_long() is --extcap-version=X.Y, not --extcap-version X.Y, so Wireshark will pass it as --extcap-version=X.Y; if you're not using getopt_long() or something compatible with it as an option parser, you must also support --extcap-version=X.Y.

To which I have replied:

I will change the argument parser to support the optional argument.

However, I am somehow lost in understanding --extcap-version. Could you please answer following questions to clarify:

When Wireshark calls --extcap-version=X.Y then the X.Y is the Wireshark version, eg. 2.9, right? Is this "should presumably check whether it supports that version and fail if it doesn't" supposed to check if the wireshark version is too old? For example, assume that some extcap makes it mandatory to use some extcap feature that is supported in Wireshark since some version A.B. Then the extcap will check the passed X.Y against the A.B and if it is lower, it'll fail. Is this the intention? If wireshark calls --extcap-version without any argument, the extcap should print its version (eg. 1.2.0.4 for the upcoming USBPcap), right? Regardless if --extcap-version is called with or without parameter, if extcap does not fail, it should print the tool version (the 1.2.0.4 for upcoming USBPcap)?

And if extcap is called without --extcap-version at all, but with the --extcap-interfaces (old version of Wireshark), is it safe to print the "extcap {version=a.b.c.d}{help=someurl}" alongside the interfaces?

pquantin commented 6 years ago

My understanding is that --extcap-version parameter is used to indicate the Wireshark version launching the extcap tool (in case the extcap tool has a dynamic behavior depending on the Wireshark version used because of new or changed features for example). To provide its own version, the extcap tool should return the version=x parameter when called with --extcap-interfaces. As far as I know --extcap-interfaces should not be called without any argument by Wireshark (that should be an error I guess). See https://www.wireshark.org/docs/wsdg_html_chunked/ChCaptureExtcap.html for details. The version parameter (to provide the extcap tool version) was introduced before --extcap-version so it should be safe to send it when invoked without --extcap-version.

guyharris commented 6 years ago

Prior to Wireshark 2.9/3.0, the --extcap-version option took no argument, and indicated that the extcap program should report its version, e.g.

$ build/run/Wireshark.app/Contents/MacOS/extcap/randpktdump --extcap-version
extcap {version=0.1.0}{help=file:///usr/local/share/wireshark/randpktdump.html}
$

Starting in 2.9/3.0, it can optionally take an argument. If no argument is specified, it still means "report the version of the extcap program"; if an argument is specified, it means "this is the version of the Wireshark/TShark/etc. that's running the extcap program".

I think it was perhaps unwise to repurpose an existing option for a separate purpose, so I may just change it so that --extcap-wireshark-version is an option with a required argument (so that it can either be specified as --extcap-wireshark-version X.Y or --extcap-wireshark-version=X.Y that indicates what version of Wireshark is running the extcap program and --extcap-version never takes an argument and always asks the extcap program to report its version.

desowin commented 6 years ago

@guyharris Could you please review the https://github.com/desowin/usbpcap/pull/54 ?

I have implemented the --extcap-version with optional argument, alongside some refactoring.

What I have noticed is that the multicheck doesn't seem to work as I would expect. Is the multicheck implemented in Qt GUI? I remember this multicheck was working in GTK+ interface, but that was long time ago.

Is it possible to inform Wireshark that the multicheck argument should be present on commandline only if there is argument to be passed alongside it?

desowin commented 6 years ago

The multicheck doesn't work properly in Qt interface. I have checked this in Wireshark 2.0.16 - the GTK+ interface works as expected, however the Qt doesn't even allow selecting the devices. The multicheck is to be fixed in Wireshark Qt interface itself.

desowin commented 6 years ago

In Wireshark 2.9.0-1285, an empty --devices parameter immadietely before --capture-from-all-devices can be triggered by doing:

  1. Open Wireshark, select USBPcap interface, open options
  2. Expand any parent USB device, click on child device name
  3. Start capture
  4. Click Stop capture button in the toolbar
  5. Click Start capture button in toolbar, continue without saving data
  6. Watch that no packets are being captured
  7. In point 5 USBPcapCMD is executed with following parameters: USBPcapCMD.exe -d .\USBPcap3 -b 1048576 -o .\pipe\wiresharkextcap.\USBPcap3_20180728132117 --devices --capture-from-all-devices --capture-from-new-devices

This makes getopt to consider --capture-from-all-devices as a parameter to --devices and thus USBPcapCMD fails with the: "Malformed address list. Invalid character: -."

I will take a look into fixing the multicheck in Wireshark Qt interface.

desowin commented 6 years ago

Well, the Wireshark Qt interface doesn't seem to be broken. In fact it does the "select by highlighting" which is confusing to me (press CTRL and click to select multiple ones). I would prefer if it displayed the tickboxes like the GTK+ interface did.

What I have noticed is that it was me who did the change that made USBPcapCMD working with GTK+ interface in a way I expected. I didn't realize however, that the implementation didn't fully follow my comment - and I screwed up hard by making all devices in USBPcapCMD --extcap-config to be listed with {enabled=false} (which means they should not be clickable). There's no point in really figuring out what exactly is broken in that change (as GTK+ interface is long gone) and/or trying to fix it.

The problem I listed in previous comment is really related to a corner case where extcap does give multicheck, but then all the options are not selectable.

EDIT: The code in USBPcapCMD is not really clean and I got confused a lot. In fact there are the correct entries with {enabled=true} so it is mostly a UI usability issue - Displaying empty checkbox next to option that is selectable would in my opinion make it significantly easier to realize which options are possible to be selected.

desowin commented 6 years ago

Kudos to whoever wrote the multicheck in Wireshark Qt interface. It really does work correctly. The only issue I see is related to the fact that it's select by highlighting, which is especially confusing in the USBPcap context where you usually have much more non-selectable items (logical driver objects) than the selectable ones (representing actual USB interface).

desowin commented 6 years ago

USBPcap 1.2.0.4 is now officially released.