Skip to content
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

Radiotap module does not correctly decode radiotap format #419

Closed
kristahi opened this issue Sep 20, 2018 · 4 comments
Closed

Radiotap module does not correctly decode radiotap format #419

kristahi opened this issue Sep 20, 2018 · 4 comments

Comments

@kristahi
Copy link

There are multiple issues with the radiotap header decoder module, which make it more or less unusable at the moment for anything but getting at the 802.11 frames trailing the radiotap header.

Major issues:

  • Endianness is reversed
    Radiotap is defined as being little-endian, yet all multibyte fields are unpacked as big-endian. Obviously this breaks all such fields.
    Importantly, this also applies to the bit ordering in the field presence bitmap.
    There is a workaround of sorts already in place for decoding encapsulated data (ntohs is called on the raw value of the length field in Radiotap.unpack).
    See also Possible endianess problem in radiotap header parsing (and others?) #132.
  • Parser only handles a single present fields bitmap
    Radiotap supports the inclusion of an arbitrary number of fields by "chaining" the presence bitmap when the most significant bit is set. This also allows fields of the same type to be repeated (e.g. to specify antenna data on radios that have multiple antennas). It isn't possible to reliably decode a radiotap header without handling this properly.
  • The tests pass because the test data are invalid radiotap headers
    Other radiotap parsers (Wireshark etc.) choke on the raw buffers supplied in the tests, which are big-endian encoded.

I am willing to provide patches to sort this out, but the extent of the breakage presents some challenges:

  • If anyone is actually using the module somehow (besides to just decode 802.11 frames), they are surely working around the problems described above themselves, so by actually returning correct output it is possible existing programs will break.
  • Also, writing a proper parser is not entirely trivial and there is already an existing, ISC-licensed Python parser library: https://github.com/radiotap/python-radiotap/
    I'm unsure if it is worth the effort the duplicate this work. dpkt currently has no dependencies and I suppose this is deliberate?

One possibility is to just remove support for parsing the contents of the radiotap header and instead offer it in raw form so users can pass it into another library such as the one above themselves. This way you can still use dpkt to decode the actual frames.

Any opinions on this?

@brifordwylie
Copy link
Collaborator

@kristahi as you may have noticed we're working with a bit of a skeleton crew here. I'll give you my input FWIW.

Looking at https://github.com/radiotap/python-radiotap there are basically just two files, one approach is to contact the author, let him know we're interested in his work and would like to include them into dpkt and add him as a contributor :) @bcopeland (https://github.com/bcopeland)

Since I don't know anything about radiotap, you'd have to manage pulling in that work (if Bob would like to contribute it).

@brifordwylie
Copy link
Collaborator

Also just making sure to 'link' this within this discussion: #306

@bcopeland
Copy link

Hi guys, thanks for pulling me in. A history on this code: from time to time I work on Linux wireless in various areas: kernel, userspace daemons, etc. I originally wrote radiotap.py for my own uses, and as such ieee80211_parse() is not a complete 802.11 dissector. However, I believe radiotap_parse() is generally correct -- I tested it against the existing test cases for the C libraries. The github radiotap account is run by the people that mostly drive the spec these days, so, to the extent that it doesn't match up with the spec, I'd like to hear about it.

As far as I'm concerned you can take what you like with or without attribution into your project. If it needs a couple of tweaks here or there (like dropping ieee80211_parse) I would consider also removing those from the "official" python-radiotap to keep them more or less in sync. It doesn't get updated very often so I wouldn't worry too much about skew.

@obormot
Copy link
Collaborator

obormot commented Dec 30, 2020

Some (but not all) of the issues addressed in #517 (now merged to master).

@obormot obormot closed this as completed Dec 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants