lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 01 Mar 2024 10:19:13 +0100
From: Roberto Sassu <roberto.sassu@...weicloud.com>
To: "Seth Forshee (DigitalOcean)" <sforshee@...nel.org>, Christian Brauner
 <brauner@...nel.org>, Serge Hallyn <serge@...lyn.com>, Paul Moore
 <paul@...l-moore.com>, Eric Paris <eparis@...hat.com>, James Morris
 <jmorris@...ei.org>, Alexander Viro <viro@...iv.linux.org.uk>, Jan Kara
 <jack@...e.cz>, Stephen Smalley <stephen.smalley.work@...il.com>, Ondrej
 Mosnacek <omosnace@...hat.com>, Casey Schaufler <casey@...aufler-ca.com>,
 Mimi Zohar <zohar@...ux.ibm.com>, Roberto Sassu <roberto.sassu@...wei.com>,
 Dmitry Kasatkin <dmitry.kasatkin@...il.com>, Eric Snowberg
 <eric.snowberg@...cle.com>, "Matthew Wilcox (Oracle)"
 <willy@...radead.org>, Jonathan Corbet <corbet@....net>, Miklos Szeredi
 <miklos@...redi.hu>,  Amir Goldstein <amir73il@...il.com>
Cc: linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org, 
	linux-security-module@...r.kernel.org, audit@...r.kernel.org, 
	selinux@...r.kernel.org, linux-integrity@...r.kernel.org, 
	linux-doc@...r.kernel.org, linux-unionfs@...r.kernel.org
Subject: Re: [PATCH v2 14/25] evm: add support for fscaps security hooks

On Wed, 2024-02-21 at 15:24 -0600, Seth Forshee (DigitalOcean) wrote:
> Support the new fscaps security hooks by converting the vfs_caps to raw
> xattr data and then handling them the same as other xattrs.

Hi Seth

I started looking at this patch set.

The first question I have is if you are also going to update libcap
(and also tar, I guess), since both deal with the raw xattr.

>From IMA/EVM perspective (Mimi will add on that), I guess it is
important that files with a signature/HMAC continue to be accessible
after applying this patch set.

Looking at the code, it seems the case (if I understood correctly,
vfs_getxattr_alloc() is still allowed).

To be sure that everything works, it would be really nice if you could
also extend our test suite:

https://github.com/mimizohar/ima-evm-utils/blob/next-testing/tests/portable_signatures.test

and

https://github.com/mimizohar/ima-evm-utils/blob/next-testing/tests/evm_hmactest


The first test we would need to extend is check_cp_preserve_xattrs,
which basically does a cp -a. We would need to set fscaps in the
origin, copy to the destination, and see if the latter is accessible.

I would also extend:

check_tar_extract_xattrs_different_owner
check_tar_extract_xattrs_same_owner
check_metadata_change
check_evm_revalidate
check_evm_portable_sig_ima_appraisal
check_evm_portable_sig_ima_measurement_list

It should not be too complicated. The purpose would be to exercise your
code below.


Regarding the second test, we would need to extend just check_evm_hmac.


Just realized, before extending the tests, it would be necessary to
modify also evmctl.c, to retrieve fscaps through the new interfaces,
and to let users provide custom fscaps the HMAC or portable signature
is calculated on.


You can run the tests locally (even with UML linux), or make a PR in
Github for both linux and ima-evm-utils, and me and Mimi will help to
run them. For Github, for now please use:

https://github.com/linux-integrity/linux
https://github.com/mimizohar/ima-evm-utils/

Thanks

Roberto

