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

PIN verification using Optiga #3296

Merged
merged 5 commits into from
Sep 27, 2023
Merged

PIN verification using Optiga #3296

merged 5 commits into from
Sep 27, 2023

Conversation

andrewkozlik
Copy link
Contributor

@andrewkozlik andrewkozlik commented Sep 21, 2023

Creating this draft PR, so we can start discussion about the cryptographic aspects of the PIN verification process using Optiga. @onvej-sl

I did some refactoring around the ui_progress, which as it turns out is not really necessary, since we decided to set PIN_STRETCH_ITERATIONS to 1, so it takes no noticeable time (unless the security monitor kicks in). But I think the refactor still has some value, so I decided to keep it. It might be used in optiga.c in the future if we have our own configuration of the security monitor which will allow us to increase PIN_STRETCH_ITERATIONS. For now I removed some of the unused code in a separate commit, so that it can be reverted when the time comes.

storage/storage.c Outdated Show resolved Hide resolved
storage/storage.c Outdated Show resolved Hide resolved
@andrewkozlik
Copy link
Contributor Author

@onvej-sl has reviewed the cryptographic solution and will do a final check tomorrow. I would like another set of eyes to go over the code, @matejcik please. I am hoping to get this merged tomorrow. The reason why it is in draft form is because it's branched off of the hashing-to-curve PR, which is not merged yet.

@andrewkozlik andrewkozlik changed the base branch from master to onvej-sl/hash_to_curve September 26, 2023 16:05
@andrewkozlik andrewkozlik force-pushed the andrewkozlik/optiga-pin branch from 2aa7bfb to d60e9a5 Compare September 26, 2023 16:27
storage/storage.c Outdated Show resolved Hide resolved
@andrewkozlik
Copy link
Contributor Author

andrewkozlik commented Sep 27, 2023

The last two commits 4419899 4ed316b that I added today reintroduce ui_progress() into the Optiga code. They don't change anything about the functionality. The execution of the Optiga part takes about 1 second to complete, so this smooths out the progress a bit better.

@onvej-sl onvej-sl force-pushed the onvej-sl/hash_to_curve branch from f89b889 to 4c144cd Compare September 27, 2023 11:09
Base automatically changed from onvej-sl/hash_to_curve to master September 27, 2023 12:04
@andrewkozlik andrewkozlik force-pushed the andrewkozlik/optiga-pin branch from 5d79134 to 121a16d Compare September 27, 2023 13:07
@andrewkozlik andrewkozlik marked this pull request as ready for review September 27, 2023 13:10
Copy link
Contributor

@matejcik matejcik left a comment

Choose a reason for hiding this comment

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

LGTM code-wise, except the progess(0) thing you discovered

@andrewkozlik
Copy link
Contributor Author

LGTM code-wise, except the progess(0) thing you discovered

Should be fixed by 38b331d

@andrewkozlik andrewkozlik force-pushed the andrewkozlik/optiga-pin branch from 96e9912 to 64a8fc3 Compare September 27, 2023 14:56
@andrewkozlik andrewkozlik merged commit b3d0fb6 into master Sep 27, 2023
7 of 8 checks passed
@andrewkozlik andrewkozlik deleted the andrewkozlik/optiga-pin branch September 27, 2023 15:17
@mmilata mmilata mentioned this pull request Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants