parallaxsecond / rust-cryptoki

Rust wrapper for the PKCS #11 API, Cryptoki
https://docs.rs/cryptoki/
Apache License 2.0
77 stars 61 forks source link

Function name as part of errors #135

Open ellerh opened 1 year ago

ellerh commented 1 year ago

What would people think about including (the name of) the called function in error values?

E.g. most error values could include a field of type cryptoki::context::Function. The error message could then look like Function::Login failed: RvError::PinIncorrect: The specified PIN is ....

Without the function name, error messages are less useful and callers of the library can't simply propagate errors. Instead they will likely create their own error types that include the function name in some way or another.

ionut-arm commented 1 year ago

Hey,

Sorry for the awful delay in replying. Yes, that sounds quite useful, though I'm not sure I fully understand what you'd like tracked. The issue is mostly that the native API of cryptoki does not necessarily correspond 1-to-1 to PKCS11 operations, so we wouldn't be able to assign a Function::... to every public operation in cryptoki (and we sometimes have multiple methods calling the same PKCS11 operation, which would also confuse things). But what we could do is for every failure coming from the actual PKCS11 backend, we could also pass along the Function::... which generated it (and add that to the Display of Error). Would that be enough to cover your use-cases?

The Pkcs11 variant of the Error enum would thus look like Pkcs11(RvError, Function),.

ionut-arm commented 1 year ago

Hmm, looking through the variants, I think Function would also make sense on NullFunctionPointer - the other seem to be fairly operation-agnostic.

ellerh commented 1 year ago

The Pkcs11 variant of the Error enum would thus look like Pkcs11(RvError, Function)

Yes, that would be perfect.