16 bit counters not handled correctly

[willem4ever]

When a device with a 16 bit counter overflows, the payload is no longer decrypted correctly. (it does seem to accept the MIC though)

[KtorZ]

Indeed, it seems that those lines:

https://github.com/TheThingsNetwork/ttn/blob/develop/core/components/broker/broker.go#L256-L280

Should change for the code below:

        // Check frame counter is in valid range

        // Check with 16-bits counters
        fhdr.FCnt = fcnt16
        ok, err := uplinkPayload.ValidateMIC(key)
        if err != nil {
            continue
        }

        if !ok { // Check with 32-bits counter
            fcnt32, err := b.NetworkController.wholeCounter(fcnt16, entry.FCntUp)
            if err != nil {
                // invalid, is device in developer mode
                if (entry.Flags & core.RelaxFcntCheck) != 0 {
                    fcnt32 = fcnt16
                    fcntReset = true
                } else {
                    continue
                }
            }
            fhdr.FCnt = fcnt32
            ok, err = uplinkPayload.ValidateMIC(key)
            if err != nil {
                continue
            }
        }

[avbentem]

I'm sure you all know what you're doing, so just for your information:

When using Over-the-Air Activation (OTAA), then the IBM LMiC documentation seems to suggest a node needs to do OTAA again whenever the counter rolls over. (In LMiC, one would get the EV_RESET event: Session reset due to rollover of sequence counters. Network will be rejoined automatically to acquire new session.) Doing OTAA again will get the node new secret keys, and will make the backend reset its counters too.

But I guess that for Activation by Personalization (ABP) counters should indeed rollover, as one cannot send new secrets to the node then.

So, if I understood correctly, then maybe this should only be fixed for ABP devices?

(On the other hand, rolling over for OTAA as well, would probably only introduce a very very tiny chance for replay attacks.)

[johanstokking]

The LoRaWAN specification doesn't prescribe taking care of roll-overs; it states that the use of 16- or 32-bit frame is defined out-of-band and may both be supported (see LoRaWAN 1.0 4.3.5.1).

To do this right, we should take the width of the frame counters into account per device registration. When using LMiC, the width is 16 bits apparently.

[johanstokking]

@KtorZ can you PR this?

[htdvisser]

This has been implemented in the refactor branch. See #199 for more info.