| Message ID | 20240802202606.12767-2-James.Bottomley@HansenPartnership.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | openssl_tpm2_engine: Add attestation functions for primary keys | expand |
On Fri Aug 2, 2024 at 11:25 PM EEST, James Bottomley wrote: > Now that we're going to be using the NULL primary to salt sessions, > the Intel TSS shim needs fixing to cope with this. In the Intel TSS, > there are two internal handles representing NULL: ESYS_TR_NONE and > ESYS_TR_RH_NULL. We translate TPM_RH_NULL to ESYS_TR_NONE because Can you split this into two paragraphs. I'm lost why it has two representations. > most of the time it does mean no value. However, for the NULL primary > handle we must use ESYS_TR_RH_NULL, so check for that specific case > and fix it. Additionally remove the intel_handle() code which was > supposed to do this: it's unused because 0 is never passed in as a > handle number. > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> > --- > src/include/intel-tss.h | 18 +++++------------- > 1 file changed, 5 insertions(+), 13 deletions(-) > > diff --git a/src/include/intel-tss.h b/src/include/intel-tss.h > index 1870b4e..5b8db20 100644 > --- a/src/include/intel-tss.h > +++ b/src/include/intel-tss.h > @@ -251,14 +251,6 @@ intel_sess_helper(TSS_CONTEXT *tssContext, TPM_HANDLE auth, TPMA_SESSION flags) > TPMA_SESSION_CONTINUESESSION | flags); > } > > -static inline TPM_HANDLE > -intel_handle(TPM_HANDLE h) > -{ > - if (h == 0) > - return ESYS_TR_NONE; > - return h; > -} > - > static inline void > TSS_Delete(TSS_CONTEXT *tssContext) > { > @@ -937,8 +929,10 @@ tpm2_CreatePrimary(TSS_CONTEXT *tssContext, TPM_HANDLE primaryHandle, > TPM2B_PUBLIC *opub; > TPM_RC rc; > > - /* FIXME will generate wrong value for NULL hierarchy */ > - primaryHandle = intel_handle(primaryHandle); > + > + /* TPM_RH_NULL is mapped to ESYS_TR_NONE, which won't work here */ I simply don't understand what E > + if (primaryHandle == TPM_RH_NULL) > + primaryHandle = INT_TPM_RH_NULL; > > outsideInfo.size = 0; > creationPcr.count = 0; > @@ -993,9 +987,7 @@ tpm2_StartAuthSession(TSS_CONTEXT *tssContext, TPM_HANDLE tpmKey, > TPM_HANDLE *sessionHandle, > const char *bindPassword) > { > - bind = intel_handle(bind); > - tpmKey = intel_handle(tpmKey); > - if (bind != ESYS_TR_NONE) > + if (bind != TPM_RH_NULL) > intel_auth_helper(tssContext, bind, bindPassword); > > return Esys_StartAuthSession(tssContext, tpmKey, bind, ESYS_TR_NONE, BR, Jarkko
On Sat, 2024-08-03 at 20:08 +0300, Jarkko Sakkinen wrote: > On Fri Aug 2, 2024 at 11:25 PM EEST, James Bottomley wrote: > > Now that we're going to be using the NULL primary to salt sessions, > > the Intel TSS shim needs fixing to cope with this. In the Intel > > TSS, there are two internal handles representing NULL: ESYS_TR_NONE > > and ESYS_TR_RH_NULL. We translate TPM_RH_NULL to ESYS_TR_NONE > > because > > Can you split this into two paragraphs. > > I'm lost why it has two representations. Well, I actually have no idea why the Intel TSS has two representations for *every* handle: an internal one (specific to the TSS) and an external one that everyone uses, like 81000001 or 40000007. As far as I can see it just adds pointless complexity to the coding. The IBM TSS only has one, so for code which works with both, the shim has to transform between internal and external handle representations before sending the command onward to the Intel TSS. Even more mysteriously the Intel TSS has three representations for the NULL handle: an internal one, an external one (40000007) and one you use for an empty session (ESYS_TR_NONE). The IBM TSS uses TPM_RH_NULL for all three so you can't just translate from external to internal you have to know if you're using the handle for a session or a hierarchy as well. James
On Sat Aug 3, 2024 at 8:51 PM EEST, James Bottomley wrote: > On Sat, 2024-08-03 at 20:08 +0300, Jarkko Sakkinen wrote: > > On Fri Aug 2, 2024 at 11:25 PM EEST, James Bottomley wrote: > > > Now that we're going to be using the NULL primary to salt sessions, > > > the Intel TSS shim needs fixing to cope with this. In the Intel > > > TSS, there are two internal handles representing NULL: ESYS_TR_NONE > > > and ESYS_TR_RH_NULL. We translate TPM_RH_NULL to ESYS_TR_NONE > > > because > > > > Can you split this into two paragraphs. > > > > I'm lost why it has two representations. > > Well, I actually have no idea why the Intel TSS has two representations > for *every* handle: an internal one (specific to the TSS) and an > external one that everyone uses, like 81000001 or 40000007. As far as I > can see it just adds pointless complexity to the coding. The IBM TSS > only has one, so for code which works with both, the shim has to > transform between internal and external handle representations before > sending the command onward to the Intel TSS. Is it possible to address this complexity and move into a single representation? I.e. use external presentation all the way. > > Even more mysteriously the Intel TSS has three representations for the > NULL handle: an internal one, an external one (40000007) and one you > use for an empty session (ESYS_TR_NONE). The IBM TSS uses TPM_RH_NULL > for all three so you can't just translate from external to internal you > have to know if you're using the handle for a session or a hierarchy as > well. Same question applies to this too. > James I'm doing my own TPM2 stack with Rust, which just re-implements the functionality of my old tpm2-scripts and tpm2.py module as nice small app called tpm2-cli. It will be more use case based interface than than protocol spec as a software interface. Main goal for now is to get this whole flow into it (with varying parameters for the key) as single function: tpm2_createprimary --hierarchy o -G ecc -c owner.txt tpm2_evictcontrol -c owner.txt 0x81000001 openssl ecparam -name prime256v1 -genkey -noout -out private.pem tpm2_import -C 0x81000001 -G ecc -i private.pem -u key.pub -r key.priv tpm2_encodeobject -C 0x81000001 -u key.pub -r key.priv -o key.priv.pem openssl asn1parse -inform pem -in key.priv.pem -noout -out key.priv.der BR, Jarkko
On Sat, 2024-08-03 at 22:31 +0300, Jarkko Sakkinen wrote: > On Sat Aug 3, 2024 at 8:51 PM EEST, James Bottomley wrote: > > On Sat, 2024-08-03 at 20:08 +0300, Jarkko Sakkinen wrote: > > > On Fri Aug 2, 2024 at 11:25 PM EEST, James Bottomley wrote: > > > > Now that we're going to be using the NULL primary to salt > > > > sessions, the Intel TSS shim needs fixing to cope with this. > > > > In the Intel TSS, there are two internal handles representing > > > > NULL: ESYS_TR_NONE and ESYS_TR_RH_NULL. We translate > > > > TPM_RH_NULL to ESYS_TR_NONE because > > > > > > Can you split this into two paragraphs. > > > > > > I'm lost why it has two representations. > > > > Well, I actually have no idea why the Intel TSS has two > > representations for *every* handle: an internal one (specific to > > the TSS) and an external one that everyone uses, like 81000001 or > > 40000007. As far as I can see it just adds pointless complexity to > > the coding. The IBM TSS only has one, so for code which works with > > both, the shim has to transform between internal and external > > handle representations before sending the command onward to the > > Intel TSS. > > Is it possible to address this complexity and move into a single > representation? I.e. use external presentation all the way. Yes, that's what the current code does. It began life as pure IBM TSS so it used what the Intel TSS would consider as all external handle representations. The external to internal shift (and back) happens inside the TSS shim. > > Even more mysteriously the Intel TSS has three representations for > > the NULL handle: an internal one, an external one (40000007) and > > one you use for an empty session (ESYS_TR_NONE). The IBM TSS uses > > TPM_RH_NULL for all three so you can't just translate from external > > to internal you have to know if you're using the handle for a > > session or a hierarchy as well. > > Same question applies to this too. Remember this is just fixing the Intel TSS Shim. The fact that we have to use three different handles for NULL isn't visible outside the shim, so a consumer of these APIs just uses TPM_RH_NULL everywhere. The fix is that the Intel TSS Shim was using the wrong handle for some operations. > > > James > > I'm doing my own TPM2 stack with Rust, which just re-implements the > functionality of my old tpm2-scripts and tpm2.py module as nice small > app called tpm2-cli. > > It will be more use case based interface than than protocol spec as > a software interface. Main goal for now is to get this whole flow > into it (with varying parameters for the key) as single function: > > tpm2_createprimary --hierarchy o -G ecc -c owner.txt > tpm2_evictcontrol -c owner.txt 0x81000001 > openssl ecparam -name prime256v1 -genkey -noout -out private.pem > tpm2_import -C 0x81000001 -G ecc -i private.pem -u key.pub -r > key.priv > tpm2_encodeobject -C 0x81000001 -u key.pub -r key.priv -o > key.priv.pem > openssl asn1parse -inform pem -in key.priv.pem -noout -out > key.priv.der Well, that's simply the library API. That's actually what the IBM TSS uses. I think the Library API is by far the simplest TPM API to use, so beyond trying to sort out the complexity of getting a compatibility shim I've not really used the Intel TSS. I think the Intel TSS is based on the ESAPI document which introduces the handle conversion complexity. I haven't read the ESAPI spec, I just tried to reverse engineer it (from the source) into supporting an API which can be compatible with the library spec/IBM TSS. James
On Sat Aug 3, 2024 at 10:47 PM EEST, James Bottomley wrote: > On Sat, 2024-08-03 at 22:31 +0300, Jarkko Sakkinen wrote: > > On Sat Aug 3, 2024 at 8:51 PM EEST, James Bottomley wrote: > > > On Sat, 2024-08-03 at 20:08 +0300, Jarkko Sakkinen wrote: > > > > On Fri Aug 2, 2024 at 11:25 PM EEST, James Bottomley wrote: > > > > > Now that we're going to be using the NULL primary to salt > > > > > sessions, the Intel TSS shim needs fixing to cope with this. > > > > > In the Intel TSS, there are two internal handles representing > > > > > NULL: ESYS_TR_NONE and ESYS_TR_RH_NULL. We translate > > > > > TPM_RH_NULL to ESYS_TR_NONE because > > > > > > > > Can you split this into two paragraphs. > > > > > > > > I'm lost why it has two representations. > > > > > > Well, I actually have no idea why the Intel TSS has two > > > representations for *every* handle: an internal one (specific to > > > the TSS) and an external one that everyone uses, like 81000001 or > > > 40000007. As far as I can see it just adds pointless complexity to > > > the coding. The IBM TSS only has one, so for code which works with > > > both, the shim has to transform between internal and external > > > handle representations before sending the command onward to the > > > Intel TSS. > > > > Is it possible to address this complexity and move into a single > > representation? I.e. use external presentation all the way. > > Yes, that's what the current code does. It began life as pure IBM TSS > so it used what the Intel TSS would consider as all external handle > representations. The external to internal shift (and back) happens > inside the TSS shim. Ah, right, OK now I'm on page, thank you. > > > > Even more mysteriously the Intel TSS has three representations for > > > the NULL handle: an internal one, an external one (40000007) and > > > one you use for an empty session (ESYS_TR_NONE). The IBM TSS uses > > > TPM_RH_NULL for all three so you can't just translate from external > > > to internal you have to know if you're using the handle for a > > > session or a hierarchy as well. > > > > Same question applies to this too. > > Remember this is just fixing the Intel TSS Shim. The fact that we have > to use three different handles for NULL isn't visible outside the shim, > so a consumer of these APIs just uses TPM_RH_NULL everywhere. The fix > is that the Intel TSS Shim was using the wrong handle for some > operations. OK, got it, thanks. BR, Jarkko
diff --git a/src/include/intel-tss.h b/src/include/intel-tss.h index 1870b4e..5b8db20 100644 --- a/src/include/intel-tss.h +++ b/src/include/intel-tss.h @@ -251,14 +251,6 @@ intel_sess_helper(TSS_CONTEXT *tssContext, TPM_HANDLE auth, TPMA_SESSION flags) TPMA_SESSION_CONTINUESESSION | flags); } -static inline TPM_HANDLE -intel_handle(TPM_HANDLE h) -{ - if (h == 0) - return ESYS_TR_NONE; - return h; -} - static inline void TSS_Delete(TSS_CONTEXT *tssContext) { @@ -937,8 +929,10 @@ tpm2_CreatePrimary(TSS_CONTEXT *tssContext, TPM_HANDLE primaryHandle, TPM2B_PUBLIC *opub; TPM_RC rc; - /* FIXME will generate wrong value for NULL hierarchy */ - primaryHandle = intel_handle(primaryHandle); + + /* TPM_RH_NULL is mapped to ESYS_TR_NONE, which won't work here */ + if (primaryHandle == TPM_RH_NULL) + primaryHandle = INT_TPM_RH_NULL; outsideInfo.size = 0; creationPcr.count = 0; @@ -993,9 +987,7 @@ tpm2_StartAuthSession(TSS_CONTEXT *tssContext, TPM_HANDLE tpmKey, TPM_HANDLE *sessionHandle, const char *bindPassword) { - bind = intel_handle(bind); - tpmKey = intel_handle(tpmKey); - if (bind != ESYS_TR_NONE) + if (bind != TPM_RH_NULL) intel_auth_helper(tssContext, bind, bindPassword); return Esys_StartAuthSession(tssContext, tpmKey, bind, ESYS_TR_NONE,
Now that we're going to be using the NULL primary to salt sessions, the Intel TSS shim needs fixing to cope with this. In the Intel TSS, there are two internal handles representing NULL: ESYS_TR_NONE and ESYS_TR_RH_NULL. We translate TPM_RH_NULL to ESYS_TR_NONE because most of the time it does mean no value. However, for the NULL primary handle we must use ESYS_TR_RH_NULL, so check for that specific case and fix it. Additionally remove the intel_handle() code which was supposed to do this: it's unused because 0 is never passed in as a handle number. Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> --- src/include/intel-tss.h | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-)