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

mgmt: mcumgr: Add settings commands #52318

Closed
nordicjm opened this issue Nov 17, 2022 · 4 comments · Fixed by #59345
Closed

mgmt: mcumgr: Add settings commands #52318

nordicjm opened this issue Nov 17, 2022 · 4 comments · Fixed by #59345
Assignees
Labels
area: mcumgr Feature Request A request for a new feature priority: low Low impact/importance bug
Milestone

Comments

@nordicjm
Copy link
Collaborator

nordicjm commented Nov 17, 2022

Is your feature request related to a problem? Please describe.
A requested feature for zephyr-based devices is being able to write/read long data over BLE to/from a device in a simple to use way. MCUmgr allows for such a system as the protocol is simple and supports data which is larger than the BLE MTU (512 bytes), so would be a good fit for this. Therefore having an MCUmgr group for reading from/writing to settings through the API would be a useful feature for this.

Describe the solution you'd like
A simple MCUmgr group with commands for reading data from/writing data to settings. This system would not have security for an initial release but may have security implemented in a future release e.g. to restrict access to some settings.

@nordicjm nordicjm added Feature Request A request for a new feature area: mcumgr labels Nov 17, 2022
@nordicjm nordicjm added the priority: low Low impact/importance bug label Nov 17, 2022
@nordicjm nordicjm changed the title mgmt: mcumgr: Add settings commands mgmt: mcumgr: Add NVS commands Nov 17, 2022
@nordicjm nordicjm self-assigned this Nov 18, 2022
@Laczen
Copy link
Collaborator

Laczen commented Nov 22, 2022

@nordicjm can't this feature be provided trough a shell command interface for mcumgr ? This would avoid doubling maintenance for a shell interface and a mcumgr interface.

@nordicjm
Copy link
Collaborator Author

@nordicjm can't this feature be provided trough a shell command interface for mcumgr ? This would avoid doubling maintenance for a shell interface and a mcumgr interface.

Then you would need to pull the whole shell system in which would add a huge bloat, and would be a big security vulnerability. Unless NVS has an API change made to it, nothing needs to be done with this.

@Laczen
Copy link
Collaborator

Laczen commented Nov 22, 2022

@nordicjm, opening the nvs system to mcumgr would be a big security vulnerability also. If there would be a change to the nvs API the work would be doubled (for the shell and for mcumgr). As the author of nvs I also consider nvs to be eol.

@nordicjm
Copy link
Collaborator Author

@nordicjm, opening the nvs system to mcumgr would be a big security vulnerability also.

Not so, with recent revamped MCUmgr callback system, the plan would be to have an (optional) callback to check if access should be granted to particular keys - similar to how the fs_mgmt one works - see https://docs.zephyrproject.org/latest/services/device_mgmt/mcumgr_callbacks.html#structfs__mgmt__file__access

As the author of nvs I also consider nvs to be eol.

What's the replacement?

@nordicjm nordicjm changed the title mgmt: mcumgr: Add NVS commands mgmt: mcumgr: Add settings commands May 19, 2023
@nordicjm nordicjm added this to the future milestone May 19, 2023
@github-project-automation github-project-automation bot moved this to Done in mcumgr Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: mcumgr Feature Request A request for a new feature priority: low Low impact/importance bug
Projects
Status: Done
2 participants