Skip to contentSkip to conversation

evm: disable EVM on overlayfs

PATCH In review Mimi Zohar 3 participants 9 messages Dec 19, 2023 – Dec 20, 2023

Subsystems (6)
lore
Share & tools
lore

Mimi Zohar

author maintainer
#0 lore raw

v2 cover evm: disable EVM on overlayfs

EVM verifies the existing 'security.evm' value, before allowing it to be updated. The EVM HMAC and the original file signatures contain filesystem specific metadata (e.g. i_ino, i_generation and s_uuid).

This poses a challenge when transitioning from the lower backing file to the upper backing file.

Until a complete solution is developed, disable EVM on overlayfs.

Changelog v2:
Addressed Amir's comments:
- Simplified security_inode_copy_up_xattr() return.
- Identified filesystems that don't support EVM based on a new SB_I flag.

Mimi Zohar (3):
evm: don't copy up 'security.evm' xattr
evm: add support to disable EVM on unsupported filesystems
overlay: disable EVM

 fs/overlayfs/super.c              |  1 +
 include/linux/evm.h               |  6 +++++
 include/linux/fs.h                |  1 +
 security/integrity/evm/evm_main.c | 42 ++++++++++++++++++++++++++++++-
 security/security.c               |  2 +-
 5 files changed, 50 insertions(+), 2 deletions(-)
Signature · 1 line
2.39.3
author maintainer
#1 lore raw

PATCH v2 1/3 evm: don't copy up 'security.evm' xattr

The security.evm HMAC and the original file signatures contain filesystem specific data. As a result, the HMAC and signature are not the same on the stacked and backing filesystems.

Don't copy up 'security.evm'.

---
 include/linux/evm.h               | 6 ++++++
 security/integrity/evm/evm_main.c | 7 +++++++
 security/security.c               | 2 +-
 3 files changed, 14 insertions(+), 1 deletion(-)
include/linux/evm.h +6 -0
index 01fc495a83e2..36ec884320d9 100644
@@ -31,6 +31,7 @@ extern void evm_inode_post_setxattr(struct dentry *dentry,
 				    const char *xattr_name,
 				    const void *xattr_value,
 				    size_t xattr_value_len);
+extern int evm_inode_copy_up_xattr(const char *name);
 extern int evm_inode_removexattr(struct mnt_idmap *idmap,
 				 struct dentry *dentry, const char *xattr_name);
 extern void evm_inode_post_removexattr(struct dentry *dentry,
@@ -117,6 +118,11 @@ static inline void evm_inode_post_setxattr(struct dentry *dentry,
 	return;
 }
 
+static inline int  evm_inode_copy_up_xattr(const char *name)
+{
+	return 0;
+}
+
 static inline int evm_inode_removexattr(struct mnt_idmap *idmap,
 					struct dentry *dentry,
 					const char *xattr_name)
security/integrity/evm/evm_main.c +7 -0
index 894570fe39bc..02adba635b02 100644
@@ -863,6 +863,13 @@ void evm_inode_post_setattr(struct dentry *dentry, int ia_valid)
 		evm_update_evmxattr(dentry, NULL, NULL, 0);
 }
 
+int evm_inode_copy_up_xattr(const char *name)
+{
+	if (strcmp(name, XATTR_NAME_EVM) == 0)
+		return 1; /* Discard */
+	return -EOPNOTSUPP;
+}
+
 /*
  * evm_inode_init_security - initializes security.evm HMAC value
  */
security/security.c +1 -1
index dcb3e7014f9b..f00ec4d988b8 100644
@@ -2539,7 +2539,7 @@ int security_inode_copy_up_xattr(const char *name)
 			return rc;
 	}
 
-	return LSM_RET_DEFAULT(inode_copy_up_xattr);
+	return evm_inode_copy_up_xattr(name);
 }
 EXPORT_SYMBOL(security_inode_copy_up_xattr);
 
Signature · 1 line
2.39.3
author maintainer
#2 lore raw

PATCH v2 2/3 evm: add support to disable EVM on unsupported filesystems

In reply to Mimi Zohar

Identify EVM unsupported filesystems by defining a new flag SB_I_EVM_UNSUPPORTED.

Don't verify, write, remove or update 'security.evm' on unsupported filesystems.

---
 include/linux/fs.h                |  1 +
 security/integrity/evm/evm_main.c | 35 ++++++++++++++++++++++++++++++-
 2 files changed, 35 insertions(+), 1 deletion(-)
include/linux/fs.h +1 -0
index 98b7a7a8c42e..db9350a734ef 100644
@@ -1164,6 +1164,7 @@ extern int send_sigurg(struct fown_struct *fown);
 #define SB_I_USERNS_VISIBLE		0x00000010 /* fstype already mounted */
 #define SB_I_IMA_UNVERIFIABLE_SIGNATURE	0x00000020
 #define SB_I_UNTRUSTED_MOUNTER		0x00000040
