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

Expose more fine-grained control over find_objects #106

Closed
jhagborgftx opened this issue Nov 18, 2022 · 6 comments
Closed

Expose more fine-grained control over find_objects #106

jhagborgftx opened this issue Nov 18, 2022 · 6 comments

Comments

@jhagborgftx
Copy link
Contributor

jhagborgftx commented Nov 18, 2022

Currently, the only way to search for objects is through Session::find_objects, which returns a vector of all objects. If there may be a huge number of objects, it's desirable to be able to find only a bounded number of objects. This is possible with PKCS#11, but the functionality is not exposed by this crate.

Further, it may be desirable to iterate through objects incrementally, i.e. calling C_FindObjects multiple times, with some processing in between each call. Also, the user may care whether the library makes several small calls to C_FindObjects, or one large one, especially if the HSM is being accessed over a network.

I believe all of these needs may be met safely by the following API:

impl Session {
    // ...

    pub fn find_objects_init<'a>(&'a mut self, template: &[Attribute]) -> Result<FindObjects<'a>> {
        // call `C_FindObjectsInit`
    }
}

pub FindObjects<'a> {
    session: '&mut Session // exclusive borrow to ensure at most one object search per session
}

impl<'a> FindObjects<'a> {
    pub fn find_next(&self, object_count: usize) -> Result<Option<Vec<ObjectHandle>>> {
        // call `C_FindObjects` once, with a buffer of size `object_count`.
        // return Ok(Some(...)) if we found objects, or Ok(None) if the search is over
    }
}

impl Drop for FindObjects { ... } // call `C_FindObjectsFinal` on drop

Although find_next could return just Result<Vec<ObjectHandle>>, I think returning an Option is more ergonomic, since

  • It's consistent with iterators and streams, which return None when finished
  • It reminds the user they need to check for termination
  • It allows for looping over all objects with while let notation

I believe the current implementation of Session::find_objects, as well as any other versions we might want to make (e.g. Session::find_objects_bounded(template, object_count)), can be implemented in terms of this one.

I will make a PR with something like this soon, but first I would like to collect some feedback about the API.

@jhagborgftx
Copy link
Contributor Author

One downside to the API above the mutable borrow means it is not possible to interleave calls to find_next with calls to other session operations. I think it's possible to recover that by adding a method session(&self) -> &Session to FindObjects. Since it only returns an & reference, it should be possible to do any other session operation except starting a new object search.

Also, I just realized that it would be necessary to change find_objects to take a mutable reference as well, to prevent someone from calling find_objects while another object search is ongoing. That would make this a breaking change.

@jhagborgftx
Copy link
Contributor Author

Another downside is that we can't handle errors from C_FindObjectsFinal, since it's called from drop. An alternative that would allow this is to use some sort of type-state: when we start a search, take the session by value, and then when you end the search you get the session back by value (only after checking for errors). One could still borrow a &Session during the search, to perform non-search operations.

I would consider this much less ergonomic than the API above. I doubt anyone cares much about handling errors from C_FindObjectsFinal, since there's not much you can do besides closing the session and trying to open a new one.

@wiktor-k
Copy link
Collaborator

The use-case is good and I like your ideas. 👍

While looking at your proposed API it makes a striking resemblance to the Iterator API. Just wondering, have you considered providing an interface like this? Of course then you'd have to return Option<Result<ObjectHandle>> (just like fallible iterators) and probably do some caching (so that FindObjects is not too frequently called).

PRs of course welcome! Sorry in advance for the delay in reviewing 🙇

@jhagborgftx
Copy link
Contributor Author

jhagborgftx commented Nov 29, 2022 via email

@keldonin
Copy link
Collaborator

I guess this issue can be closed now?

@hug-dev
Copy link
Member

hug-dev commented Sep 13, 2024

Yes, thanks a lot for your work @keldonin! @jhagborgftx, if you don't find what you want in what #223 brought, feel free to re-open 😸

@hug-dev hug-dev closed this as completed Sep 13, 2024
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

4 participants