Conversation
This commits adds the methods ToTSS2() to AK and Key to convert those keys to tss2.TPMKey. It also adds GetTSS2Signer to the tpm that returns a wrapped signer using the TSS2 key, this wrapped signer opens and closes the tpm on each sign.
This commit allows to set the TPM channel to the tss2.Signer if this was already closed.
| } | ||
|
|
||
| // ToTSS2 gets the public and private blobs and returns a [*tss2.TPMKey]. | ||
| func (k *Key) ToTSS2(ctx context.Context) (*tss2.TPMKey, error) { |
There was a problem hiding this comment.
Somewhat related to my tpm.TPM remark: maybe a ToTSS2Signer also makes sense to add on Key and AK? I think it can get the reference to the tpm.TPM in one go that way?
There was a problem hiding this comment.
Are you saying to add a ToTSS2Signer or change this name to ToTSS2Signer?
We need a method to be able to transform a Key/AK to a *tss2.TPMKey, that type by itself is not a signer. And adding a ToTSS2Signer() doesn't really make a lot of sense, we already have GetSigner():
func (t *TPM) GetSigner(ctx context.Context, name string) (csigner crypto.Signer, err error) There was a problem hiding this comment.
GetSigner only works with for Keys natively stored behind the TPM facade and identified by name, though.
I was thinking about adding an additional ToTSS2Signer method, which would function similar to the (now) CreateTSS2Signer, but would be defined on the Key and AK, and then attach the reference to the tpm.TPM in that method immediately. Then you can get the signer without having to call CreateTSS2Signer explicitly.
| // GetTSS2Signer returns a crypto.Signer using the [tss2.TPMKey]. | ||
| func (t *TPM) GetTSS2Signer(ctx context.Context, key *tss2.TPMKey) (csigner crypto.Signer, err error) { |
There was a problem hiding this comment.
I think this is not a great extension of the (public) API for the tpm.TPM. The reason is because it's now possible to provide a tss2.TPMKey that may or may not be (actually) backed by the tpm.TPM instance belonging to the TPM the TSS2 key was created for. Before this change, the tpm.TPM would be more of a facade to using the TPM, abstracting the underlying details such as key identifiers, storage format and attestation functionality.
I understand that it's useful to get a reference to the TPMs rwc, but I think it's nicer to turn this dependency around? I think you'll still need it to be part of the tpm package, but it doesn't have to be a method on the tpm.TPM. Something like Attach(key, tpm) or AttachTSS2(key, tpm), backed by roughly this method, but made private.
Another reason I think that's nicer is because we might still be able to implement a "full TSS2 format storage" that sits behind the tpm.TPM facade with all details abstracted.
There was a problem hiding this comment.
I don't get the name Attach why not just:
func CreateTSS2Signer(ctx context.Context, t *TPM, key *tss2.TPMKey) (csigner crypto.Signer, err error)There was a problem hiding this comment.
Changed to CreateTSS2Signer with 3f48274
There was a problem hiding this comment.
It's just about naming things, and about making clear what it does.
I thought of Attach because the tss2.Key "attaches" to the tpm.TPM's rwc, which makes it a complete crypto.Signer. The tss2.Key is (mostly) something external to the tpm.TPM (currently), so it somehow needs to be connected to it. It's also sufficiently different from the other methods, because it's a bit of a special case. Hence Attach.
CreateTSS2Signer at the package level is better than what it was, but I don't like that it makes TSS2 part of the top level public API surface. To me, at the moment, it's more of a utility method required because of how the tpm package is structured. I would also consider it experimental atm. Not opposed to adding it like this; it's just that I'm a bit in doubt about the right API until we have a clearer path for the native TSS2 storage.
| // SetTPM allows to change the TPM channel. This operation is useful if the | ||
| // channel set in [CreateSigner] is closed and opened again before calling [Signer.Sign]. | ||
| // | ||
| // # Experimental | ||
| // | ||
| // Notice: This API is EXPERIMENTAL and may be changed or removed in a later | ||
| // release. | ||
| func (s *Signer) SetTPM(rw io.ReadWriter) { |
There was a problem hiding this comment.
Maybe call this SetCommandChannel instead? Although it's not entirely the same as attest.CommandChannel, but it sort of has that meaning in this context.
| } | ||
|
|
||
| // ToTSS2 gets the public and private blobs and returns a [*tss2.TPMKey]. | ||
| func (ak *AK) ToTSS2(ctx context.Context) (*tss2.TPMKey, error) { |
| // - name=<name>: specify the name to identify the key with | ||
| // - path=<file>: specify the TSS2 PEM file to use |
There was a problem hiding this comment.
I'll think about this some more w.r.t. what it means for the tpm.TPM storage implementation, because there are files there too. I think adding path is OK as long as it's clear that those are for keys outside of the TPM storage directory (usually). I would also like to keep the option open for a "native TSS2 storage".
There was a problem hiding this comment.
This is similar to the path used in softkms, where you can reference a file using softkms:path=/dir/to/file.pem. So yes, it's a file outside the TPM storage.
Yes, it also makes sense to create a native TSS2 storage, but we need to think a little bit more about it, because we will need to store certificates too. But once is done I imagine it will be similar to the current one and we will be using name.
There was a problem hiding this comment.
The simplest approach might be to have a separate file (with the same or a derived name) with the cert next to the TSS2 PEM key. And possibly also a file with additional metadata, which could hold creation / attestation data.
This commit renames SetTPM to SetCommandChannel and removes GetTSS2Signer from TPM and creates tpm.CreateTSS2Signer.
This commit fixes a check like `resp.PrivateKey != nil` ton tpmkms CreateKey, because if tss2 is not passed it was (*tss2.TPMKey)(nil) instead of nil.
Description
This PR adds the methods
ToTSS2()totpm.AKandtpm.Keyto convert those keys totss2.TPMKey.It also adds the
GetTSS2Signer()method to thetpm.TPM, this will return a wrapped signer using the TSS2 key. This wrapped signer opens and closes the TPM on each sign.And finally, it adds support for getting the
tss2.TPMKeyontpmkms.CreateKey, and adds support to read TSS2 PEM files totpmkms.GetPublicKeyandtpmkms.Createsigner.