| Message ID | 9F48E1A823B03B4790B7E6E69430724D9D7ADC91@EXCH2010A.sit.fraunhofer.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Oct 03, 2015 at 10:00:59AM +0000, Fuchs, Andreas wrote: > Hi Jarkko, > > [snip] > > diff --git a/security/keys/trusted.h b/security/keys/trusted.h > index ff001a5..fc32c47 100644 > --- a/security/keys/trusted.h > +++ b/security/keys/trusted.h > @@ -12,6 +12,13 @@ > #define TPM_RETURN_OFFSET 6 > #define TPM_DATA_OFFSET 10 > > +/* Transient object handles start from 0x80000000 in TPM 2.0, which makes it > + * a sane default. > + */ > + > +#define TPM1_SRKHANDLE 0x40000000 > +#define TPM2_SRKHANDLE 0x80000000 > + > #define LOAD32(buffer, offset) (ntohl(*(uint32_t *)&buffer[offset])) > #define LOAD32N(buffer, offset) (*(uint32_t *)&buffer[offset]) > #define LOAD16(buffer, offset) (ntohs(*(uint16_t *)&buffer[offset])) > > This TPM2_SRKHANDLE is unfortunately wrong. > > Transient handles are assigned and returned by the TPM following the > commands TPM2_CreatePrimary, TPM2_LoadObject and TPM2_ContextLoad. You > can only use transient handles as returned by the TPM in order to > refer to the corresponding object created inside the TPM via these > commands. They can never assumed to be constant. The fact that TPMs > return 0x80000000 for the first loaded Object and 0x80000001 for the > second is merely a coincidence... ;-) > > TPM2 also has no (single) SRK anymore. You have to create your own SRK > / Storage Primary Keys via TPM2_CreatePrimary and use the transient > handle returned from there. This however requires SH-authorization, > usually via Policy IMHO, so not easy to manage. So IMHO, this might be > something for the future but for the moment relying on a persistent > key would be better... > > For persistent SRKs it should become a convention to have those > around. Those handles start with 0x81000000 and the SRKs (or Storage > primary Keys) shall live within 0x81000000 to 0x8100FFFF (see > http://www.trustedcomputinggroup.org/resources/registry_of_reserved_tpm_20_handles_and_localities) > > I'd recommend to rely on the existence of a handle inside this range > with an empty auth-value. So maybe install a persistent SRK to > 0x81000000 via TPM2_EvictControl and then use this from within the > kernel for anything following. > P.S. You should check for the key's TPMA_OBJECT to have fixedTPM SET. > I don't know if there is an actual test for owner-generated SRK > testing. I'll ask around though... > > Note: you can query for handles in this range via > TPM2_GetCapability(TPM_CAP_HANDLES, 0x81000000) and then look for > fitting keys. > > > Feel free to discuss other approaches. I'm fully aware of all what you said. My take was to use 0x800000000 as a default value if you don't the handle ID explicitly in 'description' parameter of the add_key() syscall. > Cheers, > Andreas /Jarkko ------------------------------------------------------------------------------
On Sat, Oct 03, 2015 at 01:26:55PM +0300, Jarkko Sakkinen wrote: > On Sat, Oct 03, 2015 at 10:00:59AM +0000, Fuchs, Andreas wrote: > > Hi Jarkko, > > > > [snip] > > > > diff --git a/security/keys/trusted.h b/security/keys/trusted.h > > index ff001a5..fc32c47 100644 > > --- a/security/keys/trusted.h > > +++ b/security/keys/trusted.h > > @@ -12,6 +12,13 @@ > > #define TPM_RETURN_OFFSET 6 > > #define TPM_DATA_OFFSET 10 > > > > +/* Transient object handles start from 0x80000000 in TPM 2.0, which makes it > > + * a sane default. > > + */ > > + > > +#define TPM1_SRKHANDLE 0x40000000 > > +#define TPM2_SRKHANDLE 0x80000000 > > + > > #define LOAD32(buffer, offset) (ntohl(*(uint32_t *)&buffer[offset])) > > #define LOAD32N(buffer, offset) (*(uint32_t *)&buffer[offset]) > > #define LOAD16(buffer, offset) (ntohs(*(uint16_t *)&buffer[offset])) > > > > This TPM2_SRKHANDLE is unfortunately wrong. > > > > Transient handles are assigned and returned by the TPM following the > > commands TPM2_CreatePrimary, TPM2_LoadObject and TPM2_ContextLoad. You > > can only use transient handles as returned by the TPM in order to > > refer to the corresponding object created inside the TPM via these > > commands. They can never assumed to be constant. The fact that TPMs > > return 0x80000000 for the first loaded Object and 0x80000001 for the > > second is merely a coincidence... ;-) > > > > TPM2 also has no (single) SRK anymore. You have to create your own SRK > > / Storage Primary Keys via TPM2_CreatePrimary and use the transient > > handle returned from there. This however requires SH-authorization, > > usually via Policy IMHO, so not easy to manage. So IMHO, this might be > > something for the future but for the moment relying on a persistent > > key would be better... > > > > For persistent SRKs it should become a convention to have those > > around. Those handles start with 0x81000000 and the SRKs (or Storage > > primary Keys) shall live within 0x81000000 to 0x8100FFFF (see > > http://www.trustedcomputinggroup.org/resources/registry_of_reserved_tpm_20_handles_and_localities) > > > > I'd recommend to rely on the existence of a handle inside this range > > with an empty auth-value. So maybe install a persistent SRK to > > 0x81000000 via TPM2_EvictControl and then use this from within the > > kernel for anything following. > > P.S. You should check for the key's TPMA_OBJECT to have fixedTPM SET. > > I don't know if there is an actual test for owner-generated SRK > > testing. I'll ask around though... > > > > Note: you can query for handles in this range via > > TPM2_GetCapability(TPM_CAP_HANDLES, 0x81000000) and then look for > > fitting keys. > > > > > > Feel free to discuss other approaches. > > I'm fully aware of all what you said. My take was to use 0x800000000 as > a default value if you don't the handle ID explicitly in 'description' > parameter of the add_key() syscall. I don't know how much you've done user space code that uses TPM2 chip. I'm not saying I've written a lot of it but here's my experience. In Haswell/PTT you can have 3 transient handles at a time. How you use them is as follows: 1. Load/create your data to TPM filling transient handles starting from 0x80000000. 2. Do your sealing/unsealing/whatever. 3. Flush transient handles. For single handle use cases transient handle is in practice always 0x80000000 so it's very convenient to have that as the default value. I think you are looking at this too much from specification point of view. I've chosen the approach that is most convenient to use even though the handle does not have exactly the same semantics as with TPM 1.x. > > Cheers, > > Andreas > > /Jarkko /Jarkko ------------------------------------------------------------------------------
Hi Jarkko, thanks for the clearification... However, I'd recommend against doing so. Furthermore, if there is a resource-manager running in userspace, applications only get virtual handles and TPM might be empty actually... If that's what you're aiming for, I'd recommend passing the pointer to a context-saved-blob and have the kernel load the key this way. That ensures no problems with resource-manager and handle-mixups. Cheers, Andreas
On Sun, Oct 04, 2015 at 06:57:42PM +0000, Fuchs, Andreas wrote: > Hi Jarkko, > > thanks for the clearification... > > However, I'd recommend against doing so. > > Furthermore, if there is a resource-manager running in userspace, > applications only get virtual handles and TPM might be empty > actually... > > If that's what you're aiming for, I'd recommend passing the pointer to > a context-saved-blob and have the kernel load the key this way. That > ensures no problems with resource-manager and handle-mixups. TPM 1.x interface has the same race if you do not use the default value for the 'keyhandle' option. In practice a processs in TCB (or root) would do all the keyctl magic so I do not see huge issue here. It can be orchestrated by the OS/distribution. From my point of view you are over-engineering in wrong place. It would be easy to add a way to provide the sealing key as blob later on if the simple approach chosen would not be sufficient. I'm confident that for 99% of all real-world use cases the interface provided by the patch set is sufficient. > Cheers, > Andreas /Jarkko > From: Jarkko Sakkinen [jarkko.sakkinen@linux.intel.com] > Sent: Saturday, October 03, 2015 12:26 > To: Fuchs, Andreas > Cc: tpmdd-devel@lists.sourceforge.net; linux-kernel@vger.kernel.org; David Howells; gregkh@linuxfoundation.org; open list:KEYS-TRUSTED; open list:KEYS-TRUSTED; James Morris; David Safford; akpm@linux-foundation.org; Serge E. Hallyn > Subject: Re: [tpmdd-devel] [PATCH 4/4] keys, trusted: seal/unseal with TPM 2.0 chips > > On Sat, Oct 03, 2015 at 10:00:59AM +0000, Fuchs, Andreas wrote: > > Hi Jarkko, > > > > [snip] > > > > diff --git a/security/keys/trusted.h b/security/keys/trusted.h > > index ff001a5..fc32c47 100644 > > --- a/security/keys/trusted.h > > +++ b/security/keys/trusted.h > > @@ -12,6 +12,13 @@ > > #define TPM_RETURN_OFFSET 6 > > #define TPM_DATA_OFFSET 10 > > > > +/* Transient object handles start from 0x80000000 in TPM 2.0, which makes it > > + * a sane default. > > + */ > > + > > +#define TPM1_SRKHANDLE 0x40000000 > > +#define TPM2_SRKHANDLE 0x80000000 > > + > > #define LOAD32(buffer, offset) (ntohl(*(uint32_t *)&buffer[offset])) > > #define LOAD32N(buffer, offset) (*(uint32_t *)&buffer[offset]) > > #define LOAD16(buffer, offset) (ntohs(*(uint16_t *)&buffer[offset])) > > > > This TPM2_SRKHANDLE is unfortunately wrong. > > > > Transient handles are assigned and returned by the TPM following the > > commands TPM2_CreatePrimary, TPM2_LoadObject and TPM2_ContextLoad. You > > can only use transient handles as returned by the TPM in order to > > refer to the corresponding object created inside the TPM via these > > commands. They can never assumed to be constant. The fact that TPMs > > return 0x80000000 for the first loaded Object and 0x80000001 for the > > second is merely a coincidence... ;-) > > > > TPM2 also has no (single) SRK anymore. You have to create your own SRK > > / Storage Primary Keys via TPM2_CreatePrimary and use the transient > > handle returned from there. This however requires SH-authorization, > > usually via Policy IMHO, so not easy to manage. So IMHO, this might be > > something for the future but for the moment relying on a persistent > > key would be better... > > > > For persistent SRKs it should become a convention to have those > > around. Those handles start with 0x81000000 and the SRKs (or Storage > > primary Keys) shall live within 0x81000000 to 0x8100FFFF (see > > http://www.trustedcomputinggroup.org/resources/registry_of_reserved_tpm_20_handles_and_localities) > > > > I'd recommend to rely on the existence of a handle inside this range > > with an empty auth-value. So maybe install a persistent SRK to > > 0x81000000 via TPM2_EvictControl and then use this from within the > > kernel for anything following. > > P.S. You should check for the key's TPMA_OBJECT to have fixedTPM SET. > > I don't know if there is an actual test for owner-generated SRK > > testing. I'll ask around though... > > > > Note: you can query for handles in this range via > > TPM2_GetCapability(TPM_CAP_HANDLES, 0x81000000) and then look for > > fitting keys. > > > > > > Feel free to discuss other approaches. > > I'm fully aware of all what you said. My take was to use 0x800000000 as > a default value if you don't the handle ID explicitly in 'description' > parameter of the add_key() syscall. > > > Cheers, > > Andreas > > /Jarkko ------------------------------------------------------------------------------
Hi Jarkko, /dev/tpm0 is single-open only. So if root wants to do anything there, you'd have to stop the tss-daemon first. Or do you see "keyctl magic" only happening in initrd ? I have to admit that I have no experience with the 1.2 trusted key features. Never used them. Is there software available that uses non-SRKs on 1.2 ? How did they solve the problem of a running TrouSerS-tcsd ? The main difference for 2.0 is the non-existence of the SRK. That's where I see the need for rethinking... Cheers, Andreas
On Mon, Oct 05, 2015 at 09:00:48AM +0000, Fuchs, Andreas wrote: > Hi Jarkko, > > /dev/tpm0 is single-open only. So if root wants to do anything there, > you'd have to stop the tss-daemon first. > Or do you see "keyctl magic" only happening in initrd ? > > I have to admit that I have no experience with the 1.2 trusted key > features. Never used them. Is there software available that uses > non-SRKs on 1.2 ? How did they solve the problem of a running > TrouSerS-tcsd ? > The main difference for 2.0 is the non-existence of the SRK. That's > where I see the need for rethinking... I see where you are getting and it is a valid queston. In the long run solution would be to have an access broker in the kernel so that you can open a separated context for transient handles if you need one and have all the eviction/loading magic in the kernel. However, this requires a lot of work, and for example for small single user embedded systems you would anyway want to disable it by using some compilation option (lets say CONFIG_TCG_TPM2_ACCESS_BROKER). Your proposal would only partly fix the issue because you need additional TPM2_Load in trusted.c in order to load the key before executing TPM2_Unseal operation. I'm not saying the solution is perfect but it works for notable coverage of systems (especially in the IoT side) and it doesn't prevent to implement access broker later on. I would be interested to get feedback from ChromeOS developers on this as they are using TPM quite extensively for user data encryption and various other use cases. > Cheers, > Andreas /Jarkko > ________________________________________ > From: Jarkko Sakkinen [jarkko.sakkinen@linux.intel.com] > Sent: Monday, October 05, 2015 10:37 > To: Fuchs, Andreas > Cc: tpmdd-devel@lists.sourceforge.net; linux-kernel@vger.kernel.org; David Howells; gregkh@linuxfoundation.org; open list:KEYS-TRUSTED; open list:KEYS-TRUSTED; James Morris; David Safford; akpm@linux-foundation.org; Serge E. Hallyn > Subject: Re: [tpmdd-devel] [PATCH 4/4] keys, trusted: seal/unseal with TPM 2.0 chips > > On Sun, Oct 04, 2015 at 06:57:42PM +0000, Fuchs, Andreas wrote: > > Hi Jarkko, > > > > thanks for the clearification... > > > > However, I'd recommend against doing so. > > > > Furthermore, if there is a resource-manager running in userspace, > > applications only get virtual handles and TPM might be empty > > actually... > > > > If that's what you're aiming for, I'd recommend passing the pointer to > > a context-saved-blob and have the kernel load the key this way. That > > ensures no problems with resource-manager and handle-mixups. > > TPM 1.x interface has the same race if you do not use the default value > for the 'keyhandle' option. > > In practice a processs in TCB (or root) would do all the keyctl magic so > I do not see huge issue here. It can be orchestrated by the > OS/distribution. From my point of view you are over-engineering in wrong > place. > > It would be easy to add a way to provide the sealing key as blob later > on if the simple approach chosen would not be sufficient. I'm confident > that for 99% of all real-world use cases the interface provided by the > patch set is sufficient. > > > Cheers, > > Andreas > > /Jarkko > > > From: Jarkko Sakkinen [jarkko.sakkinen@linux.intel.com] > > Sent: Saturday, October 03, 2015 12:26 > > To: Fuchs, Andreas > > Cc: tpmdd-devel@lists.sourceforge.net; linux-kernel@vger.kernel.org; David Howells; gregkh@linuxfoundation.org; open list:KEYS-TRUSTED; open list:KEYS-TRUSTED; James Morris; David Safford; akpm@linux-foundation.org; Serge E. Hallyn > > Subject: Re: [tpmdd-devel] [PATCH 4/4] keys, trusted: seal/unseal with TPM 2.0 chips > > > > On Sat, Oct 03, 2015 at 10:00:59AM +0000, Fuchs, Andreas wrote: > > > Hi Jarkko, > > > > > > [snip] > > > > > > diff --git a/security/keys/trusted.h b/security/keys/trusted.h > > > index ff001a5..fc32c47 100644 > > > --- a/security/keys/trusted.h > > > +++ b/security/keys/trusted.h > > > @@ -12,6 +12,13 @@ > > > #define TPM_RETURN_OFFSET 6 > > > #define TPM_DATA_OFFSET 10 > > > > > > +/* Transient object handles start from 0x80000000 in TPM 2.0, which makes it > > > + * a sane default. > > > + */ > > > + > > > +#define TPM1_SRKHANDLE 0x40000000 > > > +#define TPM2_SRKHANDLE 0x80000000 > > > + > > > #define LOAD32(buffer, offset) (ntohl(*(uint32_t *)&buffer[offset])) > > > #define LOAD32N(buffer, offset) (*(uint32_t *)&buffer[offset]) > > > #define LOAD16(buffer, offset) (ntohs(*(uint16_t *)&buffer[offset])) > > > > > > This TPM2_SRKHANDLE is unfortunately wrong. > > > > > > Transient handles are assigned and returned by the TPM following the > > > commands TPM2_CreatePrimary, TPM2_LoadObject and TPM2_ContextLoad. You > > > can only use transient handles as returned by the TPM in order to > > > refer to the corresponding object created inside the TPM via these > > > commands. They can never assumed to be constant. The fact that TPMs > > > return 0x80000000 for the first loaded Object and 0x80000001 for the > > > second is merely a coincidence... ;-) > > > > > > TPM2 also has no (single) SRK anymore. You have to create your own SRK > > > / Storage Primary Keys via TPM2_CreatePrimary and use the transient > > > handle returned from there. This however requires SH-authorization, > > > usually via Policy IMHO, so not easy to manage. So IMHO, this might be > > > something for the future but for the moment relying on a persistent > > > key would be better... > > > > > > For persistent SRKs it should become a convention to have those > > > around. Those handles start with 0x81000000 and the SRKs (or Storage > > > primary Keys) shall live within 0x81000000 to 0x8100FFFF (see > > > http://www.trustedcomputinggroup.org/resources/registry_of_reserved_tpm_20_handles_and_localities) > > > > > > I'd recommend to rely on the existence of a handle inside this range > > > with an empty auth-value. So maybe install a persistent SRK to > > > 0x81000000 via TPM2_EvictControl and then use this from within the > > > kernel for anything following. > > > P.S. You should check for the key's TPMA_OBJECT to have fixedTPM SET. > > > I don't know if there is an actual test for owner-generated SRK > > > testing. I'll ask around though... > > > > > > Note: you can query for handles in this range via > > > TPM2_GetCapability(TPM_CAP_HANDLES, 0x81000000) and then look for > > > fitting keys. > > > > > > > > > Feel free to discuss other approaches. > > > > I'm fully aware of all what you said. My take was to use 0x800000000 as > > a default value if you don't the handle ID explicitly in 'description' > > parameter of the add_key() syscall. > > > > > Cheers, > > > Andreas > > > > /Jarkko > -- > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ------------------------------------------------------------------------------
That's why I propose to give the context-save-blob into the kernel. It does not require any TPM2_Load of the key-chain or TPM2_CreatePrimary prior to key usage.
BTW, in the current TSS2-model context-save-blobs are the preferred way for "moving/copying" loaded objects between two applications or threads. The TSS2 crew did not see any value in having a "libdrm-like" flink() call. Since you have to transfer the handle anyways, transferring those few bytes of blob are actually just as easy and management inside the daemon becomes way simple without flink()ing... ;-)
Regarding the in-kernel "minimal resource manager": AFAIK there is already a tpm-mutex inside the kernel. We could use that mutex and then have the algorithm:
- get_mutex(tpm)
- If TPM_has_no_empty_slots:
- tmpBlob0 = Context_Save(some_handle)
- context_Flush(some_handle)
- if TPM_has_one_empts_slot: /* also true if "no_empty_slots" above */
- tmpBlob1 = Context_Save(another_handle)
- context_Flush(another_handle)
- keyhandle = Context_Load(keyBlob)
- datahandle = Context_Load(dataBlob)
- unseal(datahanle, keyhandle)
- Context_Flush(keyhandle)
- Context_Flush(datahandle)
- if tmpBlob0:
- h1 = Context_Load(tmpBlob0)
- h2 = Context_Load(tmpBlob0)
- assert((h1 == some_handle && h2 == another_handle) || (h2 == some_handle && h1 == another_handle))
- Context_Flush(another_handle) /* This is needed to get the original handles guaranteed */
- if tmpBlob1:
- assert(another_handle == Context_Load(tmpBlob1))
- release_mutex(tpm)
We don't need anything more fancy than this. And it should even guarantee that the old values are still present after mutex_release, so (opposed to a full-blown resource-manager) we do not need to keep track and rewrite virtual handles inside the user-space commands.
IMHO, this should be lightweight enough even for the most embedded of applications, since the 2*2k blobs are only allocated on demand...
ChromeOS with 1.2 uses a forked patched TrouSerS, right ?
I don't know about 2.0 though...
Cheers,
Andreas
I don't mean to be impolite but could line up your replies properly and avoid top-posting. I'd recommend 72 chars per line. Thanks. On Mon, Oct 05, 2015 at 12:20:47PM +0000, Fuchs, Andreas wrote: > That's why I propose to give the context-save-blob into the kernel. It > does not require any TPM2_Load of the key-chain or TPM2_CreatePrimary > prior to key usage. > > BTW, in the current TSS2-model context-save-blobs are the preferred > way for "moving/copying" loaded objects between two applications or > threads. The TSS2 crew did not see any value in having a "libdrm-like" > flink() call. Since you have to transfer the handle anyways, > transferring those few bytes of blob are actually just as easy and > management inside the daemon becomes way simple without flink()ing... > ;-) > > Regarding the in-kernel "minimal resource manager": AFAIK there is > already a tpm-mutex inside the kernel. We could use that mutex and > then have the algorithm: > > [SNIP] I don't care about one purpose hacks. Second, I don't care about pseudo code (at least not for "too big things"). It has tendency to mask unexpected details. If you want to propose something, please go through the patch process. > We don't need anything more fancy than this. And it should even > guarantee that the old values are still present after mutex_release, > so (opposed to a full-blown resource-manager) we do not need to keep > track and rewrite virtual handles inside the user-space commands. > > IMHO, this should be lightweight enough even for the most embedded of > applications, since the 2*2k blobs are only allocated on demand... It's still unnecessary functionality and increases the kernel image size and every hack requires maintenance. It would probably end up needing compilation flag as there exists efforts like: https://tiny.wiki.kernel.org/ My simple and stupid solution does not *prevent* adding better synchronization. I would go with that and implement access broker properly and not for just one use case later on. > ChromeOS with 1.2 uses a forked patched TrouSerS, right ? > I don't know about 2.0 though... Yup, patched to use UDP sockets. > Cheers, > Andreas /Jarkko ------------------------------------------------------------------------------
> > Regarding the in-kernel "minimal resource manager": AFAIK there is > > already a tpm-mutex inside the kernel. We could use that mutex and > > then have the algorithm: > > > > [SNIP] > > I don't care about one purpose hacks. Second, I don't care about pseudo > code (at least not for "too big things"). It has tendency to mask > unexpected details. If you want to propose something, please go through > the patch process. > > > We don't need anything more fancy than this. And it should even > > guarantee that the old values are still present after mutex_release, > > so (opposed to a full-blown resource-manager) we do not need to keep > > track and rewrite virtual handles inside the user-space commands. > > > > IMHO, this should be lightweight enough even for the most embedded of > > applications, since the 2*2k blobs are only allocated on demand... > > It's still unnecessary functionality and increases the kernel image size > and every hack requires maintenance. It would probably end up needing > compilation flag as there exists efforts like: > > https://tiny.wiki.kernel.org/ > > My simple and stupid solution does not *prevent* adding better > synchronization. I would go with that and implement access broker > properly and not for just one use case later on. Unfortunately, I'm not able to write up some code for this myself atm. Other priorities unfortunately. I was just pointing out, that the proposed patch will not fit in with the current approach in TSS2.0, before this user-facing kernel API is set in stone and _corrected_ new syscalls need to be added later. Also, the pseudo-code proposal should be a proper minimal access broker that should solve most accesses to TPM transient objects down the road. Session-brokering is a different beast of course. Cheers, Andreas ------------------------------------------------------------------------------
On Mon, Oct 05, 2015 at 01:36:18PM +0000, Fuchs, Andreas wrote: > > It's still unnecessary functionality and increases the kernel image size > > and every hack requires maintenance. It would probably end up needing > > compilation flag as there exists efforts like: > > > > https://tiny.wiki.kernel.org/ > > > > My simple and stupid solution does not *prevent* adding better > > synchronization. I would go with that and implement access broker > > properly and not for just one use case later on. > > Unfortunately, I'm not able to write up some code for this myself atm. > Other priorities unfortunately. > > I was just pointing out, that the proposed patch will not fit in with > the current approach in TSS2.0, before this user-facing kernel API is > set in stone and _corrected_ new syscalls need to be added later. Why you would want new system calls? Do you know how hard it is to get new system calls accepted? It's usually nearly impossible to get new system calls in. You are going wrong direction there. I do not see why couldn't survive in TSS 2.0 implementation for a while without in-kernel access broker even if the world isn't perfect and improve from that when the support becomes available. I'm not frankly following your rationale here. On the other hand I see use for the kernel images without access broker in small embdedded devices. I CC'd to Will Arthur as he has been working with TSS 2.0 for along time just in case. > Also, the pseudo-code proposal should be a proper minimal access broker > that should solve most accesses to TPM transient objects down the road. > Session-brokering is a different beast of course. I don't mean to be rude but pseudo code doesn't matter much. We know what is required from an access broker in terms of TPM 2.0 commands and locking. Only working code matters at this point. I still don't see why you couldn't add access broker later on. The patch set does not make the API worse than it is right now. > Cheers, > Andreas /Jarkko ------------------------------------------------------------------------------
> > I was just pointing out, that the proposed patch will not fit in with > > the current approach in TSS2.0, before this user-facing kernel API is > > set in stone and _corrected_ new syscalls need to be added later. > > Why you would want new system calls? Do you know how hard it is to get > new system calls accepted? It's usually nearly impossible to get new > system calls in. You are going wrong direction there. > > I do not see why couldn't survive in TSS 2.0 implementation for a while > without in-kernel access broker even if the world isn't perfect and > improve from that when the support becomes available. I'm not frankly > following your rationale here. > > On the other hand I see use for the kernel images without access broker > in small embdedded devices. > > I CC'd to Will Arthur as he has been working with TSS 2.0 for along > time just in case. > > > Also, the pseudo-code proposal should be a proper minimal access broker > > that should solve most accesses to TPM transient objects down the road. > > Session-brokering is a different beast of course. > > I don't mean to be rude but pseudo code doesn't matter much. We know > what is required from an access broker in terms of TPM 2.0 commands and > locking. Only working code matters at this point. > > I still don't see why you couldn't add access broker later on. The patch > set does not make the API worse than it is right now. OK, I guess we got stuck in the follow-up discussions and missed the points. My 1st point is: TPM1.2's 0x40000000 SRK handle was a well-known, singleton, always-present key, that could be relied upon. TPM2.0's 0x80000000 is a temporary, TPM-assigned, context-specific handle, that cannot be relied upon. Therefore, I think your patch should not use it. Instead, I'd recommend using the closest equivalent to an SRK that TPM2.0 has to offer, which is within the range 0x81000000 to 0x8100FFFF. (see http://www.trustedcomputinggroup.org/resources/registry_of_reserved_tpm_20_handles_and_localities) You might want to use TPM2_GetCapability() to find the correct one. Also User-Space could reference any of these handles in the 0x81000000-0x81FFFFFF range. This would be fine. My 2nd point is: It is IMHO a bad idea to allow user-space to provide transient handles as parameter to the TPM, because TSS2.0 will virtualize handles and /dev/tpm0 is single-access only. Instead I'd recommend passing context-saved-blobs to the kernel. Then you brought up the valid point that this requires kernel-space resource broker and I provided some sketch-idea in pseudo-code for discussion of general approach. I did not know that the access broker was solved already. Conclusion I would just like to prevent having an API that expects (and defaults to) transient handles be set in stone for the kernel, since it will not meet reality... ;-) @Will: Hope you're doing fine... Maybe we can discuss the TSS-side of things tomorrow... Cheers, Andreas ------------------------------------------------------------------------------
On Mon, Oct 05, 2015 at 02:13:15PM +0000, Fuchs, Andreas wrote: > > > I was just pointing out, that the proposed patch will not fit in with > > > the current approach in TSS2.0, before this user-facing kernel API is > > > set in stone and _corrected_ new syscalls need to be added later. > > > > Why you would want new system calls? Do you know how hard it is to get > > new system calls accepted? It's usually nearly impossible to get new > > system calls in. You are going wrong direction there. > > > > I do not see why couldn't survive in TSS 2.0 implementation for a while > > without in-kernel access broker even if the world isn't perfect and > > improve from that when the support becomes available. I'm not frankly > > following your rationale here. > > > > On the other hand I see use for the kernel images without access broker > > in small embdedded devices. > > > > I CC'd to Will Arthur as he has been working with TSS 2.0 for along > > time just in case. > > > > > Also, the pseudo-code proposal should be a proper minimal access broker > > > that should solve most accesses to TPM transient objects down the road. > > > Session-brokering is a different beast of course. > > > > I don't mean to be rude but pseudo code doesn't matter much. We know > > what is required from an access broker in terms of TPM 2.0 commands and > > locking. Only working code matters at this point. > > > > I still don't see why you couldn't add access broker later on. The patch > > set does not make the API worse than it is right now. > > OK, I guess we got stuck in the follow-up discussions and missed the points. Yup, don't get me wrong here. I like this discussion and am willing to listen to reasonable arguments. > My 1st point is: > > TPM1.2's 0x40000000 SRK handle was a well-known, singleton, always-present > key, that could be relied upon. > > TPM2.0's 0x80000000 is a temporary, TPM-assigned, context-specific handle, > that cannot be relied upon. > > Therefore, I think your patch should not use it. > > Instead, I'd recommend using the closest equivalent to an SRK that TPM2.0 > has to offer, which is within the range 0x81000000 to 0x8100FFFF. > (see http://www.trustedcomputinggroup.org/resources/registry_of_reserved_tpm_20_handles_and_localities) > You might want to use TPM2_GetCapability() to find the correct one. > > Also User-Space could reference any of these handles in the > 0x81000000-0x81FFFFFF range. This would be fine. Alright. How about requiring keyhandle as explicit option for TPM 2.0? Would that be a more reasonable solution in your opinion? That would acceptable for me. > My 2nd point is: > > It is IMHO a bad idea to allow user-space to provide transient handles as > parameter to the TPM, because TSS2.0 will virtualize handles and /dev/tpm0 > is single-access only. > Instead I'd recommend passing context-saved-blobs to the kernel. > > Then you brought up the valid point that this requires kernel-space resource > broker and I provided some sketch-idea in pseudo-code for discussion of > general approach. I did not know that the access broker was solved already. Yeah. I'm not against implementing the broker and how I've been thinking implementing it is not too far away what you just suggested. I'm not just seeing why that couldn't be done as a separate patch set later on. > Conclusion > > I would just like to prevent having an API that expects (and defaults to) > transient handles be set in stone for the kernel, since it will not meet > reality... ;-) > > > > @Will: Hope you're doing fine... Maybe we can discuss the TSS-side of things > tomorrow... > > Cheers, > Andreas-- /Jarkko ------------------------------------------------------------------------------
I'm having trouble following the arguments here. Can someone summarize the two (or more) different approaches being proposed? Will Arthur Intel Corporation Server Security Firmware 803-216-2101 -----Original Message----- From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com] Sent: Monday, October 5, 2015 10:28 AM To: Fuchs, Andreas <andreas.fuchs@sit.fraunhofer.de> Cc: tpmdd-devel@lists.sourceforge.net; linux-kernel@vger.kernel.org; David Howells <dhowells@redhat.com>; gregkh@linuxfoundation.org; open list:KEYS-TRUSTED <linux-security-module@vger.kernel.org>; open list:KEYS-TRUSTED <keyrings@vger.kernel.org>; James Morris <james.l.morris@oracle.com>; David Safford <safford@us.ibm.com>; akpm@linux-foundation.org; Serge E. Hallyn <serge@hallyn.com>; josh@joshtripplet.org; Maliszewski, Richard L <richard.l.maliszewski@intel.com>; Wiseman, Monty <monty.wiseman@intel.com>; Arthur, Will C <will.c.arthur@intel.com> Subject: Re: [tpmdd-devel] [PATCH 4/4] keys, trusted: seal/unseal with TPM 2.0 chips On Mon, Oct 05, 2015 at 02:13:15PM +0000, Fuchs, Andreas wrote: > > > I was just pointing out, that the proposed patch will not fit in > > > with the current approach in TSS2.0, before this user-facing > > > kernel API is set in stone and _corrected_ new syscalls need to be added later. > > > > Why you would want new system calls? Do you know how hard it is to > > get new system calls accepted? It's usually nearly impossible to get > > new system calls in. You are going wrong direction there. > > > > I do not see why couldn't survive in TSS 2.0 implementation for a > > while without in-kernel access broker even if the world isn't > > perfect and improve from that when the support becomes available. > > I'm not frankly following your rationale here. > > > > On the other hand I see use for the kernel images without access > > broker in small embdedded devices. > > > > I CC'd to Will Arthur as he has been working with TSS 2.0 for along > > time just in case. > > > > > Also, the pseudo-code proposal should be a proper minimal access > > > broker that should solve most accesses to TPM transient objects down the road. > > > Session-brokering is a different beast of course. > > > > I don't mean to be rude but pseudo code doesn't matter much. We know > > what is required from an access broker in terms of TPM 2.0 commands > > and locking. Only working code matters at this point. > > > > I still don't see why you couldn't add access broker later on. The > > patch set does not make the API worse than it is right now. > > OK, I guess we got stuck in the follow-up discussions and missed the points. Yup, don't get me wrong here. I like this discussion and am willing to listen to reasonable arguments. > My 1st point is: > > TPM1.2's 0x40000000 SRK handle was a well-known, singleton, > always-present key, that could be relied upon. > > TPM2.0's 0x80000000 is a temporary, TPM-assigned, context-specific > handle, that cannot be relied upon. > > Therefore, I think your patch should not use it. > > Instead, I'd recommend using the closest equivalent to an SRK that > TPM2.0 has to offer, which is within the range 0x81000000 to 0x8100FFFF. > (see > http://www.trustedcomputinggroup.org/resources/registry_of_reserved_tp > m_20_handles_and_localities) You might want to use > TPM2_GetCapability() to find the correct one. > > Also User-Space could reference any of these handles in the > 0x81000000-0x81FFFFFF range. This would be fine. Alright. How about requiring keyhandle as explicit option for TPM 2.0? Would that be a more reasonable solution in your opinion? That would acceptable for me. > My 2nd point is: > > It is IMHO a bad idea to allow user-space to provide transient handles > as parameter to the TPM, because TSS2.0 will virtualize handles and > /dev/tpm0 is single-access only. > Instead I'd recommend passing context-saved-blobs to the kernel. > > Then you brought up the valid point that this requires kernel-space > resource broker and I provided some sketch-idea in pseudo-code for > discussion of general approach. I did not know that the access broker was solved already. Yeah. I'm not against implementing the broker and how I've been thinking implementing it is not too far away what you just suggested. I'm not just seeing why that couldn't be done as a separate patch set later on. > Conclusion > > I would just like to prevent having an API that expects (and defaults > to) transient handles be set in stone for the kernel, since it will > not meet reality... ;-) > > > > @Will: Hope you're doing fine... Maybe we can discuss the TSS-side of > things tomorrow... > > Cheers, > Andreas-- /Jarkko ------------------------------------------------------------------------------
> > OK, I guess we got stuck in the follow-up discussions and missed the points. > > Yup, don't get me wrong here. I like this discussion and am willing to > listen to reasonable arguments. We could not agree more. I'm always up for a good discussion... ;-) > > My 1st point is: > > > > TPM1.2's 0x40000000 SRK handle was a well-known, singleton, always-present > > key, that could be relied upon. > > > > TPM2.0's 0x80000000 is a temporary, TPM-assigned, context-specific handle, > > that cannot be relied upon. > > > > Therefore, I think your patch should not use it. > > > > Instead, I'd recommend using the closest equivalent to an SRK that TPM2.0 > > has to offer, which is within the range 0x81000000 to 0x8100FFFF. > > (see http://www.trustedcomputinggroup.org/resources/registry_of_reserved_tpm_20_handles_and_localities) > > You might want to use TPM2_GetCapability() to find the correct one. > > > > Also User-Space could reference any of these handles in the > > 0x81000000-0x81FFFFFF range. This would be fine. > > Alright. How about requiring keyhandle as explicit option for TPM 2.0? > Would that be a more reasonable solution in your opinion? That would > acceptable for me. You mean getting rid of the default behaviour ? That sound reasonable to me as well. A later patch could add the possibility to use the TPM2_GetCapability() thing at a later stage then... > > My 2nd point is: > > > > It is IMHO a bad idea to allow user-space to provide transient handles as > > parameter to the TPM, because TSS2.0 will virtualize handles and /dev/tpm0 > > is single-access only. > > Instead I'd recommend passing context-saved-blobs to the kernel. > > > > Then you brought up the valid point that this requires kernel-space resource > > broker and I provided some sketch-idea in pseudo-code for discussion of > > general approach. I did not know that the access broker was solved already. > > Yeah. I'm not against implementing the broker and how I've been thinking > implementing it is not too far away what you just suggested. > > I'm not just seeing why that couldn't be done as a separate patch set > later on. I should have been more clear then. I agree that it can be added later on. Or rather I think it should be added at some later point... I was just trying to point out that the concept is not too difficult, since kernel-space (minimal) resource-manager makes a lot of people go "oh god, never ever, way too big, way too complicated", which IMHO it is not. Until then, I think that the call should just return -EBUSY when you cannot load the sealed data if no slots are available ? I looked at Patch 3/4 and it seems you default to -EPERM on TPM2_Create()- and TPM2_Load()-failures ? You might want to test against rc == TPM_RC_OBJECT_MEMORY and return -EBUSY in those cases. Would you agree ? (P.S. I can cross-post there if that's prefered ?) Cheers, Andreas ------------------------------------------------------------------------------
On Tue, Oct 06, 2015 at 06:22:29AM +0000, Fuchs, Andreas wrote: > > > OK, I guess we got stuck in the follow-up discussions and missed the points. > > > > Yup, don't get me wrong here. I like this discussion and am willing to > > listen to reasonable arguments. > > We could not agree more. I'm always up for a good discussion... ;-) > > > > My 1st point is: > > > > > > TPM1.2's 0x40000000 SRK handle was a well-known, singleton, always-present > > > key, that could be relied upon. > > > > > > TPM2.0's 0x80000000 is a temporary, TPM-assigned, context-specific handle, > > > that cannot be relied upon. > > > > > > Therefore, I think your patch should not use it. > > > > > > Instead, I'd recommend using the closest equivalent to an SRK that TPM2.0 > > > has to offer, which is within the range 0x81000000 to 0x8100FFFF. > > > (see http://www.trustedcomputinggroup.org/resources/registry_of_reserved_tpm_20_handles_and_localities) > > > You might want to use TPM2_GetCapability() to find the correct one. > > > > > > Also User-Space could reference any of these handles in the > > > 0x81000000-0x81FFFFFF range. This would be fine. > > > > Alright. How about requiring keyhandle as explicit option for TPM 2.0? > > Would that be a more reasonable solution in your opinion? That would > > acceptable for me. > > You mean getting rid of the default behaviour ? > That sound reasonable to me as well. A later patch could add the possibility > to use the TPM2_GetCapability() thing at a later stage then... > > > > My 2nd point is: > > > > > > It is IMHO a bad idea to allow user-space to provide transient handles as > > > parameter to the TPM, because TSS2.0 will virtualize handles and /dev/tpm0 > > > is single-access only. > > > Instead I'd recommend passing context-saved-blobs to the kernel. > > > > > > Then you brought up the valid point that this requires kernel-space resource > > > broker and I provided some sketch-idea in pseudo-code for discussion of > > > general approach. I did not know that the access broker was solved already. > > > > Yeah. I'm not against implementing the broker and how I've been thinking > > implementing it is not too far away what you just suggested. > > > > I'm not just seeing why that couldn't be done as a separate patch set > > later on. > > I should have been more clear then. I agree that it can be added later on. > Or rather I think it should be added at some later point... > > I was just trying to point out that the concept is not too difficult, since > kernel-space (minimal) resource-manager makes a lot of people go "oh god, > never ever, way too big, way too complicated", which IMHO it is not. > Until then, I think that the call should just return -EBUSY when you cannot > load the sealed data if no slots are available ? Well this is kind of argument where there is no argument. I already had plans how to do access broker back in 2014 that are more or less along the lines of the pseudo code you sent. The problem is the lack of active maintainers in the subsystem. That's why I get easily frustated discussing about access broker in the first place :) I would have implemented access broker months and months ago if I didn't have so much code in the queue for this subsystem. There can be literally months delay without any feedback. Right now I have four different patches or patch sets in the queue: - PPI support (yes you cannot enable TPM chips at the moment from Linux) - Two bug fixes (CRB command buffer alignment, dTPM2 ACPI hot plugging) - Basic trusted keys I wouldn't blame any particular person about the situation but things cannot continue like this. I've been thinking if I could acquire co-maintainership of the subsystem for TPM 2 parts (don't really have time or expertise for TPM 1.x parts). I could post my architecture (never really written it except in my head but should not take too long time) to my blog at jsakkine.blogspot.com if you are interested discussing more. > I looked at Patch 3/4 and it seems you default to -EPERM on TPM2_Create()- > and TPM2_Load()-failures ? > You might want to test against rc == TPM_RC_OBJECT_MEMORY and return -EBUSY > in those cases. Would you agree ? > (P.S. I can cross-post there if that's prefered ?) Have to check the return values. I posted this patch set already in early July. You are the first reviewer in three months for this patch set. I think the reason was that for TPM 1.x returned -EPERM in all error scenarios and I didn't want to endanger behaviour of command-line tools such as 'keyctl'. I would keep it that way unless you can guarantee that command-line tools will continue work correctly if I change it to -EBUSY. Anyway, I will recheck this part of the patch set but likely are not going to do any changes because I don't want to break the user space. I will consider revising the patch set with keyhandle required as an explicit option. > Cheers, > Andreas /Jarkko ------------------------------------------------------------------------------
> > I was just trying to point out that the concept is not too difficult, since > > kernel-space (minimal) resource-manager makes a lot of people go "oh god, > > never ever, way too big, way too complicated", which IMHO it is not. > > Until then, I think that the call should just return -EBUSY when you cannot > > load the sealed data if no slots are available ? > > Well this is kind of argument where there is no argument. I already had > plans how to do access broker back in 2014 that are more or less along > the lines of the pseudo code you sent. The problem is the lack of active > maintainers in the subsystem. That's why I get easily frustated > discussing about access broker in the first place :) > > I would have implemented access broker months and months ago if I didn't > have so much code in the queue for this subsystem. There can be literally > months delay without any feedback. Right now I have four different > patches or patch sets in the queue: > > - PPI support (yes you cannot enable TPM chips at the moment from Linux) > - Two bug fixes (CRB command buffer alignment, dTPM2 ACPI hot plugging) > - Basic trusted keys > > I wouldn't blame any particular person about the situation but things > cannot continue like this. I've been thinking if I could acquire > co-maintainership of the subsystem for TPM 2 parts (don't really have > time or expertise for TPM 1.x parts). I think I know this situation. You have all my sympathies... ;-) > I could post my architecture (never really written it except in my head > but should not take too long time) to my blog at jsakkine.blogspot.com > if you are interested discussing more. Well, I came in to tpmdd-devel rather recently and only with a small time budget to spend, but I'd be highly interested to learn about your thoughts. As you can tell, I've been involved on the userspace side of things and therefore already bent my head around some different architectures for different scenarios. Also your input might help us in the specification of userspace side as well. So please go ahead and write it up, if you can spare the time. Or let's get on the phone some time. > > I looked at Patch 3/4 and it seems you default to -EPERM on TPM2_Create()- > > and TPM2_Load()-failures ? > > You might want to test against rc == TPM_RC_OBJECT_MEMORY and return -EBUSY > > in those cases. Would you agree ? > > (P.S. I can cross-post there if that's prefered ?) > > Have to check the return values. I posted this patch set already in > early July. You are the first reviewer in three months for this patch > set. > > I think the reason was that for TPM 1.x returned -EPERM in all error > scenarios and I didn't want to endanger behaviour of command-line tools > such as 'keyctl'. I would keep it that way unless you can guarantee that > command-line tools will continue work correctly if I change it to > -EBUSY. > > Anyway, I will recheck this part of the patch set but likely are not > going to do any changes because I don't want to break the user space. > > I will consider revising the patch set with keyhandle required as an > explicit option. Hmm... Will the old keyctl work without modification with the 2.0 patches anyways ? The different keyHandle values and missing default keyHandle will yield "differences" anyways, I'd say. IMHO, we should get it as correct as possible given that TPM 2.0 is still very young. Is adding "additional" ReturnCodes considered ABI-incompatible breaking anyways ? Cheers, Andreas ------------------------------------------------------------------------------
On Tue, Oct 06, 2015 at 01:16:02PM +0000, Fuchs, Andreas wrote: > > > I was just trying to point out that the concept is not too difficult, since > > > kernel-space (minimal) resource-manager makes a lot of people go "oh god, > > > never ever, way too big, way too complicated", which IMHO it is not. > > > Until then, I think that the call should just return -EBUSY when you cannot > > > load the sealed data if no slots are available ? > > > > Well this is kind of argument where there is no argument. I already had > > plans how to do access broker back in 2014 that are more or less along > > the lines of the pseudo code you sent. The problem is the lack of active > > maintainers in the subsystem. That's why I get easily frustated > > discussing about access broker in the first place :) > > > > I would have implemented access broker months and months ago if I didn't > > have so much code in the queue for this subsystem. There can be literally > > months delay without any feedback. Right now I have four different > > patches or patch sets in the queue: > > > > - PPI support (yes you cannot enable TPM chips at the moment from Linux) > > - Two bug fixes (CRB command buffer alignment, dTPM2 ACPI hot plugging) > > - Basic trusted keys > > > > I wouldn't blame any particular person about the situation but things > > cannot continue like this. I've been thinking if I could acquire > > co-maintainership of the subsystem for TPM 2 parts (don't really have > > time or expertise for TPM 1.x parts). > > I think I know this situation. You have all my sympathies... ;-) > > > I could post my architecture (never really written it except in my head > > but should not take too long time) to my blog at jsakkine.blogspot.com > > if you are interested discussing more. > > Well, I came in to tpmdd-devel rather recently and only with a small time budget > to spend, but I'd be highly interested to learn about your thoughts. > > As you can tell, I've been involved on the userspace side of things and > therefore already bent my head around some different architectures for > different scenarios. Also your input might help us in the specification of > userspace side as well. > > So please go ahead and write it up, if you can spare the time. > Or let's get on the phone some time. > > > > I looked at Patch 3/4 and it seems you default to -EPERM on TPM2_Create()- > > > and TPM2_Load()-failures ? > > > You might want to test against rc == TPM_RC_OBJECT_MEMORY and return -EBUSY > > > in those cases. Would you agree ? > > > (P.S. I can cross-post there if that's prefered ?) > > > > Have to check the return values. I posted this patch set already in > > early July. You are the first reviewer in three months for this patch > > set. > > > > I think the reason was that for TPM 1.x returned -EPERM in all error > > scenarios and I didn't want to endanger behaviour of command-line tools > > such as 'keyctl'. I would keep it that way unless you can guarantee that > > command-line tools will continue work correctly if I change it to > > -EBUSY. > > > > Anyway, I will recheck this part of the patch set but likely are not > > going to do any changes because I don't want to break the user space. > > > > I will consider revising the patch set with keyhandle required as an > > explicit option. > > Hmm... Will the old keyctl work without modification with the 2.0 patches > anyways ? Yes it does and it should. I've been using keyctl utility to test my patch set. > The different keyHandle values and missing default keyHandle will yield > "differences" anyways, I'd say. > IMHO, we should get it as correct as possible given that TPM 2.0 is still > very young. > > Is adding "additional" ReturnCodes considered ABI-incompatible breaking > anyways ? Yes they are if they make the user space utiltiy malfunction. > Cheers, > Andreas /Jarkko ------------------------------------------------------------------------------
> > > > I looked at Patch 3/4 and it seems you default to -EPERM on TPM2_Create()- > > > > and TPM2_Load()-failures ? > > > > You might want to test against rc == TPM_RC_OBJECT_MEMORY and return -EBUSY > > > > in those cases. Would you agree ? > > > > (P.S. I can cross-post there if that's prefered ?) > > > > > > Have to check the return values. I posted this patch set already in > > > early July. You are the first reviewer in three months for this patch > > > set. > > > > > > I think the reason was that for TPM 1.x returned -EPERM in all error > > > scenarios and I didn't want to endanger behaviour of command-line tools > > > such as 'keyctl'. I would keep it that way unless you can guarantee that > > > command-line tools will continue work correctly if I change it to > > > -EBUSY. > > > > > > Anyway, I will recheck this part of the patch set but likely are not > > > going to do any changes because I don't want to break the user space. > > > > > > I will consider revising the patch set with keyhandle required as an > > > explicit option. > > > > Hmm... Will the old keyctl work without modification with the 2.0 patches > > anyways ? > > Yes it does and it should. I've been using keyctl utility to test my > patch set. > > > The different keyHandle values and missing default keyHandle will yield > > "differences" anyways, I'd say. > > IMHO, we should get it as correct as possible given that TPM 2.0 is still > > very young. > > > > Is adding "additional" ReturnCodes considered ABI-incompatible breaking > > anyways ? > > Yes they are if they make the user space utiltiy malfunction. AFAICT, keyctl just perror()s. Which is what I would have hoped. So it guess it should work with -EBUSY. Example-Trace of calls for key_adding: http://anonscm.debian.org/cgit/collab-maint/keyutils.git/tree/keyutils.c#n43 http://anonscm.debian.org/cgit/collab-maint/keyutils.git/tree/keyctl.c#n379 http://anonscm.debian.org/cgit/collab-maint/keyutils.git/tree/keyctl.c#n131 Wish I could test it myself. I understand, if you don't want to test my thoughts on this. I just cannot perform the tests myself right now... :-( Cheers, Andreas ------------------------------------------------------------------------------ Full-scale, agent-less Infrastructure Monitoring from a single dashboard Integrate with 40+ ManageEngine ITSM Solutions for complete visibility Physical-Virtual-Cloud Infrastructure monitoring from one console Real user monitoring with APM Insights and performance trend reports Learn More http://pubads.g.doubleclick.net/gampad/clk?id=247754911&iu=/4140
On Wed, Oct 07, 2015 at 10:04:40AM +0000, Fuchs, Andreas wrote: > > > > > I looked at Patch 3/4 and it seems you default to -EPERM on TPM2_Create()- > > > > > and TPM2_Load()-failures ? > > > > > You might want to test against rc == TPM_RC_OBJECT_MEMORY and return -EBUSY > > > > > in those cases. Would you agree ? > > > > > (P.S. I can cross-post there if that's prefered ?) > > > > > > > > Have to check the return values. I posted this patch set already in > > > > early July. You are the first reviewer in three months for this patch > > > > set. > > > > > > > > I think the reason was that for TPM 1.x returned -EPERM in all error > > > > scenarios and I didn't want to endanger behaviour of command-line tools > > > > such as 'keyctl'. I would keep it that way unless you can guarantee that > > > > command-line tools will continue work correctly if I change it to > > > > -EBUSY. > > > > > > > > Anyway, I will recheck this part of the patch set but likely are not > > > > going to do any changes because I don't want to break the user space. > > > > > > > > I will consider revising the patch set with keyhandle required as an > > > > explicit option. > > > > > > Hmm... Will the old keyctl work without modification with the 2.0 patches > > > anyways ? > > > > Yes it does and it should. I've been using keyctl utility to test my > > patch set. > > > > > The different keyHandle values and missing default keyHandle will yield > > > "differences" anyways, I'd say. > > > IMHO, we should get it as correct as possible given that TPM 2.0 is still > > > very young. > > > > > > Is adding "additional" ReturnCodes considered ABI-incompatible breaking > > > anyways ? > > > > Yes they are if they make the user space utiltiy malfunction. > > AFAICT, keyctl just perror()s. Which is what I would have hoped. > So it guess it should work with -EBUSY. > Example-Trace of calls for key_adding: > http://anonscm.debian.org/cgit/collab-maint/keyutils.git/tree/keyutils.c#n43 > http://anonscm.debian.org/cgit/collab-maint/keyutils.git/tree/keyctl.c#n379 > http://anonscm.debian.org/cgit/collab-maint/keyutils.git/tree/keyctl.c#n131 > > Wish I could test it myself. > I understand, if you don't want to test my thoughts on this. > I just cannot perform the tests myself right now... :-( I would submit this change as a separate patch later anyway and not include it into this patch set. If it doesn't do harm it can be added later on. This patch set has been now in queue for three months so I only make modifications that are absolutely necessary. Changing keyhandle as mandatory option seems like such changes. This doesn't. > Cheers, > Andreas-- /Jarkko ------------------------------------------------------------------------------ Full-scale, agent-less Infrastructure Monitoring from a single dashboard Integrate with 40+ ManageEngine ITSM Solutions for complete visibility Physical-Virtual-Cloud Infrastructure monitoring from one console Real user monitoring with APM Insights and performance trend reports Learn More http://pubads.g.doubleclick.net/gampad/clk?id=247754911&iu=/4140
> > > > > > I looked at Patch 3/4 and it seems you default to -EPERM on TPM2_Create()- > > > > > > and TPM2_Load()-failures ? > > > > > > You might want to test against rc == TPM_RC_OBJECT_MEMORY and return -EBUSY > > > > > > in those cases. Would you agree ? > > > > > > (P.S. I can cross-post there if that's prefered ?) > > > > > > > > > > Have to check the return values. I posted this patch set already in > > > > > early July. You are the first reviewer in three months for this patch > > > > > set. > > > > > > > > > > I think the reason was that for TPM 1.x returned -EPERM in all error > > > > > scenarios and I didn't want to endanger behaviour of command-line tools > > > > > such as 'keyctl'. I would keep it that way unless you can guarantee that > > > > > command-line tools will continue work correctly if I change it to > > > > > -EBUSY. > > > > > > > > > > Anyway, I will recheck this part of the patch set but likely are not > > > > > going to do any changes because I don't want to break the user space. > > > > > > > > > > I will consider revising the patch set with keyhandle required as an > > > > > explicit option. > > > > > > > > Hmm... Will the old keyctl work without modification with the 2.0 patches > > > > anyways ? > > > > > > Yes it does and it should. I've been using keyctl utility to test my > > > patch set. > > > > > > > The different keyHandle values and missing default keyHandle will yield > > > > "differences" anyways, I'd say. > > > > IMHO, we should get it as correct as possible given that TPM 2.0 is still > > > > very young. > > > > > > > > Is adding "additional" ReturnCodes considered ABI-incompatible breaking > > > > anyways ? > > > > > > Yes they are if they make the user space utiltiy malfunction. > > > > AFAICT, keyctl just perror()s. Which is what I would have hoped. > > So it guess it should work with -EBUSY. > > Example-Trace of calls for key_adding: > > http://anonscm.debian.org/cgit/collab-maint/keyutils.git/tree/keyutils.c#n43 > > http://anonscm.debian.org/cgit/collab-maint/keyutils.git/tree/keyctl.c#n379 > > http://anonscm.debian.org/cgit/collab-maint/keyutils.git/tree/keyctl.c#n131 > > > > Wish I could test it myself. > > I understand, if you don't want to test my thoughts on this. > > I just cannot perform the tests myself right now... :-( > > I would submit this change as a separate patch later anyway and not > include it into this patch set. If it doesn't do harm it can be added > later on. This patch set has been now in queue for three months so I > only make modifications that are absolutely necessary. > > Changing keyhandle as mandatory option seems like such changes. This > doesn't. Fine with me. P.S. do you have a git repo with all your queued and future patches at HEAD ? Cheers, Andreas ------------------------------------------------------------------------------ Full-scale, agent-less Infrastructure Monitoring from a single dashboard Integrate with 40+ ManageEngine ITSM Solutions for complete visibility Physical-Virtual-Cloud Infrastructure monitoring from one console Real user monitoring with APM Insights and performance trend reports Learn More http://pubads.g.doubleclick.net/gampad/clk?id=247754911&iu=/4140
On Wed, 2015-10-07 at 10:32 +0000, Fuchs, Andreas wrote: > > > > > > > I looked at Patch 3/4 and it seems you default to -EPERM > > > > > > > on TPM2_Create()- > > > > > > > and TPM2_Load()-failures ? > > > > > > > You might want to test against rc == TPM_RC_OBJECT_MEMORY > > > > > > > and return -EBUSY > > > > > > > in those cases. Would you agree ? > > > > > > > (P.S. I can cross-post there if that's prefered ?) > > > > > > > > > > > > Have to check the return values. I posted this patch set > > > > > > already in > > > > > > early July. You are the first reviewer in three months for > > > > > > this patch > > > > > > set. > > > > > > > > > > > > I think the reason was that for TPM 1.x returned -EPERM in > > > > > > all error > > > > > > scenarios and I didn't want to endanger behaviour of > > > > > > command-line tools > > > > > > such as 'keyctl'. I would keep it that way unless you can > > > > > > guarantee that > > > > > > command-line tools will continue work correctly if I change > > > > > > it to > > > > > > -EBUSY. > > > > > > > > > > > > Anyway, I will recheck this part of the patch set but > > > > > > likely are not > > > > > > going to do any changes because I don't want to break the > > > > > > user space. > > > > > > > > > > > > I will consider revising the patch set with keyhandle > > > > > > required as an > > > > > > explicit option. > > > > > > > > > > Hmm... Will the old keyctl work without modification with the > > > > > 2.0 patches > > > > > anyways ? > > > > > > > > Yes it does and it should. I've been using keyctl utility to > > > > test my > > > > patch set. > > > > > > > > > The different keyHandle values and missing default keyHandle > > > > > will yield > > > > > "differences" anyways, I'd say. > > > > > IMHO, we should get it as correct as possible given that TPM > > > > > 2.0 is still > > > > > very young. > > > > > > > > > > Is adding "additional" ReturnCodes considered ABI > > > > > -incompatible breaking > > > > > anyways ? > > > > > > > > Yes they are if they make the user space utiltiy malfunction. > > > > > > AFAICT, keyctl just perror()s. Which is what I would have hoped. > > > So it guess it should work with -EBUSY. > > > Example-Trace of calls for key_adding: > > > http://anonscm.debian.org/cgit/collab-maint/keyutils.git/tree/key > > > utils.c#n43 > > > http://anonscm.debian.org/cgit/collab-maint/keyutils.git/tree/key > > > ctl.c#n379 > > > http://anonscm.debian.org/cgit/collab-maint/keyutils.git/tree/key > > > ctl.c#n131 > > > > > > Wish I could test it myself. > > > I understand, if you don't want to test my thoughts on this. > > > I just cannot perform the tests myself right now... :-( > > > > I would submit this change as a separate patch later anyway and not > > include it into this patch set. If it doesn't do harm it can be > > added > > later on. This patch set has been now in queue for three months so > > I > > only make modifications that are absolutely necessary. > > > > Changing keyhandle as mandatory option seems like such changes. > > This > > doesn't. > > Fine with me. > > P.S. do you have a git repo with all your queued and future patches > at HEAD ? In separate branches: https://github.com/jsakkine/linux-tpm2/branches > Cheers, > Andreas /Jarkko ------------------------------------------------------------------------------ Full-scale, agent-less Infrastructure Monitoring from a single dashboard Integrate with 40+ ManageEngine ITSM Solutions for complete visibility Physical-Virtual-Cloud Infrastructure monitoring from one console Real user monitoring with APM Insights and performance trend reports Learn More http://pubads.g.doubleclick.net/gampad/clk?id=247754911&iu=/4140
diff --git a/security/keys/trusted.h b/security/keys/trusted.h index ff001a5..fc32c47 100644 --- a/security/keys/trusted.h +++ b/security/keys/trusted.h @@ -12,6 +12,13 @@ #define TPM_RETURN_OFFSET 6 #define TPM_DATA_OFFSET 10 +/* Transient object handles start from 0x80000000 in TPM 2.0, which makes it + * a sane default. + */ + +#define TPM1_SRKHANDLE 0x40000000 +#define TPM2_SRKHANDLE 0x80000000 + #define LOAD32(buffer, offset) (ntohl(*(uint32_t *)&buffer[offset])) #define LOAD32N(buffer, offset) (*(uint32_t *)&buffer[offset]) #define LOAD16(buffer, offset) (ntohs(*(uint16_t *)&buffer[offset]))