Thread Safety

[daxpedda]

AFAIK you can't initialize Cryptoki while one instance already is and you can't create sessions to the same slot. However, currently doing so doesn't always return CryptokiAlreadyInitialized or SessionExists, but instead sometimes causes various UBs.

Here is a simple reproducible integration test I created:

mod common;

#[test]
fn test1() {
    common::init_pins();
}

#[test]
fn test2() {
    common::init_pins();
}

This almost never succeeds in my environment.

[hug-dev]

Do you think it could be an error of the underlying PKCS11 implementation not returning those errors?

[daxpedda]

I didn't look into it, but I was just using SoftHSM. The integration test I've outlined above is using the same setup as in the CI in this repo.

I will make a PR to see if we can reproduce it in the CI here.

[bcheidemann]

Hey, FWIW I am also seeing thread unsafe behaviour when using cryptoki and SoftHSM (I haven't tried other libraries).

Sometimes I get:

signal: 11, SIGSEGV: invalid memory reference

And other times I get:

Function::Initialize: PKCS11 error: This value can only be returned by C_Initialize.  It means that the Cryptoki library has already been initialized (by a previous call to C_Initialize which did not have a matching C_Finalize call).

Is it intended that I should not instantiate Pkcs11 multiple times across different threads?

[Jakuje]

Did you have a look at my comment in #263 (comment)? Did it answer your questions? Or is there something else here to follow-up?

tldr is that the softhsm is broken. Using Initialize from different threads is probably ok, but you might need to work on some synchronization. If one of your threads will call Finalize, SoftHSM will crash.

[bcheidemann]

Did you have a look at my comment in #263 (comment)? Did it answer your questions? Or is there something else here to follow-up?

tldr is that the softhsm is broken. Using Initialize from different threads is probably ok, but you might need to work on some synchronization. If one of your threads will call Finalize, SoftHSM will crash.

Hey, thanks for the link - somehow I didn't see the PR or comment 🤦‍♂️

Anyway, yes this answers my question. I already implemented the required synchronisation primitives between posting this and seeing your (very swift) reply, but it's good to have a reference to compare my implementation with.

[ivozeba]

I think there are two separate questions that are being asked here:

1. Can I create multiple sessions to a slot?

Yes, the user guide under Section 2.6.1 General Guidance explicitly says:

An application may have one or more sessions with one or more tokens.

2. What is the expected behavior if I call C_Initialize twice?

The user guide under Section 2.6.8 Example of use of sessions appears to answer this as well:

Note that exactly one call to C_Initialize should be made for each application (as opposed to one call for every thread, for example).

Now here's the troubling part for an integrator:

a. the specification does define a very specific return value:

CKR_CRYPTOKI_ALREADY_INITIALIZED: This value can only be returned by C_Initialize. It means that the Cryptoki library has already been initialized (by a previous call to C_Initialize which did not have a matching C_Finalize call).

b. I remember reading Thales documentation and recalled the following:

If the system is currently uninitialized, this function will perform a full initialization. This means that any configuration changes since the last full initialization will now take effect. If the system is already initialized, this function will simply prepare it for the new application.

Ultimately it's up to the cryptoki library to determine what occurs:

• SoftHSM doesn't deal with multiple inits/sessions gracefully (forgive me, I didn't double check which one actually causes the crash)
• Some vendors will maintain state in their library/hsm and return CKR_CRYPTOKI_ALREADY_INITIALIZED
• Other vendors will ignore that it's already initialized and allow you to proceed

I am by no means an authoritative figure on the matter but given the outlined behaviors above, I don't think there should be any smart behavior in this project as whatever is chosen will be wrong for someone.

In case my analysis is sound, should this issue be closed?

[daxpedda]

In case my analysis is sound, should this issue be closed?

Yes.
If indeed SoftHSM is the problem here, then I also do not believe that cryptoki should compensate for that.

[hug-dev]

Thanks all and thank you @ivozeba for the summary ⭐ !