search

tpm2-software / tpm2-pkcs11

A PKCS#11 interface for TPM2 hardware
https://tpm2-software.github.io
Other
271 stars 106 forks source link

List of objects is not refreshed after import via tpm2_ptool #784

Open pepipox opened 2 years ago

pepipox commented 2 years ago

Hello The list of objects present in the database is only loaded for a given application when calling C_Initialize. When doing an import of an object via tpm2_ptool, the database is updated, but the list of objects is not updated until an application using the same database is restarted. It would be very interesting that the list becomes refreshed whenever C_FindObjectInit is called, in this way the DB and the list on a given app are kept in sync.

williamcroberts commented 2 years ago

Looks like we can use https://www.sqlite.org/c3ref/update_hook.html to get a notification that we need to reload.

pepipox commented 2 years ago

Hi @williamcroberts I am not sure of the internal structure of TPM2-PKCS11, but I suppose no internal thread is running to receive the notification for reloading, or am I wrong?

I believe what should be done, is save in some sort of global structure the number of objects present in the DB when C_Initialize is called. Then, whenever C_FindInit is called, check the number of objects in the DB and then reload the newest components, what do you think?

williamcroberts commented 2 years ago

The documentation seems to invoke the callback on a sqlite3_prepare_v2() and sqlite3_step() call. So it seems to be in that thread. Currently, as you point out we load objects early on. We could defer building the object list to object_find in object.c. Which then presents us with two options:

  1. On all C_FindObjectsInit calls, destroy and reload the object list unconditionally.
  2. On all C_FindObjectsInit calls, do a small query to see if the callback is invoked, have the callback set a flag and if the flag is tripped reload the object list.
pepipox commented 2 years ago

Hi @williamcroberts The approach 2 sounds reasonable for me, how do we proceed?

williamcroberts commented 2 years ago

Hi @williamcroberts The approach 2 sounds reasonable for me, how do we proceed?

Do you want to work on it? If so just make those changes and upload a PR.

pepipox commented 2 years ago

I will look into it later and get back to you.

pepipox commented 2 years ago

Hi @williamcroberts I have been working on this issue for a few days and would like to comment.

If I call sqlite3_update_hook in one process and then upgrade the DB via a different process, the hook function does not get called. So using that approach will not work.

I have tried approach 1. you suggested and it works, however, I do have an observation regarding thread safety and possible issues with accessing.

Imagine I call

C_FindObjectsInit C_FindObjects C_FindObjectsFinal

I work with the handles returned in FindObjects, but once I call C_FindObjectsInit again, there will be a small interval between when the list is destroyed and recreated again. Is this thread safe? will the behavior be OK? I have the patches ready but would like to get some comments before, thank you.

williamcroberts commented 2 years ago

Hi @williamcroberts I have been working on this issue for a few days and would like to comment.

If I call sqlite3_update_hook in one process and then upgrade the DB via a different process, the hook function does not get called. So using that approach will not work.

Oh darn, well thanks for looking at that.

I have tried approach 1. you suggested and it works, however, I do have an observation regarding thread safety and possible issues with accessing.

Imagine I call

C_FindObjectsInit C_FindObjects C_FindObjectsFinal

I work with the handles returned in FindObjects, but once I call C_FindObjectsInit again, there will be a small interval between when the list is destroyed and recreated again. Is this thread safe? will the behavior be OK? I have the patches ready but would like to get some comments before, thank you.

TL;DR: Don't worry it's already handled for you.

I have the ~threading~ locking set up here: https://github.com/tpm2-software/tpm2-pkcs11/blob/d00b6a7225821d1869c95e07d63aa8c97ddf5603/src/pkcs11.c#L488-L498

Essentially those macros expand to a token_lock and token_unlock call depending on how you initialize the library. Based on arguments to C_Initialize, one can use the built in locks, supply their own locks or disable locking and thus thread safety.

So each API call is locked on a per-token basis.

nwf-msr commented 1 year ago

Perhaps the correct thing to do is to use an on update trigger on the existing tables of the database to track timestamps or just version counters within the database itself? Then C_FindObjectsInit can run a very simple select to validate that its constructed cached view is still up to date or not.