+#define SB_I_EVM_UNSUPPORTED		0x00000050
 
 #define SB_I_SKIP_SYNC	0x00000100	/* Skip superblock at global sync */
 #define SB_I_PERSB_BDI	0x00000200	/* has a per-sb bdi */
security/integrity/evm/evm_main.c +34 -1
index 02adba635b02..cc7956d7878b 100644
@@ -151,6 +151,17 @@ static int evm_find_protected_xattrs(struct dentry *dentry)
 	return count;
 }
 
+static int is_unsupported_fs(struct dentry *dentry)
+{
+	struct inode *inode = d_backing_inode(dentry);
+
+	if (inode->i_sb->s_iflags & SB_I_EVM_UNSUPPORTED) {
+		pr_info_once("%s not supported\n", inode->i_sb->s_type->name);
+		return 1;
+	}
+	return 0;
+}
+
 /*
  * evm_verify_hmac - calculate and compare the HMAC with the EVM xattr
  *
@@ -181,6 +192,9 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
 		     iint->evm_status == INTEGRITY_PASS_IMMUTABLE))
 		return iint->evm_status;
 
+	if (is_unsupported_fs(dentry))
+		return INTEGRITY_UNKNOWN;
+
 	/* if status is not PASS, try to check again - against -ENOMEM */
 
 	/* first need to know the sig type */
