TheThingsArchive / ttn

The Things Network Stack V2
https://www.thethingsnetwork.org
MIT License
461 stars 279 forks source link

Stop processing subsequent MAC commands when encountering an unknown command #748

Closed avbentem closed 5 years ago

avbentem commented 5 years ago

This is a bug report for the backend.

What do you want to do?

Allow nodes to send unsupported MAC commands.

What steps did you take?

(Just browsed the source code.)

What went wrong or what is missing?

It seems that the V2 code just ignores unknown MAC commands in uplinks, and then continues processing the next command. But as it cannot know if FOpts holds any payload for the unknown command, it should not assume that the next byte in FOpts is indeed a MAC command.

The 1.0.3 specifications states:

Note: The length of a MAC command is not explicitly given and must be implicitly known by the MAC implementation. Therefore unknown MAC commands cannot be skipped and the first unknown MAC command terminates the processing of the MAC command sequence.

...and for nodes:

It is therefore advisable to order MAC commands according to the version of the LoRaWAN specification which has introduced a MAC command for the first time. This way all MAC commands up to the version of the LoRaWAN specification implemented can be processed even in the presence of MAC commands specified only in a version of the LoRaWAN specification newer than that implemented.

As an aside, given a post in the forum, it seems that TTN Console might not show the packets in the device's Data page:

@cyberman54 The packet was not displayed in console, what also means that a payload with embedded unsupported MAC command in FOpts field will get lost.

However, I don't understand how the code that processes the MAC commands might have caused that. (But Go is not my cup of tea.)

Do you have Screenshots?

N/A

What kind of OS/Browser/Gateway are you using? Which version?

N/A

What are the IDs and EUIs of your Device/Gateway?

N/A

What do your configuration files look like?

N/A

What do your log files look like?

N/A

Can you fix this yourself and submit a pull request?

Maybe, but it's probably not worth the effort for V2, so this is merely a suggestion to not copy this code verbatim to V3. (But if TTN Console indeed stops displaying the packets in the device's Data page due to unknown MAC commands, then maybe someone should report an issue for that?)

avbentem commented 5 years ago

As an aside: I've striked-out my comments about TTN Console, as another forumpost suggests:

I have a number of TTN applications where uplinks do NOT appear on the TTN web console. Nothing appears in application / data or application / devices / device / data. I know the uplinks are all working correctly because MQTT publishes the uplinks correctly - it’s just a no-show bug on the TTN web console.

If I change something trivial in the TTN web console application configuration, save, and then change it back and then save again then, sometimes, the uplinks all start to appear on the web console.

So, as expected, that seems to be unrelated.

htdvisser commented 5 years ago

We're not going to make changes to v2's MAC handling. The v3 implementation is rewritten from scratch, and should handle unknown MAC commands just fine.

avbentem commented 4 years ago

Just for the sake of future readers:

The symptoms I reported (and then removed) might be real: a new forum post seems to show that encountering DeviceTimeReq indeed makes further handling fail.

However, I think my analysis of the cause was wrong: the actual parsing of the MAC commands is done in a 3rd party library, which actually understands DeviceTimeReq (and might also correctly stop when encountering an unknown MAC command; I've not checked that).