* Re: [PATCH v2 11/12] ima: Introduce template field evmsig and write to field sig as fallback
From: Mimi Zohar @ 2020-09-17 14:25 UTC (permalink / raw)
To: Roberto Sassu, mjg59
Cc: linux-integrity, linux-security-module, linux-kernel,
silviu.vlasceanu
In-Reply-To: <20200904092643.20013-7-roberto.sassu@huawei.com>
Hi Roberto,
On Fri, 2020-09-04 at 11:26 +0200, Roberto Sassu wrote:
> With the patch to accept EVM portable signatures when the
> appraise_type=imasig requirement is specified in the policy, appraisal can
> be successfully done even if the file does not have an IMA signature.
>
> However, remote attestation would not see that a different signature type
> was used, as only IMA signatures can be included in the measurement list.
> This patch solves the issue by introducing the new template field 'evmsig'
> to show EVM portable signatures and by including its value in the existing
> field 'sig' if the IMA signature is not found.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Thank you! Just a minor comment below.
<snip>
> diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
> index c022ee9e2a4e..2c596c2a89cc 100644
> --- a/security/integrity/ima/ima_template_lib.c
> +++ b/security/integrity/ima/ima_template_lib.c
>
> @@ -438,7 +439,7 @@ int ima_eventsig_init(struct ima_event_data *event_data,
> struct evm_ima_xattr_data *xattr_value = event_data->xattr_value;
>
> if ((!xattr_value) || (xattr_value->type != EVM_IMA_XATTR_DIGSIG))
> - return 0;
> + return ima_eventevmsig_init(event_data, field_data);
>
> return ima_write_template_field_data(xattr_value, event_data->xattr_len,
> DATA_FMT_HEX, field_data);
> @@ -484,3 +485,39 @@ int ima_eventmodsig_init(struct ima_event_data *event_data,
> return ima_write_template_field_data(data, data_len, DATA_FMT_HEX,
> field_data);
> }
> +
> +/*
> + * ima_eventevmsig_init - include the EVM portable signature as part of the
> + * template data
> + */
> +int ima_eventevmsig_init(struct ima_event_data *event_data,
> + struct ima_field_data *field_data)
> +{
> + struct evm_ima_xattr_data *xattr_data = NULL;
> + int rc = 0;
> +
> + if (!event_data->file)
> + return 0;
> +
> + if (!(file_inode(event_data->file)->i_opflags & IOP_XATTR))
> + return 0;
> +
> + rc = vfs_getxattr_alloc(file_dentry(event_data->file), XATTR_NAME_EVM,
> + (char **)&xattr_data, 0, GFP_NOFS);
> + if (rc <= 0) {
> + if (!rc || rc == -ENODATA)
> + return 0;
> +
> + return rc;
We're including the EVM signature on a best effort basis to help with
attestation. Do we really care why it failed? Are we going to act on
it?
Mimi
> + }
> +
> + if (xattr_data->type != EVM_XATTR_PORTABLE_DIGSIG) {
> + kfree(xattr_data);
> + return 0;
> + }
> +
> + rc = ima_write_template_field_data((char *)xattr_data, rc, DATA_FMT_HEX,
> + field_data);
> + kfree(xattr_data);
> + return rc;
> +}
^ permalink raw reply
* [PATCH v6 1/4] KEYS: trusted: Add generic trusted keys framework
From: Sumit Garg @ 2020-09-17 13:46 UTC (permalink / raw)
To: jarkko.sakkinen, zohar, jejb
Cc: dhowells, jens.wiklander, corbet, jmorris, serge, casey,
janne.karhunen, daniel.thompson, Markus.Wamser, lhinds, keyrings,
linux-integrity, linux-security-module, linux-doc, linux-kernel,
linux-arm-kernel, op-tee, Sumit Garg
In-Reply-To: <1600350398-4813-1-git-send-email-sumit.garg@linaro.org>
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.
So this patch tries to add generic trusted keys framework where underlying
implementations like TPM, TEE etc. could be easily plugged-in.
Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
include/keys/trusted-type.h | 42 ++++
include/keys/trusted_tpm.h | 17 +-
security/keys/trusted-keys/Makefile | 1 +
security/keys/trusted-keys/trusted_core.c | 321 ++++++++++++++++++++++++++++
security/keys/trusted-keys/trusted_tpm1.c | 336 +++++-------------------------
5 files changed, 422 insertions(+), 295 deletions(-)
create mode 100644 security/keys/trusted-keys/trusted_core.c
diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index a94c03a..edd635a 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -40,6 +40,48 @@ 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);
+};
+
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("trusted_key: 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("trusted_key: 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("trusted_key: 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 a56d8e1..fb3280a 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 tpm_trusted_key_ops;
+
struct osapsess {
uint32_t handle;
unsigned char secret[SHA1_DIGEST_SIZE];
@@ -60,17 +62,6 @@ static inline void dump_options(struct trusted_key_options *o)
16, 1, o->pcrinfo, o->pcrinfo_len, 0);
}
-static inline void dump_payload(struct trusted_key_payload *p)
-{
- pr_info("trusted_key: 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("trusted_key: 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("trusted_key: migratable %d\n", p->migratable);
-}
-
static inline void dump_sess(struct osapsess *s)
{
print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE,
@@ -96,10 +87,6 @@ static inline void dump_options(struct trusted_key_options *o)
{
}
-static inline void dump_payload(struct trusted_key_payload *p)
-{
-}
-
static inline void dump_sess(struct osapsess *s)
{
}
diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
index 7b73ceb..49e3bcf 100644
--- a/security/keys/trusted-keys/Makefile
+++ b/security/keys/trusted-keys/Makefile
@@ -4,5 +4,6 @@
#
obj-$(CONFIG_TRUSTED_KEYS) += trusted.o
+trusted-y += trusted_core.o
trusted-y += trusted_tpm1.o
trusted-y += trusted_tpm2.o
diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
new file mode 100644
index 0000000..4ae3fb4
--- /dev/null
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -0,0 +1,321 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2010 IBM Corporation
+ * Copyright (c) 2019-2020, Linaro Limited
+ *
+ * See Documentation/security/keys/trusted-encrypted.rst
+ */
+
+#include <keys/user-type.h>
+#include <keys/trusted-type.h>
+#include <keys/trusted_tpm.h>
+#include <linux/capability.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/key-type.h>
+#include <linux/module.h>
+#include <linux/parser.h>
+#include <linux/rcupdate.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/uaccess.h>
+
+static struct trusted_key_ops *available_trusted_key_ops[] = {
+#if defined(CONFIG_TCG_TPM)
+ &tpm_trusted_key_ops,
+#endif
+};
+static struct trusted_key_ops *trusted_key_ops;
+
+enum {
+ Opt_err,
+ Opt_new, Opt_load, Opt_update,
+};
+
+static const match_table_t key_tokens = {
+ {Opt_new, "new"},
+ {Opt_load, "load"},
+ {Opt_update, "update"},
+ {Opt_err, NULL}
+};
+
+/*
+ * datablob_parse - parse the keyctl data and fill in the
+ * payload structure
+ *
+ * On success returns 0, otherwise -EINVAL.
+ */
+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");
+ 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");
+ if (!c)
+ return -EINVAL;
+ ret = kstrtol(c, 10, &keylen);
+ if (ret < 0 || keylen < MIN_KEY_SIZE || keylen > MAX_KEY_SIZE)
+ return -EINVAL;
+ p->key_len = keylen;
+ ret = Opt_new;
+ break;
+ case Opt_load:
+ /* first argument is sealed blob */
+ c = strsep(&datablob, " \t");
+ if (!c)
+ return -EINVAL;
+ p->blob_len = strlen(c) / 2;
+ if (p->blob_len > MAX_BLOB_SIZE)
+ return -EINVAL;
+ ret = hex2bin(p->blob, c, p->blob_len);
+ if (ret < 0)
+ return -EINVAL;
+ ret = Opt_load;
+ break;
+ case Opt_update:
+ ret = Opt_update;
+ break;
+ case Opt_err:
+ return -EINVAL;
+ }
+ return ret;
+}
+
+static struct trusted_key_payload *trusted_payload_alloc(struct key *key)
+{
+ struct trusted_key_payload *p = NULL;
+ int ret;
+
+ ret = key_payload_reserve(key, sizeof(*p));
+ if (ret < 0)
+ return p;
+ p = kzalloc(sizeof(*p), GFP_KERNEL);
+
+ p->migratable = trusted_key_ops->migratable;
+
+ return p;
+}
+
+/*
+ * trusted_instantiate - create a new trusted key
+ *
+ * Unseal an existing trusted blob or, for a new key, get a
+ * random key, then seal and create a trusted key-type key,
+ * adding it to the specified keyring.
+ *
+ * On success, return 0. Otherwise return errno.
+ */
+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 = trusted_key_ops->unseal(payload, datablob);
+ dump_payload(payload);
+ if (ret < 0)
+ pr_info("trusted_key: key_unseal failed (%d)\n", ret);
+ break;
+ case Opt_new:
+ key_len = payload->key_len;
+ ret = trusted_key_ops->get_random(payload->key, key_len);
+ if (ret != key_len) {
+ pr_info("trusted_key: key_create failed (%d)\n", ret);
+ goto out;
+ }
+
+ ret = trusted_key_ops->seal(payload, datablob);
+ if (ret < 0)
+ pr_info("trusted_key: key_seal failed (%d)\n", ret);
+ break;
+ default:
+ ret = -EINVAL;
+ }
+out:
+ kfree_sensitive(datablob);
+ if (!ret)
+ rcu_assign_keypointer(key, payload);
+ else
+ kfree_sensitive(payload);
+ return ret;
+}
+
+static void trusted_rcu_free(struct rcu_head *rcu)
+{
+ struct trusted_key_payload *p;
+
+ p = container_of(rcu, struct trusted_key_payload, rcu);
+ kfree_sensitive(p);
+}
+
+/*
+ * trusted_update - reseal an existing key with new PCR values
+ */
+static int trusted_update(struct key *key, struct key_preparsed_payload *prep)
+{
+ struct trusted_key_payload *p;
+ struct trusted_key_payload *new_p;
+ size_t datalen = prep->datalen;
+ char *datablob;
+ int ret = 0;
+
+ if (key_is_negative(key))
+ return -ENOKEY;
+ p = key->payload.data[0];
+ if (!p->migratable)
+ return -EPERM;
+ if (datalen <= 0 || datalen > 32767 || !prep->data)
+ return -EINVAL;
+
+ datablob = kmalloc(datalen + 1, GFP_KERNEL);
+ if (!datablob)
+ return -ENOMEM;
+
+ new_p = trusted_payload_alloc(key);
+ if (!new_p) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ memcpy(datablob, prep->data, datalen);
+ datablob[datalen] = '\0';
+ ret = datablob_parse(datablob, new_p);
+ if (ret != Opt_update) {
+ ret = -EINVAL;
+ kfree_sensitive(new_p);
+ goto out;
+ }
+
+ /* copy old key values, and reseal with new pcrs */
+ new_p->migratable = p->migratable;
+ new_p->key_len = p->key_len;
+ memcpy(new_p->key, p->key, p->key_len);
+ dump_payload(p);
+ dump_payload(new_p);
+
+ ret = trusted_key_ops->seal(new_p, datablob);
+ if (ret < 0) {
+ pr_info("trusted_key: key_seal failed (%d)\n", ret);
+ kfree_sensitive(new_p);
+ goto out;
+ }
+
+ rcu_assign_keypointer(key, new_p);
+ call_rcu(&p->rcu, trusted_rcu_free);
+out:
+ kfree_sensitive(datablob);
+ return ret;
+}
+
+/*
+ * trusted_read - copy the sealed blob data to userspace in hex.
+ * On success, return to userspace the trusted key datablob size.
+ */
+static long trusted_read(const struct key *key, char *buffer,
+ size_t buflen)
+{
+ const struct trusted_key_payload *p;
+ char *bufp;
+ int i;
+
+ p = dereference_key_locked(key);
+ if (!p)
+ return -EINVAL;
+
+ if (buffer && buflen >= 2 * p->blob_len) {
+ bufp = buffer;
+ for (i = 0; i < p->blob_len; i++)
+ bufp = hex_byte_pack(bufp, p->blob[i]);
+ }
+ return 2 * p->blob_len;
+}
+
+/*
+ * trusted_destroy - clear and free the key's payload
+ */
+static void trusted_destroy(struct key *key)
+{
+ kfree_sensitive(key->payload.data[0]);
+}
+
+struct key_type key_type_trusted = {
+ .name = "trusted",
+ .instantiate = trusted_instantiate,
+ .update = trusted_update,
+ .destroy = trusted_destroy,
+ .describe = user_describe,
+ .read = trusted_read,
+};
+EXPORT_SYMBOL_GPL(key_type_trusted);
+
+static int __init init_trusted(void)
+{
+ int i, ret = 0;
+
+ for (i = 0; i < sizeof(available_trusted_key_ops); i++) {
+ trusted_key_ops = available_trusted_key_ops[i];
+
+ ret = trusted_key_ops->init();
+ if (!ret)
+ break;
+ }
+
+ /*
+ * encrypted_keys.ko depends on successful load of this module even if
+ * trusted key implementation is not found.
+ */
+ if (ret == -ENODEV)
+ return 0;
+
+ return ret;
+}
+
+static void __exit cleanup_trusted(void)
+{
+ trusted_key_ops->exit();
+}
+
+late_initcall(init_trusted);
+module_exit(cleanup_trusted);
+
+MODULE_LICENSE("GPL");
diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index b9fe02e..1c5df77 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -1,29 +1,22 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
* Copyright (C) 2010 IBM Corporation
- *
- * Author:
- * David Safford <safford@us.ibm.com>
+ * Copyright (c) 2019-2020, Linaro Limited
*
* See Documentation/security/keys/trusted-encrypted.rst
*/
#include <crypto/hash_info.h>
-#include <linux/uaccess.h>
-#include <linux/module.h>
#include <linux/init.h>
#include <linux/slab.h>
#include <linux/parser.h>
#include <linux/string.h>
#include <linux/err.h>
-#include <keys/user-type.h>
#include <keys/trusted-type.h>
#include <linux/key-type.h>
-#include <linux/rcupdate.h>
#include <linux/crypto.h>
#include <crypto/hash.h>
#include <crypto/sha.h>
-#include <linux/capability.h>
#include <linux/tpm.h>
#include <linux/tpm_command.h>
@@ -703,7 +696,6 @@ static int key_unseal(struct trusted_key_payload *p,
enum {
Opt_err,
- Opt_new, Opt_load, Opt_update,
Opt_keyhandle, Opt_keyauth, Opt_blobauth,
Opt_pcrinfo, Opt_pcrlock, Opt_migratable,
Opt_hash,
@@ -712,9 +704,6 @@ enum {
};
static const match_table_t key_tokens = {
- {Opt_new, "new"},
- {Opt_load, "load"},
- {Opt_update, "update"},
{Opt_keyhandle, "keyhandle=%s"},
{Opt_keyauth, "keyauth=%s"},
{Opt_blobauth, "blobauth=%s"},
@@ -841,71 +830,6 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
return 0;
}
-/*
- * datablob_parse - parse the keyctl data and fill in the
- * payload and options structures
- *
- * On success returns 0, otherwise -EINVAL.
- */
-static int datablob_parse(char *datablob, struct trusted_key_payload *p,
- struct trusted_key_options *o)
-{
- substring_t args[MAX_OPT_ARGS];
- long keylen;
- int ret = -EINVAL;
- int key_cmd;
- char *c;
-
- /* main command */
- 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");
- if (!c)
- return -EINVAL;
- ret = kstrtol(c, 10, &keylen);
- if (ret < 0 || keylen < MIN_KEY_SIZE || keylen > MAX_KEY_SIZE)
- return -EINVAL;
- p->key_len = keylen;
- ret = getoptions(datablob, p, o);
- if (ret < 0)
- return ret;
- ret = Opt_new;
- break;
- case Opt_load:
- /* first argument is sealed blob */
- c = strsep(&datablob, " \t");
- if (!c)
- return -EINVAL;
- p->blob_len = strlen(c) / 2;
- if (p->blob_len > MAX_BLOB_SIZE)
- return -EINVAL;
- ret = hex2bin(p->blob, c, p->blob_len);
- if (ret < 0)
- return -EINVAL;
- ret = getoptions(datablob, p, o);
- if (ret < 0)
- return ret;
- ret = Opt_load;
- break;
- case Opt_update:
- /* all arguments are options */
- ret = getoptions(datablob, p, o);
- if (ret < 0)
- return ret;
- ret = Opt_update;
- break;
- case Opt_err:
- return -EINVAL;
- break;
- }
- return ret;
-}
-
static struct trusted_key_options *trusted_options_alloc(void)
{
struct trusted_key_options *options;
@@ -926,248 +850,99 @@ static struct trusted_key_options *trusted_options_alloc(void)
return options;
}
-static struct trusted_key_payload *trusted_payload_alloc(struct key *key)
+static int tpm_trusted_seal(struct trusted_key_payload *p, char *datablob)
{
- struct trusted_key_payload *p = NULL;
- int ret;
-
- ret = key_payload_reserve(key, sizeof *p);
- if (ret < 0)
- return p;
- p = kzalloc(sizeof *p, GFP_KERNEL);
- if (p)
- p->migratable = 1; /* migratable by default */
- return p;
-}
-
-/*
- * trusted_instantiate - create a new trusted key
- *
- * Unseal an existing trusted blob or, for a new key, get a
- * random key, then seal and create a trusted key-type key,
- * adding it to the specified keyring.
- *
- * On success, return 0. Otherwise return errno.
- */
-static int trusted_instantiate(struct key *key,
- struct key_preparsed_payload *prep)
-{
- struct trusted_key_payload *payload = NULL;
struct trusted_key_options *options = NULL;
- size_t datalen = prep->datalen;
- char *datablob;
int ret = 0;
- int key_cmd;
- size_t key_len;
int tpm2;
tpm2 = tpm_is_tpm2(chip);
if (tpm2 < 0)
return tpm2;
- 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';
-
options = trusted_options_alloc();
- if (!options) {
- ret = -ENOMEM;
- goto out;
- }
- payload = trusted_payload_alloc(key);
- if (!payload) {
- ret = -ENOMEM;
- goto out;
- }
+ if (!options)
+ return -ENOMEM;
- key_cmd = datablob_parse(datablob, payload, options);
- if (key_cmd < 0) {
- ret = key_cmd;
+ ret = getoptions(datablob, p, options);
+ if (ret < 0)
goto out;
- }
+ dump_options(options);
if (!options->keyhandle) {
ret = -EINVAL;
goto out;
}
- dump_payload(payload);
- dump_options(options);
+ if (tpm2)
+ ret = tpm2_seal_trusted(chip, p, options);
+ else
+ ret = key_seal(p, options);
+ if (ret < 0) {
+ pr_info("tpm_trusted_key: key_seal failed (%d)\n", ret);
+ goto out;
+ }
- switch (key_cmd) {
- case Opt_load:
- if (tpm2)
- ret = tpm2_unseal_trusted(chip, payload, options);
- else
- ret = key_unseal(payload, options);
- dump_payload(payload);
- dump_options(options);
- if (ret < 0)
- pr_info("trusted_key: key_unseal failed (%d)\n", ret);
- break;
- case Opt_new:
- key_len = payload->key_len;
- ret = tpm_get_random(chip, payload->key, key_len);
- if (ret != key_len) {
- pr_info("trusted_key: key_create failed (%d)\n", ret);
+ if (options->pcrlock) {
+ ret = pcrlock(options->pcrlock);
+ if (ret < 0) {
+ pr_info("tpm_trusted_key: pcrlock failed (%d)\n", ret);
goto out;
}
- if (tpm2)
- ret = tpm2_seal_trusted(chip, payload, options);
- else
- ret = key_seal(payload, options);
- if (ret < 0)
- pr_info("trusted_key: key_seal failed (%d)\n", ret);
- break;
- default:
- ret = -EINVAL;
- goto out;
}
- if (!ret && options->pcrlock)
- ret = pcrlock(options->pcrlock);
out:
- kfree_sensitive(datablob);
kfree_sensitive(options);
- if (!ret)
- rcu_assign_keypointer(key, payload);
- else
- kfree_sensitive(payload);
return ret;
}
-static void trusted_rcu_free(struct rcu_head *rcu)
-{
- struct trusted_key_payload *p;
-
- p = container_of(rcu, struct trusted_key_payload, rcu);
- kfree_sensitive(p);
-}
-
-/*
- * trusted_update - reseal an existing key with new PCR values
- */
-static int trusted_update(struct key *key, struct key_preparsed_payload *prep)
+static int tpm_trusted_unseal(struct trusted_key_payload *p, char *datablob)
{
- struct trusted_key_payload *p;
- struct trusted_key_payload *new_p;
- struct trusted_key_options *new_o;
- size_t datalen = prep->datalen;
- char *datablob;
+ struct trusted_key_options *options = NULL;
int ret = 0;
+ int tpm2;
- if (key_is_negative(key))
- return -ENOKEY;
- p = key->payload.data[0];
- if (!p->migratable)
- return -EPERM;
- if (datalen <= 0 || datalen > 32767 || !prep->data)
- return -EINVAL;
+ tpm2 = tpm_is_tpm2(chip);
+ if (tpm2 < 0)
+ return tpm2;
- datablob = kmalloc(datalen + 1, GFP_KERNEL);
- if (!datablob)
+ options = trusted_options_alloc();
+ if (!options)
return -ENOMEM;
- new_o = trusted_options_alloc();
- if (!new_o) {
- ret = -ENOMEM;
- goto out;
- }
- new_p = trusted_payload_alloc(key);
- if (!new_p) {
- ret = -ENOMEM;
- goto out;
- }
- memcpy(datablob, prep->data, datalen);
- datablob[datalen] = '\0';
- ret = datablob_parse(datablob, new_p, new_o);
- if (ret != Opt_update) {
- ret = -EINVAL;
- kfree_sensitive(new_p);
+ ret = getoptions(datablob, p, options);
+ if (ret < 0)
goto out;
- }
+ dump_options(options);
- if (!new_o->keyhandle) {
+ if (!options->keyhandle) {
ret = -EINVAL;
- kfree_sensitive(new_p);
goto out;
}
- /* copy old key values, and reseal with new pcrs */
- new_p->migratable = p->migratable;
- new_p->key_len = p->key_len;
- memcpy(new_p->key, p->key, p->key_len);
- dump_payload(p);
- dump_payload(new_p);
+ if (tpm2)
+ ret = tpm2_unseal_trusted(chip, p, options);
+ else
+ ret = key_unseal(p, options);
+ if (ret < 0)
+ pr_info("tpm_trusted_key: key_unseal failed (%d)\n", ret);
- ret = key_seal(new_p, new_o);
- if (ret < 0) {
- pr_info("trusted_key: key_seal failed (%d)\n", ret);
- kfree_sensitive(new_p);
- goto out;
- }
- if (new_o->pcrlock) {
- ret = pcrlock(new_o->pcrlock);
+ if (options->pcrlock) {
+ ret = pcrlock(options->pcrlock);
if (ret < 0) {
- pr_info("trusted_key: pcrlock failed (%d)\n", ret);
- kfree_sensitive(new_p);
+ pr_info("tpm_trusted_key: pcrlock failed (%d)\n", ret);
goto out;
}
}
- rcu_assign_keypointer(key, new_p);
- call_rcu(&p->rcu, trusted_rcu_free);
out:
- kfree_sensitive(datablob);
- kfree_sensitive(new_o);
+ kfree_sensitive(options);
return ret;
}
-/*
- * trusted_read - copy the sealed blob data to userspace in hex.
- * On success, return to userspace the trusted key datablob size.
- */
-static long trusted_read(const struct key *key, char *buffer,
- size_t buflen)
-{
- const struct trusted_key_payload *p;
- char *bufp;
- int i;
-
- p = dereference_key_locked(key);
- if (!p)
- return -EINVAL;
-
- if (buffer && buflen >= 2 * p->blob_len) {
- bufp = buffer;
- for (i = 0; i < p->blob_len; i++)
- bufp = hex_byte_pack(bufp, p->blob[i]);
- }
- return 2 * p->blob_len;
-}
-
-/*
- * trusted_destroy - clear and free the key's payload
- */
-static void trusted_destroy(struct key *key)
+static int tpm_trusted_get_random(unsigned char *key, size_t key_len)
{
- kfree_sensitive(key->payload.data[0]);
+ return tpm_get_random(chip, key, key_len);
}
-struct key_type key_type_trusted = {
- .name = "trusted",
- .instantiate = trusted_instantiate,
- .update = trusted_update,
- .destroy = trusted_destroy,
- .describe = user_describe,
- .read = trusted_read,
-};
-
-EXPORT_SYMBOL_GPL(key_type_trusted);
-
static void trusted_shash_release(void)
{
if (hashalg)
@@ -1182,14 +957,14 @@ static int __init trusted_shash_alloc(void)
hmacalg = crypto_alloc_shash(hmac_alg, 0, 0);
if (IS_ERR(hmacalg)) {
- pr_info("trusted_key: could not allocate crypto %s\n",
+ pr_info("tpm_trusted_key: could not allocate crypto %s\n",
hmac_alg);
return PTR_ERR(hmacalg);
}
hashalg = crypto_alloc_shash(hash_alg, 0, 0);
if (IS_ERR(hashalg)) {
- pr_info("trusted_key: could not allocate crypto %s\n",
+ pr_info("tpm_trusted_key: could not allocate crypto %s\n",
hash_alg);
ret = PTR_ERR(hashalg);
goto hashalg_fail;
@@ -1217,16 +992,13 @@ static int __init init_digests(void)
return 0;
}
-static int __init init_trusted(void)
+static int __init init_tpm_trusted(void)
{
int ret;
- /* encrypted_keys.ko depends on successful load of this module even if
- * TPM is not used.
- */
chip = tpm_default_chip();
if (!chip)
- return 0;
+ return -ENODEV;
ret = init_digests();
if (ret < 0)
@@ -1247,7 +1019,7 @@ static int __init init_trusted(void)
return ret;
}
-static void __exit cleanup_trusted(void)
+static void __exit exit_tpm_trusted(void)
{
if (chip) {
put_device(&chip->dev);
@@ -1257,7 +1029,11 @@ static void __exit cleanup_trusted(void)
}
}
-late_initcall(init_trusted);
-module_exit(cleanup_trusted);
-
-MODULE_LICENSE("GPL");
+struct trusted_key_ops tpm_trusted_key_ops = {
+ .migratable = 1, /* migratable by default */
+ .init = init_tpm_trusted,
+ .seal = tpm_trusted_seal,
+ .unseal = tpm_trusted_unseal,
+ .get_random = tpm_trusted_get_random,
+ .exit = exit_tpm_trusted,
+};
--
2.7.4
^ permalink raw reply related
* [PATCH v6 2/4] KEYS: trusted: Introduce TEE based Trusted Keys
From: Sumit Garg @ 2020-09-17 13:46 UTC (permalink / raw)
To: jarkko.sakkinen, zohar, jejb
Cc: dhowells, jens.wiklander, corbet, jmorris, serge, casey,
janne.karhunen, daniel.thompson, Markus.Wamser, lhinds, keyrings,
linux-integrity, linux-security-module, linux-doc, linux-kernel,
linux-arm-kernel, op-tee, Sumit Garg
In-Reply-To: <1600350398-4813-1-git-send-email-sumit.garg@linaro.org>
Add support for TEE based trusted keys where TEE provides the functionality
to seal and unseal trusted keys using hardware unique key.
Refer to Documentation/tee.txt for detailed information about TEE.
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
include/keys/trusted_tee.h | 55 ++++++
security/keys/trusted-keys/Makefile | 1 +
security/keys/trusted-keys/trusted_core.c | 4 +
security/keys/trusted-keys/trusted_tee.c | 278 ++++++++++++++++++++++++++++++
4 files changed, 338 insertions(+)
create mode 100644 include/keys/trusted_tee.h
create mode 100644 security/keys/trusted-keys/trusted_tee.c
diff --git a/include/keys/trusted_tee.h b/include/keys/trusted_tee.h
new file mode 100644
index 0000000..2e2bb15
--- /dev/null
+++ b/include/keys/trusted_tee.h
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2019-2020 Linaro Ltd.
+ *
+ * Author:
+ * Sumit Garg <sumit.garg@linaro.org>
+ */
+
+#ifndef __TEE_TRUSTED_KEY_H
+#define __TEE_TRUSTED_KEY_H
+
+#include <linux/tee_drv.h>
+
+#define DRIVER_NAME "tee-trusted-key"
+
+/*
+ * Get random data for symmetric key
+ *
+ * [out] memref[0] Random data
+ */
+#define TA_CMD_GET_RANDOM 0x0
+
+/*
+ * Seal trusted key using hardware unique key
+ *
+ * [in] memref[0] Plain key
+ * [out] memref[1] Sealed key datablob
+ */
+#define TA_CMD_SEAL 0x1
+
+/*
+ * Unseal trusted key using hardware unique key
+ *
+ * [in] memref[0] Sealed key datablob
+ * [out] memref[1] Plain key
+ */
+#define TA_CMD_UNSEAL 0x2
+
+/**
+ * struct trusted_key_private - TEE Trusted key private data
+ * @dev: TEE based Trusted key device.
+ * @ctx: TEE context handler.
+ * @session_id: Trusted key TA session identifier.
+ * @shm_pool: Memory pool shared with TEE device.
+ */
+struct trusted_key_private {
+ struct device *dev;
+ struct tee_context *ctx;
+ u32 session_id;
+ struct tee_shm *shm_pool;
+};
+
+extern struct trusted_key_ops tee_trusted_key_ops;
+
+#endif
diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
index 49e3bcf..012dd78 100644
--- a/security/keys/trusted-keys/Makefile
+++ b/security/keys/trusted-keys/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_TRUSTED_KEYS) += trusted.o
trusted-y += trusted_core.o
trusted-y += trusted_tpm1.o
trusted-y += trusted_tpm2.o
+trusted-y += trusted_tee.o
diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
index 4ae3fb4..83a6a15 100644
--- a/security/keys/trusted-keys/trusted_core.c
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -8,6 +8,7 @@
#include <keys/user-type.h>
#include <keys/trusted-type.h>
+#include <keys/trusted_tee.h>
#include <keys/trusted_tpm.h>
#include <linux/capability.h>
#include <linux/err.h>
@@ -24,6 +25,9 @@ static struct trusted_key_ops *available_trusted_key_ops[] = {
#if defined(CONFIG_TCG_TPM)
&tpm_trusted_key_ops,
#endif
+#if defined(CONFIG_TEE)
+ &tee_trusted_key_ops,
+#endif
};
static struct trusted_key_ops *trusted_key_ops;
diff --git a/security/keys/trusted-keys/trusted_tee.c b/security/keys/trusted-keys/trusted_tee.c
new file mode 100644
index 0000000..b414d52
--- /dev/null
+++ b/security/keys/trusted-keys/trusted_tee.c
@@ -0,0 +1,278 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019-2020 Linaro Ltd.
+ *
+ * Author:
+ * Sumit Garg <sumit.garg@linaro.org>
+ */
+
+#include <linux/err.h>
+#include <linux/key-type.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/uuid.h>
+
+#include <keys/trusted-type.h>
+#include <keys/trusted_tee.h>
+
+static struct trusted_key_private pvt_data;
+
+/*
+ * Have the TEE seal(encrypt) the symmetric key
+ */
+static int tee_trusted_seal(struct trusted_key_payload *p, char *datablob)
+{
+ int ret = 0;
+ struct tee_ioctl_invoke_arg inv_arg;
+ struct tee_param param[4];
+ struct tee_shm *reg_shm_in = NULL, *reg_shm_out = NULL;
+
+ memset(&inv_arg, 0, sizeof(inv_arg));
+ memset(¶m, 0, sizeof(param));
+
+ reg_shm_in = tee_shm_register(pvt_data.ctx, (unsigned long)p->key,
+ p->key_len, TEE_SHM_DMA_BUF |
+ TEE_SHM_KERNEL_MAPPED);
+ if (IS_ERR(reg_shm_in)) {
+ dev_err(pvt_data.dev, "key shm register failed\n");
+ return PTR_ERR(reg_shm_in);
+ }
+
+ reg_shm_out = tee_shm_register(pvt_data.ctx, (unsigned long)p->blob,
+ sizeof(p->blob), TEE_SHM_DMA_BUF |
+ TEE_SHM_KERNEL_MAPPED);
+ if (IS_ERR(reg_shm_out)) {
+ dev_err(pvt_data.dev, "blob shm register failed\n");
+ ret = PTR_ERR(reg_shm_out);
+ goto out;
+ }
+
+ inv_arg.func = TA_CMD_SEAL;
+ inv_arg.session = pvt_data.session_id;
+ inv_arg.num_params = 4;
+
+ param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT;
+ param[0].u.memref.shm = reg_shm_in;
+ param[0].u.memref.size = p->key_len;
+ param[0].u.memref.shm_offs = 0;
+ param[1].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT;
+ param[1].u.memref.shm = reg_shm_out;
+ param[1].u.memref.size = sizeof(p->blob);
+ param[1].u.memref.shm_offs = 0;
+
+ ret = tee_client_invoke_func(pvt_data.ctx, &inv_arg, param);
+ if ((ret < 0) || (inv_arg.ret != 0)) {
+ dev_err(pvt_data.dev, "TA_CMD_SEAL invoke err: %x\n",
+ inv_arg.ret);
+ ret = -EFAULT;
+ } else {
+ p->blob_len = param[1].u.memref.size;
+ }
+
+out:
+ if (reg_shm_out)
+ tee_shm_free(reg_shm_out);
+ if (reg_shm_in)
+ tee_shm_free(reg_shm_in);
+
+ return ret;
+}
+
+/*
+ * Have the TEE unseal(decrypt) the symmetric key
+ */
+static int tee_trusted_unseal(struct trusted_key_payload *p, char *datablob)
+{
+ int ret = 0;
+ struct tee_ioctl_invoke_arg inv_arg;
+ struct tee_param param[4];
+ struct tee_shm *reg_shm_in = NULL, *reg_shm_out = NULL;
+
+ memset(&inv_arg, 0, sizeof(inv_arg));
+ memset(¶m, 0, sizeof(param));
+
+ reg_shm_in = tee_shm_register(pvt_data.ctx, (unsigned long)p->blob,
+ p->blob_len, TEE_SHM_DMA_BUF |
+ TEE_SHM_KERNEL_MAPPED);
+ if (IS_ERR(reg_shm_in)) {
+ dev_err(pvt_data.dev, "blob shm register failed\n");
+ return PTR_ERR(reg_shm_in);
+ }
+
+ reg_shm_out = tee_shm_register(pvt_data.ctx, (unsigned long)p->key,
+ sizeof(p->key), TEE_SHM_DMA_BUF |
+ TEE_SHM_KERNEL_MAPPED);
+ if (IS_ERR(reg_shm_out)) {
+ dev_err(pvt_data.dev, "key shm register failed\n");
+ ret = PTR_ERR(reg_shm_out);
+ goto out;
+ }
+
+ inv_arg.func = TA_CMD_UNSEAL;
+ inv_arg.session = pvt_data.session_id;
+ inv_arg.num_params = 4;
+
+ param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT;
+ param[0].u.memref.shm = reg_shm_in;
+ param[0].u.memref.size = p->blob_len;
+ param[0].u.memref.shm_offs = 0;
+ param[1].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT;
+ param[1].u.memref.shm = reg_shm_out;
+ param[1].u.memref.size = sizeof(p->key);
+ param[1].u.memref.shm_offs = 0;
+
+ ret = tee_client_invoke_func(pvt_data.ctx, &inv_arg, param);
+ if ((ret < 0) || (inv_arg.ret != 0)) {
+ dev_err(pvt_data.dev, "TA_CMD_UNSEAL invoke err: %x\n",
+ inv_arg.ret);
+ ret = -EFAULT;
+ } else {
+ p->key_len = param[1].u.memref.size;
+ }
+
+out:
+ if (reg_shm_out)
+ tee_shm_free(reg_shm_out);
+ if (reg_shm_in)
+ tee_shm_free(reg_shm_in);
+
+ return ret;
+}
+
+/*
+ * Have the TEE generate random symmetric key
+ */
+static int tee_trusted_get_random(unsigned char *key, size_t key_len)
+{
+ int ret = 0;
+ struct tee_ioctl_invoke_arg inv_arg;
+ struct tee_param param[4];
+ struct tee_shm *reg_shm = NULL;
+
+ memset(&inv_arg, 0, sizeof(inv_arg));
+ memset(¶m, 0, sizeof(param));
+
+ reg_shm = tee_shm_register(pvt_data.ctx, (unsigned long)key, key_len,
+ TEE_SHM_DMA_BUF | TEE_SHM_KERNEL_MAPPED);
+ if (IS_ERR(reg_shm)) {
+ dev_err(pvt_data.dev, "key shm register failed\n");
+ return PTR_ERR(reg_shm);
+ }
+
+ inv_arg.func = TA_CMD_GET_RANDOM;
+ inv_arg.session = pvt_data.session_id;
+ inv_arg.num_params = 4;
+
+ param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT;
+ param[0].u.memref.shm = reg_shm;
+ param[0].u.memref.size = key_len;
+ param[0].u.memref.shm_offs = 0;
+
+ ret = tee_client_invoke_func(pvt_data.ctx, &inv_arg, param);
+ if ((ret < 0) || (inv_arg.ret != 0)) {
+ dev_err(pvt_data.dev, "TA_CMD_GET_RANDOM invoke err: %x\n",
+ inv_arg.ret);
+ ret = -EFAULT;
+ } else {
+ ret = param[0].u.memref.size;
+ }
+
+ tee_shm_free(reg_shm);
+
+ return ret;
+}
+
+static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data)
+{
+ if (ver->impl_id == TEE_IMPL_ID_OPTEE)
+ return 1;
+ else
+ return 0;
+}
+
+static int trusted_key_probe(struct device *dev)
+{
+ struct tee_client_device *rng_device = to_tee_client_device(dev);
+ int ret = 0, err = -ENODEV;
+ struct tee_ioctl_open_session_arg sess_arg;
+
+ memset(&sess_arg, 0, sizeof(sess_arg));
+
+ pvt_data.ctx = tee_client_open_context(NULL, optee_ctx_match, NULL,
+ NULL);
+ if (IS_ERR(pvt_data.ctx))
+ return -ENODEV;
+
+ memcpy(sess_arg.uuid, rng_device->id.uuid.b, TEE_IOCTL_UUID_LEN);
+ sess_arg.clnt_login = TEE_IOCTL_LOGIN_REE_KERNEL;
+ sess_arg.num_params = 0;
+
+ ret = tee_client_open_session(pvt_data.ctx, &sess_arg, NULL);
+ if ((ret < 0) || (sess_arg.ret != 0)) {
+ dev_err(dev, "tee_client_open_session failed, err: %x\n",
+ sess_arg.ret);
+ err = -EINVAL;
+ goto out_ctx;
+ }
+ pvt_data.session_id = sess_arg.session;
+
+ ret = register_key_type(&key_type_trusted);
+ if (ret < 0)
+ goto out_sess;
+
+ pvt_data.dev = dev;
+
+ return 0;
+
+out_sess:
+ tee_client_close_session(pvt_data.ctx, pvt_data.session_id);
+out_ctx:
+ tee_client_close_context(pvt_data.ctx);
+
+ return err;
+}
+
+static int trusted_key_remove(struct device *dev)
+{
+ unregister_key_type(&key_type_trusted);
+ tee_client_close_session(pvt_data.ctx, pvt_data.session_id);
+ tee_client_close_context(pvt_data.ctx);
+
+ return 0;
+}
+
+static const struct tee_client_device_id trusted_key_id_table[] = {
+ {UUID_INIT(0xf04a0fe7, 0x1f5d, 0x4b9b,
+ 0xab, 0xf7, 0x61, 0x9b, 0x85, 0xb4, 0xce, 0x8c)},
+ {}
+};
+MODULE_DEVICE_TABLE(tee, trusted_key_id_table);
+
+static struct tee_client_driver trusted_key_driver = {
+ .id_table = trusted_key_id_table,
+ .driver = {
+ .name = DRIVER_NAME,
+ .bus = &tee_bus_type,
+ .probe = trusted_key_probe,
+ .remove = trusted_key_remove,
+ },
+};
+
+static int __init init_tee_trusted(void)
+{
+ return driver_register(&trusted_key_driver.driver);
+}
+
+static void __exit exit_tee_trusted(void)
+{
+ driver_unregister(&trusted_key_driver.driver);
+}
+
+struct trusted_key_ops tee_trusted_key_ops = {
+ .migratable = 0, /* non-migratable */
+ .init = init_tee_trusted,
+ .seal = tee_trusted_seal,
+ .unseal = tee_trusted_unseal,
+ .get_random = tee_trusted_get_random,
+ .exit = exit_tee_trusted,
+};
--
2.7.4
^ permalink raw reply related
* [PATCH v6 4/4] MAINTAINERS: Add entry for TEE based Trusted Keys
From: Sumit Garg @ 2020-09-17 13:46 UTC (permalink / raw)
To: jarkko.sakkinen, zohar, jejb
Cc: dhowells, jens.wiklander, corbet, jmorris, serge, casey,
janne.karhunen, daniel.thompson, Markus.Wamser, lhinds, keyrings,
linux-integrity, linux-security-module, linux-doc, linux-kernel,
linux-arm-kernel, op-tee, Sumit Garg
In-Reply-To: <1600350398-4813-1-git-send-email-sumit.garg@linaro.org>
Add MAINTAINERS entry for TEE based Trusted Keys framework.
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
Acked-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
MAINTAINERS | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 0d0862b..0a913ba 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9668,6 +9668,14 @@ F: include/keys/trusted-type.h
F: include/keys/trusted_tpm.h
F: security/keys/trusted-keys/
+KEYS-TRUSTED-TEE
+M: Sumit Garg <sumit.garg@linaro.org>
+L: linux-integrity@vger.kernel.org
+L: keyrings@vger.kernel.org
+S: Supported
+F: include/keys/trusted_tee.h
+F: security/keys/trusted-keys/trusted_tee.c
+
KEYS/KEYRINGS
M: David Howells <dhowells@redhat.com>
M: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
--
2.7.4
^ permalink raw reply related
* [PATCH v6 3/4] doc: trusted-encrypted: updates with TEE as a new trust source
From: Sumit Garg @ 2020-09-17 13:46 UTC (permalink / raw)
To: jarkko.sakkinen, zohar, jejb
Cc: dhowells, jens.wiklander, corbet, jmorris, serge, casey,
janne.karhunen, daniel.thompson, Markus.Wamser, lhinds, keyrings,
linux-integrity, linux-security-module, linux-doc, linux-kernel,
linux-arm-kernel, op-tee, Sumit Garg
In-Reply-To: <1600350398-4813-1-git-send-email-sumit.garg@linaro.org>
Update documentation for Trusted and Encrypted Keys with TEE as a new
trust source. Following is brief description of updates:
- Add a section to demostrate a list of supported devices along with
their security properties/guarantees.
- Add a key generation section.
- Updates for usage section including differences specific to a trust
source.
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
Documentation/security/keys/trusted-encrypted.rst | 203 ++++++++++++++++++----
1 file changed, 171 insertions(+), 32 deletions(-)
diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
index 9483a74..a355045 100644
--- a/Documentation/security/keys/trusted-encrypted.rst
+++ b/Documentation/security/keys/trusted-encrypted.rst
@@ -6,30 +6,161 @@ Trusted and Encrypted Keys are two new key types added to the existing kernel
key ring service. Both of these new types are variable length symmetric keys,
and in both cases all keys are created in the kernel, and user space sees,
stores, and loads only encrypted blobs. Trusted Keys require the availability
-of a Trusted Platform Module (TPM) chip for greater security, while Encrypted
-Keys can be used on any system. All user level blobs, are displayed and loaded
-in hex ascii for convenience, and are integrity verified.
+of a Trust Source for greater security, while Encrypted Keys can be used on any
+system. All user level blobs, are displayed and loaded in hex ascii for
+convenience, and are integrity verified.
-Trusted Keys use a TPM both to generate and to seal the keys. Keys are sealed
-under a 2048 bit RSA key in the TPM, and optionally sealed to specified PCR
-(integrity measurement) values, and only unsealed by the TPM, if PCRs and blob
-integrity verifications match. A loaded Trusted Key can be updated with new
-(future) PCR values, so keys are easily migrated to new pcr values, such as
-when the kernel and initramfs are updated. The same key can have many saved
-blobs under different PCR values, so multiple boots are easily supported.
-TPM 1.2
--------
+Trust Source
+============
-By default, trusted keys are sealed under the SRK, which has the default
-authorization value (20 zeros). This can be set at takeownership time with the
-trouser's utility: "tpm_takeownership -u -z".
+Trust Source provides the source of security for the Trusted Keys, on which
+basis Trusted Keys establishes a Trust model with its user. A Trust Source could
+differ from one system to another depending on its security requirements. It
+could be either an off-chip device or an on-chip device. Following section
+demostrates a list of supported devices along with their security properties/
+guarantees:
-TPM 2.0
--------
+ * Root of trust for storage
-The user must first create a storage key and make it persistent, so the key is
-available after reboot. This can be done using the following commands.
+ (1) TPM (Trusted Platform Module: hardware device)
+
+ Rooted to Storage Root Key (SRK) which never leaves the TPM that
+ provides crypto operation to establish root of trust for storage.
+
+ (2) TEE (Trusted Execution Environment: OP-TEE based on Arm TrustZone)
+
+ Rooted to Hardware Unique Key (HUK) which is generally burnt in on-chip
+ fuses and is accessible to TEE only.
+
+ * Execution isolation
+
+ (1) TPM
+
+ Fixed set of operations running in isolated execution environment.
+
+ (2) TEE
+
+ Customizable set of operations running in isolated execution
+ environment verified via Secure/Trusted boot process.
+
+ * Optional binding to platform integrity state
+
+ (1) TPM
+
+ Keys can be optionally sealed to specified PCR (integrity measurement)
+ values, and only unsealed by the TPM, if PCRs and blob integrity
+ verifications match. A loaded Trusted Key can be updated with new
+ (future) PCR values, so keys are easily migrated to new PCR values,
+ such as when the kernel and initramfs are updated. The same key can
+ have many saved blobs under different PCR values, so multiple boots are
+ easily supported.
+
+ (2) TEE
+
+ Relies on Secure/Trusted boot process for platform integrity. It can
+ be extended with TEE based measured boot process.
+
+ * On-chip versus off-chip
+
+ (1) TPM
+
+ Off-chip device connected via serial bus (like I2C, SPI etc.) exposing
+ physical access which represents an attack surface that can be
+ mitigated via tamper detection.
+
+ (2) TEE
+
+ On-chip functionality, immune to this attack surface.
+
+ * Memory attacks (DRAM based like attaching a bus monitor etc.)
+
+ (1) TPM
+
+ Immune to these attacks as it doesn’t make use of system DRAM.
+
+ (2) TEE
+
+ An implementation based on TrustZone protected DRAM is susceptible to
+ such attacks. In order to mitigate these attacks one needs to rely on
+ on-chip secure RAM to store secrets or have the entire TEE
+ implementation based on on-chip secure RAM. An alternative mitigation
+ would be to use encrypted DRAM.
+
+ * Side-channel attacks (cache, memory, CPU or time based)
+
+ (1) TPM
+
+ Immune to side-channel attacks as its resources are isolated from the
+ main OS.
+
+ (2) TEE
+
+ A careful implementation is required to mitigate against these attacks
+ for resources which are shared (eg. shared memory) with the main OS.
+ Cache and CPU based side-channel attacks can be mitigated via
+ invalidating caches and CPU registers during context switch to and from
+ the secure world.
+ To mitigate against time based attacks, one needs to have time
+ invariant implementations (like crypto algorithms etc.).
+
+ * Resistance to physical attacks (power analysis, electromagnetic emanation,
+ probes etc.)
+
+ (1) TPM
+
+ Provides limited protection utilizing tamper resistance.
+
+ (2) TEE
+
+ Provides no protection by itself, relies on the underlying platform for
+ features such as tamper resistance.
+
+
+Key Generation
+==============
+
+Trusted Keys
+------------
+
+New keys are created from trust source generated random numbers, and are
+encrypted/decrypted using trust source storage root key.
+
+ * TPM (hardware device) based RNG
+
+ Strength of random numbers may vary from one device manufacturer to
+ another.
+
+ * TEE (OP-TEE based on Arm TrustZone) based RNG
+
+ RNG is customizable as per platform needs. It can either be direct output
+ from platform specific hardware RNG or a software based Fortuna CSPRNG
+ which can be seeded via multiple entropy sources.
+
+Encrypted Keys
+--------------
+
+Encrypted keys do not depend on a trust source, and are faster, as they use AES
+for encryption/decryption. New keys are created from kernel generated random
+numbers, and are encrypted/decrypted using a specified ‘master’ key. The
+‘master’ key can either be a trusted-key or user-key type. The main disadvantage
+of encrypted keys is that if they are not rooted in a trusted key, they are only
+as secure as the user key encrypting them. The master user key should therefore
+be loaded in as secure a way as possible, preferably early in boot.
+
+
+Usage
+=====
+
+Trusted Keys usage: TPM
+-----------------------
+
+TPM 1.2: By default, trusted keys are sealed under the SRK, which has the
+default authorization value (20 zeros). This can be set at takeownership time
+with the TrouSerS utility: "tpm_takeownership -u -z".
+
+TPM 2.0: The user must first create a storage key and make it persistent, so the
+key is available after reboot. This can be done using the following commands.
With the IBM TSS 2 stack::
@@ -79,14 +210,21 @@ TPM_STORED_DATA format. The key length for new keys are always in bytes.
Trusted Keys can be 32 - 128 bytes (256 - 1024 bits), the upper limit is to fit
within the 2048 bit SRK (RSA) keylength, with all necessary structure/padding.
-Encrypted keys do not depend on a TPM, and are faster, as they use AES for
-encryption/decryption. New keys are created from kernel generated random
-numbers, and are encrypted/decrypted using a specified 'master' key. The
-'master' key can either be a trusted-key or user-key type. The main
-disadvantage of encrypted keys is that if they are not rooted in a trusted key,
-they are only as secure as the user key encrypting them. The master user key
-should therefore be loaded in as secure a way as possible, preferably early in
-boot.
+Trusted Keys usage: TEE
+-----------------------
+
+Usage::
+
+ keyctl add trusted name "new keylen" ring
+ keyctl add trusted name "load hex_blob" ring
+ keyctl print keyid
+
+"keyctl print" returns an ascii hex copy of the sealed key, which is in format
+specific to TEE device implementation. The key length for new keys are always
+in bytes. Trusted Keys can be 32 - 128 bytes (256 - 1024 bits).
+
+Encrypted Keys usage
+--------------------
The decrypted portion of encrypted keys can contain either a simple symmetric
key or a more complex structure. The format of the more complex structure is
@@ -104,8 +242,8 @@ Where::
format:= 'default | ecryptfs | enc32'
key-type:= 'trusted' | 'user'
-
Examples of trusted and encrypted key usage:
+--------------------------------------------
Create and save a trusted key named "kmk" of length 32 bytes.
@@ -151,7 +289,7 @@ Load a trusted key from the saved blob::
f1f8fff03ad0acb083725535636addb08d73dedb9832da198081e5deae84bfaf0409c22b
e4a8aea2b607ec96931e6f4d4fe563ba
-Reseal a trusted key under new pcr values::
+Reseal (TPM specific) a trusted key under new PCR values::
$ keyctl update 268728824 "update pcrinfo=`cat pcr.blob`"
$ keyctl print 268728824
@@ -165,11 +303,12 @@ Reseal a trusted key under new pcr values::
7ef6a24defe4846104209bf0c3eced7fa1a672ed5b125fc9d8cd88b476a658a4434644ef
df8ae9a178e9f83ba9f08d10fa47e4226b98b0702f06b3b8
+
The initial consumer of trusted keys is EVM, which at boot time needs a high
-quality symmetric key for HMAC protection of file metadata. The use of a
+quality symmetric key for HMAC protection of file metadata. The use of a
trusted key provides strong guarantees that the EVM key has not been
-compromised by a user level problem, and when sealed to specific boot PCR
-values, protects against boot and offline attacks. Create and save an
+compromised by a user level problem, and when sealed to a platform integrity
+state, protects against boot and offline attacks. Create and save an
encrypted key "evm" using the above trusted key "kmk":
option 1: omitting 'format'::
--
2.7.4
^ permalink raw reply related
* [PATCH v6 0/4] Introduce TEE based Trusted Keys support
From: Sumit Garg @ 2020-09-17 13:46 UTC (permalink / raw)
To: jarkko.sakkinen, zohar, jejb
Cc: dhowells, jens.wiklander, corbet, jmorris, serge, casey,
janne.karhunen, daniel.thompson, Markus.Wamser, lhinds, keyrings,
linux-integrity, linux-security-module, linux-doc, linux-kernel,
linux-arm-kernel, op-tee, Sumit Garg
Add support for TEE based trusted keys where TEE provides the functionality
to seal and unseal trusted keys using hardware unique key. Also, this is
an alternative in case platform doesn't possess a TPM device.
This patch-set has been tested with OP-TEE based early TA which is already
merged in upstream [1].
[1] https://github.com/OP-TEE/optee_os/commit/f86ab8e7e0de869dfa25ca05a37ee070d7e5b86b
Changes in v6:
1. Revert back to dynamic detection of trust source.
2. Drop author mention from trusted_core.c and trusted_tpm1.c files.
3. Rebased to latest tpmdd/master.
Changes in v5:
1. Drop dynamic detection of trust source and use compile time flags
instead.
2. Rename trusted_common.c -> trusted_core.c.
3. Rename callback: cleanup() -> exit().
4. Drop "tk" acronym.
5. Other misc. comments.
6. Added review tags for patch #3 and #4.
Changes in v4:
1. Pushed independent TEE features separately:
- Part of recent TEE PR: https://lkml.org/lkml/2020/5/4/1062
2. Updated trusted-encrypted doc with TEE as a new trust source.
3. Rebased onto latest tpmdd/master.
Changes in v3:
1. Update patch #2 to support registration of multiple kernel pages.
2. Incoporate dependency patch #4 in this patch-set:
https://patchwork.kernel.org/patch/11091435/
Changes in v2:
1. Add reviewed-by tags for patch #1 and #2.
2. Incorporate comments from Jens for patch #3.
3. Switch to use generic trusted keys framework.
Sumit Garg (4):
KEYS: trusted: Add generic trusted keys framework
KEYS: trusted: Introduce TEE based Trusted Keys
doc: trusted-encrypted: updates with TEE as a new trust source
MAINTAINERS: Add entry for TEE based Trusted Keys
Documentation/security/keys/trusted-encrypted.rst | 203 ++++++++++---
MAINTAINERS | 8 +
include/keys/trusted-type.h | 42 +++
include/keys/trusted_tee.h | 55 ++++
include/keys/trusted_tpm.h | 17 +-
security/keys/trusted-keys/Makefile | 2 +
security/keys/trusted-keys/trusted_core.c | 325 +++++++++++++++++++++
security/keys/trusted-keys/trusted_tee.c | 278 ++++++++++++++++++
security/keys/trusted-keys/trusted_tpm1.c | 336 ++++------------------
9 files changed, 939 insertions(+), 327 deletions(-)
create mode 100644 include/keys/trusted_tee.h
create mode 100644 security/keys/trusted-keys/trusted_core.c
create mode 100644 security/keys/trusted-keys/trusted_tee.c
--
2.7.4
^ permalink raw reply
* Re: [PATCH v2 09/12] evm: Allow setxattr() and setattr() if metadata digest won't change
From: Mimi Zohar @ 2020-09-17 13:15 UTC (permalink / raw)
To: Roberto Sassu, mjg59
Cc: linux-integrity, linux-security-module, linux-kernel,
silviu.vlasceanu
In-Reply-To: <20200904092643.20013-5-roberto.sassu@huawei.com>
Hi Roberto,
"if" doesn't belong in the subject line. In this case, instead of "if
metadata ...", how about something like "for unmodified metadata"?
On Fri, 2020-09-04 at 11:26 +0200, Roberto Sassu wrote:
> With the patch to allow xattr/attr operations if a portable signature
> verification fails, cp and tar can copy all xattrs/attrs so that at the
> end of the process verification succeeds.
>
> However, it might happen that xattrs/attrs are already set to the correct
^ the xattrs/attrs
> value (taken at signing time) and signature verification succeeds before
> the copy is completed. For example, an archive might contains files owned
^ has completed.
> by root and the archive is extracted by root.
>
> Then, since portable signatures are immutable, all subsequent operations
> fail (e.g. fchown()), even if the operation is legitimate (does not alter
> the current value).
>
> This patch avoids this problem by reporting successful operation to user
> space when that operation does not alter the current value of xattrs/attrs.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
<snip>
> +/*
> + * evm_xattr_change - check if passed xattr value differs from current value
> + * @dentry: pointer to the affected dentry
> + * @xattr_name: requested xattr
> + * @xattr_value: requested xattr value
> + * @xattr_value_len: requested xattr value length
> + *
> + * Check if passed xattr value differs from current value.
> + *
> + * Returns 1 if passed xattr value differs from current value, 0 otherwise.
> + */
> +static int evm_xattr_change(struct dentry *dentry, const char *xattr_name,
> + const void *xattr_value, size_t xattr_value_len)
> +{
> + char *xattr_data = NULL;
> + int rc = 0;
> +
> + if (posix_xattr_acl(xattr_name))
> + return evm_xattr_acl_change(dentry, xattr_name, xattr_value,
> + xattr_value_len);
> +
> + rc = vfs_getxattr_alloc(dentry, xattr_name, &xattr_data, 0, GFP_NOFS);
> + if (rc < 0)
> + return 1;
> +
> + if (rc == xattr_value_len)
> + rc = memcmp(xattr_value, xattr_data, rc);
This should probably be changed to crypto_memneq(). Refer to commit
613317bd212c ("EVM: Use crypto_memneq() for digest comparisons").
thanks,
Mimi
> + else
> + rc = 1;
> +
> + kfree(xattr_data);
> + return rc;
> +}
> +
>
^ permalink raw reply
* Re: [PATCH v2 07/12] evm: Introduce EVM_RESET_STATUS atomic flag
From: Mimi Zohar @ 2020-09-17 12:01 UTC (permalink / raw)
To: Roberto Sassu, mjg59, John Johansen
Cc: linux-integrity, linux-security-module, linux-kernel,
silviu.vlasceanu, stable
In-Reply-To: <20200904092643.20013-3-roberto.sassu@huawei.com>
[Cc'ing John Johansen]
Hi Roberto,
On Fri, 2020-09-04 at 11:26 +0200, Roberto Sassu wrote:
> When EVM_ALLOW_METADATA_WRITES is set, EVM allows any operation on
> metadata. Its main purpose is to allow users to freely set metadata when
> they are protected by a portable signature, until the HMAC key is loaded.
>
> However, IMA is not notified about metadata changes and, after the first
> successful appraisal, always allows access to the files without checking
> metadata again.
>
> This patch introduces the new atomic flag EVM_RESET_STATUS in
> integrity_iint_cache that is set in the EVM post hooks and cleared in
> evm_verify_hmac(). IMA checks the new flag in process_measurement() and if
> it is set, it clears the appraisal flags.
>
> Although the flag could be cleared also by evm_inode_setxattr() and
> evm_inode_setattr() before IMA sees it, this does not happen if
> EVM_ALLOW_METADATA_WRITES is set. Since the only remaining caller is
> evm_verifyxattr(), this ensures that IMA always sees the flag set before it
> is cleared.
>
> This patch also adds a call to evm_reset_status() in
> evm_inode_post_setattr() so that EVM won't return the cached status the
> next time appraisal is performed.
>
> Cc: stable@vger.kernel.org # 4.16.x
> Fixes: ae1ba1676b88e ("EVM: Allow userland to permit modification of EVM-protected metadata")
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
> security/integrity/evm/evm_main.c | 17 +++++++++++++++--
> security/integrity/ima/ima_main.c | 8 ++++++--
> security/integrity/integrity.h | 1 +
> 3 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 4e9f5e8b21d5..05be1ad3e6f3 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -221,8 +221,15 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
> evm_status = (rc == -ENODATA) ?
> INTEGRITY_NOXATTRS : INTEGRITY_FAIL;
> out:
> - if (iint)
> + if (iint) {
> + /*
> + * EVM_RESET_STATUS can be cleared only by evm_verifyxattr()
> + * when EVM_ALLOW_METADATA_WRITES is set. This guarantees that
> + * IMA sees the EVM_RESET_STATUS flag set before it is cleared.
> + */
> + clear_bit(EVM_RESET_STATUS, &iint->atomic_flags);
> iint->evm_status = evm_status;
True IMA is currently the only caller of evm_verifyxattr() in the
upstreamed kernel, but it is an exported function, which may be called
from elsewhere. The previous version crossed the boundary between EVM
& IMA with EVM modifying the IMA flag directly. This version assumes
that IMA will be the only caller. Otherwise, I like this version.
Mimi
> + }
> kfree(xattr_data);
> return evm_status;
> }
> @@ -418,8 +425,12 @@ static void evm_reset_status(struct inode *inode)
> struct integrity_iint_cache *iint;
>
> iint = integrity_iint_find(inode);
> - if (iint)
> + if (iint) {
> + if (evm_initialized & EVM_ALLOW_METADATA_WRITES)
> + set_bit(EVM_RESET_STATUS, &iint->atomic_flags);
> +
> iint->evm_status = INTEGRITY_UNKNOWN;
> + }
> }
>
> /**
> @@ -513,6 +524,8 @@ void evm_inode_post_setattr(struct dentry *dentry, int ia_valid)
> if (!evm_key_loaded())
> return;
>
> + evm_reset_status(dentry->d_inode);
> +
> if (ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID))
> evm_update_evmxattr(dentry, NULL, NULL, 0);
> }
^ permalink raw reply
* [PATCH -next] apparmor: Fix several kernel-doc warnings
From: Huang Guobin @ 2020-09-17 12:32 UTC (permalink / raw)
To: john.johansen, jmorris, serge; +Cc: linux-security-module, linux-kernel
Fixes the following W=1 kernel build warning(s):
security/apparmor/apparmorfs.c:2127: warning: Excess function parameter 'profile' description in '__next_profile'
security/apparmor/domain.c:1292: warning: Excess function parameter 'onexec' description in 'aa_change_profile'
security/apparmor/domain.c:136: warning: Excess function parameter 'start' description in 'label_compound_match'
Rename profile to p.
@onexec in 'aa_change_profile' and @start in 'label_compound_match' are not in use, Remove it.
Signed-off-by: Huang Guobin <huangguobin4@huawei.com>
---
security/apparmor/apparmorfs.c | 2 +-
security/apparmor/domain.c | 4 ----
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index 5fd4a64e431f..f00a372be7b6 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -2116,7 +2116,7 @@ static struct aa_profile *__first_profile(struct aa_ns *root,
/**
* __next_profile - step to the next profile in a profile tree
- * @profile: current profile in tree (NOT NULL)
+ * @p: current profile in tree (NOT NULL)
*
* Perform a depth first traversal on the profile tree in a namespace
*
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index f919ebd042fd..12a9d80cd63a 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -178,7 +178,6 @@ static int label_compound_match(struct aa_profile *profile,
* @profile: profile to find perms for
* @label: label to check access permissions for
* @stack: whether this is a stacking request
- * @start: state to start match in
* @subns: whether to do permission checks on components in a subns
* @request: permissions to request
* @perms: an initialized perms struct to add accumulation to
@@ -1277,14 +1276,11 @@ static int change_profile_perms_wrapper(const char *op, const char *name,
/**
* aa_change_profile - perform a one-way profile transition
* @fqname: name of profile may include namespace (NOT NULL)
- * @onexec: whether this transition is to take place immediately or at exec
* @flags: flags affecting change behavior
*
* Change to new profile @name. Unlike with hats, there is no way
* to change back. If @name isn't specified the current profile name is
* used.
- * If @onexec then the transition is delayed until
- * the next exec.
*
* Returns %0 on success, error otherwise.
*/
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v2 08/12] evm: Allow xattr/attr operations for portable signatures if check fails
From: Mimi Zohar @ 2020-09-17 12:32 UTC (permalink / raw)
To: Roberto Sassu, mjg59
Cc: linux-integrity, linux-security-module, linux-kernel,
silviu.vlasceanu
In-Reply-To: <20200904092643.20013-4-roberto.sassu@huawei.com>
Hi Roberto,
"if check fails" in the Subject line is unnecessary.
On Fri, 2020-09-04 at 11:26 +0200, Roberto Sassu wrote:
> If files with portable signatures are copied from one location to another
> or are extracted from an archive, verification can temporarily fail until
> all xattrs/attrs are set in the destination. Only portable signatures may
> be moved or copied from one file to another, as they don't depend on
> system-specific information such as the inode generation. Instead portable
> signatures must include security.ima.
>
> Unlike other security.evm types, EVM portable signatures are also
> immutable. Thus, it wouldn't be a problem to allow xattr/attr operations
> when verification fails, as portable signatures will never be replaced with
> an HMAC on possibly corrupted xattrs/attrs.
>
> This patch first introduces a new integrity status called
> INTEGRITY_FAIL_IMMUTABLE, that allows callers of
> evm_verify_current_integrity() to detect that a portable signature didn't
> pass verification and then adds an exception in evm_protect_xattr() and
> evm_inode_setattr() for this status and returns 0 instead of -EPERM.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
< snip >
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 05be1ad3e6f3..a5dab1ac9374 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
>
> @@ -358,6 +364,12 @@ static int evm_protect_xattr(struct dentry *dentry, const char *xattr_name,
> -EPERM, 0);
> }
> out:
> + /* Writing other xattrs is safe for portable signatures, as portable
> + * signatures are immutable and can never be updated.
> + */
This is the second time I'm seeing this comment format style. Why?
What changed?
Mimi
> + if (evm_status == INTEGRITY_FAIL_IMMUTABLE)
> + return 0;
> +
> if (evm_status != INTEGRITY_PASS)
> integrity_audit_msg(AUDIT_INTEGRITY_METADATA, d_backing_inode(dentry),
> dentry->d_name.name, "appraise_metadata",
^ permalink raw reply
* [PATCH -next] apparmor: Fix several kernel-doc warnings
From: Huang Guobin @ 2020-09-17 12:24 UTC (permalink / raw)
To: john.johansen, jmorris, serge; +Cc: linux-security-module, linux-kernel
Fixes the following W=1 kernel build warning(s):
security/apparmor/apparmorfs.c:2127: warning: Excess function parameter 'profile' description in '__next_profile'
security/apparmor/domain.c:1292: warning: Excess function parameter 'onexec' description in 'aa_change_profile'
security/apparmor/domain.c:136: warning: Excess function parameter 'start' description in 'label_compound_match'
Rename profile to p.
@onexec in 'aa_change_profile' and @start in 'label_compound_match' are not in use, Remove it.
Signed-off-by: Huang Guobin <huangguobin4@huawei.com>
---
security/apparmor/apparmorfs.c | 2 +-
security/apparmor/domain.c | 4 ----
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index 5fd4a64e431f..f00a372be7b6 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -2116,7 +2116,7 @@ static struct aa_profile *__first_profile(struct aa_ns *root,
/**
* __next_profile - step to the next profile in a profile tree
- * @profile: current profile in tree (NOT NULL)
+ * @p: current profile in tree (NOT NULL)
*
* Perform a depth first traversal on the profile tree in a namespace
*
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index f919ebd042fd..12a9d80cd63a 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -178,7 +178,6 @@ static int label_compound_match(struct aa_profile *profile,
* @profile: profile to find perms for
* @label: label to check access permissions for
* @stack: whether this is a stacking request
- * @start: state to start match in
* @subns: whether to do permission checks on components in a subns
* @request: permissions to request
* @perms: an initialized perms struct to add accumulation to
@@ -1277,14 +1276,11 @@ static int change_profile_perms_wrapper(const char *op, const char *name,
/**
* aa_change_profile - perform a one-way profile transition
* @fqname: name of profile may include namespace (NOT NULL)
- * @onexec: whether this transition is to take place immediately or at exec
* @flags: flags affecting change behavior
*
* Change to new profile @name. Unlike with hats, there is no way
* to change back. If @name isn't specified the current profile name is
* used.
- * If @onexec then the transition is delayed until
- * the next exec.
*
* Returns %0 on success, error otherwise.
*/
--
2.25.1
^ permalink raw reply related
* Re: [PATCH] socket.7,ip.7: Document SO_PEERSEC for AF_INET sockets
From: Michael Kerrisk (man-pages) @ 2020-09-17 7:30 UTC (permalink / raw)
To: Stephen Smalley
Cc: mtk.manpages, linux-man, linux-security-module, selinux, smcv,
jmorris, serge, paul
In-Reply-To: <20200915163959.25334-1-stephen.smalley.work@gmail.com>
Hello Stephen
On 9/15/20 6:39 PM, Stephen Smalley wrote:
> Augment the description of SO_PEERSEC to cover AF_INET sockets
> in addition to the prior description for AF_UNIX.
>
> SO_PEERSEC for TCP sockets was introduced in Linux 2.6.17 [1], and
> SO_PEERSEC for SCTP sockets was introduced in Linux 4.17 [2].
>
> This does not cover usage of SCM_SECURITY for UDP sockets, which
> was also introduced in the same commit for 2.6.17.
(Would you by chance be able to write a patch for this also?)
> Examples of the necessary labeled IPSEC and NetLabel configurations
> to enable use of SO_PEERSEC for TCP and SCTP sockets can be found in
> the SELinux Notebook [3] and the selinux-testsuite [4].
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2c7946a7bf45ae86736ab3b43d0085e43947945c
>
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d452930fd3b9031e59abfeddb2fa383f1403d61a
>
> [3] https://github.com/SELinuxProject/selinux-notebook
>
> [4] https://github.com/SELinuxProject/selinux-testsuite
Thanks. I've applied the patch. I'll wait a few hours before pushing
in case Reviews/Acks come in.
Thanks,
Michael
> ---
> man7/ip.7 | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++
> man7/socket.7 | 2 +-
> 2 files changed, 57 insertions(+), 1 deletion(-)
>
> diff --git a/man7/ip.7 b/man7/ip.7
> index c522b219c..03a9f3f7c 100644
> --- a/man7/ip.7
> +++ b/man7/ip.7
> @@ -979,6 +979,62 @@ Argument is an
> .I ip_mreq_source
> structure as described under
> .BR IP_ADD_SOURCE_MEMBERSHIP .
> +.TP
> +.BR SO_PEERSEC " (since Linux 2.6.17)"
> +If labeled IPSEC or NetLabel is configured on both the sending and
> +receiving hosts, this read-only socket option returns the security
> +context of the peer socket connected to this socket. By default, this
> +will be the same as the security context of the process that created
> +the peer socket unless overridden by the policy or by a process with
> +the required permissions.
> +.IP
> +The argument to
> +.BR getsockopt (2)
> +is a pointer to a
> +buffer of the specified length in bytes
> +into which the security context string will be copied.
> +If the buffer length is less than the length of the security
> +context string, then
> +.BR getsockopt (2)
> +will return the required length
> +via
> +.I optlen
> +and return \-1 and sets
> +.I errno
> +to
> +.BR ERANGE .
> +The caller should allocate at least
> +.BR NAME_MAX
> +bytes for the buffer initially although this is not guaranteed
> +to be sufficient. Resizing the buffer to the returned length
> +and retrying may be necessary.
> +.IP
> +The security context string may include a terminating null character
> +in the returned length, but is not guaranteed to do so: a security
> +context "foo" might be represented as either {'f','o','o'} of length 3
> +or {'f','o','o','\\0'} of length 4, which are considered to be
> +interchangeable. It is printable, does not contain non-terminating
> +null characters, and is in an unspecified encoding (in particular it
> +is not guaranteed to be ASCII or UTF-8).
> +.IP
> +The use of this option for sockets in the
> +.B AF_INET
> +address family
> +is supported since Linux 2.6.17
> +.\" commit 2c7946a7bf45ae86736ab3b43d0085e43947945c
> +for TCP sockets and since Linux
> +4.17
> +.\" commit d452930fd3b9031e59abfeddb2fa383f1403d61a
> +for SCTP sockets.
> +.IP
> +For SELinux, NetLabel only conveys the MLS portion of the security
> +context of the peer across the wire, defaulting the rest of the
> +security context to the values defined in the policy for the
> +netmsg initial security identifier (SID). However, NetLabel can
> +be configured to pass full security contexts over loopback. Labeled
> +IPSEC always passes full security contexts as part of establishing
> +the security association (SA) and looks them up based on the association
> +for each packet.
> .SS /proc interfaces
> The IP protocol
> supports a set of
> diff --git a/man7/socket.7 b/man7/socket.7
> index c3635f95b..2f9039333 100644
> --- a/man7/socket.7
> +++ b/man7/socket.7
> @@ -693,7 +693,7 @@ For further details, see
> .BR SO_PEERSEC " (since Linux 2.6.2)"
> Return the security context of the peer socket connected to this socket.
> For further details, see
> -.BR unix (7).
> +.BR unix (7) and ip(7).
> .TP
> .B SO_PRIORITY
> Set the protocol-defined priority for all packets to be sent on
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
^ permalink raw reply
* Re: [PATCH v2] socket.7,unix.7: add initial description for SO_PEERSEC
From: Michael Kerrisk (man-pages) @ 2020-09-17 7:27 UTC (permalink / raw)
To: Stephen Smalley
Cc: mtk.manpages, linux-man, linux-security-module, selinux, smcv,
jmorris, serge
In-Reply-To: <20200914180700.11003-1-stephen.smalley.work@gmail.com>
Hello Stephen,
On 9/14/20 8:07 PM, Stephen Smalley wrote:
> SO_PEERSEC was introduced for AF_UNIX stream sockets connected via
> connect(2) in Linux 2.6.2 [1] and later augmented to support AF_UNIX stream
> and datagram sockets created via socketpair(2) in Linux 4.18 [2]. Document
> SO_PEERSEC in the socket.7 and unix.7 man pages following the example
> of the existing SO_PEERCRED descriptions. SO_PEERSEC is also supported
> on AF_INET sockets when using labeled IPSEC or NetLabel but defer
> adding a description of that support to a separate patch.
>
> The module-independent description of the security context returned
> by SO_PEERSEC is from Simon McVittie <smcv@collabora.com>.
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/commit/?id=da6e57a2e6bd7939f610d957afacaf6a131e75ed
>
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0b811db2cb2aabc910e53d34ebb95a15997c33e7
>
> Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> ---
> v2 adds kernel commit info to the description and man page and uses
> the suggested text from Simon McVittie for the description of
> the security context string in a module-neutral way.
Thanks. I've applied this patch, and added Serge's Reviewed-by.
Cheers,
Michael
>
> man7/socket.7 | 5 +++++
> man7/unix.7 | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 51 insertions(+)
>
> diff --git a/man7/socket.7 b/man7/socket.7
> index 21e891791..c3635f95b 100644
> --- a/man7/socket.7
> +++ b/man7/socket.7
> @@ -690,6 +690,11 @@ Return the credentials of the peer process connected to this socket.
> For further details, see
> .BR unix (7).
> .TP
> +.BR SO_PEERSEC " (since Linux 2.6.2)"
> +Return the security context of the peer socket connected to this socket.
> +For further details, see
> +.BR unix (7).
> +.TP
> .B SO_PRIORITY
> Set the protocol-defined priority for all packets to be sent on
> this socket.
> diff --git a/man7/unix.7 b/man7/unix.7
> index 50828a5bc..298521d4a 100644
> --- a/man7/unix.7
> +++ b/man7/unix.7
> @@ -349,6 +349,52 @@ stream sockets and for
> .B AF_UNIX
> stream and datagram socket pairs created using
> .BR socketpair (2).
> +.TP
> +.B SO_PEERSEC
> +This read-only socket option returns the
> +security context of the peer socket connected to this socket.
> +By default, this will be the same as the security context of
> +the process that created the peer socket unless overridden
> +by the policy or by a process with the required permissions.
> +.IP
> +The argument to
> +.BR getsockopt (2)
> +is a pointer to a
> +buffer of the specified length in bytes
> +into which the security context string will be copied.
> +If the buffer length is less than the length of the security
> +context string, then
> +.BR getsockopt (2)
> +will return the required length
> +via
> +.I optlen
> +and return \-1 and sets
> +.I errno
> +to
> +.BR ERANGE .
> +The caller should allocate at least
> +.BR NAME_MAX
> +bytes for the buffer initially although this is not guaranteed
> +to be sufficient. Resizing the buffer to the returned length
> +and retrying may be necessary.
> +.IP
> +The security context string may include a terminating null character
> +in the returned length, but is not guaranteed to do so: a security
> +context "foo" might be represented as either {'f','o','o'} of length 3
> +or {'f','o','o','\\0'} of length 4, which are considered to be
> +interchangeable. It is printable, does not contain non-terminating
> +null characters, and is in an unspecified encoding (in particular it
> +is not guaranteed to be ASCII or UTF-8).
> +.IP
> +The use of this option for sockets in the
> +.B AF_UNIX
> +address family
> +is supported since Linux 2.6.2 for connected stream sockets and
> +since Linux 4.18,
> +.\" commit 0b811db2cb2aabc910e53d34ebb95a15997c33e7
> +also for stream and datagram socket pairs created
> +using
> +.BR socketpair (2).
> .\"
> .SS Autobind feature
> If a
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
^ permalink raw reply
* Re: [PATCH v2 0/4] [RFC] Implement Trampoline File Descriptor
From: Florian Weimer @ 2020-09-17 1:04 UTC (permalink / raw)
To: madvenka
Cc: kernel-hardening, linux-api, linux-arm-kernel, linux-fsdevel,
linux-integrity, linux-kernel, linux-security-module, oleg, x86,
libffi-discuss
In-Reply-To: <20200916150826.5990-1-madvenka@linux.microsoft.com>
* madvenka:
> Examples of trampolines
> =======================
>
> libffi (A Portable Foreign Function Interface Library):
>
> libffi allows a user to define functions with an arbitrary list of
> arguments and return value through a feature called "Closures".
> Closures use trampolines to jump to ABI handlers that handle calling
> conventions and call a target function. libffi is used by a lot
> of different applications. To name a few:
>
> - Python
> - Java
> - Javascript
> - Ruby FFI
> - Lisp
> - Objective C
libffi does not actually need this. It currently collocates
trampolines and the data they need on the same page, but that's
actually unecessary. It's possible to avoid doing this just by
changing libffi, without any kernel changes.
I think this has already been done for the iOS port.
> The code for trampoline X in the trampoline table is:
>
> load &code_table[X], code_reg
> load (code_reg), code_reg
> load &data_table[X], data_reg
> load (data_reg), data_reg
> jump code_reg
>
> The addresses &code_table[X] and &data_table[X] are baked into the
> trampoline code. So, PC-relative data references are not needed. The user
> can modify code_table[X] and data_table[X] dynamically.
You can put this code into the libffi shared object and map it from
there, just like the rest of the libffi code. To get more
trampolines, you can map the page containing the trampolines multiple
times, each instance preceded by a separate data page with the control
information.
I think the previous patch submission has also resulted in several
comments along those lines, so I'm not sure why you are reposting
this.
> libffi
> ======
>
> I have implemented my solution for libffi and provided the changes for
> X86 and ARM, 32-bit and 64-bit. Here is the reference patch:
>
> http://linux.microsoft.com/~madvenka/libffi/libffi.v2.txt
The URL does not appear to work, I get a 403 error.
> If the trampfd patchset gets accepted, I will send the libffi changes
> to the maintainers for a review. BTW, I have also successfully executed
> the libffi self tests.
I have not seen your libffi changes, but I expect that the complexity
is about the same as a userspace-only solution.
Cc:ing libffi upstream for awareness. The start of the thread is
here:
<https://lore.kernel.org/linux-api/20200916150826.5990-1-madvenka@linux.microsoft.com/>
^ permalink raw reply
* Re: [PATCH v3] ima: Fix NULL pointer dereference in ima_file_hash
From: Mimi Zohar @ 2020-09-16 22:51 UTC (permalink / raw)
To: KP Singh, linux-kernel, linux-security-module
Cc: Florent Revest, James Morris, Kees Cook, Lakshmi Ramasubramanian,
Jann Horn
In-Reply-To: <20200916180242.430668-1-kpsingh@chromium.org>
On Wed, 2020-09-16 at 18:02 +0000, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
>
> ima_file_hash can be called when there is no iint->ima_hash available
> even though the inode exists in the integrity cache. It is fairly
> common for a file to not have a hash. (e.g. an mknodat, prior to the
> file being closed).
>
> Another example where this can happen (suggested by Jann Horn):
>
> Process A does:
>
> while(1) {
> unlink("/tmp/imafoo");
> fd = open("/tmp/imafoo", O_RDWR|O_CREAT|O_TRUNC, 0700);
> if (fd == -1) {
> perror("open");
> continue;
> }
> write(fd, "A", 1);
> close(fd);
> }
>
> and Process B does:
>
> while (1) {
> int fd = open("/tmp/imafoo", O_RDONLY);
> if (fd == -1)
> continue;
> char *mapping = mmap(NULL, 0x1000, PROT_READ|PROT_EXEC,
> MAP_PRIVATE, fd, 0);
> if (mapping != MAP_FAILED)
> munmap(mapping, 0x1000);
> close(fd);
> }
>
> Due to the race to get the iint->mutex between ima_file_hash and
> process_measurement iint->ima_hash could still be NULL.
>
> Fixes: 6beea7afcc72 ("ima: add the ability to query the cached hash of a given file")
> Signed-off-by: KP Singh <kpsingh@google.com>
> Reviewed-by: Florent Revest <revest@chromium.org>
Thanks, the patch is queued in next-integrity-testing.
Mimi
^ permalink raw reply
* Re: [PATCH v3] certs: Add EFI_CERT_X509_GUID support for dbx entries
From: Jarkko Sakkinen @ 2020-09-16 16:12 UTC (permalink / raw)
To: Eric Snowberg
Cc: dhowells, dwmw2, jmorris, serge, nayna, erichte, mpe, zohar,
keyrings, linux-kernel, linux-security-module, rdunlap
In-Reply-To: <F25F6F0E-7E13-4C9D-A7BA-33CDEF7074F2@oracle.com>
On Tue, Sep 15, 2020 at 09:42:27AM -0600, Eric Snowberg wrote:
>
> > On Sep 14, 2020, at 12:12 PM, Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote:
> >
> > On Fri, Sep 11, 2020 at 02:22:30PM -0400, Eric Snowberg wrote:
> >> The Secure Boot Forbidden Signature Database, dbx, contains a list of now
> >> revoked signatures and keys previously approved to boot with UEFI Secure
> >> Boot enabled. The dbx is capable of containing any number of
> >> EFI_CERT_X509_SHA256_GUID, EFI_CERT_SHA256_GUID, and EFI_CERT_X509_GUID
> >> entries.
> >>
> >> Currently when EFI_CERT_X509_GUID are contained in the dbx, the entries are
> >> skipped.
> >>
> >> Add support for EFI_CERT_X509_GUID dbx entries. When a EFI_CERT_X509_GUID
> >> is found, it is added as an asymmetrical key to the .blacklist keyring.
> >> Anytime the .platform keyring is used, the keys in the .blacklist keyring
> >> are referenced, if a matching key is found, the key will be rejected.
> >>
> >> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
> >> ---
> >> v3:
> >> Fixed an issue when CONFIG_PKCS7_MESSAGE_PARSER is not builtin and defined
> >> as a module instead, pointed out by Randy Dunlap
> >>
> >> v2:
> >> Fixed build issue reported by kernel test robot <lkp@intel.com>
> >> Commit message update (suggested by Jarkko Sakkinen)
> >> ---
> >> certs/blacklist.c | 33 +++++++++++++++++++
> >> certs/blacklist.h | 12 +++++++
> >> certs/system_keyring.c | 6 ++++
> >> include/keys/system_keyring.h | 11 +++++++
> >> .../platform_certs/keyring_handler.c | 11 +++++++
> >> 5 files changed, 73 insertions(+)
> >>
> >> diff --git a/certs/blacklist.c b/certs/blacklist.c
> >> index 6514f9ebc943..3d1514ba5d47 100644
> >> --- a/certs/blacklist.c
> >> +++ b/certs/blacklist.c
> >> @@ -100,6 +100,39 @@ int mark_hash_blacklisted(const char *hash)
> >> return 0;
> >> }
> >>
> >> +int mark_key_revocationlisted(const char *data, size_t size)
> >> +{
> >> + key_ref_t key;
> >> +
> >> + key = key_create_or_update(make_key_ref(blacklist_keyring, true),
> >> + "asymmetric",
> >> + NULL,
> >> + data,
> >> + size,
> >> + ((KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_VIEW),
> >> + KEY_ALLOC_NOT_IN_QUOTA | KEY_ALLOC_BUILT_IN);
> >> +
> >> + if (IS_ERR(key)) {
> >> + pr_err("Problem with revocation key (%ld)\n", PTR_ERR(key));
> >> + return PTR_ERR(key);
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +int is_key_revocationlisted(struct pkcs7_message *pkcs7)
> >> +{
> >> + int ret;
> >> +
> >> + ret = validate_trust(pkcs7, blacklist_keyring);
> >> +
> >> + if (ret == 0)
> >> + return -EKEYREJECTED;
> >> +
> >> + return -ENOKEY;
> >> +}
> >> +EXPORT_SYMBOL_GPL(is_key_revocationlisted);
> >
> > Hmm... ignore my previous comment about this. Export symbol is called
> > only by system keyring code.
> >
> > Would be best if the commit message would explicitly reason new exports.
>
> I don’t see a good reason to keep the export now, I’ll remove it from the
> next version. Thanks.
OK, great, thanks.
Was somewhat puzzled with this for a while :-)
/Jarkko
^ permalink raw reply
* [TEST]: Patches not making it to the lists
From: KP Singh @ 2020-09-16 14:51 UTC (permalink / raw)
To: Linux Security Module list; +Cc: James Morris
The mailing lists seem to be silently dropping my patches. it gets
delivered to the CC'ed folks but does not appear any archives or
delivered to the subscribers. The patch is not in HTML and was sent
using git send-email.
I don't get a bounce back either and replying-all does not help (my
colleague tried it as well and it did not appear on the list)
I can think of two possibilities:
- My email is triggering a spam filter. In which case this email won't
be delivered either.
- The body of the patch is causing something to crash on the mailserver.
Here's the patch ("ima: Fix NULL pointer dereference in
ima_file_hash") on a github gist for your reference:
https://gist.githubusercontent.com/sinkap/6dd3829a8259343a6b178cef3f59342b/raw/2b3152a48d77b1c65d0e1b5b8b2e8b6ae6d04ec5/ima.patch
- KP
^ permalink raw reply
* Re: [PATCH v2 00/12] IMA/EVM fixes
From: Mimi Zohar @ 2020-09-16 16:14 UTC (permalink / raw)
To: Roberto Sassu, mjg59
Cc: linux-integrity, linux-security-module, linux-kernel,
silviu.vlasceanu
In-Reply-To: <20200904092339.19598-1-roberto.sassu@huawei.com>
Hi Roberto,
On Fri, 2020-09-04 at 11:23 +0200, Roberto Sassu wrote:
> This patch set includes various fixes for IMA and EVM.
>
> Patches 1-3 are trivial fixes.
I've queued these patches in the next-integrity-testing branch for now.
When reposting this patch set please replace the cover letter subject
line with a more appropriate one.
> The remaining improve support and usability
> of EVM portable signatures. In particular patch 4 allows EVM to be used
> without an HMAC key.
EVM already supports using EVM portable and immutable sigatures without
an HMAC key.
I was able to apply this patch set, but patch 10/12 does not apply
cleanly due to the "fallthrough" line. Please hold off on reposting,
until I've finished reviewing the entire patch set.
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH v2] ima: Fix NULL pointer dereference in ima_file_hash
From: Mimi Zohar @ 2020-09-16 16:00 UTC (permalink / raw)
To: KP Singh, linux-kernel, linux-security-module
Cc: Florent Revest, James Morris, Kees Cook, Lakshmi Ramasubramanian,
Jann Horn
In-Reply-To: <20200916124931.1254990-1-kpsingh@chromium.org>
On Wed, 2020-09-16 at 14:49 +0200, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
>
> ima_file_hash can be called when there is no iint->ima_hash available
> even though the inode exists in the integrity cache.
>
> An example where this can happen (suggested by Jann Horn):
>
> Process A does:
>
> while(1) {
> unlink("/tmp/imafoo");
> fd = open("/tmp/imafoo", O_RDWR|O_CREAT|O_TRUNC, 0700);
> if (fd == -1) {
> perror("open");
> continue;
> }
> write(fd, "A", 1);
> close(fd);
> }
>
> and Process B does:
>
> while (1) {
> int fd = open("/tmp/imafoo", O_RDONLY);
> if (fd == -1)
> continue;
> char *mapping = mmap(NULL, 0x1000, PROT_READ|PROT_EXEC,
> MAP_PRIVATE, fd, 0);
> if (mapping != MAP_FAILED)
> munmap(mapping, 0x1000);
> close(fd);
> }
>
> Due to the race to get the iint->mutex between ima_file_hash and
> process_measurement iint->ima_hash could still be NULL.
>
> Fixes: 6beea7afcc72 ("ima: add the ability to query the cached hash of a given file")
> Signed-off-by: KP Singh <kpsingh@google.com>
> Reviewed-by: Florent Revest <revest@chromium.org>
> ---
> security/integrity/ima/ima_main.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 8a91711ca79b..4c86cd4eece0 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -531,6 +531,16 @@ int ima_file_hash(struct file *file, char *buf, size_t buf_size)
> return -EOPNOTSUPP;
>
> mutex_lock(&iint->mutex);
> +
> + /*
> + * ima_file_hash can be called when ima_collect_measurement has still
> + * not been called, we might not always have a hash.
> + */
> + if (!iint->ima_hash) {
> + mutex_unlock(&iint->mutex);
> + return -EOPNOTSUPP;
> + }
> +
Not having a file hash is rather common (e.g. mknodat, prior to the
file being closed). Before appraising the integrity of a file, it
checks whether it is a new file (eg. IMA_NEW_FILE), but, unfortunately,
the flag is only set for those files in the appraise policy.
The patch looks fine, but you might want to reflect not having a file
hash is common in the patch description.
Mimi
> if (buf) {
> size_t copied_size;
>
^ permalink raw reply
* Re: [PATCH] ima: Fix NULL pointer dereference in ima_file_hash
From: Florent Revest @ 2020-09-16 12:41 UTC (permalink / raw)
To: KP Singh, linux-kernel, linux-security-module
Cc: Mimi Zohar, James Morris, Kees Cook, Lakshmi Ramasubramanian,
Jann Horn
In-Reply-To: <20200916120548.364892-1-kpsingh@chromium.org>
Reviewed-by: Florent Revest <revest@chromium.org>
On Wed, 2020-09-16 at 12:05 +0000, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>
>
> ima_file_hash can be called when there is no iint->ima_hash available
> even though the inode exists in the integrity cache.
>
> An example where this can happen (suggested by Jann Horn):
>
> Process A does:
>
> while(1) {
> unlink("/tmp/imafoo");
> fd = open("/tmp/imafoo", O_RDWR|O_CREAT|O_TRUNC, 0700);
> if (fd == -1) {
> perror("open");
> continue;
> }
> write(fd, "A", 1);
> close(fd);
> }
>
> and Process B does:
>
> while (1) {
> int fd = open("/tmp/imafoo", O_RDONLY);
> if (fd == -1)
> continue;
> char *mapping = mmap(NULL, 0x1000, PROT_READ|PROT_EXEC,
> MAP_PRIVATE, fd, 0);
> if (mapping != MAP_FAILED)
> munmap(mapping, 0x1000);
> close(fd);
> }
>
> Due to the race to get the iint->mutex between ima_file_hash and
> process_measurement iint->ima_hash could still be NULL.
>
> Fixes: 6beea7afcc72 ("ima: add the ability to query the cached hash
> of a given file")
> Signed-off-by: KP Singh <kpsingh@google.com>
> ---
> security/integrity/ima/ima_main.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/security/integrity/ima/ima_main.c
> b/security/integrity/ima/ima_main.c
> index 8a91711ca79b..4c86cd4eece0 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -531,6 +531,16 @@ int ima_file_hash(struct file *file, char *buf,
> size_t buf_size)
> return -EOPNOTSUPP;
>
> mutex_lock(&iint->mutex);
> +
> + /*
> + * ima_file_hash can be called when ima_collect_measurement has
> still
> + * not been called, we might not always have a hash.
> + */
> + if (!iint->ima_hash) {
> + mutex_unlock(&iint->mutex);
> + return -EOPNOTSUPP;
> + }
> +
> if (buf) {
> size_t copied_size;
>
^ permalink raw reply
* Re: [PATCH v20 05/12] LSM: Infrastructure management of the superblock
From: Mickaël Salaün @ 2020-09-16 13:42 UTC (permalink / raw)
To: Stephen Smalley
Cc: Stephen Smalley, Casey Schaufler, Kees Cook, John Johansen,
linux-kernel, Al Viro, Andy Lutomirski, Anton Ivanov,
Arnd Bergmann, James Morris, Jann Horn, Jeff Dike,
Jonathan Corbet, Michael Kerrisk, Richard Weinberger,
Serge E . Hallyn, Shuah Khan, Vincent Dagonneau, kernel-hardening,
linux-api, linux-arch, linux-doc, Linux FS Devel, linux-kselftest,
LSM List, X86 ML
In-Reply-To: <CAEjxPJ7ARJO57MBW66=xsBzMMRb=9uLgqocK5eskHCaiVMx7Vw@mail.gmail.com>
On 04/09/2020 16:06, Stephen Smalley wrote:
> On Thu, Aug 13, 2020 at 2:39 PM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
>>
>> On Thu, Aug 13, 2020 at 10:17 AM Mickaël Salaün <mic@digikod.net> wrote:
>>>
>>>
>>> On 12/08/2020 21:16, Stephen Smalley wrote:
>>>> On 8/2/20 5:58 PM, Mickaël Salaün wrote:
>>>>> From: Casey Schaufler <casey@schaufler-ca.com>
>>>>>
>>>>> Move management of the superblock->sb_security blob out
>>>>> of the individual security modules and into the security
>>>>> infrastructure. Instead of allocating the blobs from within
>>>>> the modules the modules tell the infrastructure how much
>>>>> space is required, and the space is allocated there.
>>>>>
>>>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>>>>> Reviewed-by: Kees Cook <keescook@chromium.org>
>>>>> Reviewed-by: John Johansen <john.johansen@canonical.com>
>>>>> Reviewed-by: Stephen Smalley <sds@tycho.nsa.gov>
>>>>> Reviewed-by: Mickaël Salaün <mic@digikod.net>
>>>>> Link:
>>>>> https://lore.kernel.org/r/20190829232935.7099-2-casey@schaufler-ca.com
>>>>> ---
>>>>>
>>>>> Changes since v17:
>>>>> * Rebase the original LSM stacking patch from v5.3 to v5.7: I fixed some
>>>>> diff conflicts caused by code moves and function renames in
>>>>> selinux/include/objsec.h and selinux/hooks.c . I checked that it
>>>>> builds but I didn't test the changes for SELinux nor SMACK.
>>>>
>>>> You shouldn't retain Signed-off-by and Reviewed-by lines from an earlier
>>>> patch if you made non-trivial changes to it (even more so if you didn't
>>>> test them).
>>>
>>> I think I made trivial changes according to the original patch. But
>>> without reply from other people with Signed-off-by or Reviewed-by
>>> (Casey, Kees, John), I'll remove them. I guess you don't want your
>>> Reviewed-by to be kept, so I'll remove it, except if you want to review
>>> this patch (or the modified part).
>>
>> At the very least your Reviewed-by line is wrong - yours should be
>> Signed-off-by because the patch went through you and you modified it.
>> I'll try to take a look as time permits but FYI you should this
>> address (already updated in MAINTAINERS) going forward.
>
> I finally got around to reviewing your updated patch. You can drop
> the old line and add:
> Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>
>
Thanks! I'll send a new series soon.
^ permalink raw reply
* Re: [PATCH v2] ima: Fix NULL pointer dereference in ima_file_hash
From: KP Singh @ 2020-09-16 16:59 UTC (permalink / raw)
To: Mimi Zohar
Cc: open list, Linux Security Module list, Florent Revest,
James Morris, Kees Cook, Lakshmi Ramasubramanian, Jann Horn
In-Reply-To: <8cee070eed5b8a3dc9f373fc9db8d99a70b7d75a.camel@linux.ibm.com>
On Wed, Sep 16, 2020 at 6:00 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Wed, 2020-09-16 at 14:49 +0200, KP Singh wrote:
> > From: KP Singh <kpsingh@google.com>
> >
> > ima_file_hash can be called when there is no iint->ima_hash available
> > even though the inode exists in the integrity cache.
> >
> > An example where this can happen (suggested by Jann Horn):
> >
> > Process A does:
> >
> > while(1) {
> > unlink("/tmp/imafoo");
> > fd = open("/tmp/imafoo", O_RDWR|O_CREAT|O_TRUNC, 0700);
> > if (fd == -1) {
> > perror("open");
> > continue;
> > }
> > write(fd, "A", 1);
> > close(fd);
> > }
> >
> > and Process B does:
> >
> > while (1) {
> > int fd = open("/tmp/imafoo", O_RDONLY);
> > if (fd == -1)
> > continue;
> > char *mapping = mmap(NULL, 0x1000, PROT_READ|PROT_EXEC,
> > MAP_PRIVATE, fd, 0);
> > if (mapping != MAP_FAILED)
> > munmap(mapping, 0x1000);
> > close(fd);
> > }
> >
> > Due to the race to get the iint->mutex between ima_file_hash and
> > process_measurement iint->ima_hash could still be NULL.
> >
> > Fixes: 6beea7afcc72 ("ima: add the ability to query the cached hash of a given file")
> > Signed-off-by: KP Singh <kpsingh@google.com>
> > Reviewed-by: Florent Revest <revest@chromium.org>
> > ---
> > security/integrity/ima/ima_main.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > index 8a91711ca79b..4c86cd4eece0 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -531,6 +531,16 @@ int ima_file_hash(struct file *file, char *buf, size_t buf_size)
> > return -EOPNOTSUPP;
> >
> > mutex_lock(&iint->mutex);
> > +
> > + /*
> > + * ima_file_hash can be called when ima_collect_measurement has still
> > + * not been called, we might not always have a hash.
> > + */
> > + if (!iint->ima_hash) {
> > + mutex_unlock(&iint->mutex);
> > + return -EOPNOTSUPP;
> > + }
> > +
>
> Not having a file hash is rather common (e.g. mknodat, prior to the
> file being closed). Before appraising the integrity of a file, it
> checks whether it is a new file (eg. IMA_NEW_FILE), but, unfortunately,
> the flag is only set for those files in the appraise policy.
Makes sense.
>
> The patch looks fine, but you might want to reflect not having a file
> hash is common in the patch description.
>
Thanks! Will send another revision with an updated description.
- KP
> Mimi
>
> > if (buf) {
> > size_t copied_size;
> >
>
>
^ permalink raw reply
* Re: [GIT PULL] security: device_cgroup RCU warning fix
From: pr-tracker-bot @ 2020-09-16 19:00 UTC (permalink / raw)
To: James Morris; +Cc: Linus Torvalds, linux-kernel, linux-security-module
In-Reply-To: <alpine.LRH.2.21.2009160619400.8445@namei.org>
The pull request you sent on Wed, 16 Sep 2020 06:21:29 +1000 (AEST):
> git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git tags/fixes-v5.9a
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/1e484d388773b0a984236a181fb21e133630df42
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [PATCH v2] ima: Fix NULL pointer dereference in ima_file_hash
From: KP Singh @ 2020-09-16 13:36 UTC (permalink / raw)
To: Linux Security Module list
In-Reply-To: <20200916124931.1254990-1-kpsingh@chromium.org>
On Wed, Sep 16, 2020 at 2:49 PM KP Singh <kpsingh@chromium.org> wrote:
>
> From: KP Singh <kpsingh@google.com>
[...]
Another attempt to get this on the lists.
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 8a91711ca79b..4c86cd4eece0 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -531,6 +531,16 @@ int ima_file_hash(struct file *file, char *buf, size_t buf_size)
> return -EOPNOTSUPP;
>
> mutex_lock(&iint->mutex);
> +
> + /*
> + * ima_file_hash can be called when ima_collect_measurement has still
> + * not been called, we might not always have a hash.
> + */
> + if (!iint->ima_hash) {
> + mutex_unlock(&iint->mutex);
> + return -EOPNOTSUPP;
> + }
> +
> if (buf) {
> size_t copied_size;
>
> --
> 2.28.0.526.ge36021eeef-goog
>
^ permalink raw reply
* Re: [PATCH] ima: Fix NULL pointer dereference in ima_file_hash
From: KP Singh @ 2020-09-16 12:47 UTC (permalink / raw)
To: Florent Revest
Cc: open list, Linux Security Module list, Mimi Zohar, James Morris,
Kees Cook, Lakshmi Ramasubramanian, Jann Horn
In-Reply-To: <5ab256148e8ad5f9882e888dac809bc72cd3fe66.camel@chromium.org>
Somehow this patch does not seem to have been picked up by any of the
mailing list archives and I am not sure if this was delivered to the
lists. I will send a v2 and add Florent's Reviewed-by tag.
On Wed, Sep 16, 2020 at 2:41 PM Florent Revest <revest@chromium.org> wrote:
>
> Reviewed-by: Florent Revest <revest@chromium.org>
>
> On Wed, 2020-09-16 at 12:05 +0000, KP Singh wrote:
> > From: KP Singh <kpsingh@google.com>
> >
> > ima_file_hash can be called when there is no iint->ima_hash available
> > even though the inode exists in the integrity cache.
> >
> > An example where this can happen (suggested by Jann Horn):
> >
> > Process A does:
> >
> > while(1) {
> > unlink("/tmp/imafoo");
> > fd = open("/tmp/imafoo", O_RDWR|O_CREAT|O_TRUNC, 0700);
> > if (fd == -1) {
> > perror("open");
> > continue;
> > }
> > write(fd, "A", 1);
> > close(fd);
> > }
> >
> > and Process B does:
> >
> > while (1) {
> > int fd = open("/tmp/imafoo", O_RDONLY);
> > if (fd == -1)
> > continue;
> > char *mapping = mmap(NULL, 0x1000, PROT_READ|PROT_EXEC,
> > MAP_PRIVATE, fd, 0);
> > if (mapping != MAP_FAILED)
> > munmap(mapping, 0x1000);
> > close(fd);
> > }
> >
> > Due to the race to get the iint->mutex between ima_file_hash and
> > process_measurement iint->ima_hash could still be NULL.
> >
> > Fixes: 6beea7afcc72 ("ima: add the ability to query the cached hash
> > of a given file")
> > Signed-off-by: KP Singh <kpsingh@google.com>
> > ---
> > security/integrity/ima/ima_main.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/security/integrity/ima/ima_main.c
> > b/security/integrity/ima/ima_main.c
> > index 8a91711ca79b..4c86cd4eece0 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -531,6 +531,16 @@ int ima_file_hash(struct file *file, char *buf,
> > size_t buf_size)
> > return -EOPNOTSUPP;
> >
> > mutex_lock(&iint->mutex);
> > +
> > + /*
> > + * ima_file_hash can be called when ima_collect_measurement has
> > still
> > + * not been called, we might not always have a hash.
> > + */
> > + if (!iint->ima_hash) {
> > + mutex_unlock(&iint->mutex);
> > + return -EOPNOTSUPP;
> > + }
> > +
> > if (buf) {
> > size_t copied_size;
> >
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox