Re: [PATCH v9 1/4] KEYS: trusted: Add generic trusted keys framework
On Mon, 2021-03-01 at 18:41 +0530, Sumit Garg wrote:
> Current trusted keys framework is tightly coupled to use TPM device
> as an underlying implementation which makes it difficult for
> implementations like Trusted Execution Environment (TEE) etc. to
> provide trusted keys support in case platform doesn't posses a TPM
> device.
>
> Add a generic trusted keys framework where underlying implementations
> can be easily plugged in. Create struct trusted_key_ops to achieve
> this, which contains necessary functions of a backend.
>
> Also, define a module parameter in order to select a particular trust
> source in case a platform support multiple trust sources. In case its
> not specified then implementation itetrates through trust sources
> list starting with TPM and assign the first trust source as a backend
> which has initiazed successfully during iteration.
>
> Note that current implementation only supports a single trust source
> at runtime which is either selectable at compile time or during boot
> via aforementioned module parameter.
You never actually tested this, did you? I'm now getting EINVAL from
all the trusted TPM key operations because of this patch.
The reason is quite simple: this function:
> index ..0db86b44605d
> --- /dev/null
> +++ b/security/keys/trusted-keys/trusted_core.c
[...]
> +static int datablob_parse(char *datablob, struct trusted_key_payload
> *p)
> +{
> + substring_t args[MAX_OPT_ARGS];
> + long keylen;
> + int ret = -EINVAL;
> + int key_cmd;
> + char *c;
> +
> + /* main command */
> + c = strsep(&datablob, " \t");
Modifies its argument to consume tokens and separates them with NULL.
so the arguments for
keyctl add trusted kmk "new 34 keyhandle=0x8101"
Go into this function as
datablob="new 34 keyhandle=0x8101"
After we leave it, it looks like
datablob="new\034\0keyhandle=0x8101"
However here:
> +static int trusted_instantiate(struct key *key,
> +struct key_preparsed_payload *prep)
> +{
> + struct trusted_key_payload *payload = NULL;
> + size_t datalen = prep->datalen;
> + char *datablob;
> + int ret = 0;
> + int key_cmd;
> + size_t key_len;
> +
> + if (datalen <= 0 || datalen > 32767 || !prep->data)
> + return -EINVAL;
> +
> + datablob = kmalloc(datalen + 1, GFP_KERNEL);
> + if (!datablob)
> + return -ENOMEM;
> + memcpy(datablob, prep->data, datalen);
> + datablob[datalen] = '\0';
> +
> + payload = trusted_payload_alloc(key);
> + if (!payload) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + key_cmd = datablob_parse(datablob, payload);
> + if (key_cmd < 0) {
> + ret = key_cmd;
> + goto out;
> + }
> +
> + dump_payload(payload);
> +
> + switch (key_cmd) {
> + case Opt_load:
> + ret = static_call(trusted_key_unseal)(payload,
> datablob);
We're passing the unmodified
datablob="new\034\0keyhandle=0x8101"
Into the tpm trusted_key_unseal function. However, it only sees "new"
and promply gives EINVAL because you've removed the ability to process
the new option from it. What should have happened is you should have
moved data blob up to passed the consumed tokens, so it actually reads
datablob="keyhandle=0x8101"
However, to do that you'd have to have the updated pointer passed out
of your datablob_parse() above.
There's also a lost !tpm2 in the check for options->keyhandle, but I
suspect Jarkko lost that merging the two patches. I think what's below
fixes all of this, so if you can test it for trusted_tee, I'll package
it up as two separate patches fixing all of this.
James
---
diff --git a/security/keys/trusted-keys/trusted_core.c
b/security/keys/trusted-keys/trusted_core.c
index ec3a066a4b42..7c636212429b 100644
--- a/security/keys/trusted-keys/trusted_core.c
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -62,7 +62,7 @@ static const match_table_t key_tokens = {
*
* On success returns 0, otherwise -EINVAL.
*/
-static int datablob_parse(char *datablob, struct trusted_key_payload *p)
+static int datablob_parse(char **datablob, struct trusted_key_payload *p)
{
substring_t args[MAX_OPT_ARGS];
long keylen;
@@ -71,14 +71,14 @@ static int datablob_parse(char *datablob, struct
trusted_key_payload *p)
char *c;
/* main command */
- c = strsep(&datablob, " \t");
+ c = strsep(datablob, " \t");
if (!c)
return -EINVAL;
key_cmd = match_token(c, key_tokens, args);
switch (key_cmd) {
case Opt_new:
/* first argument is key size */
- c = strsep(&datablob, " \t");
+ c = strsep(datablob, " \t");
if (!c)
return -EINVAL;
ret = kstrtol(c, 10, &keylen);
@@ -89,7 +89,7 @@ static int datablob_parse(char *datablob, struct
trusted_key_
Re: [PATCH v9 1/4] KEYS: trusted: Add generic trusted keys framework
On Mon, Mar 01, 2021 at 06:41:24PM +0530, Sumit Garg wrote:
> Current trusted keys framework is tightly coupled to use TPM device as
> an underlying implementation which makes it difficult for implementations
> like Trusted Execution Environment (TEE) etc. to provide trusted keys
> support in case platform doesn't posses a TPM device.
>
> Add a generic trusted keys framework where underlying implementations
> can be easily plugged in. Create struct trusted_key_ops to achieve this,
> which contains necessary functions of a backend.
>
> Also, define a module parameter in order to select a particular trust
> source in case a platform support multiple trust sources. In case its
> not specified then implementation itetrates through trust sources list
> starting with TPM and assign the first trust source as a backend which
> has initiazed successfully during iteration.
>
> Note that current implementation only supports a single trust source at
> runtime which is either selectable at compile time or during boot via
> aforementioned module parameter.
>
> Suggested-by: Jarkko Sakkinen
> Signed-off-by: Sumit Garg
Reviewed-by: Jarkko Sakkinen
/Jarkko
> ---
> .../admin-guide/kernel-parameters.txt | 12 +
> include/keys/trusted-type.h | 53 +++
> include/keys/trusted_tpm.h| 29 +-
> security/keys/trusted-keys/Makefile | 1 +
> security/keys/trusted-keys/trusted_core.c | 354 +
> security/keys/trusted-keys/trusted_tpm1.c | 366 --
> 6 files changed, 497 insertions(+), 318 deletions(-)
> create mode 100644 security/keys/trusted-keys/trusted_core.c
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt
> b/Documentation/admin-guide/kernel-parameters.txt
> index 0ac883777318..fbc828994b06 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5459,6 +5459,18 @@
> See Documentation/admin-guide/mm/transhuge.rst
> for more details.
>
> + trusted.source= [KEYS]
> + Format:
> + This parameter identifies the trust source as a backend
> + for trusted keys implementation. Supported trust
> + sources:
> + - "tpm"
> + - "tee"
> + If not specified then it defaults to iterating through
> + the trust source list starting with TPM and assigns the
> + first trust source as a backend which is initialized
> + successfully during iteration.
> +
> tsc=Disable clocksource stability checks for TSC.
> Format:
> [x86] reliable: mark tsc clocksource as reliable, this
> diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
> index a94c03a61d8f..24016898ca41 100644
> --- a/include/keys/trusted-type.h
> +++ b/include/keys/trusted-type.h
> @@ -11,6 +11,12 @@
> #include
> #include
>
> +#ifdef pr_fmt
> +#undef pr_fmt
> +#endif
> +
> +#define pr_fmt(fmt) "trusted_key: " fmt
> +
> #define MIN_KEY_SIZE 32
> #define MAX_KEY_SIZE 128
> #define MAX_BLOB_SIZE512
> @@ -40,6 +46,53 @@ struct trusted_key_options {
> uint32_t policyhandle;
> };
>
> +struct trusted_key_ops {
> + /*
> + * flag to indicate if trusted key implementation supports migration
> + * or not.
> + */
> + unsigned char migratable;
> +
> + /* Initialize key interface. */
> + int (*init)(void);
> +
> + /* Seal a key. */
> + int (*seal)(struct trusted_key_payload *p, char *datablob);
> +
> + /* Unseal a key. */
> + int (*unseal)(struct trusted_key_payload *p, char *datablob);
> +
> + /* Get a randomized key. */
> + int (*get_random)(unsigned char *key, size_t key_len);
> +
> + /* Exit key interface. */
> + void (*exit)(void);
> +};
> +
> +struct trusted_key_source {
> + char *name;
> + struct trusted_key_ops *ops;
> +};
> +
> extern struct key_type key_type_trusted;
>
> +#define TRUSTED_DEBUG 0
> +
> +#if TRUSTED_DEBUG
> +static inline void dump_payload(struct trusted_key_payload *p)
> +{
> + pr_info("key_len %d\n", p->key_len);
> + print_hex_dump(KERN_INFO, "key ", DUMP_PREFIX_NONE,
> +16, 1, p->key, p->key_len, 0);
> + pr_info("bloblen %d\n", p->blob_len);
> + print_hex_dump(KERN_INFO, "blob ", DUMP_PREFIX_NONE,
> +16, 1, p->blob, p->blob_len, 0);
> + pr_info("migratable %d\n", p->migratable);
> +}
> +#else
> +static inline void dump_payload(struct trusted_key_payload *p)
> +{
> +}
> +#endif
> +
> #endif /* _KEYS_TRUSTED_TYPE_H */
> diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h
> index a56d8e1298f2..7769b726863a 100644
> --- a/include/keys/trusted_tp
[PATCH v9 1/4] KEYS: trusted: Add generic trusted keys framework
Current trusted keys framework is tightly coupled to use TPM device as
an underlying implementation which makes it difficult for implementations
like Trusted Execution Environment (TEE) etc. to provide trusted keys
support in case platform doesn't posses a TPM device.
Add a generic trusted keys framework where underlying implementations
can be easily plugged in. Create struct trusted_key_ops to achieve this,
which contains necessary functions of a backend.
Also, define a module parameter in order to select a particular trust
source in case a platform support multiple trust sources. In case its
not specified then implementation itetrates through trust sources list
starting with TPM and assign the first trust source as a backend which
has initiazed successfully during iteration.
Note that current implementation only supports a single trust source at
runtime which is either selectable at compile time or during boot via
aforementioned module parameter.
Suggested-by: Jarkko Sakkinen
Signed-off-by: Sumit Garg
---
.../admin-guide/kernel-parameters.txt | 12 +
include/keys/trusted-type.h | 53 +++
include/keys/trusted_tpm.h| 29 +-
security/keys/trusted-keys/Makefile | 1 +
security/keys/trusted-keys/trusted_core.c | 354 +
security/keys/trusted-keys/trusted_tpm1.c | 366 --
6 files changed, 497 insertions(+), 318 deletions(-)
create mode 100644 security/keys/trusted-keys/trusted_core.c
diff --git a/Documentation/admin-guide/kernel-parameters.txt
b/Documentation/admin-guide/kernel-parameters.txt
index 0ac883777318..fbc828994b06 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5459,6 +5459,18 @@
See Documentation/admin-guide/mm/transhuge.rst
for more details.
+ trusted.source= [KEYS]
+ Format:
+ This parameter identifies the trust source as a backend
+ for trusted keys implementation. Supported trust
+ sources:
+ - "tpm"
+ - "tee"
+ If not specified then it defaults to iterating through
+ the trust source list starting with TPM and assigns the
+ first trust source as a backend which is initialized
+ successfully during iteration.
+
tsc=Disable clocksource stability checks for TSC.
Format:
[x86] reliable: mark tsc clocksource as reliable, this
diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index a94c03a61d8f..24016898ca41 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -11,6 +11,12 @@
#include
#include
+#ifdef pr_fmt
+#undef pr_fmt
+#endif
+
+#define pr_fmt(fmt) "trusted_key: " fmt
+
#define MIN_KEY_SIZE 32
#define MAX_KEY_SIZE 128
#define MAX_BLOB_SIZE 512
@@ -40,6 +46,53 @@ struct trusted_key_options {
uint32_t policyhandle;
};
+struct trusted_key_ops {
+ /*
+* flag to indicate if trusted key implementation supports migration
+* or not.
+*/
+ unsigned char migratable;
+
+ /* Initialize key interface. */
+ int (*init)(void);
+
+ /* Seal a key. */
+ int (*seal)(struct trusted_key_payload *p, char *datablob);
+
+ /* Unseal a key. */
+ int (*unseal)(struct trusted_key_payload *p, char *datablob);
+
+ /* Get a randomized key. */
+ int (*get_random)(unsigned char *key, size_t key_len);
+
+ /* Exit key interface. */
+ void (*exit)(void);
+};
+
+struct trusted_key_source {
+ char *name;
+ struct trusted_key_ops *ops;
+};
+
extern struct key_type key_type_trusted;
+#define TRUSTED_DEBUG 0
+
+#if TRUSTED_DEBUG
+static inline void dump_payload(struct trusted_key_payload *p)
+{
+ pr_info("key_len %d\n", p->key_len);
+ print_hex_dump(KERN_INFO, "key ", DUMP_PREFIX_NONE,
+ 16, 1, p->key, p->key_len, 0);
+ pr_info("bloblen %d\n", p->blob_len);
+ print_hex_dump(KERN_INFO, "blob ", DUMP_PREFIX_NONE,
+ 16, 1, p->blob, p->blob_len, 0);
+ pr_info("migratable %d\n", p->migratable);
+}
+#else
+static inline void dump_payload(struct trusted_key_payload *p)
+{
+}
+#endif
+
#endif /* _KEYS_TRUSTED_TYPE_H */
diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h
index a56d8e1298f2..7769b726863a 100644
--- a/include/keys/trusted_tpm.h
+++ b/include/keys/trusted_tpm.h
@@ -16,6 +16,8 @@
#define LOAD32N(buffer, offset)(*(uint32_t *)&buffer[offset])
#define LOAD16(buffer, offset) (ntohs(*(uint16_t *)&buffer[offset]))
+extern struct trusted_key_ops trusted_key_tpm_ops;
+
struct osapsess {