> Signed-off-by: Seth Forshee (DigitalOcean) <sforshee@...nel.org>
> ---
>  include/linux/evm.h               | 39 +++++++++++++++++++++++++
>  security/integrity/evm/evm_main.c | 60 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 99 insertions(+)
> 
> diff --git a/include/linux/evm.h b/include/linux/evm.h
> index 36ec884320d9..aeb9ff52ad22 100644
> --- a/include/linux/evm.h
> +++ b/include/linux/evm.h
> @@ -57,6 +57,20 @@ static inline void evm_inode_post_set_acl(struct dentry *dentry,
>  {
>  	return evm_inode_post_setxattr(dentry, acl_name, NULL, 0);
>  }
> +extern int evm_inode_set_fscaps(struct mnt_idmap *idmap,
> +				struct dentry *dentry,
> +				const struct vfs_caps *caps, int flags);
> +static inline int evm_inode_remove_fscaps(struct dentry *dentry)
> +{
> +	return evm_inode_set_fscaps(&nop_mnt_idmap, dentry, NULL, XATTR_REPLACE);
> +}
> +extern void evm_inode_post_set_fscaps(struct mnt_idmap *idmap,
> +				      struct dentry *dentry,
> +				      const struct vfs_caps *caps, int flags);
> +static inline void evm_inode_post_remove_fscaps(struct dentry *dentry)
> +{
> +	return evm_inode_post_set_fscaps(&nop_mnt_idmap, dentry, NULL, 0);
> +}
>  
>  int evm_inode_init_security(struct inode *inode, struct inode *dir,
>  			    const struct qstr *qstr, struct xattr *xattrs,
> @@ -164,6 +178,31 @@ static inline void evm_inode_post_set_acl(struct dentry *dentry,
>  	return;
>  }
>  
> +static inline int evm_inode_set_fscaps(struct mnt_idmap *idmap,
> +				       struct dentry *dentry,
> +				       const struct vfs_caps *caps, int flags)
> +{
> +	return 0;
> +}
> +
> +static inline int evm_inode_remove_fscaps(struct dentry *dentry)
> +{
> +	return 0;
> +}
> +
> +static inline void evm_inode_post_set_fscaps(struct mnt_idmap *idmap,
> +					     struct dentry *dentry,
> +					     const struct vfs_caps *caps,
> +					     int flags)
> +{
> +	return;
> +}
> +
> +static inline void evm_inode_post_remove_fscaps(struct dentry *dentry)
> +{
> +	return;
> +}
> +
>  static inline int evm_inode_init_security(struct inode *inode, struct inode *dir,
>  					  const struct qstr *qstr,
>  					  struct xattr *xattrs,
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index cc7956d7878b..ecf4634a921a 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -805,6 +805,66 @@ void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name)
>  	evm_update_evmxattr(dentry, xattr_name, NULL, 0);
>  }
>  
> +int evm_inode_set_fscaps(struct mnt_idmap *idmap, struct dentry *dentry,
> +			 const struct vfs_caps *caps, int flags)
> +{
> +	struct inode *inode = d_inode(dentry);
> +	struct vfs_ns_cap_data nscaps;
> +	const void *xattr_data = NULL;
> +	int size = 0;
> +
> +	/* Policy permits modification of the protected xattrs even though
> +	 * there's no HMAC key loaded
> +	 */
> +	if (evm_initialized & EVM_ALLOW_METADATA_WRITES)
> +		return 0;
> +
> +	if (caps) {
> +		size = vfs_caps_to_xattr(idmap, i_user_ns(inode), caps, &nscaps,
> +					 sizeof(nscaps));
> +		if (size < 0)
> +			return size;
> +		xattr_data = &nscaps;
> +	}
> +
> +	return evm_protect_xattr(idmap, dentry, XATTR_NAME_CAPS, xattr_data, size);
> +}
> +
> +void evm_inode_post_set_fscaps(struct mnt_idmap *idmap, struct dentry *dentry,
> +			       const struct vfs_caps *caps, int flags)
> +{
> +	struct inode *inode = d_inode(dentry);
> +	struct vfs_ns_cap_data nscaps;
> +	const void *xattr_data = NULL;
> +	int size = 0;
> +
> +	if (!evm_revalidate_status(XATTR_NAME_CAPS))
> +		return;
> +
> +	evm_reset_status(dentry->d_inode);
> +
> +	if (!(evm_initialized & EVM_INIT_HMAC))
> +		return;
> +
> +	if (is_unsupported_fs(dentry))
> +		return;
> +
> +	if (caps) {
> +		size = vfs_caps_to_xattr(idmap, i_user_ns(inode), caps, &nscaps,
> +					 sizeof(nscaps));
> +		/*
> +		 * The fscaps here should have been converted to an xattr by
> +		 * evm_inode_set_fscaps() already, so a failure to convert
> +		 * here is a bug.
> +		 */
> +		if (WARN_ON_ONCE(size < 0))
> +			return;
> +		xattr_data = &nscaps;
> +	}
> +
> +	evm_update_evmxattr(dentry, XATTR_NAME_CAPS, xattr_data, size);
> +}
> +
>  static int evm_attr_change(struct mnt_idmap *idmap,
>  			   struct dentry *dentry, struct iattr *attr)
>  {
> 


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