All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] EVM: Allow runtime modification of the set of verified xattrs
@ 2018-04-13 22:52 Matthew Garrett
  2018-04-15 14:05 ` Mimi Zohar
  0 siblings, 1 reply; 8+ messages in thread
From: Matthew Garrett @ 2018-04-13 22:52 UTC (permalink / raw)
  To: linux-integrity; +Cc: zohar, Matthew Garrett

Sites may wish to provide additional metadata alongside files in order
to make more fine-grained security decisions[1]. The security of this is
enhanced if this metadata is protected, something that EVM makes
possible. However, the kernel cannot know about the set of extended
attributes that local admins may wish to protect, and hardcoding this
policy in the kernel makes it difficult to change over time and less
convenient for distributions to enable.

This patch adds a new /sys/kernel/security/evm_xattrs node, which can be
read to obtain the current set of EVM-protected extended attributes or
written to in order to add new entries. Once EVM is enabled (by writing
to /sys/kernel/security/evm), further writes are blocked. This should
make no difference to security considerations around EVM - even if an
attacker is able to get additional extended attributes added to this
list, it will simply result in existing signatures failing to validate.

[1] For instance, a package manager could install information about the
package uploader in an additional extended attribute. Local LSM policy
could then be associated with that extended attribute in order to
restrict the privileges available to packages from less trusted
uploaders.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 Documentation/ABI/testing/evm      |  16 +++++
 security/integrity/evm/evm.h       |   3 +-
 security/integrity/evm/evm_main.c  |   4 +-
 security/integrity/evm/evm_secfs.c | 111 +++++++++++++++++++++++++++--
 4 files changed, 128 insertions(+), 6 deletions(-)

diff --git a/Documentation/ABI/testing/evm b/Documentation/ABI/testing/evm
index d12cb2eae9ee..5f60c263462c 100644
--- a/Documentation/ABI/testing/evm
+++ b/Documentation/ABI/testing/evm
@@ -57,3 +57,19 @@ Description:
 		dracut (via 97masterkey and 98integrity) and systemd (via
 		core/ima-setup) have support for loading keys at boot
 		time.
+
+What:		security/evm_xattrs
+Date:		April 2018
+Contact:	Matthew Garrett <mjg59@google.com>
+Description:
+		Shows the set of extended attributes used to calculate or
+		validate the EVM signature, and allows additional attributes
+		to be added at runtime. Adding additional extended attributes
+		will result in any existing signatures generated without the
+		additional attributes becoming invalid, and any signatures
+		generated after additional attributes are added will only be
+		valid if the same additional attributes are configured on
+		system boot.
+
+		This list cannot be modified after EVM has been enabled at
+		runtime.
\ No newline at end of file
diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
index 45c4a89c02ff..b134358860d2 100644
--- a/security/integrity/evm/evm.h
+++ b/security/integrity/evm/evm.h
@@ -40,7 +40,8 @@ extern struct crypto_shash *hmac_tfm;
 extern struct crypto_shash *hash_tfm;
 
 /* List of EVM protected security xattrs */
-extern char *evm_config_xattrnames[];
+extern char **evm_config_xattrnames;
+extern char **evm_config_extra_xattrnames;
 
 int evm_init_key(void);
 int evm_update_evmxattr(struct dentry *dentry,
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 9ea9c19a545c..debc8f836a9c 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -35,7 +35,7 @@ static const char * const integrity_status_msg[] = {
 };
 int evm_hmac_attrs;
 
-char *evm_config_xattrnames[] = {
+char *evm_config_default_xattrnames[] = {
 #ifdef CONFIG_SECURITY_SELINUX
 	XATTR_NAME_SELINUX,
 #endif
@@ -56,6 +56,8 @@ char *evm_config_xattrnames[] = {
 	XATTR_NAME_CAPS,
 	NULL
 };
+char **evm_config_xattrnames = evm_config_default_xattrnames;
+char **evm_config_extra_xattrnames;
 
 static int evm_fixmode;
 static int __init evm_set_fixmode(char *str)
diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
index feba03bbedae..2db800076728 100644
--- a/security/integrity/evm/evm_secfs.c
+++ b/security/integrity/evm/evm_secfs.c
@@ -20,6 +20,7 @@
 #include "evm.h"
 
 static struct dentry *evm_init_tpm;
+static struct dentry *evm_xattrs;
 
 /**
  * evm_read_key - read() for <securityfs>/evm
@@ -107,13 +108,115 @@ static const struct file_operations evm_key_ops = {
 	.write		= evm_write_key,
 };
 
-int __init evm_init_secfs(void)
+/**
+ * evm_read_xattrs - read() for <securityfs>/evm_xattrs
+ *
+ * @filp: file pointer, not actually used
+ * @buf: where to put the result
+ * @count: maximum to send along
+ * @ppos: where to start
+ *
+ * Returns number of bytes read or error code, as appropriate
+ */
+static ssize_t evm_read_xattrs(struct file *filp, char __user *buf,
+			       size_t count, loff_t *ppos)
+{
+	char *temp;
+	char **xattr;
+	int offset = 0;
+	ssize_t rc, size = 0;
+
+	if (*ppos != 0)
+		return 0;
+
+	for (xattr = evm_config_xattrnames; *xattr != NULL; xattr++)
+		size += strlen(*xattr) + 1;
+
+	temp = kmalloc(size + 1, GFP_KERNEL);
+	if (!temp)
+		return -ENOMEM;
+
+	for (xattr = evm_config_xattrnames; *xattr != NULL; xattr++) {
+		sprintf(temp + offset, "%s\n", *xattr);
+		offset += strlen(*xattr) + 1;
+	}
+
+	rc = simple_read_from_buffer(buf, count, ppos, temp, strlen(temp));
+
+	return rc;
+}
+
+/**
+ * evm_write_xattrs - write() for <securityfs>/evm_xattrs
+ * @file: file pointer, not actually used
+ * @buf: where to get the data from
+ * @count: bytes sent
+ * @ppos: where to start
+ *
+ * Returns number of bytes written or error code, as appropriate
+ */
+static ssize_t evm_write_xattrs(struct file *file, const char __user *buf,
+				size_t count, loff_t *ppos)
 {
-	int error = 0;
+	bool populate = false;
+	int len, entries = 0;
+	char **xattr, **temp;
+
+	if (!capable(CAP_SYS_ADMIN) || (evm_initialized & EVM_SETUP_COMPLETE))
+		return -EPERM;
+
+	if (*ppos != 0)
+		return -EINVAL;
+
+	for (xattr = evm_config_xattrnames; *xattr != NULL; xattr++)
+		entries++;
+
+	if (!evm_config_extra_xattrnames)
+		populate = true;
+
+	temp = krealloc(evm_config_extra_xattrnames,
+			(entries + 2) * sizeof(char *),
+			GFP_KERNEL);
+
+	if (!temp)
+		return -ENOMEM;
 
+	evm_config_extra_xattrnames = temp;
+
+	if (populate) {
+		memcpy(evm_config_extra_xattrnames, evm_config_xattrnames,
+		       entries * sizeof(char *));
+		evm_config_xattrnames = evm_config_extra_xattrnames;
+	}
+
+	evm_config_extra_xattrnames[entries] = memdup_user_nul(buf, count);
+	evm_config_extra_xattrnames[entries + 1] = NULL;
+
+	/* Remove any trailing newline */
+	len = strlen(evm_config_extra_xattrnames[entries]);
+	if (evm_config_extra_xattrnames[entries][len-1] == '\n')
+		evm_config_extra_xattrnames[entries][len-1] = '\0';
+	return count;
+}
+
+static const struct file_operations evm_xattr_ops = {
+	.read		= evm_read_xattrs,
+	.write		= evm_write_xattrs,
+};
+
+int __init evm_init_secfs(void)
+{
 	evm_init_tpm = securityfs_create_file("evm", S_IRUSR | S_IRGRP,
 					      NULL, NULL, &evm_key_ops);
 	if (!evm_init_tpm || IS_ERR(evm_init_tpm))
-		error = -EFAULT;
-	return error;
+		return -EFAULT;
+
+	evm_xattrs = securityfs_create_file("evm_xattrs", S_IRUSR | S_IRGRP,
+					    NULL, NULL, &evm_xattr_ops);
+	if (!evm_xattrs || IS_ERR(evm_xattrs)) {
+		securityfs_remove(evm_init_tpm);
+		return -EFAULT;
+	}
+
+	return 0;
 }
-- 
2.17.0.484.g0c8726318c-goog

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] EVM: Allow runtime modification of the set of verified xattrs
  2018-04-13 22:52 [PATCH] EVM: Allow runtime modification of the set of verified xattrs Matthew Garrett
@ 2018-04-15 14:05 ` Mimi Zohar
  2018-04-16 18:32   ` Matthew Garrett
  0 siblings, 1 reply; 8+ messages in thread
From: Mimi Zohar @ 2018-04-15 14:05 UTC (permalink / raw)
  To: Matthew Garrett, linux-integrity

On Fri, 2018-04-13 at 15:52 -0700, Matthew Garrett wrote:
> Sites may wish to provide additional metadata alongside files in order
> to make more fine-grained security decisions[1]. The security of this is
> enhanced if this metadata is protected, something that EVM makes
> possible. However, the kernel cannot know about the set of extended
> attributes that local admins may wish to protect, and hardcoding this
> policy in the kernel makes it difficult to change over time and less
> convenient for distributions to enable.
> 
> This patch adds a new /sys/kernel/security/evm_xattrs node, which can be
> read to obtain the current set of EVM-protected extended attributes or
> written to in order to add new entries. Once EVM is enabled (by writing
> to /sys/kernel/security/evm), further writes are blocked. This should
> make no difference to security considerations around EVM - even if an
> attacker is able to get additional extended attributes added to this
> list, it will simply result in existing signatures failing to validate.

Up to now, the security.evm HMAC was calculated based on the existing
set of protected xattrs, but did not require all of the xattrs to
exist.  Here, after adding a new xattr to the set of protected xattrs,
the HMAC/signature verification of existing security.evm xattrs should
continue to validate, just as it currently does with non-existent
xattrs.  Writing an existing, currently non-existent or new xattrs
would cause the signature/HMAC to be re-calculate and written out as
an HMAC.  Only the new EVM portable and immutable signatures would
result in signature verification failures. 

Although evm_config_xattrnames is not currently defined as a const, it
should have been.  Including additional xattrs via a securityfs file,
limits how the memory for the entire list of xattrs and the pointer to
that list can be protected.

Does this extra list of xattrs need to be run time or build time
configurable?  If it's build time configurable you'd be able to use
__ro_after_init.  For run time configurable, perhaps the proposed
"post-init read-only memory" (https://lwn.net/Articles/750215/) could
be used.

Mimi

> 
> [1] For instance, a package manager could install information about the
> package uploader in an additional extended attribute. Local LSM policy
> could then be associated with that extended attribute in order to
> restrict the privileges available to packages from less trusted
> uploaders.
> 
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> ---
>  Documentation/ABI/testing/evm      |  16 +++++
>  security/integrity/evm/evm.h       |   3 +-
>  security/integrity/evm/evm_main.c  |   4 +-
>  security/integrity/evm/evm_secfs.c | 111 +++++++++++++++++++++++++++--
>  4 files changed, 128 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/evm b/Documentation/ABI/testing/evm
> index d12cb2eae9ee..5f60c263462c 100644
> --- a/Documentation/ABI/testing/evm
> +++ b/Documentation/ABI/testing/evm
> @@ -57,3 +57,19 @@ Description:
>  		dracut (via 97masterkey and 98integrity) and systemd (via
>  		core/ima-setup) have support for loading keys at boot
>  		time.
> +
> +What:		security/evm_xattrs
> +Date:		April 2018
> +Contact:	Matthew Garrett <mjg59@google.com>
> +Description:
> +		Shows the set of extended attributes used to calculate or
> +		validate the EVM signature, and allows additional attributes
> +		to be added at runtime. Adding additional extended attributes
> +		will result in any existing signatures generated without the
> +		additional attributes becoming invalid, and any signatures
> +		generated after additional attributes are added will only be
> +		valid if the same additional attributes are configured on
> +		system boot.
> +
> +		This list cannot be modified after EVM has been enabled at
> +		runtime.
> \ No newline at end of file
> diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
> index 45c4a89c02ff..b134358860d2 100644
> --- a/security/integrity/evm/evm.h
> +++ b/security/integrity/evm/evm.h
> @@ -40,7 +40,8 @@ extern struct crypto_shash *hmac_tfm;
>  extern struct crypto_shash *hash_tfm;
> 
>  /* List of EVM protected security xattrs */
> -extern char *evm_config_xattrnames[];
> +extern char **evm_config_xattrnames;
> +extern char **evm_config_extra_xattrnames;
> 
>  int evm_init_key(void);
>  int evm_update_evmxattr(struct dentry *dentry,
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 9ea9c19a545c..debc8f836a9c 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -35,7 +35,7 @@ static const char * const integrity_status_msg[] = {
>  };
>  int evm_hmac_attrs;
> 
> -char *evm_config_xattrnames[] = {
> +char *evm_config_default_xattrnames[] = {
>  #ifdef CONFIG_SECURITY_SELINUX
>  	XATTR_NAME_SELINUX,
>  #endif
> @@ -56,6 +56,8 @@ char *evm_config_xattrnames[] = {
>  	XATTR_NAME_CAPS,
>  	NULL
>  };
> +char **evm_config_xattrnames = evm_config_default_xattrnames;
> +char **evm_config_extra_xattrnames;
> 
>  static int evm_fixmode;
>  static int __init evm_set_fixmode(char *str)
> diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
> index feba03bbedae..2db800076728 100644
> --- a/security/integrity/evm/evm_secfs.c
> +++ b/security/integrity/evm/evm_secfs.c
> @@ -20,6 +20,7 @@
>  #include "evm.h"
> 
>  static struct dentry *evm_init_tpm;
> +static struct dentry *evm_xattrs;
> 
>  /**
>   * evm_read_key - read() for <securityfs>/evm
> @@ -107,13 +108,115 @@ static const struct file_operations evm_key_ops = {
>  	.write		= evm_write_key,
>  };
> 
> -int __init evm_init_secfs(void)
> +/**
> + * evm_read_xattrs - read() for <securityfs>/evm_xattrs
> + *
> + * @filp: file pointer, not actually used
> + * @buf: where to put the result
> + * @count: maximum to send along
> + * @ppos: where to start
> + *
> + * Returns number of bytes read or error code, as appropriate
> + */
> +static ssize_t evm_read_xattrs(struct file *filp, char __user *buf,
> +			       size_t count, loff_t *ppos)
> +{
> +	char *temp;
> +	char **xattr;
> +	int offset = 0;
> +	ssize_t rc, size = 0;
> +
> +	if (*ppos != 0)
> +		return 0;
> +
> +	for (xattr = evm_config_xattrnames; *xattr != NULL; xattr++)
> +		size += strlen(*xattr) + 1;
> +
> +	temp = kmalloc(size + 1, GFP_KERNEL);
> +	if (!temp)
> +		return -ENOMEM;
> +
> +	for (xattr = evm_config_xattrnames; *xattr != NULL; xattr++) {
> +		sprintf(temp + offset, "%s\n", *xattr);
> +		offset += strlen(*xattr) + 1;
> +	}
> +
> +	rc = simple_read_from_buffer(buf, count, ppos, temp, strlen(temp));
> +
> +	return rc;
> +}
> +
> +/**
> + * evm_write_xattrs - write() for <securityfs>/evm_xattrs
> + * @file: file pointer, not actually used
> + * @buf: where to get the data from
> + * @count: bytes sent
> + * @ppos: where to start
> + *
> + * Returns number of bytes written or error code, as appropriate
> + */
> +static ssize_t evm_write_xattrs(struct file *file, const char __user *buf,
> +				size_t count, loff_t *ppos)
>  {
> -	int error = 0;
> +	bool populate = false;
> +	int len, entries = 0;
> +	char **xattr, **temp;
> +
> +	if (!capable(CAP_SYS_ADMIN) || (evm_initialized & EVM_SETUP_COMPLETE))
> +		return -EPERM;
> +
> +	if (*ppos != 0)
> +		return -EINVAL;
> +
> +	for (xattr = evm_config_xattrnames; *xattr != NULL; xattr++)
> +		entries++;
> +
> +	if (!evm_config_extra_xattrnames)
> +		populate = true;
> +
> +	temp = krealloc(evm_config_extra_xattrnames,
> +			(entries + 2) * sizeof(char *),
> +			GFP_KERNEL);
> +
> +	if (!temp)
> +		return -ENOMEM;
> 
> +	evm_config_extra_xattrnames = temp;
> +
> +	if (populate) {
> +		memcpy(evm_config_extra_xattrnames, evm_config_xattrnames,
> +		       entries * sizeof(char *));
> +		evm_config_xattrnames = evm_config_extra_xattrnames;
> +	}
> +
> +	evm_config_extra_xattrnames[entries] = memdup_user_nul(buf, count);
> +	evm_config_extra_xattrnames[entries + 1] = NULL;
> +
> +	/* Remove any trailing newline */
> +	len = strlen(evm_config_extra_xattrnames[entries]);
> +	if (evm_config_extra_xattrnames[entries][len-1] == '\n')
> +		evm_config_extra_xattrnames[entries][len-1] = '\0';
> +	return count;
> +}
> +
> +static const struct file_operations evm_xattr_ops = {
> +	.read		= evm_read_xattrs,
> +	.write		= evm_write_xattrs,
> +};
> +
> +int __init evm_init_secfs(void)
> +{
>  	evm_init_tpm = securityfs_create_file("evm", S_IRUSR | S_IRGRP,
>  					      NULL, NULL, &evm_key_ops);
>  	if (!evm_init_tpm || IS_ERR(evm_init_tpm))
> -		error = -EFAULT;
> -	return error;
> +		return -EFAULT;
> +
> +	evm_xattrs = securityfs_create_file("evm_xattrs", S_IRUSR | S_IRGRP,
> +					    NULL, NULL, &evm_xattr_ops);
> +	if (!evm_xattrs || IS_ERR(evm_xattrs)) {
> +		securityfs_remove(evm_init_tpm);
> +		return -EFAULT;
> +	}
> +
> +	return 0;
>  }

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] EVM: Allow runtime modification of the set of verified xattrs
  2018-04-15 14:05 ` Mimi Zohar
