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/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. With the kernel-only locality selected at
compile (or possibly kernel command line) time, a reboot with different
measurements would be required for userspace to gain access to the
locality used to seal a secret intended for use by the kernel alone -
and the secret would presumably be sealed to those original
measurements.

> Would:
>   user_default_locality
>   kernel_default_locality
>   user_allowed_localities (bitmask)
>   supported_localities (bitmask)
>   a GET_LOCALITY/SET_LOCALITY IOCTL to change localities of an open'd
>    /dev/tpmX
>
> Do the job?

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.

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.

> At first glance anyhow. I wonder what in-kernel users would be
> impacted by localities..

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.

IMA could potentially be impacted by the locality selection if it were
configured to use a locality-restricted PCR; however, the default (10) is
not restricted and there is generally no need to use a locality-restricted
PCR for this.

> Any thoughts on root vs not-root? Would middelware want to use
> localities?

I think permissions on the /dev/tpmX node suffices for this distinction.
The TCS daemon would need to be trusted to separate multiple user-space
localities since it will be keeping /dev/tpmX open anyway.

> Do you know anyone on the userspace SW side who could look at this?
>
> Jason

I should be able to find someone.

-- 
Daniel De Graaf
National Security Agency