Fix TPM 2.0 trusted keys
Subsystems (7)
James Bottomley
maintainerv2 cover Fix TPM 2.0 trusted keys
This fixes a wide array of problems with the current TPM 2.0 implementation of trusted keys. Since policy based trusted keys never worked in the current implementation, I've rewritten the policy implementation to make it easier to use and so the trusted key handler can understand what elements of a policy are failing and why.
Apart from fixing bugs like volatile object leakage, I've changed the output format to use the standardised ASN.1 coding for TPM2 keys, meaning they should interoperate with userspace TPM2 key implementations. Apart from interoperability, another advantage of the existing key format is that it carries all parameters like parent and hash with it and it is capable of carrying policy directives in a way that mean they're tied permanently to the key (no having to try to remember what the policy was and reconstruct it from userspace). This actually allows us to support the TPM 1.2 commands like pcrinfo easily in 2.0.
The big problem with this patch is still that we can't yet combine policy with authorization because that requires proper session handling, but at least with this rewrite it becomes possible (whereas it was never possible with the old external policy session code). Thus, when we have the TPM 2.0 security patch upstream, we'll be able to use the session logic from that patch to imlement authorizations.
James
---
v2: Fix all the code review issues noticed by David Woodhouse and redo
the ASN.1 encoder API to allow in-place encoding for short tags and sequences
James Bottomley (8):
security: keys: trusted: flush the key handle after use
lib: add asn.1 encoder
oid_registry: Add TCG defined OIDS for TPM keys
security: keys: trusted: use ASN.1 tpm2 key format for the blobs
security: keys: trusted: Make sealed key properly interoperable
security: keys: trusted: add PCR policy to TPM2 keys
security: keys: trusted: add ability to specify arbitrary policy
security: keys: trusted: implement counter/timer policy
Documentation/security/keys/trusted-encrypted.rst | 70 +++- drivers/char/tpm/tpm.h | 1 - drivers/char/tpm/tpm2-cmd.c | 1 + include/keys/trusted-type.h | 6 +- include/linux/asn1_encoder.h | 21 ++ include/linux/oid_registry.h | 5 + include/linux/tpm.h | 8 + lib/Makefile | 2 +- lib/asn1_encoder.c | 258 ++++++++++++++ security/keys/Kconfig | 2 + security/keys/trusted-keys/Makefile | 2 +- security/keys/trusted-keys/tpm2-policy.c | 405 ++++++++++++++++++++++ security/keys/trusted-keys/tpm2-policy.h | 31 ++ security/keys/trusted-keys/tpm2key.asn1 | 23 ++ security/keys/trusted-keys/trusted_tpm1.c | 40 +-- security/keys/trusted-keys/trusted_tpm2.c | 307 ++++++++++++++-- 16 files changed, 1124 insertions(+), 58 deletions(-) create mode 100644 include/linux/asn1_encoder.h create mode 100644 lib/asn1_encoder.c create mode 100644 security/keys/trusted-keys/tpm2-policy.c create mode 100644 security/keys/trusted-keys/tpm2-policy.h create mode 100644 security/keys/trusted-keys/tpm2key.asn1
Signature · 1 line
2.16.4
James Bottomley
maintainerPATCH v2 1/8 security: keys: trusted: flush the key handle after use
The trusted keys code currently loads a blob into the TPM and unseals on the handle. However, it never flushes the handle meaning that volatile contexts build up until the TPM becomes unusable. Fix this by flushing the handle after the unseal.
---
v2: use EXPORT_SYMBOL_GPL
--- drivers/char/tpm/tpm.h | 1 - drivers/char/tpm/tpm2-cmd.c | 1 + include/linux/tpm.h | 1 + security/keys/trusted-keys/trusted_tpm2.c | 1 + 4 files changed, 3 insertions(+), 1 deletion(-)
drivers/char/tpm/tpm.h +0 -1
@@ -218,7 +218,6 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,int tpm2_pcr_extend (struct tpm_chip *chip,u32 pcr_idx,struct tpm_digest *digests);int tpm2_get_random (struct tpm_chip *chip,u8 *dest,size_t max); -void tpm2_flush_context (struct tpm_chip *chip,u32 handle);ssize_t tpm2_get_tpm_pt (struct tpm_chip *chip,u32 property_id,u32 *value,const char *desc);
drivers/char/tpm/tpm2-cmd.c +1 -0
@@ -362,6 +362,7 @@ void tpm2_flush_context(struct tpm_chip *chip, u32 handle)tpm_transmit_cmd (chip, &buf,0 ,"flushing context" );tpm_buf_destroy (&buf); } +EXPORT_SYMBOL_GPL (tpm2_flush_context);struct tpm2_get_cap_out {u8 more_data;
include/linux/tpm.h +1 -0
@@ -403,6 +403,7 @@ extern int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,extern int tpm_send (struct tpm_chip *chip,void *cmd,size_t buflen);extern int tpm_get_random (struct tpm_chip *chip,u8 *data,size_t max);extern struct tpm_chip *tpm_default_chip (void ); +void tpm2_flush_context (struct tpm_chip *chip,u32 handle);#else static inline int tpm_is_tpm2 (struct tpm_chip *chip) {
security/keys/trusted-keys/trusted_tpm2.c +1 -0
@@ -309,6 +309,7 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,return rc; rc =tpm2_unseal_cmd (chip, payload, options, blob_handle); +tpm2_flush_context (chip, blob_handle);return rc; }
Signature · 1 line
2.16.4
James Bottomley
maintainerPATCH v2 2/8 lib: add asn.1 encoder
In reply to James BottomleyWe have a need in the TPM trusted keys to return the ASN.1 form of the TPM key blob so it can be operated on by tools outside of the kernel. To do that, we have to be able to read and write the key format. The current ASN.1 decoder does fine for reading, but we need pieces of an ASN.1 encoder to return the key blob.
The current implementation only encodes the ASN.1 bits we actually need.
---
v2: updated API to use indefinite length, and made symbol exports gpl
--- include/linux/asn1_encoder.h | 21 ++++ lib/Makefile | 2 +- lib/asn1_encoder.c | 258 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 280 insertions(+), 1 deletion(-) create mode 100644 include/linux/asn1_encoder.h create mode 100644 lib/asn1_encoder.c
include/linux/asn1_encoder.h new file +21 -0
@@ -0,0 +1,21 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef _LINUX_ASN1_ENCODER_H +#define _LINUX_ASN1_ENCODER_H + +#include <linux/types.h> +#include <linux/asn1.h> +#include <linux/asn1_ber_bytecode.h> + +#define asn1_oid_len (oid) (sizeof(oid)/sizeof(u32)) +void asn1_encode_integer (unsigned char **_data,s64 integer,int len); +void asn1_encode_oid (unsigned char **_data,u32 oid[],int oid_len); +void asn1_encode_tag (unsigned char **data,u32 tag, +const unsigned char *string,int len); +void asn1_encode_octet_string (unsigned char **data,const unsigned char *string, +u32 len); +void asn1_encode_sequence (unsigned char **data,const unsigned char *seq, +int len); +void asn1_encode_boolean (unsigned char **data,bool val); + +#endif
lib/Makefile +1 -1
@@ -233,7 +233,7 @@ obj-$(CONFIG_INTERVAL_TREE_TEST) += interval_tree_test.o obj-$(CONFIG_PERCPU_TEST) += percpu_test.o -obj-$(CONFIG_ASN1) += asn1_decoder.o +obj-$(CONFIG_ASN1) += asn1_decoder.o asn1_encoder.o obj-$(CONFIG_FONT_SUPPORT) += fonts/
lib/asn1_encoder.c new file +258 -0
258 lines not on this page · raw message
Signature · 1 line
2.16.4
Re: [PATCH v2 2/8] lib: add asn.1 encoder
Quoted 46 lines · James Bottomley
lib/asn1_encoder.c new file +46
-0+/** +* asn1_encode_integer - encode positive integer to ASN.1 +* @_data: pointer to the pointer to the data +* @integer: integer to be encoded +* @len: length of buffer +* +* This is a simplified encoder: it only currently does +* positive integers, but it should be simple enough to add the +* negative case if a use comes along. +*/ +void asn1_encode_integer (unsigned char **_data,s64 integer,int len) +{ +unsigned char *data = *_data, *d = &data[2 ]; +int i; +bool found = false; + +if (WARN (integer <0 , +"BUG: asn1_encode_integer only supports positive integers" )) +return ; + +if (WARN (len <3 , +"BUG: buffer for integers must have at least 3 bytes" )) +return ; + + len =-2 ; + + data[0 ] =_tag (UNIV ,PRIM ,INT ); +if (integer ==0 ) { + *d++ =0 ; + goto out; + } +for (i =sizeof (integer); i >0 ; i--) { +int byte = integer >> (8 *(i-1 )); + +if (!found && byte ==0 ) +continue ; + found = true; +if (byte &0x80 ) { +/* +* no check needed here, we already know we +* have len >= 1 +*/ + *d++ =0 ; + len--; + } + if (WARN(len == 0,
lib/asn1_encoder.c line 68
Quoted 10 lines
lib/asn1_encoder.c new file +10
-0+ "BUG buffer too short in asn1_encode_integer")) +return ; + *d++ = byte; + len--; + } + out: + data[1 ] = d - data -2 ; + *_data = d; +} +EXPORT_SYMBOL_GPL (asn1_encode_integer);
Didn't you say you were going to make it return an error when it ran out of space or was asked to encode a negative number?
There are other encoding functions which you haven't yet added the buffer length field to, and they'll want to be able to return -ENOSPC too.
James Bottomley
maintainerRe: [PATCH v2 2/8] lib: add asn.1 encoder
Quoted 57 lines · David Woodhouse
Quoted 56 lines · James Bottomley
lib/asn1_encoder.c new file +11
-0+/** +* asn1_encode_integer - encode positive integer to ASN.1 +* @_data: pointer to the pointer to the data +* @integer: integer to be encoded +* @len: length of buffer +* +* This is a simplified encoder: it only currently does +* positive integers, but it should be simple enough to add the +* negative case if a use comes along. +*/ +void asn1_encode_integer(unsigned char **_data, s64 integer, intlen)
lib/asn1_encoder.c new file +7
-0+{ +unsigned char *data = *_data, *d = &data[2 ]; +int i; +bool found = false; + + if (WARN(integer < 0, + "BUG: asn1_encode_integer only supports positiveintegers"))
lib/asn1_encoder.c new file +4
-0+return ; + + if (WARN(len < 3, + "BUG: buffer for integers must have at least 3bytes"))
lib/asn1_encoder.c new file +25
-0+return ; + + len =-2 ; + + data[0 ] =_tag (UNIV ,PRIM ,INT ); +if (integer =0 ) { + *d++ =0 ; + goto out; + } +for (i =sizeof (integer); i >0 ; i--) { +int byte = integer >> (8 *(i-1 )); + +if (!found && byte =0 ) +continue ; + found = true; +if (byte &0x80 ) { +/* +* no check needed here, we already know we +* have len >= 1 +*/ + *d++ =0 ; + len--; + } + if (WARN(len = 0, + "BUG buffer too short in
asn1_encode_integer"))lib/asn1_encoder.c new file +5
-0+return ; + *d++ = byte; + len--; + } + out:
Quoted 8 lines
lib/asn1_encoder.c line 68
Quoted 4 lines
lib/asn1_encoder.c new file +4
-0+ data[1 ] = d - data -2 ; + *_data = d; +} +EXPORT_SYMBOL_GPL (asn1_encode_integer);Didn't you say you were going to make it return an error when it ran out of space or was asked to encode a negative number?
it follows the pattern of all the other functions in that it dumps a kernel warning on problems and bails. I don't really want to add error handling for my use case, since it's not expected to have any problems.
My main problem case is a malicious user tricking the kernel into
trying to overflow the output buffer and in that case I don't really care that the ASN.1 output will be malformed as long as the buffer doesn't overflow.
James
Quoted 3 lines
There are other encoding functions which you haven't yet added the buffer length field to, and they'll want to be able to return
-ENOSPCtoo.
David Howells
maintainerRe: [PATCH v2 2/8] lib: add asn.1 encoder
Quoted 5 lines · James Bottomley
Quoted 2 lines
Didn't you say you were going to make it return an error when it ran out of space or was asked to encode a negative number?
it follows the pattern of all the other functions in that it dumps a kernel warning on problems and bails.
I don't see any bounds checking there, let alone anything that dumps a kernel warning and bails - except if the length is so large that the ASN.1 cannot be constructed. That said, there is a check in patch 4.
Quoted 1 line
... as long as the buffer doesn't overflow.
Yes, but that's Dave's point.
[from patch 4]
Review
Quoted 1 line
(quoted diff) +1
-0+ * Assume both ovtet strings will encode to a 2 byte definite length
octet, btw.
At least I've found a preliminary bounds check there
Review
Quoted 4 lines
(quoted diff) +4
-0+ */ + if (WARN(work - scratch + pub_len + priv_len + 8 > SCRATCH_SIZE, + "BUG: scratch buffer is too small")) + return -EINVAL;
which I presume makes the correct calculation.
I thought TPM comms were slow - slow enough that putting bounds checking in your asn1_encode_* routines will be insignificant.
Further, you're promoting this ASN.1 encoder as a general library within the kernel, presumably so that other people can make use of it. Please therefore put bounds checking and error handling in it. And please *don't* just produce broken ASN.1 when something goes wrong.
David
James Bottomley
maintainerRe: [PATCH v2 2/8] lib: add asn.1 encoder
Quoted 10 lines · David Howells
Quoted 5 lines · James Bottomley
Quoted 2 lines
Didn't you say you were going to make it return an error when it ran out of space or was asked to encode a negative number?
it follows the pattern of all the other functions in that it dumps a kernel warning on problems and bails.
I don't see any bounds checking there, let alone anything that dumps a kernel warning and bails
It's in the if (WARN part of asn1_encode_integer.
Quoted 2 lines
- except if the length is so large that the ASN.1 cannot be constructed. That said, there is a check in patch 4.
However, I think you'd like both a length and a buffer length to each function to cope with definite length encoding overflows? I can do that.
Quoted 10 lines
Quoted 1 line
... as long as the buffer doesn't overflow.
Yes, but that's Dave's point.
[from patch 4]
Quoted 2 lines
(quoted diff) +1
-0+ * Assume both ovtet strings will encode to a 2 bytedefinite length
octet, btw.
Heh, yes, noticed that mere seconds after I pressed send ...
Quoted 10 lines
At least I've found a preliminary bounds check there
Review
Quoted 5 lines
(quoted diff) +2
-0+ */ + if (WARN(work - scratch + pub_len + priv_len + 8 >
SCRATCH_SIZE,(quoted diff) +2
-0+ "BUG: scratch buffer is too small")) + return -EINVAL;which I presume makes the correct calculation.
Quoted 2 lines
I thought TPM comms were slow - slow enough that putting bounds checking in your asn1_encode_* routines will be insignificant.
Yes, I'm not bothered about timings. I can add bounds checking on the buffer length like the integer case. For the other routines, I'll make it decrement the data length in place as it increments the data pointer
Quoted 3 lines
Further, you're promoting this ASN.1 encoder as a general library within the kernel, presumably so that other people can make use of it.
Well, I did notice the TPM 1.2 asymmetric key code rolled its own ASN.1 encoding, yes.
Quoted 2 lines
Please therefore put bounds checking and error handling in it. And
please *don't* just produce broken ASN.1 when something goes wrong.
OK, I'll make it return an error and add a wrapper for my use case that warns on error and causes the function to bail.
James
Re: [PATCH v2 2/8] lib: add asn.1 encoder
Quoted 6 lines · James Bottomley
Quoted 2 lines · David Howells
Please therefore put bounds checking and error handling in it. And
please *don't* just produce broken ASN.1 when something goes wrong.OK, I'll make it return an error and add a wrapper for my use case that warns on error and causes the function to bail.
Traditionally we call that "error handling" :p
Signature · 1 line
Sent from my Android device with K-9 Mail. Please excuse my brevity.
James Bottomley
maintainerRe: [PATCH v2 2/8] lib: add asn.1 encoder
Quoted 9 lines · David Woodhouse
Quoted 6 lines · James Bottomley
Quoted 4 lines · David Howells
Please therefore put bounds checking and error handling in
it. And
please *don't* just produce broken ASN.1 when something goes wrong.
Quoted 5 lines
Quoted 3 lines
OK, I'll make it return an error and add a wrapper for my use case that
warns on error and causes the function to bail.Traditionally we call that "error handling" :p
This is what I'm thinking (still reworking the rest of the series ... I found out that AA doesn't habitually have power sockets and my laptop dying in the middle of a rebase turned out not to be such a good thing).
James
---
From 7b0c52cf07ca2b8b8ddbe0442a6d3f9de30f7b1b Mon Sep 17 00:00:00 2001
From: James Bottomley <[email protected]>
Date: Wed, 4 Dec 2019 15:19:01 -0800
Subject: [PATCH] lib: add asn.1 encoder
We have a need in the TPM trusted keys to return the ASN.1 form of the TPM key blob so it can be operated on by tools outside of the kernel. To do that, we have to be able to read and write the key format. The current ASN.1 decoder does fine for reading, but we need pieces of an ASN.1 encoder to return the key blob.
The current implementation only encodes the ASN.1 bits we actually need.
---
v2: updated API to use indefinite length, and made symbol exports gpl
include/linux/asn1_encoder.h new file +24 -0
@@ -0,0 +1,24 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef _LINUX_ASN1_ENCODER_H +#define _LINUX_ASN1_ENCODER_H + +#include <linux/types.h> +#include <linux/asn1.h> +#include <linux/asn1_ber_bytecode.h> +#include <linux/bug.h> + +#define asn1_oid_len (oid) (sizeof(oid)/sizeof(u32)) +int asn1_encode_integer (unsigned char **_data,int *data_len, +s64 integer); +int asn1_encode_oid (unsigned char **_data,int *data_len, +u32 oid[],int oid_len); +int asn1_encode_tag (unsigned char **data,int *data_len,u32 tag, +const unsigned char *string,int len); +int asn1_encode_octet_string (unsigned char **data,int *data_len, +const unsigned char *string,u32 len); +int asn1_encode_sequence (unsigned char **data,int *data_len, +const unsigned char *seq,int len); +int asn1_encode_boolean (unsigned char **data,int *data_len,bool val); + +#endif
lib/Makefile +1 -1
@@ -233,7 +233,7 @@ obj-$(CONFIG_INTERVAL_TREE_TEST) += interval_tree_test.o obj-$(CONFIG_PERCPU_TEST) += percpu_test.o -obj-$(CONFIG_ASN1) += asn1_decoder.o +obj-$(CONFIG_ASN1) += asn1_decoder.o asn1_encoder.o obj-$(CONFIG_FONT_SUPPORT) += fonts/
lib/asn1_encoder.c new file +362 -0
362 lines not on this page · raw message
David Howells
maintainerRe: [PATCH v2 2/8] lib: add asn.1 encoder
lib/asn1_encoder.c line 267
Quoted 11 lines · James Bottomley
lib/asn1_encoder.c new file +11
-0+/** +* asn1_encode_octet_string - encode an ASN.1 OCTET STRING +* @data: pointer to encode at +* @data_len: bytes remaining in @data buffer +* @string: string to be encoded +* @len: length of string +* +* Note ASN.1 octet strings may contain zeros, so the length is obligatory. +*/ +int asn1_encode_octet_string (unsigned char **data,int *data_len, +const unsigned char *string,u32 len)
I wonder if it makes more sense to pass in an end pointer and return the new data pointer (or an error), ie.:
unsigned char *asn1_encode_octet_string(unsigned char *data,
unsigned char *data_end, const unsigned char *string, u32 len)
Further, I wonder - does it actually make more sense to encode backwards, ie. start at the end of the buffer and do the last element first, working towards the front.
The disadvantage being that the data start would likely not be coincident with the buffer start.
David
James Bottomley
maintainerRe: [PATCH v2 2/8] lib: add asn.1 encoder
Quoted 18 lines · David Howells
lib/asn1_encoder.c line 267
Quoted 12 lines · James Bottomley
+/**
+ *asn1_encode_octet_string- encode an ASN.1 OCTET STRING
+ * @data: pointer to encode at
+ * @data_len: bytes remaining in @data buffer
+ * @string: string to be encoded
+ * @len: length of string
+ *
+ * Note ASN.1 octet strings may contain zeros, so the length is obligatory.
+ */lib/asn1_encoder.c new file +2
-0+int asn1_encode_octet_string (unsigned char **data,int *data_len, +const unsigned char *string,u32 len)I wonder if it makes more sense to pass in an end pointer and return the new data pointer (or an error), ie.:
Quoted 4 lines
unsigned char *asn1_encode_octet_string(unsigned char *data,
unsigned char*data_end, const unsigned char *string,
u32 len)
On the first point: people are prone to get off by one confusion on data_end pointers (should they point to the last byte in the buffer or one beyond). If I look at how I use the API, I've no real use for either length remaining or the end pointer, so I think it makes no difference to the consumer, it's just stuff you have to do for the API.
If I look at the internal API use, we definitely need the length
remaining, so I've a marginal preference for that format, but since it's easy to work out it is very marginal.
Quoted 3 lines
Further, I wonder - does it actually make more sense to encode backwards, ie. start at the end of the buffer and do the last element first, working towards the front.
Heh, let me ask you this: do you use a reverse polish notation calculator ... The problem is that it makes the ASN.1 hard to construct
for the API user and hard to read for the reviewer because of the
order reversal. Debugging is going to be a pain because you're going to get the output of asn1parse and have to read it backwards to see where the problems are.
Quoted 2 lines
The disadvantage being that the data start would likely not be coincident with the buffer start.
This would be a big issue: in several routines I allocate a buffer, fill it with ASN.1 and pass it back and the receiving routine has to free it. Now the buffer won't be freeable by the pointer I pass back because that may not be where the allocation was done.
For these two reasons, I'd like to keep the work forwards behaviour. I'm reasonably ambivalent on the end pointer with a marginal preference for passing in the length remaining instead.
James
James Bottomley
maintainerRe: [PATCH v2 2/8] lib: add asn.1 encoder
Quoted 43 lines · James Bottomley
Quoted 21 lines · David Howells
Quoted 13 lines · James Bottomley
+/**
+ *asn1_encode_octet_string- encode an ASN.1 OCTET STRING
+ * @data: pointer to encode at
+ * @data_len: bytes remaining in @data buffer
+ * @string: string to be encoded
+ * @len: length of string
+ *
+ * Note ASN.1 octet strings may contain zeros, so the length is obligatory.
+ */
+int asn1_encode_octet_string(unsigned char **data, int*data_len,(quoted diff) +1
-0+ const unsigned char *string, u32Quoted 1 line
len)
I wonder if it makes more sense to pass in an end pointer and return
the new data pointer (or an error), ie.:Quoted 4 lines
unsigned char *asn1_encode_octet_string(unsigned char *data,
unsigned char*data_end, const unsigned char *string,
u32 len)On the first point: people are prone to get off by one confusion on
data_endpointers (should they point to the last byte in the buffer or
one beyond). If I look at how I use the API, I've no real use for either length remaining or the end pointer, so I think it makes no difference to the consumer, it's just stuff you have to do for the API.
If I look at the internal API use, we definitely need the length
remaining, so I've a marginal preference for that format, but since it's easy to work out it is very marginal.Quoted 4 lines
Further, I wonder - does it actually make more sense to encode backwards, ie. start at the end of the buffer and do the last element
first, working towards the front.
Quoted 6 lines
Heh, let me ask you this: do you use a reverse polish notation calculator ... The problem is that it makes the ASN.1 hard to construct for the API user and hard to read for the reviewer because of the order reversal. Debugging is going to be a pain because you're going to get the output of asn1parse and have to read it backwards to see where the problems are.
I coded this up to see what it would look like, and I think it can all be made to work with error pass through. The latter is because you want to build up sequences of
data = asn1_encode...(data, ...);
data = asn1_encode...(data, ...);
data = asn1_encode...(data, ...);
And only check for errors when you're finished. I think the interface looks nicer than a modifying pointer, so if you wait for the v4 patches they'll show this new interface with the consumers.
James
James Bottomley
maintainerPATCH v2 3/8 oid_registry: Add TCG defined OIDS for TPM keys
In reply to James BottomleyThe TCG has defined an OID prefix "2.23.133.10.1" for the various TPM key uses. We've defined three of the available numbers:
2.23.133.10.1.3 TPM Loadable key. This is an asymmetric key (Usually
RSA2048 or Elliptic Curve) which can be imported by a TPM2_Load() operation.
2.23.133.10.1.4 TPM Importable Key. This is an asymmetric key (Usually
RSA2048 or Elliptic Curve) which can be imported by a TPM2_Import() operation.
Both loadable and importable keys are specific to a given TPM, the difference is that a loadable key is wrapped with the symmetric secret, so must have been created by the TPM itself. An importable key is wrapped with a DH shared secret, and may be created without access to the TPM provided you know the public part of the parent key.
2.23.133.10.1.5 TPM Sealed Data. This is a set of data (up to 128
bytes) which is sealed by the TPM. It usually represents a symmetric key and must be unsealed before use.
--- include/linux/oid_registry.h | 5 +++++ 1 file changed, 5 insertions(+)
include/linux/oid_registry.h +5 -0
@@ -107,6 +107,11 @@ enum OID { OID_gostTC26Sign512B,/* 1.2.643.7.1.2.1.2.2 */ OID_gostTC26Sign512C,/* 1.2.643.7.1.2.1.2.3 */ +/* TCG defined OIDS for TPM based keys */ + OID_TPMLoadableKey,/* 2.23.133.10.1.3 */ + OID_TPMImporableKey,/* 2.23.133.10.1.4 */ + OID_TPMSealedData,/* 2.23.133.10.1.5 */ + OID__NR };
Signature · 1 line
2.16.4
Re: [PATCH v2 3/8] oid_registry: Add TCG defined OIDS for TPM keys
include/linux/oid_registry.h enum OID { · line 112
Quoted 3 lines · James Bottomley
include/linux/oid_registry.h +3
-0+/* TCG defined OIDS for TPM based keys */ + OID_TPMLoadableKey,/* 2.23.133.10.1.3 */ + OID_TPMImporableKey, /* 2.23.133.10.1.4 */
There's a t missing from OID_TPMImpoTableKey. Sorry, missed that last time.
James Bottomley
maintainerRe: [PATCH v2 3/8] oid_registry: Add TCG defined OIDS for TPM keys
Quoted 8 lines · David Woodhouse
include/linux/oid_registry.h enum OID { · line 112
Quoted 3 lines · James Bottomley
include/linux/oid_registry.h +3
-0+/* TCG defined OIDS for TPM based keys */ + OID_TPMLoadableKey,/* 2.23.133.10.1.3 */ + OID_TPMImporableKey, /* 2.23.133.10.1.4 */There's a t missing from
OID_TPMImpoTableKey. Sorry, missed that last time.
Heh, yes, will fix.
James
James Bottomley
maintainerPATCH v2 4/8 security: keys: trusted: use ASN.1 tpm2 key format for the blobs
In reply to James BottomleyModify the tpm2 key format blob output to export and import in the ASN.1 form for tpm2 sealed object keys. For compatibility with prior trusted keys, the importer will also accept two tpm2b quantities representing the public and private parts of the key. However, the export via keyctl pipe will only output the ASN.1 format.
The benefit of the ASN.1 format is that it's a standard and thus the exported key can be used by userspace tools. The format includes policy specifications, thus it gets us out of having to construct policy handles in userspace and the format includes the parent meaning you don't have to keep passing it in each time.
This patch only implements basic handling for the ASN.1 format, so keys with passwords but no policy.
v2: Updated encode API, added length checks
--- security/keys/trusted-keys/Makefile | 2 +- security/keys/trusted-keys/tpm2key.asn1 | 23 ++++ security/keys/trusted-keys/trusted_tpm1.c | 2 +- security/keys/trusted-keys/trusted_tpm2.c | 181 +++++++++++++++++++++++++++++- 4 files changed, 201 insertions(+), 7 deletions(-) create mode 100644 security/keys/trusted-keys/tpm2key.asn1
security/keys/trusted-keys/Makefile +1 -1
@@ -5,4 +5,4 @@ obj-$(CONFIG_TRUSTED_KEYS) += trusted.o trusted-y += trusted_tpm1.o -trusted-y += trusted_tpm2.o +trusted-y += trusted_tpm2.o tpm2key.asn1.o
security/keys/trusted-keys/tpm2key.asn1 new file +23 -0
@@ -0,0 +1,23 @@ +--- +--- Note: This isn't quite the definition in the standard +--- However, the Linux asn.1 parser doesn't understand +--- [2] EXPLICIT SEQUENCE OF OPTIONAL +--- So there's an extra intermediate TPMPolicySequence +--- definition to work around this + +TPMKey ::= SEQUENCE { + type OBJECT IDENTIFIER ({tpmkey_type}), + emptyAuth [0] EXPLICIT BOOLEAN OPTIONAL, + policy [1] EXPLICIT TPMPolicySequence OPTIONAL, + secret [2] EXPLICIT OCTET STRING OPTIONAL, + parent INTEGER ({tpmkey_parent}), + pubkey OCTET STRING ({tpmkey_pub}), + privkey OCTET STRING ({tpmkey_priv}) + } + +TPMPolicySequence ::= SEQUENCE OF TPMPolicy + +TPMPolicy ::= SEQUENCE { + commandCode [0] EXPLICIT INTEGER, + commandPolicy [1] EXPLICIT OCTET STRING + }
security/keys/trusted-keys/trusted_tpm1.c +1 -1
@@ -991,7 +991,7 @@ static int trusted_instantiate(struct key *key, goto out; } -if (!options->keyhandle) { +if (!options->keyhandle && !tpm2) { ret = -EINVAL ; goto out; }
security/keys/trusted-keys/trusted_tpm2.c +175 -5
239 lines not on this page · raw message
Signature · 1 line
2.16.4
James Bottomley
maintainerPATCH v2 5/8 security: keys: trusted: Make sealed key properly interoperable
In reply to James BottomleyThe current implementation appends a migratable flag to the end of a key, meaning the format isn't exactly interoperable because the using party needs to know to strip this extra byte. However, all other consumers of TPM sealed blobs expect the unseal to return exactly the key. Since TPM2 keys have a key property flag that corresponds to migratable, use that flag instead and make the actual key the only sealed quantity. This is secure because the key properties are bound to a hash in the private part, so if they're altered the key won't load.
Backwards compatibility is implemented by detecting whether we're loading a new format key or not and correctly setting migratable from the last byte of old format keys.
---
v2: added length checks to untrusted payload
--- include/keys/trusted-type.h | 1 + include/linux/tpm.h | 2 + security/keys/trusted-keys/trusted_tpm2.c | 69 ++++++++++++++++++++++--------- 3 files changed, 53 insertions(+), 19 deletions(-)
include/keys/trusted-type.h +1 -0
@@ -22,6 +22,7 @@ struct trusted_key_payload {unsigned int key_len;unsigned int blob_len;unsigned char migratable; +unsigned char old_format;unsigned char key[MAX_KEY_SIZE +1 ];unsigned char blob[MAX_BLOB_SIZE ]; };
include/linux/tpm.h +2 -0
@@ -297,6 +297,8 @@ struct tpm_buf { };enum tpm2_object_attributes { +TPM2_OA_FIXED_TPM =BIT (1 ), +TPM2_OA_FIXED_PARENT =BIT (4 ),TPM2_OA_USER_WITH_AUTH =BIT (6 ), };
security/keys/trusted-keys/trusted_tpm2.c +50 -19
124 lines not on this page · raw message
Signature · 1 line
2.16.4
James Bottomley
maintainerPATCH v2 6/8 security: keys: trusted: add PCR policy to TPM2 keys
In reply to James BottomleyThis commit adds the ability to specify a PCR lock policy to TPM2 keys. There is a complexity in that the creator of the key must chose either to use a PCR lock policy or to use authentication. At the current time they can't use both due to a complexity with the way authentication works when policy registers are in use. The way to construct a pcrinfo statement for a key is simply to use the TPMS_PCR_SELECT structure to specify the PCRs and follow this by a hash of all their values in order of ascending PCR number.
For simplicity, we require the policy name hash and the hash used for the PCRs to be the same. Thus to construct a policy around the value of the resettable PCR 16 using the sha1 bank, first reset the pcr to zero giving a hash of all zeros as:
6768033e216468247bd031a0a2d9876d79818f8f
Then the TPMS_PCR_SELECT value for PCR 16 is
03000001
So create a new 32 byte key with a policy policy locking the key to this value of PCR 16 with a parent key of 81000001 would be:
keyctl new 32 keyhandle=0x81000001 hash=sha1 pcrinfo0000016768033e216468247bd031a0a2d9876d79818f8f" @u
---
v2: fix for new ASN.1 API eliminating hack in place and check lengths
--- Documentation/security/keys/trusted-encrypted.rst | 25 +- include/keys/trusted-type.h | 5 +- include/linux/tpm.h | 5 + security/keys/Kconfig | 2 + security/keys/trusted-keys/Makefile | 2 +- security/keys/trusted-keys/tpm2-policy.c | 340 ++++++++++++++++++++++ security/keys/trusted-keys/tpm2-policy.h | 30 ++ security/keys/trusted-keys/tpm2key.asn1 | 4 +- security/keys/trusted-keys/trusted_tpm1.c | 32 +- security/keys/trusted-keys/trusted_tpm2.c | 86 +++++- 10 files changed, 480 insertions(+), 51 deletions(-) create mode 100644 security/keys/trusted-keys/tpm2-policy.c create mode 100644 security/keys/trusted-keys/tpm2-policy.h
Documentation/security/keys/trusted-encrypted.rst +18 -7
@@ -60,19 +60,16 @@ Usage:: (40 ascii zeros) blobauth= ascii hex auth for sealed data default 0x00... (40 ascii zeros) - pcrinfo= ascii hex of PCR_INFO or PCR_INFO_LONG (no default) + pcrinfo= ascii hex of PCR_INFO or PCR_INFO_LONG (no + default) on TPM 1.2 and a TPMS_PCR_SELECTION + coupled with a hash of all the selected PCRs on + TPM 2.0 using the selected hash. pcrlock= pcr number to be extended to "lock" blob migratable= 0|1 indicating permission to reseal to new PCR values, default 1 (resealing allowed) hash= hash algorithm name as a string. For TPM 1.x the only allowed value is sha1. For TPM 2.x the allowed values are sha1, sha256, sha384, sha512 and sm3-256. - policydigest= digest for the authorization policy. must be calculated - with the same hash algorithm as specified by the 'hash=' - option. - policyhandle= handle to an authorization policy session that defines the - same policy and with the same hash algorithm as was used to - seal the key. "keyctl print" returns an ascii hex copy of the sealed key, which is in standard TPM_STORED_DATA format. The key length for new keys are always in bytes.
@@ -151,6 +148,20 @@ Load a trusted key from the saved blob:: f1f8fff03ad0acb083725535636addb08d73dedb9832da198081e5deae84bfaf0409c22b e4a8aea2b607ec96931e6f4d4fe563ba +Create a trusted key on TPM 2.0 using an all zero value of PCR16 and +using the NV storage root 81000001 as the parent:: + + $ keyctl add trusted kmk "new 32 keyhandle=0x81000001 hash=sha1 pcrinfo0000016768033e216468247bd031a0a2d9876d79818f8f" @u + +Note the TPMS_PCR_SELECT value for PCR 16 is 03000001 because all +current TPMs have 24 PCRs, so the initial 03 says there are three +following bytes of selection and then because the bytes are big +endian, 16 is bit zero of byte 2. the hash is the sha1 sum of all +zeros (the value of PCR 16):: + + $ dd if=/dev/zero bs=1 count 2>/dev/null|sha1sum + 6768033e216468247bd031a0a2d9876d79818f8f + Reseal a trusted key under new pcr values:: $ keyctl update 268728824 "update pcrinfo=`cat pcr.blob`"
include/keys/trusted-type.h +4 -1
@@ -14,9 +14,11 @@#define MIN_KEY_SIZE 32#define MAX_KEY_SIZE 128#define MAX_BLOB_SIZE 512 -#define MAX_PCRINFO_SIZE 64 +#define MAX_PCRINFO_SIZE 128#define MAX_DIGEST_SIZE 64 +#define TPM2_MAX_POLICIES 16 +struct trusted_key_payload {struct rcu_head rcu;unsigned int key_len;
@@ -25,6 +27,7 @@ struct trusted_key_payload {unsigned char old_format;unsigned char key[MAX_KEY_SIZE +1 ];unsigned char blob[MAX_BLOB_SIZE ]; +struct tpm2_policies *policies; };struct trusted_key_options {
include/linux/tpm.h +5 -0
@@ -222,10 +222,14 @@ enum tpm2_command_codes {TPM2_CC_CONTEXT_LOAD =0x0161 ,TPM2_CC_CONTEXT_SAVE =0x0162 ,TPM2_CC_FLUSH_CONTEXT =0x0165 , +TPM2_CC_POLICY_AUTHVALUE =0x016B , +TPM2_CC_POLICY_COUNTER_TIMER =0x016D , +TPM2_CC_START_AUTH_SESS =0x0176 ,TPM2_CC_VERIFY_SIGNATURE =0x0177 ,TPM2_CC_GET_CAPABILITY =0x017A ,TPM2_CC_GET_RANDOM =0x017B ,TPM2_CC_PCR_READ =0x017E , +TPM2_CC_POLICY_PCR =0x017F ,TPM2_CC_PCR_EXTEND =0x0182 ,TPM2_CC_EVENT_SEQUENCE_COMPLETE =0x0185 , TPM2_CC_HASH_SEQUENCE_START = 0x0186,
@@ -234,6 +238,7 @@ enum tpm2_command_codes { };enum tpm2_permanent_handles { +TPM2_RH_NULL =0x40000007 ,TPM2_RS_PW =0x40000009 , };
security/keys/Kconfig +2 -0
@@ -80,6 +80,8 @@ config TRUSTED_KEYS select CRYPTO select CRYPTO_HMAC select CRYPTO_SHA1 + select CRYPTO_SHA256 + select CRYPTO_SHA512 select CRYPTO_HASH_INFO help This option provides support for creating, sealing, and unsealing
security/keys/trusted-keys/Makefile +1 -1
@@ -5,4 +5,4 @@ obj-$(CONFIG_TRUSTED_KEYS) += trusted.o trusted-y += trusted_tpm1.o -trusted-y += trusted_tpm2.o tpm2key.asn1.o +trusted-y += trusted_tpm2.o tpm2key.asn1.o tpm2-policy.o
security/keys/trusted-keys/tpm2-policy.c new file +340 -0
340 lines not on this page · raw message
security/keys/trusted-keys/tpm2-policy.h new file +30 -0
@@ -0,0 +1,30 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +struct tpm2key_context { +u32 parent; +const u8 *pub; +u32 pub_len; +const u8 *priv; +u32 priv_len; +const u8 *policies[TPM2_MAX_POLICIES ]; +u32 policy_code[TPM2_MAX_POLICIES ]; +u16 policy_len[TPM2_MAX_POLICIES ]; +u8 policy_count; +}; + +struct tpm2_policies { +u32 code[TPM2_MAX_POLICIES ]; +u8 *policies[TPM2_MAX_POLICIES ]; +u16 len[TPM2_MAX_POLICIES ]; +u8 count; +u16 hash; +u16 hash_size; +}; + +int tpmkey_policy_process (struct tpm2key_context *ctx, +struct trusted_key_payload *payload); +int tpm2_generate_policy_digest (struct tpm2_policies *pols,u32 hash, +u8 *policydigest,u32 *plen); +int tpm2_encode_policy (struct tpm2_policies *pols,u8 **data,u32 *len); +int tpm2_get_policy_session (struct tpm_chip *chip,struct tpm2_policies *pols, +u32 *handle);
security/keys/trusted-keys/tpm2key.asn1 +2 -2
@@ -18,6 +18,6 @@ TPMKey ::= SEQUENCE { TPMPolicySequence ::= SEQUENCE OF TPMPolicy TPMPolicy ::= SEQUENCE { - commandCode [0] EXPLICIT INTEGER, - commandPolicy [1] EXPLICIT OCTET STRING + commandCode [0] EXPLICIT INTEGER ({tpmkey_code}), + commandPolicy [1] EXPLICIT OCTET STRING ({tpmkey_policy}) }
security/keys/trusted-keys/trusted_tpm1.c +6 -26
@@ -707,8 +707,6 @@ enum { Opt_keyhandle, Opt_keyauth, Opt_blobauth, Opt_pcrinfo, Opt_pcrlock, Opt_migratable, Opt_hash, - Opt_policydigest, - Opt_policyhandle, }; static const match_table_t key_tokens = {
@@ -722,8 +720,6 @@ static const match_table_t key_tokens = { {Opt_pcrlock, "pcrlock=%s"}, {Opt_migratable, "migratable=%s"}, {Opt_hash, "hash=%s"}, - {Opt_policydigest, "policydigest=%s"}, - {Opt_policyhandle, "policyhandle=%s"}, {Opt_err, NULL} };
@@ -738,7 +734,6 @@ static int getoptions(char *c, struct trusted_key_payload *pay,unsigned long handle;unsigned long lock;unsigned long token_mask =0 ; -unsigned int digest_len;int i;int tpm2;
@@ -801,8 +796,6 @@ static int getoptions(char *c, struct trusted_key_payload *pay, opt->pcrlock = lock;break ;case Opt_hash: -if (test_bit (Opt_policydigest, &token_mask)) -return -EINVAL ;for (i =0 ; i <HASH_ALGO__LAST ; i++) {if (!strcmp (args[0 ].from, hash_algo_name[i])) { opt->hash = i;
@@ -816,24 +809,6 @@ static int getoptions(char *c, struct trusted_key_payload *pay,return -EINVAL ; }break ; -case Opt_policydigest: - digest_len = hash_digest_size[opt->hash]; -if (!tpm2 ||strlen (args[0 ].from) != (2 * digest_len)) -return -EINVAL ; - res =hex2bin (opt->policydigest, args[0 ].from, - digest_len); -if (res <0 ) -return -EINVAL ; - opt->policydigest_len = digest_len; -break ; -case Opt_policyhandle: -if (!tpm2) -return -EINVAL ; - res =kstrtoul (args[0 ].from,16 , &handle); -if (res <0 ) -return -EINVAL ; - opt->policyhandle = handle; -break ;default :return -EINVAL ; }
@@ -1045,6 +1020,7 @@ static void trusted_rcu_free(struct rcu_head *rcu)struct trusted_key_payload *p; p = container_of(rcu, struct trusted_key_payload, rcu); +kzfree (p->policies);kzfree (p); }
@@ -1164,7 +1140,11 @@ static long trusted_read(const struct key *key, char __user *buffer,*/ static void trusted_destroy (struct key *key) { -kzfree (key->payload.data[0 ]); +struct trusted_key_payload *p; + + p = key->payload.data[0 ]; +kzfree (p->policies); +kzfree (p); }struct key_type key_type_trusted = {
security/keys/trusted-keys/trusted_tpm2.c +72 -14
148 lines not on this page · raw message
Signature · 1 line
2.16.4
James Bottomley
maintainerPATCH v2 7/8 security: keys: trusted: add ability to specify arbitrary policy
In reply to James BottomleyThis patch adds a policy= argument to key creation. The policy is the standard tss policymaker format and each separate policy line must have a newline after it.
Thus to construct a policy requiring authorized value and pcr 16 locking using a sha256 hash, the policy (policy.txt) file would be two lines:
0000017F00000001000B03000001303095B49BE85E381E5B20E557E46363EF55B0F43B132C2D8E3DE9AC436656F2 0000016b
This can be inserted into the key with
keyctl add trusted kmk "new 32 policy=cat policy.txt keyhandle=0x81000001 hash=sha256" @u
Note that although a few policies work like this, most require special handling which must be added to the kernel policy construction routine.
--- Documentation/security/keys/trusted-encrypted.rst | 16 ++++++++ security/keys/trusted-keys/tpm2-policy.c | 46 +++++++++++++++++++++++ security/keys/trusted-keys/tpm2-policy.h | 1 + security/keys/trusted-keys/trusted_tpm1.c | 14 ++++++- 4 files changed, 76 insertions(+), 1 deletion(-)
Documentation/security/keys/trusted-encrypted.rst +16 -0
@@ -70,6 +70,9 @@ Usage:: hash= hash algorithm name as a string. For TPM 1.x the only allowed value is sha1. For TPM 2.x the allowed values are sha1, sha256, sha384, sha512 and sm3-256. + policy= specify an arbitrary set of policies. These must + be in policymaker format with each separate + policy line newline terminated. "keyctl print" returns an ascii hex copy of the sealed key, which is in standard TPM_STORED_DATA format. The key length for new keys are always in bytes.
@@ -162,6 +165,19 @@ zeros (the value of PCR 16):: $ dd if=/dev/zero bs=1 count 2>/dev/null|sha1sum 6768033e216468247bd031a0a2d9876d79818f8f +You can also specify arbitrary policy in policymaker format, so a two +value policy (the pcr example above and authvalue) would look like +this in policymaker format:: + + 0000017F000000010004030000016768033e216468247bd031a0a2d9876d79818f8f + 0000016b + +This can be placed in a file (say policy.txt) and then added to the key as:: + + $ keyctl add trusted kmk "new 32 keyhandle=0x81000001 hash=sha1 policy=`cat policy.txt`" @u + +The newlines in the file policy.txt will be automatically processed. + Reseal a trusted key under new pcr values:: $ keyctl update 268728824 "update pcrinfo=`cat pcr.blob`"
security/keys/trusted-keys/tpm2-policy.c +46 -0
49 lines not on this page · raw message
security/keys/trusted-keys/tpm2-policy.h +1 -0
@@ -28,3 +28,4 @@ int tpm2_generate_policy_digest(struct tpm2_policies *pols, u32 hash,int tpm2_encode_policy (struct tpm2_policies *pols,u8 **data,u32 *len);int tpm2_get_policy_session (struct tpm_chip *chip,struct tpm2_policies *pols,u32 *handle); +int tpm2_parse_policies (struct tpm2_policies **ppols,char *str);
security/keys/trusted-keys/trusted_tpm1.c +13 -1
@@ -29,6 +29,8 @@#include <keys/trusted_tpm.h> +#include "tpm2-policy.h" +static const char hmac_alg[] ="hmac(sha1)" ;static const char hash_alg[] ="sha1" ;static struct tpm_chip *chip;
@@ -706,7 +708,7 @@ enum { Opt_new, Opt_load, Opt_update, Opt_keyhandle, Opt_keyauth, Opt_blobauth, Opt_pcrinfo, Opt_pcrlock, Opt_migratable, - Opt_hash, + Opt_hash, Opt_policy, }; static const match_table_t key_tokens = {
@@ -720,6 +722,7 @@ static const match_table_t key_tokens = { {Opt_pcrlock, "pcrlock=%s"}, {Opt_migratable, "migratable=%s"}, {Opt_hash, "hash=%s"}, + {Opt_policy, "policy=%s"}, {Opt_err, NULL} };
@@ -809,6 +812,15 @@ static int getoptions(char *c, struct trusted_key_payload *pay,return -EINVAL ; }break ; +case Opt_policy: +if (pay->policies) +return -EINVAL ; +if (!tpm2) +return -EINVAL ; + res =tpm2_parse_policies (&pay->policies, args[0 ].from); +if (res) +return res; +break ;default :return -EINVAL ; }
Signature · 1 line
2.16.4
James Bottomley
maintainerPATCH v2 8/8 security: keys: trusted: implement counter/timer policy
In reply to James BottomleyThis is actually a generic policy allowing a range of comparisons against any value set in the TPM Clock, which includes things like the reset count, a monotonic millisecond count and the restart count. The most useful comparison is against the millisecond count for expiring keys. However, you have to remember that currently Linux doesn't try to sync the epoch timer with the TPM, so the expiration is actually measured in how long the TPM itself has been powered on ... the TPM timer doesn't count while the system is powered down. The millisecond counter is a u64 quantity found at offset 8 in the timer structure, and the <= comparision operand is 9, so a policy set to expire after the TPM has been up for 100 seconds would look like
0000016d00000000000f424000080009
Where 0x16d is the counter timer policy code and 0xf4240 is 100 000 in hex.
--- Documentation/security/keys/trusted-encrypted.rst | 29 +++++++++++++++++++++++ security/keys/trusted-keys/tpm2-policy.c | 19 +++++++++++++++ 2 files changed, 48 insertions(+)
Documentation/security/keys/trusted-encrypted.rst +29 -0
@@ -235,3 +235,32 @@ about the usage can be found in the file Another new format 'enc32' has been defined in order to support encrypted keys with payload size of 32 bytes. This will initially be used for nvdimm security but may expand to other usages that require 32 bytes payload. + +Appendix +-------- + +TPM 2.0 Policies +---------------- + +The current TPM supports PCR lock policies as documented above and +CounterTimer policies which can be used to create expiring keys. One +caveat with expiring keys is that the TPM millisecond counter does not +update while a system is powered off and Linux does not sync the TPM +millisecond count with its internal clock, so the best you can expire +in is in terms of how long any given TPM has been powered on. (FIXME: +Linux should simply update the millisecond clock to the current number +of seconds past the epoch on boot). + +A CounterTimer policy is expressed in terms of length and offset +against the TPM clock structure (TPMS_TIME_INFO), which looks like the +packed structure:: + + struct tpms_time_info { + u64 uptime; /* time in ms since last start or reset */ + u64 clock; /* cumulative uptime in ms */ + u32 resetcount; /* numer of times the TPM has been reset */ + u32 restartcount; /* number of times the TPM has been restarted */ + u8 safe /* time was safely loaded from NVRam */ + }; + +The usual comparison for expiring keys is against clock, at offset 8.
security/keys/trusted-keys/tpm2-policy.c +19 -0
@@ -322,6 +322,25 @@ int tpm2_get_policy_session(struct tpm_chip *chip, struct tpm2_policies *pols,tpm_buf_append (&buf, pols->policies[i], pols->len[i] - pols->hash_size);break ; +case TPM2_CC_POLICY_COUNTER_TIMER : { +/* +* the format of this is the last two u16 +* quantities are the offset and operation +* respectively. The rest is operandB which +* must be zero padded in a hash digest +*/ +u16 opb_len = pols->len[i] -4 ; + +if (opb_len > pols->hash_size) +return -EINVAL ; + +tpm_buf_append_u16 (&buf, opb_len); +tpm_buf_append (&buf, pols->policies[i], opb_len); +/* offset and operand*/ +tpm_buf_append (&buf, pols->policies[i] + opb_len,4 ); + failure ="Counter Timer" ; +break ; + }default : failure ="unknown policy" ;break ;
Signature · 1 line
2.16.4
Quoted 17 lines · James Bottomley
This fixes a wide array of problems with the current TPM 2.0 implementation of trusted keys. Since policy based trusted keys never worked in the current implementation, I've rewritten the policy implementation to make it easier to use and so the trusted key handler can understand what elements of a policy are failing and why.
Apart from fixing bugs like volatile object leakage, I've changed the output format to use the standardised ASN.1 coding for TPM2 keys, meaning they should interoperate with userspace TPM2 key implementations. Apart from interoperability, another advantage of the existing key format is that it carries all parameters like parent and hash with it and it is capable of carrying policy directives in a way that mean they're tied permanently to the key (no having to try to remember what the policy was and reconstruct it from userspace). This actually allows us to support the TPM 1.2 commands like pcrinfo easily in 2.0.
Quoted 6 lines
The big problem with this patch is still that we can't yet combine policy with authorization because that requires proper session handling, but at least with this rewrite it becomes possible (whereas it was never possible with the old external policy session code). Thus, when we have the TPM 2.0 security patch upstream, we'll be able to use the session logic from that patch to imlement authorizations.
Testing as soon as we have more urgent issues out of the table.
/Jarkko
James Bottomley
maintainerOn Mon, 2019-12-09 at 16:04 -0800, James Bottomley wrote:
[...]
Quoted 6 lines
The big problem with this patch is still that we can't yet combine policy with authorization because that requires proper session handling, but at least with this rewrite it becomes possible (whereas it was never possible with the old external policy session code). Thus, when we have the TPM 2.0 security patch upstream, we'll be able to use the session logic from that patch to imlement authorizations.
I had a discussion with Ken Goldman on Friday where he told me this wasn't true: we can actually persuade a policy session to do a non-HMAC authorization (for the interested, the trick is to use TPM2_PolicyPassword in place of TPM2_PolicyAuthValue. It hashes to the same policy but the former sets the session up for non-HMAC and the latter for HMAC) so I'll add password based authorization to policies when I respin the patch set.
James