@ 2018-04-16 18:32   ` Matthew Garrett
  2018-04-16 20:16     ` Mimi Zohar
  0 siblings, 1 reply; 8+ messages in thread
From: Matthew Garrett @ 2018-04-16 18:32 UTC (permalink / raw)
  To: Mimi Zohar; +Cc: linux-integrity

On Sun, Apr 15, 2018 at 7:05 AM Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:

> On Fri, 2018-04-13 at 15:52 -0700, Matthew Garrett wrote:
> > Sites may wish to provide additional metadata alongside files in order
> > to make more fine-grained security decisions[1]. The security of this is
> > enhanced if this metadata is protected, something that EVM makes
> > possible. However, the kernel cannot know about the set of extended
> > attributes that local admins may wish to protect, and hardcoding this
> > policy in the kernel makes it difficult to change over time and less
> > convenient for distributions to enable.
> >
> > This patch adds a new /sys/kernel/security/evm_xattrs node, which can be
> > read to obtain the current set of EVM-protected extended attributes or
> > written to in order to add new entries. Once EVM is enabled (by writing
> > to /sys/kernel/security/evm), further writes are blocked. This should
> > make no difference to security considerations around EVM - even if an
> > attacker is able to get additional extended attributes added to this
> > list, it will simply result in existing signatures failing to validate.

> Up to now, the security.evm HMAC was calculated based on the existing
> set of protected xattrs, but did not require all of the xattrs to
> exist.  Here, after adding a new xattr to the set of protected xattrs,
> the HMAC/signature verification of existing security.evm xattrs should
> continue to validate, just as it currently does with non-existent
> xattrs.  Writing an existing, currently non-existent or new xattrs
> would cause the signature/HMAC to be re-calculate and written out as
> an HMAC.  Only the new EVM portable and immutable signatures would
> result in signature verification failures.

Yes, you're right - this is a misleading statement in the description, and
this behaviour isn't actually changed by the patch.

> Although evm_config_xattrnames is not currently defined as a const, it
> should have been.  Including additional xattrs via a securityfs file,
> limits how the memory for the entire list of xattrs and the pointer to
> that list can be protected.

Right, I did wonder about that.

> Does this extra list of xattrs need to be run time or build time
> configurable?  If it's build time configurable you'd be able to use
> __ro_after_init.  For run time configurable, perhaps the proposed
> "post-init read-only memory" (https://lwn.net/Articles/750215/) could
> be used.

Runtime. I'll look into the post-init stuff, but given that this doesn't
change the current security position do you think it's a blocker?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] EVM: Allow runtime modification of the set of verified xattrs
  2018-04-16 18:32   ` Matthew Garrett
@ 2018-04-16 20:16     ` Mimi Zohar
  2018-04-16 20:22       ` Matthew Garrett
  0 siblings, 1 reply; 8+ messages in thread
From: Mimi Zohar @ 2018-04-16 20:16 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: linux-integrity

On Mon, 2018-04-16 at 18:32 +0000, Matthew Garrett wrote:
> On Sun, Apr 15, 2018 at 7:05 AM Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:

> > Although evm_config_xattrnames is not currently defined as a const, it
> > should have been.  Including additional xattrs via a securityfs file,
> > limits how the memory for the entire list of xattrs and the pointer to
> > that list can be protected.
> 
> Right, I did wonder about that.
> 
> > Does this extra list of xattrs need to be run time or build time
> > configurable?  If it's build time configurable you'd be able to use
> > __ro_after_init.  For run time configurable, perhaps the proposed
> > "post-init read-only memory" (https://lwn.net/Articles/750215/) could
> > be used.
> 
> Runtime. I'll look into the post-init stuff, but given that this doesn't
> change the current security position do you think it's a blocker?

I would probably make the existing evm_config_xattrnames a const and
create a link list.  As new xattrs are defined, append them to the
tail.

Is there a reason for adding one additional xattrs one at a time, as
opposed to parsing a string?

Is it better to define a securityfs file, rather than a boot command
line argument?  With a boot command line argument, the list of xattrs
could be defined as __ro_after_init.

Mimi

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] EVM: Allow runtime modification of the set of verified xattrs
  2018-04-16 20:16     ` Mimi Zohar
@ 2018-04-16 20:22       ` Matthew Garrett
  2018-04-24 20:03         ` Matthew Garrett
  0 siblings, 1 reply; 8+ messages in thread
From: Matthew Garrett @ 2018-04-16 20:22 UTC (permalink / raw)
  To: Mimi Zohar; +Cc: linux-integrity

On Mon, Apr 16, 2018 at 1:16 PM Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:

> On Mon, 2018-04-16 at 18:32 +0000, Matthew Garrett wrote:
> > Runtime. I'll look into the post-init stuff, but given that this doesn't
> > change the current security position do you think it's a blocker?

> I would probably make the existing evm_config_xattrnames a const and
> create a link list.  As new xattrs are defined, append them to the
> tail.

Ok, that's definitely an option. But thinking about it some more - if an
attacker has arbitrary memory overwrite of writable pages, wouldn't it be
easier for them to just overwrite the policy and disable appraisal?

> Is there a reason for adding one additional xattrs one at a time, as
> opposed to parsing a string?

Mostly to avoid introducing more string parsing into the kernel.

> Is it better to define a securityfs file, rather than a boot command
> line argument?  With a boot command line argument, the list of xattrs
> could be defined as __ro_after_init.

I could go either way on this - I think that doing it on the command line
would satisfy all my use cases.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] EVM: Allow runtime modification of the set of verified xattrs
  2018-04-16 20:22       ` Matthew Garrett
@ 2018-04-24 20:03         ` Matthew Garrett
  2018-04-25 14:51           ` Mimi Zohar
  0 siblings, 1 reply; 8+ messages in thread
From: Matthew Garrett @ 2018-04-24 20:03 UTC (permalink / raw)
  To: Mimi Zohar; +Cc: linux-integrity

On Mon, Apr 16, 2018 at 1:22 PM Matthew Garrett <mjg59@google.com> wrote:
> I could go either way on this - I think that doing it on the command line
> would satisfy all my use cases.

Thinking about this some more - I think being able to do this at runtime is
actually important. If we add an additional xattr to the signatures then we
want to be able to update machine policy without forcing a reboot first,
otherwise we have a chicken and egg problem where we have to gate any new
package update against having a machine rebooted with an updated command
line (otherwise the signature validation will fail for packages that
contain new signatures)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] EVM: Allow runtime modification of the set of verified xattrs
  2018-04-24 20:03         ` Matthew Garrett
@ 2018-04-25 14:51           ` Mimi Zohar
  2018-04-25 16:54             ` Matthew Garrett
  0 siblings, 1 reply; 8+ messages in thread
From: Mimi Zohar @ 2018-04-25 14:51 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: linux-integrity, Igor Stoppa

[CC'ing Igor]

On Tue, 2018-04-24 at 20:03 +0000, Matthew Garrett wrote:
> On Mon, Apr 16, 2018 at 1:22 PM Matthew Garrett <mjg59@google.com> wrote:
> > I could go either way on this - I think that doing it on the command line
> > would satisfy all my use cases.
> 
> Thinking about this some more - I think being able to do this at runtime is
> actually important. If we add an additional xattr to the signatures then we
> want to be able to update machine policy without forcing a reboot first,
> otherwise we have a chicken and egg problem where we have to gate any new
> package update against having a machine rebooted with an updated command
> line (otherwise the signature validation will fail for packages that
> contain new signatures)

If the list of xattr names is append only, there is no reason for re-
allocating the entire xattr name list each time.  As long as the xattr
name list pointer is defined as __ro_after_init, we can work with Igor
on using "protectable memory" once it is upstreamed.

Mimi

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] EVM: Allow runtime modification of the set of verified xattrs
  2018-04-25 14:51           ` Mimi Zohar
@ 2018-04-25 16:54             ` Matthew Garrett
  0 siblings, 0 replies; 8+ messages in thread
From: Matthew Garrett @ 2018-04-25 16:54 UTC (permalink / raw)
  To: Mimi Zohar; +Cc: linux-integrity, igor.stoppa

On Wed, Apr 25, 2018 at 7:51 AM Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:

> [CC'ing Igor]

> On Tue, 2018-04-24 at 20:03 +0000, Matthew Garrett wrote:
> > Thinking about this some more - I think being able to do this at
runtime is
> > actually important. If we add an additional xattr to the signatures
then we
> > want to be able to update machine policy without forcing a reboot first,
> > otherwise we have a chicken and egg problem where we have to gate any
new
> > package update against having a machine rebooted with an updated command
> > line (otherwise the signature validation will fail for packages that
> > contain new signatures)

> If the list of xattr names is append only, there is no reason for re-
> allocating the entire xattr name list each time.  As long as the xattr
> name list pointer is defined as __ro_after_init, we can work with Igor
> on using "protectable memory" once it is upstreamed.

Ok, I'll refactor this into a list.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-04-25 16:55 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-13 22:52 [PATCH] EVM: Allow runtime modification of the set of verified xattrs Matthew Garrett
2018-04-15 14:05 ` Mimi Zohar
2018-04-16 18:32   ` Matthew Garrett
2018-04-16 20:16     ` Mimi Zohar
2018-04-16 20:22       ` Matthew Garrett
2018-04-24 20:03         ` Matthew Garrett
2018-04-25 14:51           ` Mimi Zohar
2018-04-25 16:54             ` Matthew Garrett

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.