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

Check authorization in Basics Station discovery #6734

Merged
merged 2 commits into from
Dec 1, 2023

Conversation

adriansmares
Copy link
Contributor

@adriansmares adriansmares commented Dec 1, 2023

Summary

References https://github.com/TheThingsIndustries/lorawan-stack-support/issues/1007
References https://github.com/TheThingsIndustries/lorawan-stack/pull/3974

Changes

  • Allow individual gateway protocol frontends to assert the rights of the caller, without connecting per se as the gateway.
  • Assert that the caller has the required rights over a gateway while calling the LNS discovery endpoint.
    • Currently, the gateway doesn't realize that its credentials may need to be updated, because the LNS gives mixed signals - it accepts the credentials during discovery, but not during traffic. As such, the gateway falls back to doing a discovery round again, instead of doing a CUPS lookup.
    • The changes proposed here allow the gateway to correctly fall back to CUPS in order to update both its credentials and the LNS address.

Testing

Unit tests and local testing. This is currently live on staging1.

Testing steps:

  1. Register a Basic Station gateway using CUPS.
  2. Connect said gateway.
  3. Invalidate (delete or just remove the linking right) the LNS key.
  4. Generate new LNS key and set it as LNS key.
  5. Trigger a gateway reconnect remotely (i.e. toggle the 'disable Packet Broker forwarding' option).
  6. Expect the gateway to reconnect using the new LNS key before the next periodic CUPS lookup (i.e. in less than 24 hour, preferably way faster).
Regressions

All of the implementations derived form the Semtech reference implementation provide the credentials during discovery. Tektelic also provides the credentials during discovery. So does Kerlink.

I've ran the above test sequence for all of the vendors.

Kerlink unfortunately doesn't re-trigger CUPS if discovery fails, but that's unrelated to this fix. They run CUPS periodically every hour, so should someone need to migrate, it should more manageable. For reference, the reference implementation runs CUPS every 24 hours if discovery never fails.

Notes for Reviewers

We cannot unfortunately return a 'good' HTTP status code, as the gateway identifiers are not available before the WebSocket upgrade occurs. We have to return the error via WebSockets.

No need to review.

Checklist

  • Scope: The referenced issue is addressed, there are no unrelated changes.
  • Compatibility: The changes are backwards compatible with existing API, storage, configuration and CLI, according to the compatibility commitments in README.md for the chosen target branch.
  • Documentation: Relevant documentation is added or updated.
  • The steps/process to test this feature are clearly explained including testing for regressions.
  • Changelog: Significant features, behavior changes, deprecations and fixes are added to CHANGELOG.md.
  • Commits: Commit messages follow guidelines in CONTRIBUTING.md, there are no fixup commits left.

@adriansmares adriansmares self-assigned this Dec 1, 2023
@adriansmares adriansmares added this to the v3.28.2 milestone Dec 1, 2023
@adriansmares adriansmares added the bug Something isn't working label Dec 1, 2023
@github-actions github-actions bot added the c/gateway server This is related to the Gateway Server label Dec 1, 2023
@adriansmares adriansmares marked this pull request as ready for review December 1, 2023 10:33
@adriansmares adriansmares requested a review from a team as a code owner December 1, 2023 10:33
@adriansmares adriansmares merged commit 28f39d3 into v3.28 Dec 1, 2023
12 of 13 checks passed
@adriansmares adriansmares deleted the fix/lns-discovery-os branch December 1, 2023 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working c/gateway server This is related to the Gateway Server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant