Re: [tpmdd-devel] [PATCH 09/13] tpm: Pull everything related to sysfs into tpm-sysfs.c

[Daniel De Graaf <dgdegra@tycho.nsa.gov>]

On 09/30/2013 02:10 PM, Jason Gunthorpe wrote:
> On Tue, Sep 24, 2013 at 10:28:41AM -0400, Daniel De Graaf wrote:
>> On 09/23/2013 06:23 PM, Jason Gunthorpe wrote:
>>> On Mon, Sep 23, 2013 at 06:00:46PM -0400, Daniel De Graaf wrote:
>>>
>>>> In a PC client TPM, normal OS code (as opposed to firmware or microcode)
>>>> is already restricted to locality 0-2. It may make sense to restrict
>>>> locality 2 to the kernel, which would allow an in-kernel TPM seal
>>>> command to be able to bind data so that userspace cannot unseal it.
>>>> However, only allowing localities 0 and 1 to userspace may be too
>>>> restrictive if userspace also wishes to use locality for separation,
>>>> since locality 1 does not have the ability to reset any PCRs that
>>>> locality 0 cannot also reset.
>>>> The kernel could reserve only locality 1 for its own use, giving it the
>>>> ability to seal data but not interfering with the ability to reset PCRs.
>>>> This would be my preference, although it is less intuitive to allow code
>>>> of lower privilege (userspace) to control the higher numbered locality
>>>> 2.
>>>
>>> This matches my vague understanding (we don't use localities here)
>>>
>>>>> Perhaps a .config option would be useful to allow the system designer to
>>>>> choose what, if any, locality to reserve for kernel use?
>>>
>>> A runtime sysfs seems reasonable..
>>
>> Allowing a userspace application to change which locality is kernel- and
>> userspace-only will eliminate the primary benefit of having a locality
>> restricted to the kernel.
>
> Right, I was sort of thinking these sysfs files would be write-once or
> trapped doored to prevent going backwards (like how secure level
> worked). Ideally the write would be in the initramfs, which is part of
> the PCR computation, so it should have the same properties as CONFIG_?
>
> I think using CONFIG_ options would make this feature unavaiable to
> distro kernel users...

This just moves the problem - now you need a custom initrd instead of
a custom kernel. Other TPM options like IMA's PCR selection also must
be changed at CONFIG_ time, although that seems to be more justified
since IMA in TCB mode is not usable on any distro kernel that makes
the TPM driver a module (i.e. most or all of them).

>> At least "supported_localities" should be generated by the driver if it
>> is generated at all. There are a few different proposals for handling
>> localities over 4 in virtual TPMs; one is that locality numbers between
>> 32-255 would be permitted and 5-31 made non-addressable. While this
>> would work with a bitmask, I'm not sure that is the best solution.
>
> Hmm, a 256 bit wide mask isn't impossible, not sure what other
> options are good. Do you think userspace needs to know what localities
> are valid or is an ioctl scan sufficient?

I don't think userspace needs to know this directly. The system designer
will need know in order to create any kind of policy about what locality
is being used (even if that's just setting the default), but they would
also need to be aware of the hardware or virtual environment and could
always enumerate them manually as a last resort.

There is also the fact that the driver may not be able to tell if a
locality is available without doing some kind of test command. The Xen
TPM interface doesn't expose what localities are available, for example,
and the TIS interface may need to test to see if locality 3 and 4 are
actually blocked by the chipset - at least 3 might be available on some
systems (the spec leaves this "implementation dependent").

>> Perhaps:
>> 	default_locality - default to CONFIG_USER_DEFAULT_LOCALITY
>> 		sysfs node permissions 0644
>> 	kernel_locality - default to #CONFIG_KERNEL_DEFAULT_LOCALITY
>> 		0444 if CONFIG_KERNEL_ONLY_LOCALITY=y
>> 		0644 if CONFIG_KERNEL_ONLY_LOCALITY=n
>> 	ioctl TPM_{GET,SET}_LOCALITY on an open /dev/tpmX
>>
>> If CONFIG_KERNEL_ONLY_LOCALITY=y, the userspace locality is not
>> permitted to be equal to kernel_locality (but may take any other valid
>> value).  Drivers may reject locality values that they consider invalid
>> (the default should be to only allow 0-4, which is all that is defined
>> in the spec) or may fail on attempted use of the TPM by passing down an
>> error from the hardware - I would expect the latter to be the case on
>> attempts to use locality 4 in the tpm_tis driver.
>
> Seems resonable, CONFIG_KERNEL_ONLY_LOCALITY could be
> CONFIG_TPM_ONE_TIME_LOCALITY (eg you get to set kernel_locality only
> once)

Hmm, how much trouble would it be to make this a menu selection? Even
with the one-time-set option, you still need a default set either in
the code or by CONFIG_ so that the TPM is not unavailable before the
sysfs write. The options would be:

  - CONFIG_TPM_KERNEL_DEFAULT_LOCALITY = [int]
  - CONFIG_TPM_KERNEL_LOCALITY_FIXED - no changes from userspace
  - CONFIG_TPM_KERNEL_LOCALITY_ONESHOT - only one change possible
  - CONFIG_TPM_KERNEL_LOCALITY_ANY - may be changed freely

The userspace locality is not allowed to use the kernel locality if
the mode is either FIXED or ONESHOT, but may share locality if ANY
is used.

Or, for more flexibility (I actually like this one better):

  - CONFIG_TPM_KERNEL_DEFAULT_LOCALITY = [int]
  - CONFIG_TPM_KERNEL_LOCALITY_FIXED = [bool]

And sysfs contains:
  - kernel_locality [0644, int; 0444 if FIXED=y or when locked(?)]
  - lock_kernel_locality [write-once; only exists if FIXED=n]

Where kernel_locality may be changed until a write is made to
local_kernel_locality, at which time the value of kernel_locality
becomes read-only and no longer available via /dev/tpmX.

>> The only one I see immediately is seal/unseal (security/keys/trusted.c).
>> The invocation of the seal command would need to be changed to seal the
>> trusted keys to the kernel-only locality in order to take advantage of
>> the increased protection provided by a kernel-only locality.
>
> Right

Actually, only the invocation needs to be changed - the PCR selection
is passed in from userspace, which will just need to use PCR_INFO_LONG
with the proper locality mask.

>>> Do you know anyone on the userspace SW side who could look at this?
>
>> I should be able to find someone.
>
> Okay, let me know. I'd like to get a few more clean ups done before
> mucking with the sysfs, but the way forward for locality looks pretty
> clear..
>
> Thanks,
> Jason

So far, nobody I have talked to has offered any strong opinions on
what locality should be used or how it should be set. I think finding
a developer of trousers may be the most useful to talk about how the
ioctl portion of this would need to be set up - if someone is actually
needed.

-- 
Daniel De Graaf
National Security Agency