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

Add a get_application_key method #21

Merged
merged 3 commits into from
Apr 25, 2023
Merged

Add a get_application_key method #21

merged 3 commits into from
Apr 25, 2023

Conversation

sosthene-nitrokey
Copy link
Contributor

This method is meant to encrypt data stored on the external flash so that it can't be accessed just but plugging into it.
Multiple keys can be obtained with an info parameter.

This adds one step after the get_app_key step described in #10 to add a per-application salt and an info parameter: HMAC(application_key, application_salt || len(info) || info).
With the application_key being the result of get_app_key and the salt being a per-application salt, that is deleted with delete_all_pins, so that the keys change.

@sosthene-nitrokey
Copy link
Contributor Author

Close #20

@szszszsz
Copy link

What's the ETA on this?

@sosthene-nitrokey
Copy link
Contributor Author

Only needs review

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.

I think it is counter-intuitive that delete_all_pins also affects the pin-less application key. We should either rename the syscall or change it to only reset the PINs and introduce a separate syscall to clear all data.

@sosthene-nitrokey
Copy link
Contributor Author

Ok, I'll make delete_all_pins only delete pins, and add two syscall:

  • reset_application_keys (delete app keys)
  • reset_auth_data (delete everything)

@sosthene-nitrokey
Copy link
Contributor Author

Hurh, trussed's filestore doesn't have a remove_dir_all_where

@sosthene-nitrokey
Copy link
Contributor Author

And now I'm finding bugs in the littlefs bindings and trussed

@sosthene-nitrokey
Copy link
Contributor Author

See trussed-dev/littlefs2#36 (review), which is required to properly implement remove_dir_all_where in the filestore which is required for this.

But this also mean we will have to have a new release of littlefs2 and merge trussed-dev/trussed#96 to benefit from it

@robin-nitrokey
Copy link
Member

We’re still using a patched littlefs2 in nitrokey-3-firmware so we should be able to cherry-pick the fix for our fork:

https://github.com/Nitrokey/littlefs2
https://github.com/Nitrokey/nitrokey-3-firmware/blob/edfeef921c951ec00e97513f0d4e74e9c70f8406/Cargo.toml#L16

@szszszsz
Copy link

szszszsz commented Apr 12, 2023

@sosthene-nitrokey @robin-nitrokey
Can you prepare version handles to use in Cargo.toml to develop against in the meantime?
Is only the littlefs update needed?

@daringer daringer linked an issue Apr 18, 2023 that may be closed by this pull request
@szszszsz
Copy link

szszszsz commented Apr 19, 2023

Just tested and now it works, without changing anything additionally, specifically littlefs dep.
See below for my use case:

szszszsz added a commit to Nitrokey/trussed-secrets-app that referenced this pull request Apr 21, 2023
@daringer daringer merged commit 6beabd1 into main Apr 25, 2023
@daringer daringer deleted the get-app-key branch April 25, 2023 14:19
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.

Expose the Application key
4 participants