Add pReserved and pkcs11 flags

[placintaalexandru]

#321: Add pReserved to CInitializeArgs and other flags as as stated in PKCS#11 initialize doc

[placintaalexandru]

There is still the docstring to change. I'll do that after I've confirmed the final state of CInitializeArgs

[hug-dev]

Thanks for the PR! As said with the comments I think we don't have to continue with the enum type as it is now but start from scratch with a more straightforward implementation :)

[hug-dev]

I think to start with a new abstraction of this type it would be nice to go with a "dumber" 1-1 mapping with the C type. I think it's nicer to start simple and straightforward and then improve over time depending on usage.

I mean the following:

• making CInitializeArgs a struct with the same fields as in the standard but no need to have pReserved for now (it can be set to null always in the From implementation).
• The CInitializeFlags flags could be created as a bitflags definition, similar than what is done for MechanismInfoFlags. That way it can expend easily with new flags in future versions.
• The CInitializeArgs basic constructor would take the five arguments as inputs (Option for the mutex functions)

[placintaalexandru]

I agree with the 1st point, but the purpose of the PR was to add the pReserved pointer. I see 2 ways here:

• allowing the pointer and using unsafe
• restraining the user to some known cryptokis that use pReserved and the type of those structs. We would create safe bindings to those structs. The pointer would be then obtained via Box::into_raw(Box::new(safe_object)) and we would have to manage that pointer

pub enum Reserved {
    None,
    IBM(IBMInitParams),
    Utimaco(UtimacoInitParams),
}

let p_reserved = Box::into_raw(Box::new(Reserved::IBM(...)));

Regarding points 2 and 3 I see the following problems:

• for point 2: one could set both CKF_OS_LOCKING_OK and CKF_LIBRARY_CANT_CREATE_OS_THREADS. Is that a valid situation?
• for point 3: all 4 mutex functions must be provided according to PKCS#11 docs. Allowing Option would add unnecessary runtime validation so that all match Some

[hug-dev]

but the purpose of the PR was to add the pReserved pointer.

Yes 🤦 ! I forgot the whole purpose of the related issue 😬

for point 2: one could set both CKF_OS_LOCKING_OK and CKF_LIBRARY_CANT_CREATE_OS_THREADS. Is that a valid situation?

I think the PKCS11 API does not prevent it so I guess we should not limit either... But agree that some combinations of those two are pretty weird.

for point 3: all 4 mutex functions must be provided according to PKCS#11 docs. Allowing Option would add unnecessary runtime validation that can be avoided

Let's see what we decide with the other comment to see how we proceed here

[placintaalexandru]

They mention that if CKF_LIBRARY_CANT_CREATE_OS_THREADS is set and the lib can't work with it then C_Initialize will return with code CKR_NEED_TO_CREATE_THREADS. So I guess I can go with bitflags

[hug-dev]

I think it would be nice to also create abstraction types for those functions, which would be safe abstractions over the CK_LOCKMUTEX, CK_UNLOCKMUTEX, CK_DESTROYMUTEX and CK_CREATEMUTEX types. They would take in and return abstracted types and be converted to the C function in our code.

[placintaalexandru]

yes, I agree with that. the thing is I do not know how they would look like (I've never had to pass those functions when i initialize cryptoki)

[hug-dev]

Those are defined here but I could not come up with a nice way to do this is pure Rust.
I was thinking of defining a trait Mutex with all those methods but not sure how one could define trait methods as types 🤔

I am wondering if to not make it easier for now we could ignore those functions? As in always set them to None and remove it as parameters. Until someone needs them and then we can think about how to do it?

Unless you come up with a good way to do this.