tpm2-software / tpm2-pkcs11

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

Add support for C_DestroyObject #144

Closed PeterHuewe closed 5 years ago

PeterHuewe commented 5 years ago

Since we (soon) can create keys, it would be great to also get rid of them again.

williamcroberts commented 5 years ago

@PeterHuewe what should the behavior be if C_SignInit(handle=42) is going on when a call to C_DestroyObject(handle=42) is invoked?

Should subsequent multi-part object operations, like sign start failing or should C_DestroyObject() fail?

We could make C_DestroyObject() remove the tobject from the in-memory list, and track all removed objects in a to-be-deleted list and defer the del/free to C_Finalize(). This way any references to the object are stable in the lifetime of the application.

The spec didn't have anything on this I could find.

PeterHuewe commented 5 years ago

Hi, The way I interpret the standard, is that once C_SignInit is called, "[t]he signature operation is active until the application uses a call to C_Sign or C_SignFinal to actually obtain the signature". So if an application calls C_DestroyObject after C_SignInit, C_Sign must still work.

C_DestroyObject could be implemented in a way [1] like you described (marking stuff for deletion and running a 'garbage collector' once the last reference is used.)

or it could simply fail - either with the general [2] CKR_FUNCTION_FAILED (possible according to spec), or [3] CKR_OPERATION_ACTIVE (not specified as a return value for C_DestroyObject, but for similar functions like C_FindObject).

Option 1 is probably the cleanest way, but most likely also the most complicated and most error prone. Also it might lead to some interesting cases, where the caller just wants to retrieve the signature size (i.e. calling with a NULL pointer), getting the result back, and suddenly (on the real operation) the handle is gone.

Option 3 sounds like the 'most logical' return value, but is not specified. -> Option 2 is probably the way to go. How likely is that a sane application removes the handle it currently does a sign operation on.

williamcroberts commented 5 years ago

How likely is that a sane application removes the handle it currently does a sign operation on.

It shouldn't be, but the fact the spec doesn't describe this situation and how to handle it is a glaring issue.

PeterHuewe commented 5 years ago

Correct - would be interesting how other pkcs11 libraries handle this situation.

williamcroberts commented 5 years ago

Looks like my decision to have public and private key objects point to the same backing tobject bit me in the a$$. But it's also tricky on how the TPM works. I need both the public and private portion, and you can always derive the public portion from the private anyways (if you actually had the key material). So C_Destroy(public) seems weird. Not sure what to do with it at that point, I could just hide it in the interface and never expose it.

williamcroberts commented 5 years ago

I guess what I need to do is kill the link and actually make public objects their own separate thing exposed in the interface and call TPM2_LoadExternal on them. On C_Destroy() do the following:

private public trigger result new state
0 1 pub del del db entry 0 0
1 0 priv del del db entry 0 0
1 1 pub del mark pub hidden in db 1 0
1 1 priv del del priv portion of db entry - use load external 0 1
williamcroberts commented 5 years ago

Or better yet, after sleeping on it, manage it as two separate objects. When we create a db pair create one for the private portion (tpm pub/priv blobs) and one for the public portion (just pub, use tpm2_loadexternal).

This will be a db scheme back to a single attribute column.