TheThingsNetwork / lorawan-stack

The Things Stack, an Open Source LoRaWAN Network Server
https://www.thethingsindustries.com/stack/
Apache License 2.0
975 stars 306 forks source link

Cleanup Join Server session keys #5086

Closed johanstokking closed 2 years ago

johanstokking commented 2 years ago

Summary

Cleanup the Join Server session keys

References https://github.com/TheThingsIndustries/lorawan-stack/issues/2229

Why do we need this?

To avoid unbound growth of the Join Server session key store.

What is already there? What do you see now?

They are currently persisted without limits. Also there isn't an index.

What is missing? What do you want to see?

We should keep 3 session keys. That should be enough for Application Servers to recover the AppSKey.

How do you propose to implement this?

  1. Store the session keys in two lists. The first list is for join-requests and rejoin-requests type 2. In that case, the end device is either on join mode (has no session keys anymore) or is requested to force rejoin via ForceRejoinReq so its existing session keys are void. The second list is for rejoin-requests type 0 and 1. The end device may continuously send rejoin-requests type 0 and 1 which are not necessarily answered or may vanish in the air. That should not lead to rolling over the first list. Rejoin type 0 may also be the result of ForceRejoinReq, but it is unknown to the Join Server what the reason was for the end device to transmit the rejoin-request, so it's safe to store that with rejoin-request type 1
  2. In a database migration script, enumerate all session keys per end device. The JoinEUI and DevEUI can only be parsed from the Redis key. It's safe to sort the keys ascending as the session key ID is an ULID
    • Since the existing key registry doesn't record the rejoin-request type, we should store the last 3 keys in both lists
    • The other keys can be discarded

How do you propose to test this?

Local testing

Can you do this yourself and submit a Pull Request?

Yes

johanstokking commented 2 years ago

This needs a database migration, so we'll do this for 3.18.0