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

Stateful initialization of crypto backends #248

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

geonnave
Copy link
Collaborator

Fixes #94.

Copy link
Collaborator

@chrysn chrysn left a comment

Choose a reason for hiding this comment

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

That sounds like a lot of unsafe and not-thread-safe comments with little guidance on how to use right. Wouldn't this be way easier to do in a constructor to Crypto, which has a static_cell as part of the initializer?

@geonnave
Copy link
Collaborator Author

lot of unsafe

In the cc310 backend, I must resort to unsafe in order to access the C functions. Making the whole function unsafe extern "C" would automatically also make it available for the C wrapper. Nevertheless, I think it is fine to drop the function-wise unsafe and have lakers-c add a wrapper for that.

constructor to Crypto

I thought of that, the issue is how to provide a constructor that is generic for all backends (what should it take as parameter?).
For example in rustcrypto we would want to pass rand_core::OsRng, while in cc310 it would be either nothing or the static_cell you mentioned.

@chrysn
Copy link
Collaborator

chrysn commented Mar 15, 2024

must resort to unsafe

Yeah, but that should be limited to the C or register calls; having a static mut is unsafe independently of that (and, moreover, about to be deprecated in favor of static S: SyncUnsafeCell<X>; or any safer wrappers around it).

rovide a constructor that is generic for all backends

There's two ways to answer that:

  • I don't think we need a constructor that is generic over all backends. It is convenient for the tests suite and the build tests to have a single lakers-crypto dispatch crate whose features can be placed in a matrix, but that's really just that. Any application building on it can make its choice of which crypto implementation to use (possibly guided by features of its own, but picking concrete crypto and not the dispatch crate), or defer to some platform specific dispatch that knows how to construct any backends, but in the end, the library part just takes an impl instance of it.
  • For this concrete case, the constructor doesn't need to take anything (and can thus be just be implementing Default): the static_cell can be a static local of the constructor.

@geonnave geonnave added the enhancement New feature or request label May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stateful initialization of crypto backends
2 participants