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

Adds Admin session_passkey to prevent replay of admin packets #558

Merged
merged 2 commits into from
Aug 15, 2024

Conversation

jp-bennett
Copy link
Contributor

Adds a session passkey for admin operations. When the remote node responds to any get_x request, it will include the session passkey in the response. When the local client tries to make any admin changes, the remote node expects that session key to be set. The session key will expire after a short time, currently 120 seconds.

@GUVWAF
Copy link
Member

GUVWAF commented Aug 15, 2024

@garthvh Do you request the config when a user goes into the settings or only after "Save"? Would also be good to know this for other clients, as otherwise 120 seconds will be too tight, especially with slow LoRa settings and when it goes over multiple hops.

@garthvh
Copy link
Member

garthvh commented Aug 15, 2024

@garthvh Do you request the config when a user goes into the settings or only after "Save"? Would also be good to know this for other clients, as otherwise 120 seconds will be too tight, especially with slow LoRa settings and when it goes over multiple hops.

I don't do anything yet, so we can set it up however makes sense and it will likely get used by the other clients

@GUVWAF
Copy link
Member

GUVWAF commented Aug 15, 2024

I mean how remote admin is currently implemented. Probably you first request the config to be able to show the current settings? If the user then afterwards still has to choose the new config, that will likely take too much time.

@jp-bennett
Copy link
Contributor Author

We could even add new admin get_sessionkey and get_sessionkey_response messages, just to get an updated sessionkey.

@GUVWAF
Copy link
Member

GUVWAF commented Aug 15, 2024

That sounds a bit too much of a hassle. I think going from 120 to 300 should give enough margin either way, and it's not a big deal that someone can re-apply the changes you made 5 min. ago.

Copy link
Member

@garthvh garthvh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

300 seems better

@jp-bennett jp-bennett requested a review from garthvh August 15, 2024 21:32
@jp-bennett jp-bennett merged commit 06d7ca5 into 2.5 Aug 15, 2024
2 checks passed
@jp-bennett jp-bennett deleted the admin-passkey branch August 15, 2024 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants