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

nk3 set-config: Add support for opcard.use_se050_backend #488

Merged
merged 1 commit into from
Dec 18, 2023

Conversation

robin-nitrokey
Copy link
Member

This patch refactors the set-config command:

  1. It checks whether the config option is supported by the device using the GET_CONFIG command before any other steps are performed.
  2. For known keys that trigger a reset, currently only opcard.use_se050_backend, a warning and a confirmation prompt are shown.
  3. Unknown keys are rejected unless --force is set, trigger a warning and require a confirmation prompt.
  4. It adds a --dry-run option to check the infos and prompts that are printed for a config change.
  5. It adds an automatic reboot if required.

Potential improvements:

  • The metadata for config values should be put into data structures that can also be used by nitrokey-app2.
  • Information on whether a command triggers a reset and requires touch confirmation could be queried from the device using a new command.
  • We could validate the configuration values before sending them to the device.
  • We could compare the current and the new value to detect no-op calls before printing the sermon about side effects.

This patch refactors the set-config command:

1. It checks whether the config option is supported by the device using
   the GET_CONFIG command before any other steps are performed.
2. For known keys that trigger a reset, currently only
   opcard.use_se050_backend, a warning and a confirmation prompt are
   shown.
3. Unknown keys are rejected unless --force is set, trigger a warning
   and require a confirmation prompt.
4. It adds a --dry-run option to check the infos and prompts that are
   printed for a config change.
5. It adds an automatic reboot if required.

Potential improvements:
- The metadata for config values should be put into data structures that
  can also be used by nitrokey-app2.
- Information on whether a command triggers a reset and requires touch
  confirmation could be queried from the device using a new command.
- We could validate the configuration values before sending them to the
  device.
- We could compare the current and the new value to detect no-op calls
  before printing the sermon about side effects.
@daringer
Copy link
Collaborator

lgtm!
tested on v1.6-test-20231218 with nk3am:

  • setting opcard.use_se050_backend
  • getting opcard.use_se050_backend
  • getting fido.disable_skip_up_timeout
  • --dry-run

@robin-nitrokey robin-nitrokey merged commit e8fe4f9 into master Dec 18, 2023
10 checks passed
@robin-nitrokey robin-nitrokey deleted the config-se050 branch December 18, 2023 22:48
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.

2 participants