rust-cryptoki icon indicating copy to clipboard operation
rust-cryptoki copied to clipboard

Allow working with a non-finalizing `Pkcs11`

Open wiktor-k opened this issue 6 months ago • 4 comments

Hi folks,

There is much interest in a Pkcs11 that does not finalize the underlying implementation and I thought I'll try out one idea I had: splitting the Pkcs11 into an owning and ref structs (similarly to how Rust does with String and str). I've made the Pkcs11 finalize itself and the Pkcs11Impl just work on what's passed in. This way, folks that manage the lifecycle themselves can construct a new instance via Pkcs11Impl::new_unchecked which should resolve https://github.com/parallaxsecond/rust-cryptoki/issues/208.

I've also dropped inner Arc from Pkcs11 (so that the concurrency is now managed by the client outside of Pkcs11) and made Session own a reference to the client. I'd further rename Pkcs11Impl into something like Pkcs11Ref but I want to keep the diff as small as possible, and in case you don't like it it's not worth the refactor.

See what you think about it @hug-dev and @Jakuje.

The alternative posted on Slack by @testingapisname is adding additional finalization flag: https://github.com/testingapisname/rust-cryptoki/commit/6a4e58df0229312c8967a5d1aaab825f1a63615d

wiktor-k avatar Jun 23 '25 11:06 wiktor-k

I do not have this use case and I do not feel knowledgeable enough of rust to be able to judge which of the approaches is better. But this one looks more straightforward, while the other more "bolted on top of the existing things".

Generally, I think it is a good idea, especially when we have people asking for this.

Jakuje avatar Jun 23 '25 12:06 Jakuje

Yep, good point. I've pinged folks in https://github.com/parallaxsecond/rust-cryptoki/issues/208 to check if this makes sense (I'd rather avoid making changes without backing approval from real-world users).

Thanks for your help!

wiktor-k avatar Jun 23 '25 13:06 wiktor-k

I think this is a step in the right direction, although I see follow-up work (just outlining here so I don't forget):

  • considering running initialize in Pkcs11::new so the object is always initialized (just like it's always finalized on drop)
  • moving functions from Pkcs11 to Pkcs11Impl since currently even with Pkcs11Impl::new_unchecked it's actually not possible to call further functions :grimacing: (since they are on Pkcs11 and it's not possible to create that one from Pkcs11Impl)... or maybe I forgot something
  • Pkcs11::finalize could return an error... that'd give the caller the ability to finalize and get the finalization error... which is currently impossible (the error is only logged)... this means Pkcs11::finalize would need to directly call finalize_ref and suppress running the Drop (not to have finalization running twice).

Phew :sweat_smile: hopefully I didn't forget about anything!

wiktor-k avatar Oct 23 '25 10:10 wiktor-k

moving functions from Pkcs11 to Pkcs11Impl since currently even with Pkcs11Impl::new_unchecked it's actually not possible to call further functions 😬 (since they are on Pkcs11 and it's not possible to create that one from Pkcs11Impl)... or maybe I forgot something

ahh that's a good point :sweat_smile: Seems like @testingapisname could also be a cheap solution with just a new finalization flag

hug-dev avatar Oct 24 '25 13:10 hug-dev