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

Capture Information Block (CIB) addition#51

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

ryankurte
Copy link

@ryankurte ryankurte commented Jun 14, 2018

Adds a Capture Information Block (CIB) with global location/orientation/velocity vectors. CIBs are associated with an IDB using the interface number. It is intended that this will be extended with other useful fields in the future.

This began in #48, which has now been split into two components.

  • The CIB (this PR) for appending information to a capture interface (that applies to all packets captured after the CIB, until the next CIB).
  • New EPB options (Additional RF information options for EPBs #56), for appending RSSI etc. to EPBs.

The proposal with changes is viewable here (thanks @guyharris!).

Please read the RFC or check out the diff for an up to date view of the changes

Adds CIB with location/orientation/velocity vectors, this is optionally attached to an IDB or referenced from EPBs.
@ryankurte
Copy link
Author

ryankurte commented Jun 14, 2018

See [here](http://xml2rfc.tools.ietf.org/cgi-bin/xml2rfc.cgi?url=https://raw.githubusercontent.com/ryankurte/pcapng/proposed-wireless-fields/draft-tuexen-opsawg-pcapng.xml&modeAsFormat=html/ascii#rfc.section.4.7) for the RFC with proposed changes.

@guyharris
Copy link
Collaborator

guyharris commented Jun 15, 2018

I was hoping this would be viewable here, but there seems to be something wrong with my formatting at the moment :-(

It's not the formatting, it's the URL.

Neither the URL https://github.com/ryankurte/pcapng/blob/proposed-wireless-fields/draft-tuexen-opsawg-pcapng.xml&modeAsFormat=html/ascii nor the URL https://github.com/ryankurte/pcapng/blob/proposed-wireless-fields/draft-tuexen-opsawg-pcapng.xml, when handed to curl, download an XML document in the form of the pcapng spec with your changes. The first of those produces a 404; the second of those produces a display of the modified pcapng spec, with line numbers and a whole bunch of GitHub decoration around it.

If I go to the proposed-wireless-fields branch of your pcapng repository, click on draft-tuexen-opsawg-pcapng.xml, and then click the "Raw" button, I get the XML without the GitHub decoration, with the URL https://raw.githubusercontent.com/ryankurte/pcapng/proposed-wireless-fields/draft-tuexen-opsawg-pcapng.xml.

So this url gives you the draft, with your changes, formatted as HTML.

@guyharris
Copy link
Collaborator

I've added some copy editor's notes as comments.

@ryankurte
Copy link
Author

Awesome, thanks! Plan to look at it again tomorrow and fix them up.


<t hangText="cib_velocity"><vspace blankLines="0"/>The
cib_velocity option specifies the location of the packet
capture or interface; location is stored as three 32-bit floats
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"velocity is stored as three 32-bit floats representing the rates of change of latitude and longitude in degrees per second, and the rate of change of altitude in metres/second."

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes more sense for velocity to be in meters per second against a global reference frame (orientation zero, Y facing north, -Z down). It's easier to compare and use in calculations, and works for non-global locations (see PR update introducing local locations too).

@guyharris
Copy link
Collaborator

More copy-and-pasteos indicated in comments.

@jkcko
Copy link

jkcko commented Dec 31, 2018

Will the CIB options also be permitted in the EPB? i.e. Options appearing in an EPB override the values in a CIB? For a wireless capture device which is in motion it may be better/simpler to attach location information to every EPB instead of preceding every EPB with a new CIB.

@guyharris
Copy link
Collaborator

Will the CIB options also be permitted in the EPB?

Only if you assign them different numbers, otherwise some of the option numbers given for the CIB collide with already-assigned numbers for EPB options.

Otherwise...

i.e. Options appearing in an EPB override the values in a CIB?

...I don't see any problem with allowing them in either type of block.

@guyharris
Copy link
Collaborator

Adds a Capture Information Block (CIB) with global location/orientation/velocity vectors, as proposed in #48. CIBs are associated with an IDB (via an optional interface id field) or with an EPB through the addition of a CIB ID option to the EPB.

It is intended that the CIB will be extended to provide further useful capture metadata within PCAP files (for example, wireless channels, frequencies, and received RSSIs).

Presumably the CIB is useful only for information that could change over time; otherwise, you'd just attach it to the IDB.

Would the RSSI be better as an option for an EPB? Would the same apply to channel/frequency options?

@jkcko
Copy link

jkcko commented Dec 31, 2018

I'm thinking of a mobile multi-channel wireless sniffer (one IDB per channel) which could be used to survey channels and relative signal strength at given locations. Additionally, in my case one set of location details could apply to multiple interfaces and RSSI would be per actual EPB received.

Speaking of conflicting option codes, the "global" comment option code (1) should still be permitted in a CIB so other options should start at 2 or other non-conflicting offset.

@packetfoo
Copy link
Contributor

I have no problem with overriding options in the EPB if that's helpful. But I agree with Guy that the option numbers must be unique in that case. It's too confusing if the same option has a different number for the same thing when present both in CIB and EPB - even if that means we skip some option numbers in the CIB options spec.

Also, regarding CIB vs. MDB - I say let's call it CIB, because MDB is too generic and people might want to try and add tons of non-related-to-capture (but still "meta") information to that block. As an example I can think of storing data like pre-computed endpoint/conversation values found in the capture file, and we'll end up with a monster "block-of-all-trades" :-)

@jkcko
Copy link

jkcko commented Jan 1, 2019

"CIBs are associated with an IDB (via an optional interface id field) or with an EPB through the addition of a CIB ID option to the EPB."
if we can overload options of a CIB in an EPB then we don't need addition of a CIB ID option to the EPB. Otherwise that sounds like a CIB would have either an Interface ID or other unique ID which the option could reference. The packet already has an Interface ID. I'm sure we don't want a CIB to have unique ID numbers of their own.

Also, what was the objection, if any, to allowing these options in the IDB itself and perhaps having the CIB as an overloaded/update IDB options block? (IOB)

Too bad IDBs are sequential in order of appearance the pcapng doesn't allow updating an IDB and keeping the same interface number.

Also, would an overloaded CIB option appearing in an EPB apply to just that packet or that and all subsequent packets until further update. I figure the latter makes sense.

@ryankurte
Copy link
Author

ryankurte commented Jan 5, 2019

Alright, there appears to be some confusion arising as I hadn't updated the PR description with all the changes from the discussion here and in #48.

As a quick overview (correct me if you disagree):

  • we decided that RSSI etc. are better as EPB options, and that Location etc. work well attached to an interface.
  • this PR is the addition of CIBs that are associated with an interface ID for attaching location information and other interface-specific options that are not synchronized with packet receipt
  • i just opened a new PR for the additional EPB options (Additional RF information options for EPBs #56)

@b1tninja

I guess I like CIB when the data is associated with the particular packet/interface.

This is the only case in the current PR, sorry I had not updated the PR description.

@guyharris

Only if you assign them different numbers, otherwise some of the option numbers given for the CIB collide with already-assigned numbers for EPB options.

Right, I'll re-assign the option numbers to be non-colliding?

...I don't see any problem with allowing them in either type of block.

It seems simpler to only allow the options in the EPB or CIB depending on their relevance to the interface or captured packet / removes the need for us to describe a heirachy and support overriding. I would prefer exclusive options personally.

Presumably the CIB is useful only for information that could change over time; otherwise, you'd just attach it to the IDB.

Yep, except it's simpler to have one-true-way than define and handle levels of inheritance for options imo.

Would the RSSI be better as an option for an EPB? Would the same apply to channel/frequency options?

Yep

@jkcko

Speaking of conflicting option codes, the "global" comment option code (1) should still be permitted in a CIB so other options should start at 2 or other non-conflicting offset.

Definitely makes sense to re-index the options to be globally unique.

Also, what was the objection, if any, to allowing these options in the IDB itself and perhaps having the CIB as an overloaded/update IDB options block? (IOB)

See above, that is the approach i initially proposed, but imo it's preferable to have one (simple) way of doing things and not impose parsing / encoding complexity.

Also, would an overloaded CIB option appearing in an EPB apply to just that packet or that and all subsequent packets until further update. I figure the latter makes sense.

The latter, check the diff or the rendered RFC for more information.

@jkcko
Copy link

jkcko commented Jan 18, 2019

The CIB block type code in Figure 15 is incorrect. It is showing the same number as the EPB.
The CIB block type code should also be added to the list in 11.1. Standardized Block Type Codes.

@ryankurte
Copy link
Author

@jkcko thanks for catching that ^_^

I re-indexed the options to be unique within the CIB, do we need them to be globally unique given they can only occur in the CIB / having the CIB as the only mechanism for attaching that information simplifies encoding and parsing?

Change CIB block ID to 0x0000000B to avoid newly included block
@ryankurte
Copy link
Author

@guyharris merged and re-indexed to avoid the new journal block

are there any outstanding things you need resolved to get this merged?

@guyharris
Copy link
Collaborator

are there any outstanding things you need resolved to get this merged?

See comments.

@guyharris
Copy link
Collaborator

Presumably if a measurement option is present, but the corresponding error option isn't present, the error is unknown?

Presumably if a measurement option is absent, but the corresponding error option is present, whoever decided (directly, or indirectly by writing software to write a CIB) to provide an error estimate for a non-existent measurement is confused (i.e., ignore the error option)?

@guyharris
Copy link
Collaborator

@ryankurte
Copy link
Author

ryankurte commented Feb 13, 2019

See comments.

thanks! resolved all of those i think, and added options examples. a couple more questions:

- [ ] do we want globally unique indicies on the CIB options? (and if so, at what number should I start?)

  • would it make more sense for orientations to be represented as euler angles..?

@guyharris
Copy link
Collaborator

  • do we want globally unique indicies on the CIB options? (and if so, at what number should I start?)

I would say "no", because 1) if the idea is that we'd want to be able to add that information to packet blocks as well, assigning separate option numbers for the EPB versions of the various information items isn't a huge problem in code reading pcapng files and 2) the second question you ask is a reason why it's a pain to pick globally unique option numbers - you have to pick a number that's larger than the maximum option number for all blocks to which the option would apply, and now you have holes in the available option number space for some of those block types.

<t>Example: '41 24 cc cd 41 73 33 33 3f cc cc cd' decodes to
x: 10.3m, y: 15.2m, z: 1.6m</t>

<t hangText="cib_local_location_error"><vspace blankLines="0"/>The
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it should indicate that having a cib_local_location_error option without a cib_local_location makes no sense, and that having a cib_local_location option without a cib_local_location_error option means that the error is unknown.

If so, the same should be done for the other options as well.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added that an overarching comment at line 2125 rather than adding it per-option, though I can put it on each option if you would prefer?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A global comment suffices.

What's the correct interpretation of a measurement without a corresponding error? "It's unknown how accurate this measurement is"?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the absence of error information only indicates that there is no error information.
As to whether it is unknown, unknowable, elided because the capturer doesn't care, or for any other reason, would be outside the scope of the capture file?

@b1tninja
Copy link

b1tninja commented Mar 2, 2019

Came across a blackhat presentation where they had implemented similar functionality, posting here so that people might consider various facets https://media.blackhat.com/bh-us-11/Cache/BH_US_11_Cache_PPI-Geolocation_WP.pdf

@ryankurte
Copy link
Author

hey hey, are there any other things i can do to help get this merged?

@guyharris
Copy link
Collaborator

hey hey, are there any other things i can do to help get this merged?

Well, step 1 would be to change it to update the Markdown file rather than the XML file; @mcr converted the spec to extended Markdown - it's at draft-tuexen-opsawg-pcapng.md now. See cabo/kramdown-rfc2629 for the extensions.

@mcr
Copy link
Collaborator

mcr commented Dec 21, 2020 via email

@mcr
Copy link
Collaborator

mcr commented Jul 23, 2023

@ryankurte can you rebase? I know this is years old, but sometimes it takes awhile for the IETF to get its ducks into the right geometric pattern.

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

Successfully merging this pull request may close these issues.

None yet

6 participants