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

async API for Env? #685

Open
antonok-edm opened this issue May 6, 2024 · 3 comments
Open

async API for Env? #685

antonok-edm opened this issue May 6, 2024 · 3 comments
Assignees

Comments

@antonok-edm
Copy link

antonok-edm commented May 6, 2024

Hi! I'm working on a project using the RTIC framework on an STM32F4 microcontroller, and I'm interested in porting OpenSK to that architecture if at all possible.

RTIC leans heavily on Rust's async with interrupts for resource sharing and task scheduling. Right now it looks like the current definition of traits required by Env exclusively use blocking function calls. After a first review for my use-case it looks like HidConnection::send_and_maybe_recv, AttestationStore::{get,set}, Storage::{read_slice,write_slice,erase_page}, and UserPresence would all need to access some kind of shared I/O peripherals. Under RTIC, non-blocking implementations of these traits would be awkward at best (and maybe highly unsafe at worst).

Before I go down this route, I wanted to check in with the maintainers - is this something you'd be willing to merge if a PR was provided? Any other thoughts?

@kaczmarczyck kaczmarczyck self-assigned this May 6, 2024
@kaczmarczyck
Copy link
Collaborator

Thank you for your interest. The current Env API has evolved with needs from different platforms. The API calls you mentioned are broadly in 2 categories.

USB HID related

HidConnection and UserPresence.
Our current API is mostly taken from pre-library code and hasn't been redesigned yet. I could see us moving to an async API here.
We haven't touched it yet since we might want to be compatible with other transports like NFC in the future, and the new design should work for that.

Storage related

AttestationStore::{get,set}, Storage::{read_slice,write_slice,erase_page}
First comment: This is work in progress, e.g., #679. I suggest you look at the develop branch, not the current main branch 2.1 which is for people who want to build and use OpenSK more than it is for developers. Not too different yet, but more recent. AttestationStore moved into Persist.

Being async here is more problematic, since some of our security guarantees are tied to storage writes having persisted when execution continues. Can you help me understand why a blocking API doesn't work for you?

@antonok-edm
Copy link
Author

USB HID related

Our current API is mostly taken from pre-library code and hasn't been redesigned yet. I could see us moving to an async API here.

Cool, then there's a chance I may propose a PR or two later 🙂

Storage related

Can you help me understand why a blocking API doesn't work for you?

On my hardware, I'm using external eMMC memory for persistent storage, rather than embedded flash. It gives me quite a bit more capacity to work with, but it's also a slower channel where data is transferred over a bus 1 byte at a time with a checksum at the end for verification.

some of our security guarantees are tied to storage writes having persisted when execution continues

In my case, that's exactly why I'd prefer async - so that execution can switch to other unrelated tasks (e.g. updating the display) while a read/write is pending, and yield back to OpenSK once the operation has been confirmed by a successful handshake from the eMMC.

@kaczmarczyck
Copy link
Collaborator

Question is how you'd design an Env that doesn't break all sync users and supports async use cases at the same time? To make sure I don't overpromise: I don't merge a PR that only moves our API to async.

So maybe it is indeed easier if you can use the sync OpenSK as is? Outside of user presence checking, our USB HID code is in the main loop, which is fully in your control. So the limitation that you can't run, e.g., display updates in parallel only applies when OpenSK processes a command.

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

No branches or pull requests

2 participants