@@ -408,6 +422,9 @@ enum integrity_status evm_verifyxattr(struct dentry *dentry,
 	if (!evm_key_loaded() || !evm_protected_xattr(xattr_name))
 		return INTEGRITY_UNKNOWN;
 
+	if (is_unsupported_fs(dentry))
+		return INTEGRITY_UNKNOWN;
+
 	if (!iint) {
 		iint = integrity_iint_find(d_backing_inode(dentry));
 		if (!iint)
@@ -491,15 +508,21 @@ static int evm_protect_xattr(struct mnt_idmap *idmap,
 	if (strcmp(xattr_name, XATTR_NAME_EVM) == 0) {
 		if (!capable(CAP_SYS_ADMIN))
 			return -EPERM;
+		if (is_unsupported_fs(dentry))
+			return -EPERM;
 	} else if (!evm_protected_xattr(xattr_name)) {
 		if (!posix_xattr_acl(xattr_name))
 			return 0;
+		if (is_unsupported_fs(dentry))
+			return 0;
+
 		evm_status = evm_verify_current_integrity(dentry);
 		if ((evm_status == INTEGRITY_PASS) ||
 		    (evm_status == INTEGRITY_NOXATTRS))
 			return 0;
 		goto out;
-	}
+	} else if (is_unsupported_fs(dentry))
+		return 0;
 
 	evm_status = evm_verify_current_integrity(dentry);
 	if (evm_status == INTEGRITY_NOXATTRS) {
@@ -750,6 +773,9 @@ void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name,
 	if (!(evm_initialized & EVM_INIT_HMAC))
 		return;
 
+	if (is_unsupported_fs(dentry))
+		return;
+
 	evm_update_evmxattr(dentry, xattr_name, xattr_value, xattr_value_len);
 }
 
@@ -814,8 +840,12 @@ int evm_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 	if (evm_initialized & EVM_ALLOW_METADATA_WRITES)
 		return 0;
 
+	if (is_unsupported_fs(dentry))
+		return 0;
+
 	if (!(ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID)))
 		return 0;
+
 	evm_status = evm_verify_current_integrity(dentry);
 	/*
 	 * Writing attrs is safe for portable signatures, as portable signatures
@@ -859,6 +889,9 @@ void evm_inode_post_setattr(struct dentry *dentry, int ia_valid)
 	if (!(evm_initialized & EVM_INIT_HMAC))
 		return;
 
+	if (is_unsupported_fs(dentry))
+		return;
+
 	if (ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID))
 		evm_update_evmxattr(dentry, NULL, NULL, 0);
 }
Signature · 1 line
2.39.3
author maintainer
#3 lore raw

Re: [PATCH v2 2/3] evm: add support to disable EVM on unsupported filesystems

include/linux/fs.h extern int send_sigurg(struct fown_struct *fown); · line 1167

Quoted 9 lines · Mimi Zohar
include/linux/fs.h +1 -0
index 98b7a7a8c42e..db9350a734ef 100644
@@ -1164,6 +1164,7 @@ extern int send_sigurg(struct fown_struct *fown);
 #define SB_I_USERNS_VISIBLE		0x00000010 /* fstype already mounted */
 #define SB_I_IMA_UNVERIFIABLE_SIGNATURE	0x00000020
 #define SB_I_UNTRUSTED_MOUNTER		0x00000040
+#define SB_I_EVM_UNSUPPORTED		0x00000050

This needs to be fixed.

Mimi

Quoted 4 lines
include/linux/fs.h +0 -0
 #define SB_I_SKIP_SYNC	0x00000100	/* Skip superblock at global sync */
 #define SB_I_PERSB_BDI	0x00000200	/* has a per-sb bdi */
security/integrity/evm/evm_main.c +0 -0
maintainer Acked-by
#4 lore raw

Re: [PATCH v2 2/3] evm: add support to disable EVM on unsupported filesystems

Quoted 15 lines · Mimi Zohar

include/linux/fs.h extern int send_sigurg(struct fown_struct *fown); · line 1167

Quoted 9 lines · Mimi Zohar
include/linux/fs.h +1 -0
index 98b7a7a8c42e..db9350a734ef 100644
@@ -1164,6 +1164,7 @@ extern int send_sigurg(struct fown_struct *fown);
 #define SB_I_USERNS_VISIBLE          0x00000010 /* fstype already mounted */
 #define SB_I_IMA_UNVERIFIABLE_SIGNATURE      0x00000020
 #define SB_I_UNTRUSTED_MOUNTER               0x00000040
+#define SB_I_EVM_UNSUPPORTED         0x00000050

This needs to be fixed.

With this fixed, you may add:

Thanks,
Amir.

author maintainer
#5 lore raw

PATCH v2 3/3 overlay: disable EVM

In reply to Mimi Zohar

Until a complete solution is developed, update 'sb->s_iflags' to disable EVM.

---
 fs/overlayfs/super.c | 1 +
 1 file changed, 1 insertion(+)
fs/overlayfs/super.c +1 -0
index a0967bb25003..e3d9c6c80a47 100644
@@ -1454,6 +1454,7 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
 	 * lead to unexpected results.
 	 */
 	sb->s_iflags |= SB_I_NOUMASK;
+	sb->s_iflags |= SB_I_EVM_UNSUPPORTED;
 
 	err = -ENOMEM;
 	root_dentry = ovl_get_root(sb, ctx->upper.dentry, oe);
Signature · 1 line
2.39.3
maintainer Acked-by on v2 3/3
#6 lore raw

Re: [PATCH v2 3/3] overlay: disable EVM

Quoted 5 lines · Mimi Zohar

Until a complete solution is developed, update 'sb->s_iflags' to disable EVM.

Quoted 19 lines
---
 fs/overlayfs/super.c | 1 +
 1 file changed, 1 insertion(+)
fs/overlayfs/super.c +1 -1
index a0967bb25003..e3d9c6c80a47 100644
@@ -1454,6 +1454,7 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
         * lead to unexpected results.
         */
        sb->s_iflags |= SB_I_NOUMASK;
+       sb->s_iflags |= SB_I_EVM_UNSUPPORTED;
 
        err = -ENOMEM;
        root_dentry = ovl_get_root(sb, ctx->upper.dentry, oe);
--

2.39.3

maintainer Reviewed-by on the v2 series
#7 lore raw
In reply to Mimi Zohar
Quoted 7 lines · Mimi Zohar

EVM verifies the existing 'security.evm' value, before allowing it to be updated. The EVM HMAC and the original file signatures contain filesystem specific metadata (e.g. i_ino, i_generation and s_uuid).

This poses a challenge when transitioning from the lower backing file to the upper backing file.

Quoted 6 lines

Until a complete solution is developed, disable EVM on overlayfs.

Changelog v2:
Addressed Amir's comments:
- Simplified security_inode_copy_up_xattr() return.
- Identified filesystems that don't support EVM based on a new SB_I flag.

We're wasting a flag for a single filesystem but we do have enough of them left so I think this is ok,

author maintainer
#8 lore raw
Quoted 10 lines · Christian Brauner
Quoted 9 lines · Mimi Zohar

EVM verifies the existing 'security.evm' value, before allowing it to be updated. The EVM HMAC and the original file signatures contain filesystem specific metadata (e.g. i_ino, i_generation and s_uuid).

This poses a challenge when transitioning from the lower backing file to the upper backing file.

Until a complete solution is developed, disable EVM on overlayfs.

Quoted 7 lines
Quoted 4 lines

Changelog v2:
Addressed Amir's comments:
- Simplified security_inode_copy_up_xattr() return.
- Identified filesystems that don't support EVM based on a new SB_I flag.

We're wasting a flag for a single filesystem but we do have enough of them left so I think this is ok,

Thanks, Christian.

Quoted 2 lines