Skip to content

Conversation

@danieluliedguevara
Copy link

Previous GeoNet was outdated and didn't comply with current standards, making tcpdump wrongly dissect GeoNet packets (issue) .

This PR updates ETSI GeoNetworking protocol to version 1 (ETSI EN 302 636-4-1 V1.4.1 (2020-01)). The update includes dissectors for GeoNet Beacon messages and TopoScopeBcast-SH. Additionally, the Basic Transport Protocol (ETSI EN 302 636-5-1 V2.2.1 (2019-05)) has also been updated. It includes BTP-A and BTP-B in all its variants.

@infrastation
Copy link
Member

Please see CONTRIBUTING.md, specifically:

touch .devel
./build_matrix.sh

After all the fix-ups have been made, please make this one clean commit.

@infrastation
Copy link
Member

Thank you. At a glance, this code needs to use tok2str() more instead of implementing a number of functions that take an integer and return a string, and needs to use named constants instead of hard-coding integer values. This problem isn't new, but in the old code it is much smaller because the old code is much smaller.

Also the use of clock_gettime() does not look right. For any protocol decoder the result is supposed to be a function of the packet data only (except reverse lookups in DNS when enabled).

Also most of the details that are now in the pull request message concern the commit, not the pull request, so should be in the commit message.

Also this removes support for GeoNet version 0 altogether, correct? And the new code prints the version, but does not check that it is version 1, correct?

@infrastation
Copy link
Member

Also this removes 4 existing tests.

@danieluliedguevara
Copy link
Author

I'll look into using tok2str() for some of the functions and also using make sure to use more constants. I'll also modify the commit message once I solve the issues.

As for the use of clock_gettime(), the GeoNet message header includes a "timestamp", the timestamp, in milliseconds, is encoded in 32 bits and hence can only express aproximately 49 days. So, instead of printing the raw value of the timestamp, which is of little use, I process it and generate the actual (human-readable) timestamp. Is this okay?

This does remove support for GeoNet version 0. There are two things I would point out, one is that GeoNet version 0 was drafted for a preliminary ETSI technical spec (as mentioned in this issue) and a new version was published in 2013 (12 years ago). Second, seeing that GeoNet-related services are rare, and all providers follow current standards I think it's better to remove support for GeoNet version 0 (for simplicity).

You are correct, the current PR doesn't check it's version 1... I'll make sure to fix that.

In the meantime, can you confirm that it's okay to remove support for GeoNet version 0? I can add it, but don't know if it will be used much.

@infrastation
Copy link
Member

Thank you for explaining. Regarding the 32-bit TST field, the standard defines it to wrap this often and to have this much precision, it does not include a notion of a calendar date or a wrap counter. It would be wrong for a protocol decoder to present the TS field as if it has more data than it actually has. If the user needs to interpret the recorded 32-bit value of TST relative to "real" date and time, that's what the libpcap packet timestamp is for. So the best this decoder could do is to print TST exactly as it is (an integer number of milliseconds, or something formatted as "seconds.millieconds" or "dd:hh:mm:ss.ms" or whatever is the common notation for this 32-bit value in this practice).

Regarding the versions, to comprehend this better, I had a look at the following specifications available at etsi.org:

As far as these documents define it, version 1 did not exist until 2017. As far as I understand V1.2.1 Section 8.4, digital signatures are optional. So, seemingly, version 0 could be not entirely irrelevant, but the problem is, the two version 0 specifications indeed define GeoNet packet structure differently:

  • TS 102 636-4-1 V1.1.1 Section 8.3 says it is Common Header plus Extended header. Ibid., Section 8.5.1 says Version is a Common Header field.
  • EN 302 636-4-1 V1.2.1 Section 8.3 says it is Basic Header plus Common Header plus Extended Header (optional). Ibid., Section 8.6.1 says Version is a Basic Header field.
  • Beyond the first octet (Version + NH) the V1.1.1 Common Header and V1.2.1 Basic Header diverge in their structure.

This way, I agree that the best move is to lose the current V1.1.1 (version 0) code in the tcpdump decoder, whether anybody gets to implement V1.2.1 (version 0) code in future or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants