wolfTPM is not thread-safe (`gActiveTPM` ought to be a thread-local)

[nicowilliams]

static TPM2_CTX* gActiveTPM;

TPM2_CTX* TPM2_GetActiveCtx(void)
{
    return gActiveTPM;
}

void TPM2_SetActiveCtx(TPM2_CTX* ctx)
{
    gActiveTPM = ctx;
}

This is not thread-safe.

There's probably other thread-safety issues, and I'm not looking for them. I just wanted to know what the wolfTPM APIs look like, and noticed the lack of any context arguments, and then this, and that's as far as I want to look.

I think it would be totally fine to have a single explicit context argument to all TPM2 functions, but at the very least any hidden context should be thread-local.

Not that anyone is likely to need wolfTPM to be usable with multiple active threads in one process, but that it's easier to build a library to be usable that way than to fix it to be that way later.

Thinking of it, I can imagine scenarios where I'd want to support threads. Without a high-performance TPM with lots of resources, those scenarios are contrived. Perhaps one might build applications or libraries that use remote software TPMs running on physically secure, fast, HSM-like hardware, and then thread-safety might really matter.

One use case where thread-safety is not that important would be PAM. PAM can be used in threaded applications, but generally such applications a) use the same thread for all PAM calls, b) only maintain one active PAM handle, so wolfTPM not being exactly thread-safe wouldn't preclude using it in a PAM.

At any rate, I highly recommend designing libraries to be thread-safe by design.

[dgarske]

Hi @nicowilliams ,

Thank you for your feedback! I agree have a Thread Local Storage (TLS) option on the TPM2_CTX makes sense. I've logged this feature request.

We originally designed this library for bare-metal and single threaded applications. Later we did extend support for multiple-threads and processing using the --enable-tislock (WOLFTPM_TIS_LOCK) option, which enables a Linux Named Semaphore for locking access to SPI/I2C device for concurrent access between processes.

Can you tell us more about your use of wolfTPM?

Thanks, David Garske, wolfSSL

[nicowilliams]

Later we did extend support for multiple-threads and processing using the --enable-tislock (WOLFTPM_TIS_LOCK) option, which enables a Linux Named Semaphore for locking access to SPI/I2C device for concurrent access between processes.

Using a TPM resource manager (itself speaking the TPM protocol) would address concurrent access to the same TPM. That "tislock" essentially lets the library function as a limited resource manager, but the callers will still need to re-create/re-load key objects as needed, while a resource manager would do that automatically.

Can you tell us more about your use of wolfTPM?

Not at this time.

[dgarske]

@jpbland1 . Will you look at adding a Thread Local Storage (see THREAD_LS_T in wolfSSL) option to wolfTPM for the gActiveTPM?

[jpbland1]

Thanks for the heads up @nicowilliams. Here is a link the pr with the fix https://github.com/wolfSSL/wolfTPM/pull/253. It's been merged in so you can pull from master as well.

[dgarske]

For reference this was fixed in https://github.com/wolfSSL/wolfTPM/pull/253