tpm2-software / tpm2-tss

OSS implementation of the TCG TPM2 Software Stack (TSS2)
https://tpm2-software.github.io
BSD 2-Clause "Simplified" License
730 stars 359 forks source link

Define protocol for dlopening and querying TCTI for common init #690

Closed flihp closed 6 years ago

flihp commented 6 years ago

The common initialization function should have a function prototype something like:

TSS2_RC (*Tss2_Tcti_Init) (TSS2_TCTI_CONTEXT *context, size_t *size, char *config);

To allow users to discover information about the TCTI module we include a common info function with the prototype:

TSS2_RC (*Tss2_Tcti_Info)(TSS2_TCTI_INFO *info);

The TCTI header (tss2_tcti.h) should define a type for each, as well as string representations of the symbol for each:

Currently we need an interface to query a TCTI for information about the TCTI. This should include probably 3 fields: 1) name - simple string name for the module( ex. tcti-device) 2) description - a sentence or two describing what the TCTI module is / does 3) config_help - a help message describing the format of the configuration string Additionally a function pointer to the modules common init function.

Each TCTI module should expose these symbols if the intended use case allows (may not make sense for things like UEFI or SGX). Not sure what to do about Windows LoadLibrary / LoadLibraryEx?

williamcroberts commented 6 years ago

I would just export that info struct, and in the info struct have a function pointer to the init routine. Less dl lookups....

We would also probably want to implement a generic tcti-dynamic module (like tcti-abrmd) but takes a "path:argument" as the configuration string. Loads the module provided by path, and passes argument as the configuration string.

This way projects don't have to keep implementing this logic every-time they need dynamic tcti support. We could use this in the tools.

williamcroberts commented 6 years ago

Oh and getting the initialization to a common interface means that I can drop all my tcti wrappers in the tools, which is a nice win.

flihp commented 6 years ago

Yeah getting all of the contortions we had to do to support multiple TCTI libraries is what this is all about. The factory object I had to build for the tabrmd was more work than it's worth. I hacked something up that exposed the symbol for the info struct directly from the .so file. It definitely works but with the compiler flags dialed up it starts to complain about the unused symbol at compile time. Ignoring this error starts to get into the realm of compiler specific attributes or disabling the warning.

How about we just expose the static object from the .so using a function that just returns a pointer to it:

static TSS2_TCTI_INFO tcti_info = { ... };
TSS2_TCTI_INFO* Tss2_Tcti_Info (void) { return &tcti_info; };

With the info struct holding a reference to the init function then there's only ever 1 symbol look up required. It will however require one additional line of code to invoke the info function. If we ever want to change the Exposing any symbols beyond the Tss2_Tcti_Info function wouldn't be required, but it may be useful to expose a version of the init function using a TCTI specific symbol like Tss2_Tcti_<name>_Init for those that want to link direclty against the shared object. I imagine that these functions will replace the current TCTI specific init functions in the next release since we're breaking API anyways.

I've got topic branches for the tss and tools with a minimal implementation. The tss has the required symbols / structures and functions implemented in the two TCTIs. The tools has two new ... tools, one tha prettyprints the info struct, the other take a path to a TCTI so and config string and uses this new interface to instantiate a TCTI context:

Is this middle ground reasonable @williamcroberts?

williamcroberts commented 6 years ago

That looks fine, one DL lookup and gets the compiler to stop complaining.

flihp commented 6 years ago

Great. I just realized that my branches aren't building on account of a broken dependency. I'll get those fixed up and turn this into a PR. The PR queue has backed up a bit so I've gotta clean that up before we'll make much progress on this in the code.