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.
Mimi Zohar (2):
evm: don't copy up 'security.evm' xattr
evm: add support to disable EVM on unsupported filesystems
include/linux/evm.h | 6 +++++
security/integrity/evm/evm_main.c | 42 ++++++++++++++++++++++++++++++-
security/security.c | 4 +++
3 files changed, 51 insertions(+), 1 deletion(-)
--
2.39.3
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'.
Signed-off-by: Mimi Zohar <[email protected]>
---
include/linux/evm.h | 6 ++++++
security/integrity/evm/evm_main.c | 7 +++++++
security/security.c | 4 ++++
3 files changed, 17 insertions(+)
diff --git a/include/linux/evm.h b/include/linux/evm.h
index 01fc495a83e2..36ec884320d9 100644
--- a/include/linux/evm.h
+++ b/include/linux/evm.h
@@ -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)
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 894570fe39bc..02adba635b02 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -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
*/
diff --git a/security/security.c b/security/security.c
index dcb3e7014f9b..a02e78c45007 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2539,6 +2539,10 @@ int security_inode_copy_up_xattr(const char *name)
return rc;
}
+ rc = evm_inode_copy_up_xattr(name);
+ if (rc != LSM_RET_DEFAULT(inode_copy_up_xattr))
+ return rc;
+
return LSM_RET_DEFAULT(inode_copy_up_xattr);
}
EXPORT_SYMBOL(security_inode_copy_up_xattr);
--
2.39.3
Don't verify, write, remove or update 'security.evm' on unsupported
filesystems.
Temporarily define overlayfs as an unsupported filesystem until
a complete solution is developed.
Signed-off-by: Mimi Zohar <[email protected]>
---
security/integrity/evm/evm_main.c | 35 ++++++++++++++++++++++++++++++-
1 file changed, 34 insertions(+), 1 deletion(-)
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 02adba635b02..aa6d32a07d20 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -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 (strcmp(inode->i_sb->s_type->name, "overlay") == 0) {
+ pr_info_once("overlayfs not supported\n");
+ 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);
}
--
2.39.3
On Tue, Dec 19, 2023 at 3:49 PM Mimi Zohar <[email protected]> wrote:
>
> 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'.
>
> Signed-off-by: Mimi Zohar <[email protected]>
> ---
> include/linux/evm.h | 6 ++++++
> security/integrity/evm/evm_main.c | 7 +++++++
> security/security.c | 4 ++++
> 3 files changed, 17 insertions(+)
>
> diff --git a/include/linux/evm.h b/include/linux/evm.h
> index 01fc495a83e2..36ec884320d9 100644
> --- a/include/linux/evm.h
> +++ b/include/linux/evm.h
> @@ -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)
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 894570fe39bc..02adba635b02 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -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
> */
> diff --git a/security/security.c b/security/security.c
> index dcb3e7014f9b..a02e78c45007 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2539,6 +2539,10 @@ int security_inode_copy_up_xattr(const char *name)
> return rc;
> }
>
> + rc = evm_inode_copy_up_xattr(name);
> + if (rc != LSM_RET_DEFAULT(inode_copy_up_xattr))
> + return rc;
> +
> return LSM_RET_DEFAULT(inode_copy_up_xattr);
The rest of the hooks call evm last, e.g.:
return evm_inode_setattr(idmap, dentry, attr);
return evm_inode_remove_acl(idmap, dentry, acl_name);
evm_inode_post_setxattr(dentry, name, value, size);
return evm_inode_removexattr(idmap, dentry, name);
best keep a consistent LSM order.
Other than that, feel free to add:
Reviewed-by: Amir Goldstein <[email protected]>
Thanks,
Amir.
On Tue, Dec 19, 2023 at 3:49 PM Mimi Zohar <[email protected]> wrote:
>
> Don't verify, write, remove or update 'security.evm' on unsupported
> filesystems.
>
> Temporarily define overlayfs as an unsupported filesystem until
> a complete solution is developed.
>
> Signed-off-by: Mimi Zohar <[email protected]>
> ---
> security/integrity/evm/evm_main.c | 35 ++++++++++++++++++++++++++++++-
> 1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 02adba635b02..aa6d32a07d20 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -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 (strcmp(inode->i_sb->s_type->name, "overlay") == 0) {
> + pr_info_once("overlayfs not supported\n");
> + return 1;
> + }
Please do not special case overlayfs in and please do not use the
fs name to detect support.
Please define an sb flag like SB_I_IMA_UNVERIFIABLE_SIGNATURE
to disable EVM and set this flag in ovl_fill_super().
Thanks,
Amir.