Test parallel access

[daxpedda]

This PR just tests #260 in the CI. It might be turned into a useful test later down the line when the bug can be reproduced and fixed.

[daxpedda]

It is indeed reproducible in the CI:

      Running tests/parallel.rs (target/debug/deps/parallel-9574901f18cd4e9f)

running 2 tests
error: test failed, to rerun pass `-p cryptoki --test parallel`

Caused by:
  process didn't exit successfully: `/home/runner/work/rust-cryptoki/rust-cryptoki/target/debug/deps/parallel-9574901f18cd4e9f` (signal: 11, SIGSEGV: invalid memory reference)

[hug-dev]

Thanks for putting it together!

Does Rust run test1 and test2 as two different binaries? Or different threads? If that is the earlier, this could work as written in the documentation:

If several applications are using Cryptoki, each one should call C_Initialize. Every call to C_Initialize should (eventually) be succeeded by a single call to C_Finalize. See [PKCS11-UG] for further details.

But if that is the later then I agree that the initialize call of one of them should have returned an error :thinking: Could we double check that this is working correctly by having a test trying to initialize twice in a row?

[daxpedda]

Does Rust run test1 and test2 as two different binaries? Or different threads?

Each test is run in its own thread as part of the same process. See https://doc.rust-lang.org/1.86.0/cargo/commands/cargo-test.html#description.

Could we double check that this is working correctly by having a test trying to initialize twice in a row?

#[test]
fn test() {
    let (pkcs11, slot) = common::init_pins();
    let _session = pkcs11.open_rw_session(slot).unwrap();

    let (pkcs11, slot) = common::init_pins();
    let _session = pkcs11.open_rw_session(slot).unwrap();
}

This correctly always returns CryptokiAlreadyInitialized. Considering we can only recreate UB in a threaded scenario, I assume its some kind of race condition.

[Jakuje]

I think this can be an issue on three different levels:

• this library
• the pkcs11 module you are using
• the PKCS#11 specification

I did not manage to dig into the PKCS#11 specs deep enough, but there is a locking mechanism when you need to access a single token from different threads

The softhsm tests with your reproducer crash (I think interface functions do not have enough checks for threaded-access and finalize frees something they expect it will be around):

process didn't exit successfully: `/home/runner/work/rust-cryptoki/rust-cryptoki/target/debug/deps/parallel-9574901f18cd4e9f` (signal: 11, SIGSEGV: invalid memory reference)

but kryoptic just fails:

thread 'test1' panicked at cryptoki/tests/common.rs:41:52:
called `Result::unwrap()` on an `Err` value: Pkcs11(CryptokiNotInitialized, OpenSession)

This means that the tests got further, but the first test likely called c_finalize between the second one called the init_token() and open_session(). If you want to run run the tests in different threads, the initialization and finalization needs to be adjusted. Likely something like we do in kryoptic now:

• the TestToken makes sure the cryptoki is initialized (just once)
• for each test separate slot is created
• the finalize is called only when the last test ends

https://github.com/latchset/kryoptic/blob/main/src/tests/keys.rs#L11

This way, we are able to run the tests in parallel. But we have a bit more control of the token as we call some internals to create separate slot for each test.

With generic pkcs11 tokens, you will likely have issues if one thread will be creating objects and the other will be expecting different ones in the single slot, but I think it should still be possible to adjust the tests to work.

[hug-dev]

Closing since the related issue has been closed. Thanks!