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

Make configure method public #31

Merged
merged 3 commits into from
Jun 6, 2024
Merged

Make configure method public #31

merged 3 commits into from
Jun 6, 2024

Conversation

sosthene-nitrokey
Copy link
Collaborator

This method will now be called in the init process, based on the last configured version

This method will now be called in the init process, based on the last configured version
@sosthene-nitrokey
Copy link
Collaborator Author

This PR also replaces #29

@robin-nitrokey
Copy link
Member

For the filesystem versions we just use an integer which is easier to handle than the crate version. Would that make sense in this case too?

@sosthene-nitrokey
Copy link
Collaborator Author

We could, but that seemed less error prone. The filesystem was versioned because it batches migrations from multiple backends at the same time, and filesystem migrations are much more expensive/risky than SE050 re-configuration

Copy link
Member

@robin-nitrokey robin-nitrokey left a comment

Choose a reason for hiding this comment

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

Okay, if you thing this is easier than let’s do it that way. For me the main argument for a counter would have been that we currently don’t always bump the crate version for every change. This is especially true for testing, but also for the regular firmware. So if you want to test changes, you need to bump the crate version every time instead of just changing a counter.

@sosthene-nitrokey
Copy link
Collaborator Author

Right. it's true that we generally bump the version number only very late in the development cycle.

@sosthene-nitrokey
Copy link
Collaborator Author

Maybe the struct could also be a bitflag of configured curves. This could also make future configure calls faster, by only configuring non-configured curves.

@robin-nitrokey
Copy link
Member

I’m fine with both approaches. I would default to using a counter because it is simpler, the additional work to re-initialize curves is negligible and it gives us more flexibility if we need to change the configuration more fundamentally than just adding curves.

@sosthene-nitrokey sosthene-nitrokey merged commit a8cf2ad into main Jun 6, 2024
2 checks passed
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