-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Adds Wireshark pipes support, cleanup OSX tcpdump handling. #1959
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
9089554 to
a0c4d29
Compare
|
Really like this PR ! If you could rebase, it will fix the AppVeyor failure |
Codecov Report
@@ Coverage Diff @@
## master #1959 +/- ##
==========================================
+ Coverage 85.92% 85.96% +0.04%
==========================================
Files 187 187
Lines 42937 42976 +39
==========================================
+ Hits 36892 36946 +54
+ Misses 6045 6030 -15
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great PR, all good to me apart:
- https://ci.appveyor.com/project/secdev/scapy/builds/23671224/job/cpf5sdranaeooj9k seem to be a random failure of the
WiresharkSinktest. Probably some racing problems :/ it should be stabilized - (optional): add a tiny reference in the pipetool documentation (advanced usage) to the new Pipes
|
Found another issue, tcpdump, wireshark and tshark doesn't like integer linktypes... :( |
|
In case it isn't clear already, please don't merge this yet... Yesterday, I was trying to debug that Windows test flake. I ran the test about 1300 times (automated, of course) in Python 3.7 on Linux, and couldn't trigger a single failure. Interestingly only the |
6eea710 to
348bf26
Compare
|
I'm going to amend the docs a little more, squash and rebase this before it's ready. |
* SniffSource now only opens a socket on calling `start`.
* WrpcapSink only opens a PcapWriter on calling `start`.
* Adds WiresharkSink for streaming packets to Wireshark through a pipe.
* Adds `fd` parameter to `utils.get_temp_file`, which allows a temporary
file to be used without closing and re-opening it.
* Adds `utils.get_temp_dir` (used for tests)
* Adds `use_tempfile` parameter to `utils.tcpdump`, which causes it to
use a temporary file to store packets.
* `utils.tcpdump` now only uses a temporary file by default for calling
tcpdump on OSX (to work around Apple's broken version of tcpdump).
stdin is now used with other tools which are not impacted by this bug
(eg: tshark).
* Adds `read_stdin_opts` parameter to `utils.tcpdump`, which allows
callers to control the options used for reading packets over stdin
(previously hard-coded to `-r -`).
* `utils.wireshark` now uses a pipe rather than a temporary file.
Wireshark itself has options to save this file to disk if desired.
Also cleans up the tests a little:
* Removes `os.remove("test.png")` (which seems to be unused).
* Uses a temporary directory for some pipetool tests.
Includes fixup:
* spelling
* Enable `manpages_url`, so we can use manpage links.
* Pipetools: Revise Sinks section to use `py:class` style documentation.
This makes cross-references to specific sinks much easier.
* Add documentation for `WiresharkSink`.
* Pipetools: Revise Link object section with some word-smithing.
* Usage/wireshark:
* improve wordsmithing
* remove hard coded references to `google.com` (which trigger DNS
lookups), and use an RFC 5737 netmask instead.
* set a source IP address (otherwise, this gets the host's IP address).
* remove `Ether` layer, as Wireshark supports `DLT_RAW` (which triggers
getting the host's MAC address).
* elaborate on mixed `linktype` issues.
* `QueueSink.recv`: add `block` and `timeout` parameters * `WrpcapSink`, `WiresharkSink`: add `linktype` parameter * `WiresharkSink`: add `args` parameter * tests: sleep longer to work-around race conditions on Windows
* `{Raw,}PcapWriter._write_packet`:
* Remove unused support for `packet` as tuple, as `write` will always
unroll iterators for us (and do it better).
* Always set the `usec` parameter if `sec` was unset.
* Set `usec=0` if `sec` was set, but `usec` was unset (instead of using
the current time's usec value
* PEP-8 fixup, add docstring.
* Only write the header if there is a packet
* `PcapWriter._write_packet`: support packet as bytes
* `RawPcapWriter.close`: write the file header here if not already written
* `tcpdump()`:
* Add `linktype` parameter, like `wrpcap(linktype=...)`
* Add `wait` parameter, which controls whether a program should be run in
the background. Defaults to `True` (run in foreground).
* Throw an error if `prog` is not a string.
* Copy `read_stdin_opts` (for thread safety).
* `tdecode()`: Add `args` parameter (defaults to `-V`, as before), pass
other `tcpdump()` kwargs.
* `wireshark()`: Run in the background by default, pass other `tcpdump()`
kwargs.
* Add tests that hit `wireshark`, `tdecode`, `tcpdump` with new parameters,
and try to pass packets as bytes.
847e884 to
33c471c
Compare
|
@gpotter2: I squashed fixups into commits that make a bit more sense, and fixed the issues you mentioned. I've updated the original change description with the extra changes since approval, please take a look! |
|
the About linktypes, we could hack our way through with something similar to >>> def hacky_linktype_name(value):
...: return next(k for k, v in six.iteritems(scapy.data.__dict__) if k[:3] == "DLT" and v == value)
>>>
>>> hacky_linktype_name(0) 'DLT_NULL'as a fallback, though that's a bit messy, it could be useful... what do you think ? Otherwise, the doc improvement is great and PR looks good overall |
New:
WiresharkSink, a PipeTools Sink for streaming packets to Wiresharkutils.get_temp_dir: creates a temporary directoryExtensions:
tcpdump(): Addsuse_tempfile, to explicitly control use of a tempfile for packets.tcpdump(): Addsread_stdin_optsparameter, which allows callers to control the options used for reading packets overstdin(previously hard-coded to-r -). This auto-detectswiresharkto use-ki -instead.tcpdump(),WrpcapSink: Addslinktypeparameter.get_temp_file(): Addsfdparameter, which allows a temporary file to be used without closing and re-opening it.wireshark(),SourceandPipedocumentation.tdecode(): Addargsparameter (defaults to-V, as before), pass kwargs totcpdump().QueueSink.recv: addblockandtimeoutparameters.Fixes:
tcpdump()now only uses a temporary file by default for calling tcpdump on OSX (to work around Apple's broken version oftcpdump).stdinis now used with other tools which are not impacted by this bug (eg:tshark).wireshark()now uses a pipe rather than a temporary file. Wireshark itself has options to save this file to disk if desired.SniffSourcenow only opens a socket on callingstart.WrpcapSinkonly opens aPcapWriteron callingstart.RawPcapWriterno longer writes a header onwritecalls with no packets specified.RawPcapWriterwrites a header oncloseif no header has been written.QueueSink.recv()no longer busy-loops.PcapWriter,wrpcap(): support packets as bytesDocumentation fixes:
wireshark()andPipespy:function/py:classfor Sinks andwireshark()to allow easy cross-referencing, and expand this documentation.wireshark(): Remove references togoogle.com, and use RFC 5737 IP addresses insteadwireshark(): Set a source IP address (to avoid looking up host's IP address).wireshark(): RemoveEtherlayer, as Wireshark works fine withDLT_RAWforIPpackets, which removes the need to lookup MAC addressesmanpages_urlto configuration, to support:manpage:directives. This is pointing at Debian's server, which should have a good spread of versions of tools, and Debian generally adds manpages for tools that don't have them.Test fixes:
os.remove("test.png")(which seems to be unused).Other changes:
{Raw,}PcapWriter._write_packet: Remove unused support forpacketas tuple, aswritewill always unroll iterators for us (and do it better).{Raw,}PcapWriter._write_packet: Always set theusecparameter ifsecwas unset.{Raw,}PcapWriter._write_packet: Setusec=0ifsecwas set, butusecwas unset (instead of using the current time's usec value